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

[Philipp Stanner]

+Cc Matthew, again :)

On Thu, 2025-05-22 at 18:19 +0200, 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.

@Matthew, would be really nice if you could give some insights to that
and maybe give advice on a path how to best do that with xarray in a
canonical way.


Thanks,
P.


> 
> Regards,
> Christian.
> 
> > 
> > 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);
> > >   
> > 
>