On Mon, Feb 04, 2019 at 09:24:49PM -0700, Jason Gunthorpe wrote:
> > > +#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?
Yes, we could use an xa_err value, but all the users we had actively
didn't want to see the NULL values ... so that was the interface
provided ;-)
> > 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.
Fair. The "Advanced" (I should have called it Plumbing) API is less
well-documented. If you were going to use the client_data then you'd
theoretically need to care about retry values, but seeing a retry value
means "There was something at this entry, but it has now moved".
You don't care -- all you care about is there was something stored
at this index.
> > 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.
Cool. Just remember to call xas_reset() if you drop the rcu read lock
and continue the loop.
