Re: [PATCH rdma-next v7 0/8] RDMA resource tracking

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

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

On Mon, Jan 29, 2018 at 08:34:36PM -0700, Jason Gunthorpe wrote:
> On Mon, Jan 29, 2018 at 03:11:53PM -0500, Doug Ledford wrote:
> > On Sun, 2018-01-28 at 14:05 -0700, Jason Gunthorpe wrote:
> > > On Sun, Jan 28, 2018 at 11:17:17AM +0200, Leon Romanovsky wrote:
> > >
> > > > The original goal of this series was to allow ability to view connection
> > > > (QP) information about running processes, however I used this opportunity and
> > > > created common infrastructure to track and report various resources. The report
> > > > part is implemented in netlink (nldev), but smart ULPs can now create
> > > > advanced usage models based on device utilization.
> > > >
> > > > The current implementation relies on one lock per-object per-device, so
> > > > creation/destroying of various objects (CQ, PD, e.t.c) on various or the
> > > > same devices doesn't interfere each with another.
> > > >
> > > > The data protection is performed with SRCU and its reader-writer model
> > > > ensures that resource won't be destroyed till readers will finish their
> > > > work.
> > >
> > > Well, this cover letter isn't quite right anymore.. but no matter.
> > >
> > > My small comments aside it looks OK to me.
> >
> > Likewise.  I'm happy with it at this point.
>
> Okay, I fixed up the small things and applied the patches to for-next
>
> Leon: Please validate I didn't screw it up. Here is the diff against
> what you sent:
>
> - Success path on the main execution flow, not under an if
> - constify static structure
> - Remove confusing comment about locking, ib_enum_all_devs
>   obtains locks to iterate its list and rdma_restrack_count holds
>   res->rwsem so everything is accounted for directly without
>   trickyness
> - Speeling
> - Remove extra lock in rdma_restrack_del
> - Restore pd = NULL in ib_create_xrc_qp. This scraed me a bit, xrc is
>   wonky. But ib_create_xrc_q is only called in cases where
>   rdma_restrack_add is not added, so keeping things as-they-are should
>   not impact restrack. If restrack needs the pd for a XRC someday it
>   should get it from qp->real_qp
> - Remove SET/NEW/DEL cargo cult, please send a patch for rest?
>
> Branch is here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=wip/jgg-for-next
>
> Still unhappy with the kref-as-not-a-kref.

Thanks Doug and Jason for accepting it.

The "qp->pd = NULL" assignment is not needed. PD is NULL
for the XRCD and you are setting "qp->pd = pd" before returning
from _ib_create_qp() call, so it is actually the same.

Everything works as expected, it passed my tests with ud/rc/xsrq
pingpongs and shutdowns during traffic.

Steve, I created the stable tag for you: rdma-next-2018-01-30

Thanks

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