[PATCH 1/2] nfs41: fix nfs4_callback_recallslot

[PATCH 1/2] nfs41: fix nfs4_callback_recallslot

[andros]

From: Andy Adamson <andros@netapp.com>

Return NFS4_OK if target high slotid equals enforced high slotid.
Fix nfs_client reference leak.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/callback_proc.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index e5155d9..c79e18c 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -381,13 +381,17 @@ unsigned nfs4_callback_recallslot(struct cb_recallslotargs *args, void *dummy)
 	fc_tbl = &clp->cl_session->fc_slot_table;
 
 	status = htonl(NFS4ERR_BAD_HIGH_SLOT);
-	if (args->crsa_target_max_slots >= fc_tbl->max_slots ||
+	if (args->crsa_target_max_slots > fc_tbl->max_slots ||
 	    args->crsa_target_max_slots < 1)
-		goto out;
+		goto out_putclient;
+
+	status = htonl(NFS4_OK);
+	if (args->crsa_target_max_slots == fc_tbl->max_slots)
+		goto out_putclient;
 
 	fc_tbl->target_max_slots = args->crsa_target_max_slots;
 	nfs41_handle_recall_slot(clp);
-	status = htonl(NFS4_OK);
+out_putclient:
 	nfs_put_client(clp);	/* balance nfs_find_client */
 out:
 	dprintk("%s: exit with status = %d\n", __func__, ntohl(status));
-- 
1.6.6

[PATCH 2/2] nfs41: clear NFS4CLNT_RECALL_SLOT bit on session reset

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4state.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 069dcb3..c7f2944 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1291,9 +1291,12 @@ static int nfs4_reset_session(struct nfs_client *clp)
 
 	memset(clp->cl_session->sess_id.data, 0, NFS4_MAX_SESSIONID_LEN);
 	status = nfs4_proc_create_session(clp);
-	if (status)
+	if (status) {
 		status = nfs4_recovery_handle_error(clp, status);
-
+		goto out;
+	}
+	/* create_session negotiated new slot table */
+	clear_bit(NFS4CLNT_RECALL_SLOT, &clp->cl_state);
 out:
 	/*
 	 * Let the state manager reestablish state
-- 
1.6.6

Re: [PATCH 2/2] nfs41: clear NFS4CLNT_RECALL_SLOT bit on session reset

[Trond Myklebust]

On Thu, 2010-01-21 at 14:19 -0500, andros@netapp.com wrote: 
> From: Andy Adamson <andros@netapp.com>
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/nfs4state.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index 069dcb3..c7f2944 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -1291,9 +1291,12 @@ static int nfs4_reset_session(struct nfs_client *clp)
>  
>  	memset(clp->cl_session->sess_id.data, 0, NFS4_MAX_SESSIONID_LEN);
>  	status = nfs4_proc_create_session(clp);
> -	if (status)
> +	if (status) {
>  		status = nfs4_recovery_handle_error(clp, status);
> -
> +		goto out;
> +	}
> +	/* create_session negotiated new slot table */
> +	clear_bit(NFS4CLNT_RECALL_SLOT, &clp->cl_state);
>  out:
>  	/*
>  	 * Let the state manager reestablish state

Errm.... Looking at nfs4_reset_session, wouldn't it make sense to move
that 'out:' label down one line, so that we can get rid of that test for
'status == 0'?

Trond

Re: [PATCH 2/2] nfs41: clear NFS4CLNT_RECALL_SLOT bit on session reset

[William A. (Andy) Adamson]

On Thu, Jan 21, 2010 at 2:42 PM, Trond Myklebust
<Trond.Myklebust@netapp.com> wrote:
> On Thu, 2010-01-21 at 14:19 -0500, andros@netapp.com wrote:
>> From: Andy Adamson <andros@netapp.com>
>>
>> Signed-off-by: Andy Adamson <andros@netapp.com>
>> ---
>> =A0fs/nfs/nfs4state.c | =A0 =A07 +++++--
>> =A01 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>> index 069dcb3..c7f2944 100644
>> --- a/fs/nfs/nfs4state.c
>> +++ b/fs/nfs/nfs4state.c
>> @@ -1291,9 +1291,12 @@ static int nfs4_reset_session(struct nfs_clie=
nt *clp)
>>
>> =A0 =A0 =A0 memset(clp->cl_session->sess_id.data, 0, NFS4_MAX_SESSIO=
NID_LEN);
>> =A0 =A0 =A0 status =3D nfs4_proc_create_session(clp);
>> - =A0 =A0 if (status)
>> + =A0 =A0 if (status) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 status =3D nfs4_recovery_handle_error(cl=
p, status);
>> -
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>> + =A0 =A0 }
>> + =A0 =A0 /* create_session negotiated new slot table */
>> + =A0 =A0 clear_bit(NFS4CLNT_RECALL_SLOT, &clp->cl_state);
>> =A0out:
>> =A0 =A0 =A0 /*
>> =A0 =A0 =A0 =A0* Let the state manager reestablish state
>
> Errm.... Looking at nfs4_reset_session, wouldn't it make sense to mov=
e
> that 'out:' label down one line, so that we can get rid of that test =
for
> 'status =3D=3D 0'?

yes indeed it would.

-->Andy

>
> Trond
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" =
in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 1/2] nfs41: fix nfs4_callback_recallslot

[Trond Myklebust]

On Thu, 2010-01-21 at 14:19 -0500, andros@netapp.com wrote: 
> From: Andy Adamson <andros@netapp.com>
> 
> Return NFS4_OK if target high slotid equals enforced high slotid.
> Fix nfs_client reference leak.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/callback_proc.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index e5155d9..c79e18c 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -381,13 +381,17 @@ unsigned nfs4_callback_recallslot(struct cb_recallslotargs *args, void *dummy)
>  	fc_tbl = &clp->cl_session->fc_slot_table;
>  
>  	status = htonl(NFS4ERR_BAD_HIGH_SLOT);

This is not related directly to your patch, but "sparse" is going to
have a field day with all these scattered around the place. Are there
any plans to clean all this up, and replace those 'int' and 'unsigned'
declarations that are being assigned big ended values with the correct
'__be32' type?

> -	if (args->crsa_target_max_slots >= fc_tbl->max_slots ||
> +	if (args->crsa_target_max_slots > fc_tbl->max_slots ||
>  	    args->crsa_target_max_slots < 1)
> -		goto out;
> +		goto out_putclient;
> +
> +	status = htonl(NFS4_OK);
> +	if (args->crsa_target_max_slots == fc_tbl->max_slots)
> +		goto out_putclient;
>  
>  	fc_tbl->target_max_slots = args->crsa_target_max_slots;
>  	nfs41_handle_recall_slot(clp);
> -	status = htonl(NFS4_OK);
> +out_putclient:
>  	nfs_put_client(clp);	/* balance nfs_find_client */
>  out:
>  	dprintk("%s: exit with status = %d\n", __func__, ntohl(status));

Re: [PATCH 1/2] nfs41: fix nfs4_callback_recallslot

[William A. (Andy) Adamson]

On Thu, Jan 21, 2010 at 2:47 PM, Trond Myklebust
<Trond.Myklebust@netapp.com> wrote:
> On Thu, 2010-01-21 at 14:19 -0500, andros@netapp.com wrote:
>> From: Andy Adamson <andros@netapp.com>
>>
>> Return NFS4_OK if target high slotid equals enforced high slotid.
>> Fix nfs_client reference leak.
>>
>> Signed-off-by: Andy Adamson <andros@netapp.com>
>> ---
>> =A0fs/nfs/callback_proc.c | =A0 10 +++++++---
>> =A01 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
>> index e5155d9..c79e18c 100644
>> --- a/fs/nfs/callback_proc.c
>> +++ b/fs/nfs/callback_proc.c
>> @@ -381,13 +381,17 @@ unsigned nfs4_callback_recallslot(struct cb_re=
callslotargs *args, void *dummy)
>> =A0 =A0 =A0 fc_tbl =3D &clp->cl_session->fc_slot_table;
>>
>> =A0 =A0 =A0 status =3D htonl(NFS4ERR_BAD_HIGH_SLOT);
>
> This is not related directly to your patch, but "sparse" is going to
> have a field day with all these scattered around the place. Are there
> any plans to clean all this up, and replace those 'int' and 'unsigned=
'
> declarations that are being assigned big ended values with the correc=
t
> '__be32' type?

I could do that.

-->Andy

>
>> - =A0 =A0 if (args->crsa_target_max_slots >=3D fc_tbl->max_slots ||
>> + =A0 =A0 if (args->crsa_target_max_slots > fc_tbl->max_slots ||
>> =A0 =A0 =A0 =A0 =A0 args->crsa_target_max_slots < 1)
>> - =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto out_putclient;
>> +
>> + =A0 =A0 status =3D htonl(NFS4_OK);
>> + =A0 =A0 if (args->crsa_target_max_slots =3D=3D fc_tbl->max_slots)
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto out_putclient;
>>
>> =A0 =A0 =A0 fc_tbl->target_max_slots =3D args->crsa_target_max_slots=
;
>> =A0 =A0 =A0 nfs41_handle_recall_slot(clp);
>> - =A0 =A0 status =3D htonl(NFS4_OK);
>> +out_putclient:
>> =A0 =A0 =A0 nfs_put_client(clp); =A0 =A0/* balance nfs_find_client *=
/
>> =A0out:
>> =A0 =A0 =A0 dprintk("%s: exit with status =3D %d\n", __func__, ntohl=
(status));
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" =
in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>

[PATCH 2/2] nfs41: clear NFS4CLNT_RECALL_SLOT bit on session reset

[andros]

From: Andy Adamson <andros@netapp.com>

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4state.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 069dcb3..294827a 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1291,17 +1291,17 @@ static int nfs4_reset_session(struct nfs_client *clp)
 
 	memset(clp->cl_session->sess_id.data, 0, NFS4_MAX_SESSIONID_LEN);
 	status = nfs4_proc_create_session(clp);
-	if (status)
+	if (status) {
 		status = nfs4_recovery_handle_error(clp, status);
+		goto out;
+	}
+	/* create_session negotiated new slot table */
+	clear_bit(NFS4CLNT_RECALL_SLOT, &clp->cl_state);
 
-out:
-	/*
-	 * Let the state manager reestablish state
-	 */
-	if (!test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) &&
-	    status == 0)
+	 /* Let the state manager reestablish state */
+	if (!test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state))
 		nfs41_setup_state_renewal(clp);
-
+out:
 	return status;
 }
 
-- 
1.6.6