Hi Swen, Are you going to re-push your recent closed PR as 2 new PRs as planned?
Dan has made some time this week to review and test, so if you have time, it will be easier to get to merge. Cheers, Matt ----- Original Message ----- > From: "Matt Benjamin" <mbenja...@redhat.com> > To: "Swen Schillig" <s...@vnet.ibm.com> > Cc: "Daniel Gryniewicz" <d...@redhat.com>, "nfs-ganesha-devel" > <nfs-ganesha-devel@lists.sourceforge.net> > Sent: Tuesday, September 6, 2016 4:16:39 PM > Subject: Re: [ntirpc] refcount issue ? > > Hi, > > inline > > ----- Original Message ----- > > From: "Swen Schillig" <s...@vnet.ibm.com> > > To: "Matt Benjamin" <mbenja...@redhat.com>, "Daniel Gryniewicz" > > <d...@redhat.com> > > Cc: "nfs-ganesha-devel" <nfs-ganesha-devel@lists.sourceforge.net> > > Sent: Tuesday, September 6, 2016 4:10:32 PM > > Subject: [ntirpc] refcount issue ? > > > > Matt, Dan. > > > > Could you please have a look at the following code areas and verify > > what I think is a refcount issue. > > > > clnt_vc_ncreate2() > > { > > ... > > if ((oflags & RPC_DPLX_LKP_OFLAG_ALLOC) || (!rec->hdl.xd)) { > > xd = rec->hdl.xd = alloc_x_vc_data(); > > ... > > } else { > > xd = rec->hdl.xd; > > ++(xd->refcnt); <=== this is not right. we're not taking an > > addtl' > > ref > > Aren't we? We now share a ref to previously allocated rec->hdl.xd. > > > here. > > } > > ... > > clnt->cl_p1 = xd; <=== but here we should increment the > > refocunt. > > } > > > > another code section with the same handling. > > > > makefd_xprt() > > { > > ... > > if ((oflags & RPC_DPLX_LKP_OFLAG_ALLOC) || (!rec->hdl.xd)) { > > newxd = true; > > xd = rec->hdl.xd = alloc_x_vc_data(); > > ... > > } else { > > xd = (struct x_vc_data *)rec->hdl.xd; > > /* dont return destroyed xprts */ > > if (!(xd->flags & X_VC_DATA_FLAG_SVC_DESTROYED)) { > > if (rec->hdl.xprt) { > > xprt = rec->hdl.xprt; > > /* inc xprt refcnt */ > > SVC_REF(xprt, SVC_REF_FLAG_NONE); > > } else > > ++(xd->refcnt); <==== not right, no addtl' > > ref to xd taken. > > so this looks more likely to be incorrect, need to review > > > } > > /* return extra ref */ > > rpc_dplx_unref(rec, > > RPC_DPLX_FLAG_LOCKED | RPC_DPLX_FLAG_UNLOCK); > > *allocated = FALSE; > > > > /* return ref'd xprt */ > > goto done_xprt; > > } > > ... > > xprt->xp_p1 = xd; <==== but here we should increment the refcount > > > > ... > > } > > > > Both areas handle the refcount'ing wrong, but it might balance out > > sometimes. > > > > > > What do you think ? > > > > Cheers Swen > > > > > > -- > Matt Benjamin > Red Hat, Inc. > 315 West Huron Street, Suite 140A > Ann Arbor, Michigan 48103 > > http://www.redhat.com/en/technologies/storage > > tel. 734-707-0660 > fax. 734-769-8938 > cel. 734-216-5309 > -- Matt Benjamin Red Hat, Inc. 315 West Huron Street, Suite 140A Ann Arbor, Michigan 48103 http://www.redhat.com/en/technologies/storage tel. 734-707-0660 fax. 734-769-8938 cel. 734-216-5309 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ Nfs-ganesha-devel mailing list Nfs-ganesha-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel