<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<p style="font-family:Calibri;font-size:10pt;color:#0000FF;margin:5pt;font-style:normal;font-weight:normal;text-decoration:none;" align="Left">
[AMD Official Use Only - AMD Internal Distribution Only]<br>
</p>
<br>
<div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
Hi Philipp,</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
I updated the commit message to better clarify the issue. Would you mind reviewing if this revised description adequately explains the bug and the rationale behind the fix?</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="appendonsend" style="color: inherit;"></div>
<div class="elementToProof" style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
When an entity from application B is killed, drm_sched_entity_kill() </div>
<div class="elementToProof" style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
removes all jobs belonging to that entity through </div>
<div class="elementToProof" style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
drm_sched_entity_kill_jobs_work(). If application A's job depends on a </div>
<div class="elementToProof" style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
scheduled fence from application B's job, and that fence is not properly </div>
<div class="elementToProof" style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
signaled during the killing process, application A's dependency cannot be </div>
<div class="elementToProof" style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
cleared. </div>
<div class="elementToProof" style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
</div>
<div class="elementToProof" style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
This leads to application A hanging indefinitely while waiting for a </div>
<div class="elementToProof" style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
dependency that will never be resolved. Fix this issue by ensuring that </div>
<div class="elementToProof" style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
scheduled fences are properly signaled when an entity is killed, allowing </div>
<div style="margin-left: 40px; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
dependent applications to continue execution.</div>
<div class="elementToProof"><br>
</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
Thanks,</div>
<div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
Lin</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<hr style="display: inline-block; width: 98%;">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<b>From:</b> Koenig, Christian <Christian.Koenig@amd.com><br>
<b>Sent:</b> Thursday, May 15, 2025 20:39<br>
<b>To:</b> phasta@kernel.org <phasta@kernel.org>; cao, lin <lin.cao@amd.com>; dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>; aamd-gfx@lists.freedesktop.org <aamd-gfx@lists.freedesktop.org><br>
<b>Cc:</b> Chang, HaiJun <HaiJun.Chang@amd.com>; Yin, ZhenGuo (Chris) <ZhenGuo.Yin@amd.com>; Danilo Krummrich <dakr@kernel.org><br>
<b>Subject:</b> Re: [PATCH] drm/scheduler: signal scheduled fence when kill job </div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-size: 11pt;">On 5/15/25 11:05, Philipp Stanner wrote:<br>
> On Thu, 2025-05-15 at 10:48 +0200, Christian König wrote:<br>
>> Explicitly adding the scheduler maintainers.<br>
>><br>
>> On 5/15/25 04:07, Lin.Cao wrote:<br>
>>> Previously we only signaled finished fence which may cause some<br>
>>> submission's dependency cannot be cleared the cause benchmark hang.<br>
>>> Signal both scheduled fence and finished fence could fix this<br>
>>> issue.<br>
><br>
> Code seems legit to me; but be so kind and also pimp up the commit<br>
> message a bit, Christian. It's not very clear what the bug is and why<br>
> setting the parent to NULL solves it. Or is the issue simply that the<br>
> fence might be dropped unsignaled, being a bug by definition? Needs to<br>
> be written down.<br>
<br>
The later, we simply forgot to signal the scheduled fence when an application was killed.<br>
<br>
> Grammar is also a bit too broken.<br>
><br>
> And running the unit tests before pushing is probably also a good idea.<br>
<br>
And maybe even writing a new unit test for that.<br>
<br>
Regards,<br>
Christian.<br>
<br>
><br>
>>><br>
>>> Signed-off-by: Lin.Cao <lincao12@amd.com><br>
><br>
> Acked-by: Philipp Stanner <phasta@kernel.org><br>
><br>
>><br>
>> Reviewed-by: Christian König <christian.koenig@amd.com><br>
>><br>
>> Danilo & Philipp can we quickly get an rb for that? I'm volunteering<br>
>> to push it to drm-misc-fixes and add the necessary stable tags since<br>
>> this is a fix for a rather ugly bug.<br>
>><br>
>> Regards,<br>
>> Christian.<br>
>><br>
>><br>
>>> ---<br>
>>> drivers/gpu/drm/scheduler/sched_entity.c | 1 +<br>
>>> 1 file changed, 1 insertion(+)<br>
>>><br>
>>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c<br>
>>> b/drivers/gpu/drm/scheduler/sched_entity.c<br>
>>> index bd39db7bb240..e671aa241720 100644<br>
>>> --- a/drivers/gpu/drm/scheduler/sched_entity.c<br>
>>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c<br>
>>> @@ -176,6 +176,7 @@ static void<br>
>>> drm_sched_entity_kill_jobs_work(struct work_struct *wrk)<br>
>>> {<br>
>>> struct drm_sched_job *job = container_of(wrk,<br>
>>> typeof(*job), work);<br>
>>> <br>
>>> + drm_sched_fence_scheduled(job->s_fence, NULL);<br>
>>> drm_sched_fence_finished(job->s_fence, -ESRCH);<br>
>>> WARN_ON(job->s_fence->parent);<br>
>>> job->sched->ops->free_job(job);<br>
>><br>
><br>
<br>
</div>
</div>
</body>
</html>