On Sun, Sep 13, 2020 at 11:02:33AM +0300, Leon Romanovsky wrote:
> On Fri, Sep 11, 2020 at 04:52:21PM -0300, Jason Gunthorpe wrote:
> > On Thu, Sep 10, 2020 at 05:22:03PM +0300, Leon Romanovsky wrote:
> > > From: Avihai Horon <avihaih@xxxxxxxxxx>
> > >
> > > Introduce rdma_query_gid_table which enables querying all the GID tables
> > > of a given device and copying the attributes of all valid GID entries to
> > > a provided buffer.
> > >
> > > This API provides a faster way to query a GID table using single call and
> > > will be used in libibverbs to improve current approach that requires
> > > multiple calls to open, close and read multiple sysfs files for a single
> > > GID table entry.
> > >
> > > Signed-off-by: Avihai Horon <avihaih@xxxxxxxxxx>
> > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
> > > drivers/infiniband/core/cache.c | 93 +++++++++++++++++++++++++
> > > include/rdma/ib_cache.h | 5 ++
> > > include/uapi/rdma/ib_user_ioctl_verbs.h | 8 +++
> > > 3 files changed, 106 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> > > index cf49ac0b0aa6..175e229eccd3 100644
> > > +++ b/drivers/infiniband/core/cache.c
> > > @@ -1247,6 +1247,99 @@ rdma_get_gid_attr(struct ib_device *device, u8 port_num, int index)
> > > }
> > > EXPORT_SYMBOL(rdma_get_gid_attr);
> > >
> > > +/**
> > > + * rdma_get_ndev_ifindex - Reads ndev ifindex of the given gid attr.
> > > + * @gid_attr: Pointer to the GID attribute.
> > > + * @ndev_ifindex: Pointer through which the ndev ifindex is returned.
> > > + *
> > > + * Returns 0 on success or appropriate error code. The netdevice must be in UP
> > > + * state.
> > > + */
> > > +int rdma_get_ndev_ifindex(const struct ib_gid_attr *gid_attr, u32 *ndev_ifindex)
> > > +{
> > > + struct net_device *ndev;
> > > + int ret = 0;
> > > +
> > > + if (rdma_protocol_ib(gid_attr->device, gid_attr->port_num)) {
> > > + *ndev_ifindex = 0;
> > > + return 0;
> > > + }
> > > +
> > > + rcu_read_lock();
> > > + ndev = rcu_dereference(gid_attr->ndev);
> > > + if (!ndev || (READ_ONCE(ndev->flags) & IFF_UP) == 0) {
> > > + ret = -ENODEV;
> > > + goto out;
> > > + }
> >
> > None of this is necessary to read the ifindex, especially since the
> > read_lock is being held.
>
> I see same rcu_read_lock->rcu_dereference->rcu_read_unlock pattern in
> rdma_read_gid_l2_fields(), why this function is different?
It doesn't hold the read_lock so it is using rcu to access
attr->ndev. With the read_lock held attr->ndev is stable and none of
this is needed.
Jason
