On 5/15/25 11:05, Philipp Stanner wrote:
> On Thu, 2025-05-15 at 10:48 +0200, Christian König wrote:
>> Explicitly adding the scheduler maintainers.
>>
>> On 5/15/25 04:07, Lin.Cao wrote:
>>> Previously we only signaled finished fence which may cause some
>>> submission's dependency cannot be cleared the cause benchmark hang.
>>> Signal both scheduled fence and finished fence could fix this
>>> issue.
>
> Code seems legit to me; but be so kind and also pimp up the commit
> message a bit, Christian. It's not very clear what the bug is and why
> setting the parent to NULL solves it. Or is the issue simply that the
> fence might be dropped unsignaled, being a bug by definition? Needs to
> be written down.
The later, we simply forgot to signal the scheduled fence when an application
was killed.
> Grammar is also a bit too broken.
>
> And running the unit tests before pushing is probably also a good idea.
And maybe even writing a new unit test for that.
Regards,
Christian.
>
>>>
>>> Signed-off-by: Lin.Cao <linca...@amd.com>
>
> Acked-by: Philipp Stanner <pha...@kernel.org>
>
>>
>> Reviewed-by: Christian König <christian.koe...@amd.com>
>>
>> Danilo & Philipp can we quickly get an rb for that? I'm volunteering
>> to push it to drm-misc-fixes and add the necessary stable tags since
>> this is a fix for a rather ugly bug.
>>
>> Regards,
>> Christian.
>>
>>
>>> ---
>>> drivers/gpu/drm/scheduler/sched_entity.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
>>> b/drivers/gpu/drm/scheduler/sched_entity.c
>>> index bd39db7bb240..e671aa241720 100644
>>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>>> @@ -176,6 +176,7 @@ static void
>>> drm_sched_entity_kill_jobs_work(struct work_struct *wrk)
>>> {
>>> struct drm_sched_job *job = container_of(wrk,
>>> typeof(*job), work);
>>>
>>> + drm_sched_fence_scheduled(job->s_fence, NULL);
>>> drm_sched_fence_finished(job->s_fence, -ESRCH);
>>> WARN_ON(job->s_fence->parent);
>>> job->sched->ops->free_job(job);
>>
>
