Re: [PATCH drm-misc-next v6] drm/sched: implement dynamic job-flow control

[Luben Tuikov]

On 2023-11-09 19:57, Luben Tuikov wrote:
> On 2023-11-09 19:16, Danilo Krummrich wrote:
[snip]
>> @@ -667,6 +771,8 @@ EXPORT_SYMBOL(drm_sched_resubmit_jobs);
>>   * drm_sched_job_init - init a scheduler job
>>   * @job: scheduler job to init
>>   * @entity: scheduler entity to use
>> + * @credits: the number of credits this job contributes to the schedulers
>> + * credit limit
>>   * @owner: job owner for debugging
>>   *
>>   * Refer to drm_sched_entity_push_job() documentation
>> @@ -684,7 +790,7 @@ EXPORT_SYMBOL(drm_sched_resubmit_jobs);
>>   */
>>  int drm_sched_job_init(struct drm_sched_job *job,
>>  		       struct drm_sched_entity *entity,
>> -		       void *owner)
>> +		       u32 credits, void *owner)
>>  {
>>  	if (!entity->rq) {
>>  		/* This will most likely be followed by missing frames
>> @@ -695,7 +801,11 @@ int drm_sched_job_init(struct drm_sched_job *job,
>>  		return -ENOENT;
>>  	}
>>  
>> +	if (unlikely(!credits))
>> +		return -EINVAL;
>> +
> 
> This will most likely result in bad user experience (read: blank screen),
> and debugging this would be really hard without something to go by
> in the kernel log.
> 
> (This was exactly the case with amdgpu when 56e449603f0ac5
> ("drm/sched: Convert the GPU scheduler to variable number of run-queues") 
> was being worked on and merged. Without the drm_err() on missing rq in
> the lines immediately before the hunk above returning -ENOENT, there
> was no indication why setting up an fb was failing very early on (blank screen).)
> 
> So it is best to print a "[drm] *ERROR* "-equivalent string in the logs,
> so that we can make a note of this, without relying on drivers, old and new, logging
> that drm_sched_job_init() failed.

If you add _exactly_ this,

	if (unlikely(!credits)) {
		pr_err("*ERROR* %s: credits cannot be 0!\n", __func__)
		return -EINVAL;
	}

You can add my,

Reviewed-by: Luben Tuikov <ltuikov89@xxxxxxxxx>

and push it.
-- 
Regards,
Luben

Attachment: OpenPGP_0x4C15479431A334AF.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature