[PATCH 1/4] drm/sched: optimize drm_sched_job_add_dependency a bit

[Tvrtko Ursulin]

On 22/05/2025 17:19, Christian König wrote:
> On 5/22/25 16:27, Tvrtko Ursulin wrote:
>>
>> On 22/05/2025 14:41, Christian König wrote:
>>> Since we already iterated over the xarray we know at which index the new
>>> entry should be stored. So instead of using xa_alloc use xa_store and
>>> write into the index directly.
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> ---
>>>    drivers/gpu/drm/scheduler/sched_main.c | 12 ++++++------
>>>    1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
>>> index f7118497e47a..d2d64bf17c96 100644
>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>> @@ -871,10 +871,8 @@ EXPORT_SYMBOL(drm_sched_job_arm);
>>>    int drm_sched_job_add_dependency(struct drm_sched_job *job,
>>>                     struct dma_fence *fence)
>>>    {
>>> +    unsigned long index = -1;
>>>        struct dma_fence *entry;
>>> -    unsigned long index;
>>> -    u32 id = 0;
>>> -    int ret;
>>>          if (!fence)
>>>            return 0;
>>> @@ -896,11 +894,13 @@ int drm_sched_job_add_dependency(struct drm_sched_job *job,
>>>            return 0;
>>>        }
>>>    -    ret = xa_alloc(&job->dependencies, &id, fence, xa_limit_32b, GFP_KERNEL);
>>> -    if (ret != 0)
>>> +    entry = xa_store(&job->dependencies, index + 1, fence, GFP_KERNEL);
>>
>>  From the code it looks index does not "move" for NULL slots?
> 
> Correct, but I just found out that the macro initializes index to zero, so that approach also doesn't work.
> 
> *sigh* going to look into this again tomorrow. It looks like this use case is somehow not well supported at all by xarray.

Back to the stub fence and __dma_fence_is_signaled()? ;)

Regards,

Tvrtko

>> That is, if someone:
>>
>> 1) Preallocates one entry, when trying to populate it index will be -1 after xa_for_each?
>>
>> 2) Add one, preallocate one, then add one more - index will be 0 after xa_for_each?
>>
>> Regards,
>>
>> Tvrtko
>>
>>> +    if (xa_is_err(entry))
>>>            dma_fence_put(fence);
>>> +    else
>>> +        WARN_ON(entry);
>>>    -    return ret;
>>> +    return xa_err(entry);
>>>    }
>>>    EXPORT_SYMBOL(drm_sched_job_add_dependency);
>>>    
>>
>