On Tue, Mar 3, 2020 at 10:57 AM Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> On Fri, Feb 21, 2020 at 11:47:00AM +0100, Jack Wang wrote:
> > From: Jack Wang <jinpu.wang@xxxxxxxxxxxxxxx>
> >
> > This is a set of library functions existing as a rtrs-core module,
> > used by client and server modules.
> >
> > Mainly these functions wrap IB and RDMA calls and provide a bit higher
> > abstraction for implementing of RTRS protocol on client or server
> > sides.
> >
> > Signed-off-by: Danil Kipnis <danil.kipnis@xxxxxxxxxxxxxxx>
> > Signed-off-by: Jack Wang <jinpu.wang@xxxxxxxxxxxxxxx>
> > ---
> > drivers/infiniband/ulp/rtrs/rtrs.c | 594 +++++++++++++++++++++++++++++
> > 1 file changed, 594 insertions(+)
> > create mode 100644 drivers/infiniband/ulp/rtrs/rtrs.c
>
> <...>
>
> > +
> > +static void dev_free(struct kref *ref)
> > +{
> > + struct rtrs_rdma_dev_pd *pool;
> > + struct rtrs_ib_dev *dev;
> > +
> > + dev = container_of(ref, typeof(*dev), ref);
> > + pool = dev->pool;
> > +
> > + mutex_lock(&pool->mutex);
> > + list_del(&dev->entry);
> > + mutex_unlock(&pool->mutex);
> > +
> > + if (pool->ops && pool->ops->deinit)
> > + pool->ops->deinit(dev);
> > +
> > + ib_dealloc_pd(dev->ib_pd);
> > +
> > + if (pool->ops && pool->ops->free)
> > + pool->ops->free(dev);
> > + else
> > + kfree(dev);
> > +}
> > +
> > +int rtrs_ib_dev_put(struct rtrs_ib_dev *dev)
> > +{
> > + return kref_put(&dev->ref, dev_free);
> > +}
> > +EXPORT_SYMBOL(rtrs_ib_dev_put);
> > +
> > +static int rtrs_ib_dev_get(struct rtrs_ib_dev *dev)
> > +{
> > + return kref_get_unless_zero(&dev->ref);
> > +}
> > +
> > +struct rtrs_ib_dev *
> > +rtrs_ib_dev_find_or_add(struct ib_device *ib_dev,
> > + struct rtrs_rdma_dev_pd *pool)
> > +{
> > + struct rtrs_ib_dev *dev;
> > +
> > + mutex_lock(&pool->mutex);
>
> The scope of this mutex is unclear, you protected everything here with
> this mutex, but in dev_free() you guarded list_del() only.
Looks the mutex only need to guard poll->list, I will run some tests to verify.
Thanks!
