RE: [PATCH rdma-next] IB/core: Avoid deadlock during netlink message handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Leon Romanovsky <leon@xxxxxxxxxx>
> Sent: Thursday, October 24, 2019 11:13 AM
> To: Jason Gunthorpe <jgg@xxxxxxxx>
> Cc: Doug Ledford <dledford@xxxxxxxxxx>; Parav Pandit
> <parav@xxxxxxxxxxxx>; RDMA mailing list <linux-rdma@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH rdma-next] IB/core: Avoid deadlock during netlink message
> handling
> 
> On Thu, Oct 24, 2019 at 01:08:10PM -0300, Jason Gunthorpe wrote:
> > On Thu, Oct 24, 2019 at 07:02:52PM +0300, Leon Romanovsky wrote:
> > > On Thu, Oct 24, 2019 at 10:50:17AM -0300, Jason Gunthorpe wrote:
> > > > On Thu, Oct 24, 2019 at 04:26:07PM +0300, Leon Romanovsky wrote:
> > > > > On Thu, Oct 24, 2019 at 10:17:43AM -0300, Jason Gunthorpe wrote:
> > > > > > On Tue, Oct 15, 2019 at 11:07:33AM +0300, Leon Romanovsky wrote:
> > > > > >
> > > > > > > diff --git a/drivers/infiniband/core/netlink.c
> > > > > > > b/drivers/infiniband/core/netlink.c
> > > > > > > index 81dbd5f41bed..a3507b8be569 100644
> > > > > > > +++ b/drivers/infiniband/core/netlink.c
> > > > > > > @@ -42,9 +42,12 @@
> > > > > > >  #include <linux/module.h>
> > > > > > >  #include "core_priv.h"
> > > > > > >
> > > > > > > -static DEFINE_MUTEX(rdma_nl_mutex);  static struct {
> > > > > > > -	const struct rdma_nl_cbs   *cb_table;
> > > > > > > +	const struct rdma_nl_cbs __rcu *cb_table;
> > > > > > > +	/* Synchronizes between ongoing netlink commands and
> netlink client
> > > > > > > +	 * unregistration.
> > > > > > > +	 */
> > > > > > > +	struct srcu_struct unreg_srcu;
> > > > > >
> > > > > > A srcu in every index is serious overkill for this. Lets just
> > > > > > us a
> > > > > > rwsem:
> > > > >
> > > > > I liked previous variant more than rwsem, but it is Parav's patch.
> > > >
> > > > Why? srcu is a huge data structure and slow on unregister
> > >
> > > The unregister time is not so important for those IB/core modules.
> > > I liked SRCU because it doesn't have *_ONCE() macros and smb_* calls.
> >
> > It does, they are just hidden under other macros..
> >
Its better that they are hidden. So that we don't need open code them.
Also with srcu, we don't need lock annotations in get_cb_table() which releases and acquires semaphore.
Additionally lock nesting makes overall more complex.
Given that there are only 3 indices, out of which only 2 are outside of the ib_core module and unlikely to be unloaded, I also prefer srcu version.


> > > Maybe wrong here, but the extra advantage of SRCU is that we are
> > > already using that mechanism in uverbs and my assumption that SRCU
> > > will greatly enjoy shared grace period.
> >
> > Hm, I'm not sure it works like that, the grace periods would be
> > consecutive
> 
> Whatever, the most important part of Parav's patch that we removed global
> lock from RDMA netlink interface.
> 
> Thanks
> 
> >
> > Jason




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux