Re: [PATCH rdma-next 5/8] RDMA/core: Provide locked variant of device name to index function

Leon Romanovsky
2017-12-28T05:01:59Z

[-- Attachment #1: Type: text/plain, Size: 2047 bytes --]

On Wed, Dec 27, 2017 at 04:02:27PM -0700, Jason Gunthorpe wrote:
> On Sun, Dec 24, 2017 at 03:43:25PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > Add self-contained with locks device name to index function.
> >
> > Reviewed-by: Mark Bloch <markb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > drivers/infiniband/core/core_priv.h | 1 +
> > drivers/infiniband/core/device.c | 16 ++++++++++++++++
> > 2 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h
> > index ded3850721e0..e71dd1814bf0 100644
> > +++ b/drivers/infiniband/core/core_priv.h
> > @@ -301,6 +301,7 @@ static inline int ib_mad_enforce_security(struct ib_mad_agent_private *map,
> > #endif
> >
> > struct ib_device *__ib_device_get_by_index(u32 ifindex);
> > +struct ib_device *ib_device_get_by_index(u32 ifindex);
> > /* RDMA device netlink */
> > void nldev_init(void);
> > void nldev_exit(void);
> > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > index cb69357a1909..adf3a4ca038b 100644
> > +++ b/drivers/infiniband/core/device.c
> > @@ -150,6 +150,22 @@ struct ib_device *__ib_device_get_by_index(u32 index)
> > return NULL;
> > }
> >
> > +/*
> > + * Caller is responsible to return refrerence count by calling put_device()
> > + */
> > +struct ib_device *ib_device_get_by_index(u32 index)
> > +{
> > + struct ib_device *device;
> > +
> > + down_write(&lists_rwsem);
> > + device = __ib_device_get_by_index(index);
> > + if (device)
> > + get_device(&device->dev);
> > +
> > + up_write(&lists_rwsem);
> > + return device;
> > +}
>
> If you hold the write side of lists_rwsem you must also hold
> device_mutex.
>
> But the write side is only needed if the code mutates the device_list
> or client list, and this does neither, so it should be the read lock, right?

Right, I'll fix.

Thanks

>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]