[PATCH 1/3] drm/sched: add drm_sched_prealloc_dependency_slots v3

[Christian König]

On 5/15/25 18:17, Tvrtko Ursulin wrote:
> 
> On 15/05/2025 16:00, Christian König wrote:
>> Sometimes drivers need to be able to submit multiple jobs which depend on
>> each other to different schedulers at the same time, but using
>> drm_sched_job_add_dependency() can't fail any more after the first job is
>> initialized.
>>
>> This function preallocate memory for dependency slots so that no ENOMEM
>> can come later while adding dependencies.
>>
>> v2: rework implementation an documentation
>> v3: rework from scratch, use separate function to add preallocated deps
>>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>>   drivers/gpu/drm/scheduler/sched_main.c | 45 ++++++++++++++++++++++++++
>>   include/drm/gpu_scheduler.h            |  4 +++
>>   2 files changed, 49 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
>> index f7118497e47a..b95e7089aa70 100644
>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>> @@ -858,6 +858,51 @@ void drm_sched_job_arm(struct drm_sched_job *job)
>>   }
>>   EXPORT_SYMBOL(drm_sched_job_arm);
>>   +/**
>> + * drm_sched_job_prealloc_dependency_slot - avoid ENOMEM on adding dependencies
>> + * @job: scheduler job where dependencies will be added
>> + * @id: id for the allocated slot
>> +  *
>> + * Sometimes drivers need to be able to submit multiple jobs which depend on
>> + * each other to different schedulers at the same time, but using
>> + * drm_sched_job_add_dependency() can't fail any more after the first job is
>> + * initialized.
>> + *
>> + * This function preallocate memory for a dependency slot so that no ENOMEM can
>> + * come later while adding dependencies. The index of the preallocated slot is
>> + * returned in @id.
>> + *
>> + * Return:
>> + * 0 on success, or an error on failing to expand the array.
>> + */
>> +int drm_sched_job_prealloc_dependency_slot(struct drm_sched_job *job,
>> +                       u32 *id)
>> +{
>> +    return xa_alloc(&job->dependencies, id, NULL, xa_limit_32b, GFP_KERNEL);
>> +}
>> +EXPORT_SYMBOL(drm_sched_job_prealloc_dependency_slot);
>> +
>> +/**
>> + * drm_sched_job_add_prealloc_dep - add dependency to preallocated slot
>> + * @job: scheduler job where dependencies will be added
>> + * @id: the preallocated slot index
>> + * @fence: the dependency to add
>> + *
>> + * Consumes @fence and adds it to the preallocated slot dependency.
>> + */
>> +void drm_sched_job_add_prealloc_dep(struct drm_sched_job *job, u32 id,
>> +                    struct dma_fence *fence)
>> +{
>> +    fence = xa_store(&job->dependencies, id, fence, GFP_ATOMIC);
> 
> Add assert that the passed id exists (was preallocated) and is NULL?

That is partially done by the WARN_ON below, but I couldn't find a way to check if the slot was pre-allocated.

> Also, if someone preallocates and does not consume the slot will that confuse the iteration in drm_sched_job_dependency()?

No, slots with NULL in them are skipped by xa_for_each(). I will add a comment explaining that.

Thanks,
Christian.

> 
> Regards,
> 
> Tvrtko
> 
>> +    /*
>> +     * Be defensive just in case driver messed it up and used preallocated
>> +     * slot twice.
>> +     */
>> +    if (WARN_ON(fence))
>> +        dma_fence_put(fence);
>> +}
>> +EXPORT_SYMBOL(drm_sched_job_add_prealloc_dep);
>> +
>>   /**
>>    * drm_sched_job_add_dependency - adds the fence as a job dependency
>>    * @job: scheduler job to add the dependencies to
>> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
>> index d860db087ea5..0286e0934317 100644
>> --- a/include/drm/gpu_scheduler.h
>> +++ b/include/drm/gpu_scheduler.h
>> @@ -632,6 +632,10 @@ int drm_sched_job_init(struct drm_sched_job *job,
>>                  u32 credits, void *owner);
>>   void drm_sched_job_arm(struct drm_sched_job *job);
>>   void drm_sched_entity_push_job(struct drm_sched_job *sched_job);
>> +int drm_sched_job_prealloc_dependency_slot(struct drm_sched_job *job,
>> +                       u32 *id);
>> +void drm_sched_job_add_prealloc_dep(struct drm_sched_job *job, u32 id,
>> +                    struct dma_fence *fence);
>>   int drm_sched_job_add_dependency(struct drm_sched_job *job,
>>                    struct dma_fence *fence);
>>   int drm_sched_job_add_syncobj_dependency(struct drm_sched_job *job,
>