David any objections that I take this patch and make the necessary 
modifications?

People are pinging me about the problem.

Regards,
Christian.

On 09.10.25 17:04, Christian König wrote:
> FYI
> 
> On 09.10.25 09:01, Christian König wrote:
>> On 02.10.25 17:05, David Rosca wrote:
>>> drm_sched_entity_flush should only kill the entity if the current
>>> process is the last user of the entity. The last_user is only set
>>> when adding new job, so entities that had no jobs submitted to them
>>> have NULL last_user and would always be killed.
>>> Another issue is setting last_user to NULL from drm_sched_entity_flush,
>>> which causes subsequent calls to kill the entity.
>>>
>>> Signed-off-by: David Rosca <[email protected]>
>>> Fixes: 51564e9f06f0 ("drm/amdgpu: Avoid extra evict-restore process.")
>>
>> Good catch, but in general please CC the relevant maintainers and mailing 
>> lists for scheduler patches.
>>
>>> ---
>>>  drivers/gpu/drm/scheduler/sched_entity.c | 5 ++---
>>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c 
>>> b/drivers/gpu/drm/scheduler/sched_entity.c
>>> index 8867b95ab089..a325e4a59990 100644
>>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>>> @@ -70,6 +70,7 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
>>>     entity->guilty = guilty;
>>>     entity->num_sched_list = num_sched_list;
>>>     entity->priority = priority;
>>> +   entity->last_user = current->group_leader;
>>>     /*
>>>      * It's perfectly valid to initialize an entity without having a valid
>>>      * scheduler attached. It's just not valid to use the scheduler before 
>>> it
>>> @@ -278,7 +279,6 @@ static void drm_sched_entity_kill(struct 
>>> drm_sched_entity *entity)
>>>  long drm_sched_entity_flush(struct drm_sched_entity *entity, long timeout)
>>>  {
>>>     struct drm_gpu_scheduler *sched;
>>> -   struct task_struct *last_user;
>>>     long ret = timeout;
>>>  
>>>     if (!entity->rq)
>>> @@ -301,8 +301,7 @@ long drm_sched_entity_flush(struct drm_sched_entity 
>>> *entity, long timeout)
>>>     }
>>>  
>>>     /* For killed process disable any more IBs enqueue right now */
>>> -   last_user = cmpxchg(&entity->last_user, current->group_leader, NULL);
>>> -   if ((!last_user || last_user == current->group_leader) &&
>>> +   if (entity->last_user == current->group_leader &&
>>
>> You still need the cmpxchg() here or otherwise drm_sched_entity_kill() would 
>> run multiple times.
>>
>> Regards,
>> Christian.
>>
>>>         (current->flags & PF_EXITING) && (current->exit_code == SIGKILL))
>>>             drm_sched_entity_kill(entity);
>>>  
>>
> 

Reply via email to