<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 8/16/2024 4:00 PM, Rodrigo Vivi
wrote:<br>
</div>
<blockquote type="cite" cite="mid:Zr9bjsY8zcnciW0M@intel.com">
<pre wrap="" class="moz-quote-pre">On Fri, Aug 16, 2024 at 01:33:55PM +0530, <a class="moz-txt-link-abbreviated" href="mailto:apoorva.singh@intel.com">apoorva.singh@intel.com</a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">From: Apoorva Singh <a class="moz-txt-link-rfc2396E" href="mailto:apoorva.singh@intel.com"><apoorva.singh@intel.com></a>
- lrc->bo NULL check is not needed in xe_lrc_snapshot_capture() as
its already been taken care of in xe_lrc_init().
</pre>
</blockquote>
<pre wrap="" class="moz-quote-pre">
I'm afraid this is not a good reason.
This snapshot capture is coming from other places and apparently
with risks of paths where bo was already freed?!</pre>
</blockquote>
<p><br>
</p>
<p>I didn't think about it while suggesting this while reviewing
<a class="moz-txt-link-freetext" href="https://patchwork.freedesktop.org/patch/608210/?series=137289&rev=1">https://patchwork.freedesktop.org/patch/608210/?series=137289&rev=1</a></p>
<p>If the bo is null then we will hit NPD while doing <span
style="white-space: pre-wrap">xe_lrc_ggtt_addr() in the next like. so I guess it should be then</span></p>
<p><span style="white-space: pre-wrap">if (lrc->bo) {</span></p>
<p><span style="white-space: pre-wrap"> snapshot->context_desc = xe_lrc_ggtt_addr(lrc);
</span></p>
<p><span style="white-space: pre-wrap"> snapshot->lrc_bo = xe_bo_get(lrc->bo);</span></p>
<p><span style="white-space: pre-wrap">....
</span></p>
<p><span style="white-space: pre-wrap">}
</span></p>
<p><br>
</p>
<p>Regards,</p>
<p>Nirmoy<br>
</p>
<blockquote type="cite" cite="mid:Zr9bjsY8zcnciW0M@intel.com">
<pre wrap="" class="moz-quote-pre">
why is this check bothering you so much?
</pre>
<blockquote type="cite">
<pre wrap="" class="moz-quote-pre">
Signed-off-by: Apoorva Singh <a class="moz-txt-link-rfc2396E" href="mailto:apoorva.singh@intel.com"><apoorva.singh@intel.com></a>
---
drivers/gpu/drm/xe/xe_lrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
index 974a9cd8c379..aec7db39c061 100644
--- a/drivers/gpu/drm/xe/xe_lrc.c
+++ b/drivers/gpu/drm/xe/xe_lrc.c
@@ -1649,7 +1649,7 @@ struct xe_lrc_snapshot *xe_lrc_snapshot_capture(struct xe_lrc *lrc)
if (!snapshot)
return NULL;
- if (lrc->bo && lrc->bo->vm)
+ if (lrc->bo->vm)
xe_vm_get(lrc->bo->vm);
snapshot->context_desc = xe_lrc_ggtt_addr(lrc);
--
2.34.1
</pre>
</blockquote>
</blockquote>
</body>
</html>