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.koe...@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,
>
