Hi Malahal, Thanks for your reply.
#1. I'd like to post a patch to delete it, because I have some cleanups for drc. #2/#3. Sorry for my missing of nfs_rpc_free_user_data(). With it, everything is okay. thanks, Kinglong Mee On 10/13/2017 15:52, Malahal Naineni wrote: > #1. Looks like a bug! Lines 629 and 630 should be deleted > #2. See nfs_rpc_free_user_data(). It sets xp_u2 to NULL and drc ref is > decremented there. > #3. Life time of drc should start when it is allocated in > nfs_dupreq_get_drc() using alloc_tcp_drc(). > It can live beyond xprt's xp_u2 setting to NULL. It will live until we > decide to free in drc_free_expired() using free_tcp_drc(). > > Regards, Malahal. > PS: The comment "drc cache maintains a ref count." seems to imply that it > will have a refcount for keeping it in the hash table itself. I may have kept > those two lines because of that but It doesn't make sense as refcnt will > never go to zero this way. > > On Thu, Oct 12, 2017 at 3:48 PM, Kinglong Mee <kinglong...@gmail.com > <mailto:kinglong...@gmail.com>> wrote: > > Describes in src/RPCAL/nfs_dupreq.c, > > * The life of tcp drc: it gets allocated when we process the first > * request on the connection. It is put into rbtree (tcp_drc_recycle_t). > * drc cache maintains a ref count. Every request as well as the xprt > * holds a ref count. Its ref count should go to zero when the > * connection's xprt gets freed (all requests should be completed on the > * xprt by this time). When the ref count goes to zero, it is also put > * into a recycle queue (tcp_drc_recycle_q). When a reconnection > * happens, we hope to find the same drc that was used before, and the > * ref count goes up again. At the same time, the drc will be removed > * from the recycle queue. Only drc's with ref count zero end up in the > * recycle queue. If a reconnection doesn't happen in time, the drc gets > * freed by drc_free_expired() after some period of inactivety. > > Some questions about the life time of tcp drc, > 1. The are two references of drc for xprt in nfs_dupreq_get_drc(). > 629 /* xprt ref */ > 630 drc->refcnt = 1; > ... > 638 (void)nfs_dupreq_ref_drc(drc); /* xprt > ref */ > ... > 653 req->rq_xprt->xp_u2 = (void *)drc; > > I think it's a bug. The first one needs remove. Right? > > 2. The is no place to decrease the reference of drc for xprt. > The xprt argument in nfs_dupreq_put_drc() is unused. > Should it be used to decrease the ref? > I think it's the right place to decrease the ref in > nfs_dupreq_put_drc(). > > 3. My doubts is that, the life time of drc stored in req->rq_xprt->xp_u2 ? > Start at #1, end at #2 (req->rq_xprt->xp_u2 = NULL) ? > If that, the bad case is always lookup drc from tcp_drc_recycle_t. > > Otherwise, don't put the reference at #2, when to put it? > the bad case is the drc ref always be 1 forever, am I right? > > thanks, > Kinglong Mee > > > ------------------------------------------------------------------------------ > 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 > <mailto:Nfs-ganesha-devel@lists.sourceforge.net> > https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel > <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
