What is the purpose of those put_rpccred() statements?

What is the purpose of those put_rpccred() statements?

[Trond Myklebust]

I'm finding a bunch of random calls of the form
   put_rpccred(task->tk_msg.rpc_cred);

in functions like nfs4_renew_done(), nfs41_sequence_call_done() with
apparently no corresponding get_rpccred(). Could you please enlighten me
what they are all about? They're not mentioned at all in the changelogs
of commits 29fba38b, and fc01cea9...

Trond

Re: What is the purpose of those put_rpccred() statements?

[Trond Myklebust]

On Fri, 2009-10-23 at 15:56 -0400, Trond Myklebust wrote:
> I'm finding a bunch of random calls of the form
>    put_rpccred(task->tk_msg.rpc_cred);
> 
> in functions like nfs4_renew_done(), nfs41_sequence_call_done() with
> apparently no corresponding get_rpccred(). Could you please enlighten me
> what they are all about? They're not mentioned at all in the changelogs
> of commits 29fba38b, and fc01cea9...
> 
> Trond
--------------------------------------------------------------------
NFSv4: Fix two unbalanced put_rpccred() issues.

From: Trond Myklebust <Trond.Myklebust@netapp.com>

Commits 29fba38b (nfs41: lease renewal) and fc01cea9 (nfs41: sequence
operation) introduce a couple of put_rpccred() calls on credentials for
which there is no corresponding get_rpccred().

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/nfs4proc.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)


diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 65c2527..ff37454 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3065,9 +3065,6 @@ static void nfs4_renew_done(struct rpc_task *task, void *data)
 	if (time_before(clp->cl_last_renewal,timestamp))
 		clp->cl_last_renewal = timestamp;
 	spin_unlock(&clp->cl_lock);
-	dprintk("%s calling put_rpccred on rpc_cred %p\n", __func__,
-				task->tk_msg.rpc_cred);
-	put_rpccred(task->tk_msg.rpc_cred);
 }
 
 static const struct rpc_call_ops nfs4_renew_ops = {
@@ -4882,7 +4879,6 @@ void nfs41_sequence_call_done(struct rpc_task *task, void *data)
 	nfs41_sequence_free_slot(clp, task->tk_msg.rpc_resp);
 	dprintk("%s rpc_cred %p\n", __func__, task->tk_msg.rpc_cred);
 
-	put_rpccred(task->tk_msg.rpc_cred);
 	kfree(task->tk_msg.rpc_argp);
 	kfree(task->tk_msg.rpc_resp);
 

Re: What is the purpose of those put_rpccred() statements?

[Benny Halevy]

On Oct. 23, 2009, 22:09 +0200, Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
> On Fri, 2009-10-23 at 15:56 -0400, Trond Myklebust wrote:
>> I'm finding a bunch of random calls of the form
>>    put_rpccred(task->tk_msg.rpc_cred);
>>
>> in functions like nfs4_renew_done(), nfs41_sequence_call_done() with
>> apparently no corresponding get_rpccred(). Could you please enlighten me
>> what they are all about? They're not mentioned at all in the changelogs
>> of commits 29fba38b, and fc01cea9...

The original intent of this is:
http://linux-nfs.org/pipermail/pnfs/2007-December/002093.html

my bad rebase:
54cf1c6e937fd6e2e9724adba2af39eeaa7939d9

on top of your patch:
b0d3ded1a21dc3057daff5a488469d9e6aa1b567
NFSv4: Clean up nfs_expire_all_delegations()

left the call to put_rpccred() in nfs4_renew_state()

That said, since rpcauth_bindcred gets the rpc cred
when called from rpc_init_task
and rpc_put_task eventually puts the creds we don't
need to hold a reference on the creds the way
http://linux-nfs.org/pipermail/pnfs/2007-December/002093.html
intended...

Your fix below makes sense to me.

Benny

>>
>> Trond
> --------------------------------------------------------------------
> NFSv4: Fix two unbalanced put_rpccred() issues.
> 
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> Commits 29fba38b (nfs41: lease renewal) and fc01cea9 (nfs41: sequence
> operation) introduce a couple of put_rpccred() calls on credentials for
> which there is no corresponding get_rpccred().
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> 
>  fs/nfs/nfs4proc.c |    4 ----
>  1 files changed, 0 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 65c2527..ff37454 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3065,9 +3065,6 @@ static void nfs4_renew_done(struct rpc_task *task, void *data)
>  	if (time_before(clp->cl_last_renewal,timestamp))
>  		clp->cl_last_renewal = timestamp;
>  	spin_unlock(&clp->cl_lock);
> -	dprintk("%s calling put_rpccred on rpc_cred %p\n", __func__,
> -				task->tk_msg.rpc_cred);
> -	put_rpccred(task->tk_msg.rpc_cred);
>  }
>  
>  static const struct rpc_call_ops nfs4_renew_ops = {
> @@ -4882,7 +4879,6 @@ void nfs41_sequence_call_done(struct rpc_task *task, void *data)
>  	nfs41_sequence_free_slot(clp, task->tk_msg.rpc_resp);
>  	dprintk("%s rpc_cred %p\n", __func__, task->tk_msg.rpc_cred);
>  
> -	put_rpccred(task->tk_msg.rpc_cred);
>  	kfree(task->tk_msg.rpc_argp);
>  	kfree(task->tk_msg.rpc_resp);
>  
> 
>