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

------------------------------------------------------------------------------
_______________________________________________
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Reply via email to