Re: [Nfs-ganesha-devel] The life of tcp drc

2017-10-13 Thread Matt Benjamin 
Yes, nfs_rpc_free_user_data() is the secret :)

Matt

On Fri, Oct 13, 2017 at 4:11 AM, Kinglong Mee  wrote:
> 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 > > 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 
>> 
>> 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



-- 

Matt Benjamin
Red Hat, Inc.
315 West Huron Street, Suite 140A
Ann Arbor, Michigan 48103

http://www.redhat.com/en/technologies/storage

tel.  734-821-5101
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

Re: [Nfs-ganesha-devel] The life of tcp drc

2017-10-13 Thread Kinglong Mee 
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  > 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 
> 
> 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

Re: [Nfs-ganesha-devel] The life of tcp drc

2017-10-13 Thread Malahal Naineni 
#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  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
> 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

[Nfs-ganesha-devel] The life of tcp drc

2017-10-12 Thread Kinglong Mee 
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
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] The life of tcp drc

2017-10-12 Thread Kinglong Mee 
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
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel