On Mon, Feb 04, 2019 at 04:24:01PM -0800, Matthew Wilcox wrote:
> On Mon, Feb 04, 2019 at 02:17:50PM -0700, Jason Gunthorpe wrote:
> > +/*
> > + * xarray has this annoying behavior where it won't iterate over NULL values
> > + * stored in allocated arrays. So we need our own iterator to see all values
> > + * stored in the array. This does the same thing as xa_for_each except that it
> > + * also returns NULL valued entries if the array is allocating. Simplified to
> > + * only work on simple xarrays.
> > + */
> > +static void *xan_find(struct xarray *xa, unsigned long *indexp,
> > + unsigned long max, xa_mark_t filter)
> > +{
> > + XA_STATE(xas, xa, *indexp);
> > + void *entry;
> > +
> > + rcu_read_lock();
> > + do {
> > + if ((__force unsigned int)filter < XA_MAX_MARKS)
> > + entry = xas_find_marked(&xas, max, filter);
> > + else
> > + entry = xas_find(&xas, max);
> > + if (xa_is_zero(entry) && !xas_get_mark(&xas, XA_FREE_MARK))
> > + break;
> > + } while (xas_retry(&xas, entry));
> > + rcu_read_unlock();
> > +
> > + if (entry) {
> > + *indexp = xas.xa_index;
> > + if (xa_is_zero(entry))
> > + return NULL;
> > + return entry;
> > + }
> > + return ERR_PTR(-ENOENT);
> > +}
> > +#define xan_for_each(xa, index, entry, filter) \
> > + for (index = 0, entry = xan_find(xa, &(index), ULONG_MAX, filter); \
> > + !IS_ERR(entry); \
> > + (index)++, entry = xan_find(xa, &(index), ULONG_MAX, filter))
>
> We can't do this in the generic XArray code because ERR_PTR(-ENOENT) might
> overlap with an XArray value entry.
Oh, I guess it could overlap with a user value here in some bizzaro
case.. Maybe xa_err would be better?
Ah, but earlier versios had more users of this, so open coding
xas_for_each_marked is not so bad now that there is only one, I'll
just do that instead.
> Now, what you can do is iterate using xas_for_each(). That will return the
> zeroed entries.
>
> XA_STATE(xas, &dev->client_data, 0);
> ...
> rcu_read_lock();
> xas_for_each_marked(&xas, client_data, ULONG_MAX,
> CLIENT_DATA_REGISTERED) {
> struct ib_client *client = xa_load(&clients, xas.xa_index);
> ...
Is this all that is needed? Do I have to worry about XA_FREE_MARK or
xas_retry()? It wasn't totally clear to me from the docs what was
required to use xas_for_each.
> I'm not familiar enough with your code to figure out where you'd want
> to rcu_read_unlock() :-)
Hum, it barely matters, but just after the if would be fine and
slightly fewer pauses. This thing only has about 5 entries in it. I
often wonder why it was made dynamic at all.
Jason
