On Di, 2016-09-06 at 16:16 -0400, Matt Benjamin wrote:
> Hi,
>
> inline
>
> ----- Original Message -----
> >
> > From: "Swen Schillig" <s...@vnet.ibm.com>
> > To: "Matt Benjamin" <mbenja...@redhat.com>, "Daniel Gryniewicz" <da
> > n...@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.
No, we're just locally using the reference from rec
which is already refcounted and rec is protected to not change
by the REFLOCK.
>
> >
> > here.
> > }
> > ...
> > clnt->cl_p1 = xd; <=== but here we should increment
> > the refocunt.
but this reference is not counted.
> > }
> >
> > 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
> >
> >
------------------------------------------------------------------------------
_______________________________________________
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
