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

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- 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 --]