On Wed, Oct 30, 2024 at 04:51:40PM +0000, Matthew Wilcox wrote:
> On Wed, Oct 30, 2024 at 10:15:13AM -0300, Jason Gunthorpe wrote:
> > Hi Matthew,
> >
> > Nicolin pointed this out and I was wondering what is the right thing.
> >
> > For instance this:
> >
> > xa_init(&xa);
> > ret = xa_reserve(&xa, 1, GFP_KERNEL);
> > printk("xa_reserve() = %d\n", ret);
> > old = xa_cmpxchg(&xa, 1, NULL, &xa, GFP_KERNEL);
>
> You're really not supposed to be doing xa_cmpxchg() here. Just use
> xa_store(). That's the intended way to use these APIs.
xa_store() also looses the XA_ZERO_ENTRY, it doesn't help to write an
assertion that the index was reserved.
> > The general purpose of code like the above is to just validate that
> > the xa has not been corrupted, that the index we are storing to has
> > been reserved. Maybe we can't sleep or something.
>
> Thr intent is to provide you with an array abstraction. You don't
> cmpxchg() pointers into an array, do you? Almost everybody just does
> array[i] = p.
Sort of, what is desired here is test and store, not cmpxchg. You
would do that in normal arrays:
if (WARN_ON(array[i] == NULL)
return;
array[i] = foo;
In xarray it would be nice to do both those under a single walk and
lock.
Jason
