On Mon, Feb 04, 2019 at 10:15:20PM -0700, Jason Gunthorpe wrote:
> > > > 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.
>
> xas_reset? Did you mean xas_pause?
>
> I guess the full version looks like this:
>
> down_read(&dev->client_data_rwsem);
> rcu_read_lock();
> xas_for_each_marked(&xas, client_data, ULONG_MAX,
> CLIENT_DATA_REGISTERED) {
> struct ib_client *client;
>
> if (!xa_is_zero(entry) && xas_retry(&xas, client_data))
> continue;
Rather looks like this needs to check for XA_FREE_MARK too...
> client = xa_load(&clients, xas.xa_index);
> if (!client || !client->get_net_dev_by_params)
> continue;
>
> xas_pause();
> rcu_read_unlock();
>
> net_dev = client->get_net_dev_by_params(dev, port, pkey, gid,
> addr, client_data);
> rcu_read_lock();
> if (net_dev)
> break;
>
> }
> rcu_read_unlock();
> up_read(&dev->client_data_rwsem);
>
> Doesn't do much for readability :)
Ah.. Unless you feel strongly I think I'll stick with just changing
the macro version to use xa_err .. this becomes too much confounding
stuff in what should have been a simple function.
/*
* xarray has this 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_marked(struct xarray *xa, unsigned long *indexp,
xa_mark_t filter)
{
XA_STATE(xas, xa, *indexp);
void *entry;
rcu_read_lock();
do {
entry = xas_find_marked(&xas, ULONG_MAX, filter);
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 XA_ERROR(-ENOENT);
}
#define xan_for_each_marked(xa, index, entry, filter) \
for (index = 0, entry = xan_find_marked(xa, &(index), filter); \
!xa_is_err(entry); \
(index)++, entry = xan_find_marked(xa, &(index), filter))
Thanks,
Jason
