<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:DengXian;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Aptos;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
@font-face
{font-family:"\@DengXian";
panose-1:2 1 6 0 3 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:12.0pt;
font-family:"Aptos",sans-serif;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:"Consolas",serif;}
span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Arial",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;
mso-ligatures:none;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="#467886" vlink="#96607D" style="word-wrap:break-word">
<p style="font-family:Calibri;font-size:10pt;color:#008000;margin:5pt;font-style:normal;font-weight:normal;text-decoration:none;" align="Left">
[Public]<br>
</p>
<br>
<div>
<div class="WordSection1">
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Koenig, Christian <Christian.Koenig@amd.com>
<br>
<b>Sent:</b> Tuesday, April 22, 2025 9:26 PM<br>
<b>To:</b> Liang, Prike <Prike.Liang@amd.com>; Yadav, Arvind <Arvind.Yadav@amd.com><br>
<b>Cc:</b> Deucher, Alexander <Alexander.Deucher@amd.com>; amd-gfx@lists.freedesktop.org; Christian König <ckoenig.leichtzumerken@gmail.com><br>
<b>Subject:</b> Re: [PATCH 4/4] drm/amdgpu: free the evf when the attached bo release<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Am 22.04.25 um 15:09 schrieb Liang, Prike:<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>Stop, wait a second. That shouldn't happen at the first place. Why is the eviction<o:p></o:p></pre>
</blockquote>
<pre>fence considered a dependency for page table updates?<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<pre>When it is added only as bookkeep then we should never consider that here.<o:p></o:p></pre>
</blockquote>
<pre>Looks like something in the sync obj is messed up.<o:p></o:p></pre>
<pre>It is like this. Here, amdgpu_sync_resv is using<o:p></o:p></pre>
<pre>DMA_RESV_USAGE_BOOKKEEP.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre> int amdgpu_sync_resv() {<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre> ..<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre> /* TODO: Use DMA_RESV_USAGE_READ here */<o:p></o:p></pre>
</blockquote>
</blockquote>
<p class="MsoNormal"><br>
That here is the core of the problem.<br>
<br>
I've added this TODO item 4 years ago to switch over to DMA_RESV_USAGE_READ here when moved all TTM use cases to using drm_sched_job_add_resv_dependencies().<br>
<br>
That was done, but this TODO here forgotten.<br>
<br>
<b><i>So here the user space submission sync only requires syncing the fences with less than DMA_RESV_USAGE_READ usage in the amdgpu_sync_resv(). If so, then the eviction fence will not be added to the sync with kernel queue submission. I can submit a patch
for this change. <o:p></o:p></i></b></p>
<p class="MsoNormal"><b><i><span style="font-size:11.0pt;font-family:"Arial",sans-serif"><o:p> </o:p></span></i></b></p>
<p class="MsoNormal"><b><i><span style="font-size:11.0pt;font-family:"Arial",sans-serif">Thanks,<o:p></o:p></span></i></b></p>
<p class="MsoNormal"><b><i><span style="font-size:11.0pt;font-family:"Arial",sans-serif">Prike</span></i></b><span style="font-size:11.0pt;font-family:"Arial",sans-serif"><o:p></o:p></span></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<pre> dma_resv_for_each_fence(&cursor, resv,<o:p></o:p></pre>
<pre>DMA_RESV_USAGE_BOOKKEEP, f) {<o:p></o:p></pre>
<pre> dma_fence_chain_for_each(f, f) {<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre> ..<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>}<o:p></o:p></pre>
<pre>during PT update amdgpu_vm_bo_update() is using sync to moving<o:p></o:p></pre>
<pre>fences(Eviction fence) before mapping anything. Because of this Eviction fence will<o:p></o:p></pre>
<pre>act as dependency.<o:p></o:p></pre>
</blockquote>
<pre><o:p> </o:p></pre>
<pre>Yes, since the amdgpu_sync_resv() uses the bookkeep usage, then all the BOs reservation fences along with the eviction fence will be returned and added to the sync.<o:p></o:p></pre>
</blockquote>
<p class="MsoNormal"><br>
Yeah, but that is incorrect.<br>
<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>And with the attached patch, the eviction fence can be released properly when the kq and uq are enabled.<o:p></o:p></pre>
</blockquote>
<p class="MsoNormal"><br>
We need to fix the underlying bug first before we can work on the next step.<br>
<br>
Regards,<br>
Christian.<br>
<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<pre><o:p> </o:p></pre>
<pre>Thanks,<o:p></o:p></pre>
<pre>Prike<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>~arvind<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>Regards,<o:p></o:p></pre>
<pre>Christian.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>, and then the eviction fence will be added as a dependent fence by<o:p></o:p></pre>
</blockquote>
</blockquote>
<pre>propagating with amdgpu_sync_push_to_job(). With removing the eviction fence<o:p></o:p></pre>
<pre>from the VM sync at amdgpu_sync_resv(), then the eviction fence can be released<o:p></o:p></pre>
<pre>properly.<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre><o:p> </o:p></pre>
<pre>Thanks,<o:p></o:p></pre>
<pre>Prike<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>Thanks,<o:p></o:p></pre>
<pre>Christian.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>PS: Please stop calling the eviction fence evf.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre> return 0;<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre> free_err:<o:p></o:p></pre>
<pre>@@ -237,6 +234,30 @@ void amdgpu_eviction_fence_detach(struct<o:p></o:p></pre>
</blockquote>
<pre>amdgpu_eviction_fence_mgr *evf_mgr,<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre> dma_fence_put(stub);<o:p></o:p></pre>
<pre> }<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre>+void amdgpu_userq_remove_all_eviction_fences(struct amdgpu_bo<o:p></o:p></pre>
<pre>+*bo)<o:p></o:p></pre>
</blockquote>
<pre>Please name that amdgpu_eviction_fence_remove_all().<o:p></o:p></pre>
</blockquote>
<pre>Noted.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>Regards,<o:p></o:p></pre>
<pre>Christian.<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>+{<o:p></o:p></pre>
<pre>+ struct dma_resv *resv = &bo->tbo.base._resv;<o:p></o:p></pre>
<pre>+ struct dma_fence *fence, *stub;<o:p></o:p></pre>
<pre>+ struct dma_resv_iter cursor;<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre>+ dma_resv_assert_held(resv);<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre>+ stub = dma_fence_get_stub();<o:p></o:p></pre>
<pre>+ dma_resv_for_each_fence(&cursor, resv,<o:p></o:p></pre>
</blockquote>
<pre>DMA_RESV_USAGE_BOOKKEEP, fence) {<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre>+ struct amdgpu_eviction_fence *ev_fence;<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre>+ ev_fence = fence_to_evf(fence);<o:p></o:p></pre>
<pre>+ if (!ev_fence || !dma_fence_is_signaled(&ev_fence->base))<o:p></o:p></pre>
<pre>+ continue;<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre>+ dma_resv_replace_fences(resv, fence->context, stub,<o:p></o:p></pre>
<pre>+ DMA_RESV_USAGE_BOOKKEEP);<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre>+ }<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre>+ dma_fence_put(stub);<o:p></o:p></pre>
<pre>+}<o:p></o:p></pre>
<pre>+<o:p></o:p></pre>
<pre> int amdgpu_eviction_fence_init(struct amdgpu_eviction_fence_mgr<o:p></o:p></pre>
<pre>*evf_mgr) {<o:p></o:p></pre>
<pre> /* This needs to be done one time per open */ diff --git<o:p></o:p></pre>
<pre>a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h<o:p></o:p></pre>
<pre>b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h<o:p></o:p></pre>
<pre>index fcd867b7147d..da99ac322a2e 100644<o:p></o:p></pre>
<pre>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h<o:p></o:p></pre>
<pre>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h<o:p></o:p></pre>
<pre>@@ -66,4 +66,5 @@ amdgpu_eviction_fence_signal(struct<o:p></o:p></pre>
<pre>amdgpu_eviction_fence_mgr *evf_mgr, int<o:p></o:p></pre>
<pre>amdgpu_eviction_fence_replace_fence(struct<o:p></o:p></pre>
<pre>amdgpu_eviction_fence_mgr<o:p></o:p></pre>
</blockquote>
<pre>*evf_mgr,<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre> struct drm_exec *exec);<o:p></o:p></pre>
<pre>+void amdgpu_userq_remove_all_eviction_fences(struct amdgpu_bo<o:p></o:p></pre>
<pre>+*bo);<o:p></o:p></pre>
<pre> #endif<o:p></o:p></pre>
<pre>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c<o:p></o:p></pre>
<pre>b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c<o:p></o:p></pre>
<pre>index 1e73ce30d4d7..f001018a01eb 100644<o:p></o:p></pre>
<pre>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c<o:p></o:p></pre>
<pre>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c<o:p></o:p></pre>
<pre>@@ -1392,6 +1392,7 @@ void amdgpu_bo_release_notify(struct<o:p></o:p></pre>
</blockquote>
<pre>ttm_buffer_object *bo)<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre> amdgpu_vram_mgr_set_cleared(bo->resource);<o:p></o:p></pre>
<pre> dma_resv_add_fence(&bo->base._resv, fence,<o:p></o:p></pre>
</blockquote>
<pre>DMA_RESV_USAGE_KERNEL);<o:p></o:p></pre>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<pre> dma_fence_put(fence);<o:p></o:p></pre>
<pre>+ amdgpu_userq_remove_all_eviction_fences(abo);<o:p></o:p></pre>
<pre><o:p> </o:p></pre>
<pre> out:<o:p></o:p></pre>
<pre> dma_resv_unlock(&bo->base._resv);<o:p></o:p></pre>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
</blockquote>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</body>
</html>