Re: [PATCH v3 1/2] drm/sched: Avoid job cleanup if sched thread is parked.

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 25.11.19 um 17:51 schrieb Steven Price:
> On 25/11/2019 14:10, Andrey Grodzovsky wrote:
>> When the sched thread is parked we assume ring_mirror_list is
>> not accessed from here.
> FWIW I don't think this is necessary. kthread_park() will wait until the
> thread is parked, at which point the thread is stuck in kthread_parkme()
> until unparked.
>
> So all this does is avoid waiting for any cleanup jobs before parking -
> which might be a reasonable goal in itself, but if so lets at least
> document that.

Now that you mention it that is indeed wrong.

The real problem is that in the main thread we mangled the call to 
kthread_parkme() into drm_sched_blocked() which can be called in atomic 
context.

I suggest to rework this so that the kthread_should_park() and 
kthread_should_stop() test in wait_event_interruptible() come first and 
then call kthread_parkme() outside of the wait_event_interruptible().

Regards,
Christian.

>
> Steve
>
>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>> Reviewed-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/scheduler/sched_main.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
>> index d4cc728..6774955 100644
>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>> @@ -635,9 +635,13 @@ drm_sched_get_cleanup_job(struct drm_gpu_scheduler *sched)
>>   	struct drm_sched_job *job;
>>   	unsigned long flags;
>>   
>> -	/* Don't destroy jobs while the timeout worker is running */
>> -	if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>> -	    !cancel_delayed_work(&sched->work_tdr))
>> +	/*
>> +	* Don't destroy jobs while the timeout worker is running  OR thread
>> +	* is being parked and hence assumed to not touch ring_mirror_list
>> +	*/
>> +	if ((sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>> +	     !cancel_delayed_work(&sched->work_tdr)) ||
>> +	     __kthread_should_park(sched->thread))
>>   		return NULL;
>>   
>>   	spin_lock_irqsave(&sched->job_list_lock, flags);
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel