I do not believe it's reorganizing code. I think the code in the first section is wrong, and the second version is needed.

Once you've dec'd the refcount, you cannot access the pointer. At that point, another thread could deref and free, and you'd be left with freed memory. You *must* store that value in a local variable.

Daniel

On 08/03/2017 02:13 AM, William Allen Simpson wrote:
It's improperly reorganizing code.

===

Accessing freed memory at ==>

Logically, this couldn't happen!

int free_nfs_request(request_data_t *reqdata)
{
         atomic_dec_uint32_t(&reqdata->r_d_refs);

         LogDebug(COMPONENT_DISPATCH,
                  "%s: %p fd %d xp_refs %" PRIu32 " r_d_refs %" PRIu32,
                  __func__,
                  reqdata->r_u.req.svc.rq_xprt,
                  reqdata->r_u.req.svc.rq_xprt->xp_fd,
                  reqdata->r_u.req.svc.rq_xprt->xp_refs,
                  reqdata->r_d_refs);

         if (reqdata->r_d_refs)
==>             return reqdata->r_d_refs;

         switch (reqdata->rtype) {
         case NFS_REQUEST:
                 /* dispose RPC header */
                 if (reqdata->r_u.req.svc.rq_auth)
                         SVCAUTH_RELEASE(&(reqdata->r_u.req.svc));
                 XDR_DESTROY(reqdata->r_u.req.svc.rq_xdrs);
                 break;
         default:
                 break;
         }
         SVC_RELEASE(reqdata->r_u.req.svc.rq_xprt, SVC_RELEASE_FLAG_NONE);
         pool_free(request_pool, reqdata);
         return 0;
}

===

This works by using local variables.  Kinda pointlessly, when not
running log debug.

int free_nfs_request(request_data_t *reqdata)
{
         SVCXPRT *xprt = reqdata->r_u.req.svc.rq_xprt;
         uint32_t refs = atomic_dec_uint32_t(&reqdata->r_d_refs);

         LogDebug(COMPONENT_DISPATCH,
                  "%s: %p fd %d xp_refs %" PRIu32 " r_d_refs %" PRIu32,
                  __func__,
                  xprt, xprt->xp_fd, xprt->xp_refs,
                  refs);

         if (refs)
                 return refs;

         switch (reqdata->rtype) {
         case NFS_REQUEST:
                 /* dispose RPC header */
                 if (reqdata->r_u.req.svc.rq_auth)
                         SVCAUTH_RELEASE(&(reqdata->r_u.req.svc));
                 XDR_DESTROY(reqdata->r_u.req.svc.rq_xdrs);
                 break;
         default:
                 break;
         }
         SVC_RELEASE(xprt, SVC_RELEASE_FLAG_NONE);
         pool_free(request_pool, reqdata);
         return 0;
}

------------------------------------------------------------------------------
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


------------------------------------------------------------------------------
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

Reply via email to