[RFC 03/18] drm/sched: Remove one local variable

[Tvrtko Ursulin]

On 09/01/2025 12:49, Christian König wrote:
> Am 08.01.25 um 19:35 schrieb Tvrtko Ursulin:
>> It is not helping readability nor it is required to keep the line length
>> in check.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
>> Cc: Christian König <christian.koenig at amd.com>
>> Cc: Danilo Krummrich <dakr at redhat.com>
>> Cc: Matthew Brost <matthew.brost at intel.com>
>> Cc: Philipp Stanner <pstanner at redhat.com>
>> ---
>>   drivers/gpu/drm/scheduler/sched_main.c | 5 +----
>>   1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c 
>> b/drivers/gpu/drm/scheduler/sched_main.c
>> index 1734c17aeea5..01e0d6e686d1 100644
>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>> @@ -1175,7 +1175,6 @@ static void drm_sched_run_job_work(struct 
>> work_struct *w)
>>           container_of(w, struct drm_gpu_scheduler, work_run_job);
>>       struct drm_sched_entity *entity;
>>       struct dma_fence *fence;
>> -    struct drm_sched_fence *s_fence;
>>       struct drm_sched_job *sched_job;
>>       int r;
>> @@ -1194,15 +1193,13 @@ static void drm_sched_run_job_work(struct 
>> work_struct *w)
>>           return;
>>       }
>> -    s_fence = sched_job->s_fence;
>> -
>>       atomic_add(sched_job->credits, &sched->credit_count);
>>       drm_sched_job_begin(sched_job);
>>       trace_drm_run_job(sched_job, entity);
>>       fence = sched->ops->run_job(sched_job);
>>       complete_all(&entity->entity_idle);
>> -    drm_sched_fence_scheduled(s_fence, fence);
>> +    drm_sched_fence_scheduled(sched_job->s_fence, fence);
> 
> Originally that was not for readability but for correctness.
> 
> As soon as complete_all(&entity->entity_idle); was called the sched_job 
> could have been released.

And without a comment ouch.

> But we changed that so that the sched_job is released from a separate 
> worker instead, so that is most likely not necessary any more.

Very subtle. Especially given some drivers use unordered queue.

And for them sched_job is dereferenced a few more times in the block 
below so not sure how it is safe.

Move complete_all() to the end of it all?

Regards,

Tvrtko

>>       if (!IS_ERR_OR_NULL(fence)) {
>>           /* Drop for original kref_init of the fence */
>