[v6,12/29] NFSv4.1: Allow revoked stateids to skip the call to TEST_STATEID
diff mbox

Message ID 1474390571-17106-13-git-send-email-trond.myklebust@primarydata.com
State New
Headers show

Commit Message

Trond Myklebust Sept. 20, 2016, 4:55 p.m. UTC
In some cases (e.g. when the SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED sequence
flag is set) we may already know that the stateid was revoked and that the
only valid operation we can call is FREE_STATEID. In those cases, allow
the stateid to carry the information in the type field, so that we skip
the redundant call to TEST_STATEID.
---
 fs/nfs/nfs4proc.c    | 32 +++++++++++++++++++++++---------
 include/linux/nfs4.h |  1 +
 2 files changed, 24 insertions(+), 9 deletions(-)

Comments

Schumaker, Anna Sept. 21, 2016, 4:03 p.m. UTC | #1
Hi Trond,

On 09/20/2016 12:55 PM, Trond Myklebust wrote:
> In some cases (e.g. when the SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED sequence
> flag is set) we may already know that the stateid was revoked and that the
> only valid operation we can call is FREE_STATEID. In those cases, allow
> the stateid to carry the information in the type field, so that we skip
> the redundant call to TEST_STATEID.

Would you like to add a Signed-off-by to this patch? :)

Thanks,
Anna

> ---
>  fs/nfs/nfs4proc.c    | 32 +++++++++++++++++++++++---------
>  include/linux/nfs4.h |  1 +
>  2 files changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 97a614370604..3c1b8cb7dd95 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2422,18 +2422,29 @@ static int nfs41_test_and_free_expired_stateid(struct nfs_server *server,
>  {
>  	int status;
>  
> -	status = nfs41_test_stateid(server, stateid, cred);
> +	switch (stateid->type) {
> +	default:
> +		break;
> +	case NFS4_INVALID_STATEID_TYPE:
> +	case NFS4_SPECIAL_STATEID_TYPE:
> +		return -NFS4ERR_BAD_STATEID;
> +	case NFS4_REVOKED_STATEID_TYPE:
> +		goto out_free;
> +	}
>  
> +	status = nfs41_test_stateid(server, stateid, cred);
>  	switch (status) {
>  	case -NFS4ERR_EXPIRED:
>  	case -NFS4ERR_ADMIN_REVOKED:
>  	case -NFS4ERR_DELEG_REVOKED:
> -		/* Ack the revoked state to the server */
> -		nfs41_free_stateid(server, stateid, cred);
> -	case -NFS4ERR_BAD_STATEID:
> +		break;
> +	default:
>  		return status;
>  	}
> -	return NFS_OK;
> +out_free:
> +	/* Ack the revoked state to the server */
> +	nfs41_free_stateid(server, stateid, cred);
> +	return -NFS4ERR_EXPIRED;
>  }
>  
>  static void nfs41_check_delegation_stateid(struct nfs4_state *state)
> @@ -2468,7 +2479,7 @@ static void nfs41_check_delegation_stateid(struct nfs4_state *state)
>  	rcu_read_unlock();
>  	status = nfs41_test_and_free_expired_stateid(server, &stateid, cred);
>  	trace_nfs4_test_delegation_stateid(state, NULL, status);
> -	if (status != NFS_OK)
> +	if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID)
>  		nfs_finish_clear_delegation_stateid(state, &stateid);
>  
>  	put_rpccred(cred);
> @@ -2497,7 +2508,7 @@ static int nfs41_check_open_stateid(struct nfs4_state *state)
>  
>  	status = nfs41_test_and_free_expired_stateid(server, stateid, cred);
>  	trace_nfs4_test_open_stateid(state, NULL, status);
> -	if (status != NFS_OK) {
> +	if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID) {
>  		clear_bit(NFS_O_RDONLY_STATE, &state->flags);
>  		clear_bit(NFS_O_WRONLY_STATE, &state->flags);
>  		clear_bit(NFS_O_RDWR_STATE, &state->flags);
> @@ -6124,7 +6135,7 @@ out:
>   */
>  static int nfs41_check_expired_locks(struct nfs4_state *state)
>  {
> -	int status, ret = -NFS4ERR_BAD_STATEID;
> +	int status, ret = NFS_OK;
>  	struct nfs4_lock_state *lsp;
>  	struct nfs_server *server = NFS_SERVER(state->inode);
>  
> @@ -6136,9 +6147,12 @@ static int nfs41_check_expired_locks(struct nfs4_state *state)
>  					&lsp->ls_stateid,
>  					cred);
>  			trace_nfs4_test_lock_stateid(state, lsp, status);
> -			if (status != NFS_OK) {
> +			if (status == -NFS4ERR_EXPIRED ||
> +			    status == -NFS4ERR_BAD_STATEID)
>  				clear_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags);
> +			else if (status != NFS_OK) {
>  				ret = status;
> +				break;
>  			}
>  		}
>  	};
> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> index c6564ada9beb..9094faf0699d 100644
> --- a/include/linux/nfs4.h
> +++ b/include/linux/nfs4.h
> @@ -67,6 +67,7 @@ struct nfs4_stateid_struct {
>  		NFS4_DELEGATION_STATEID_TYPE,
>  		NFS4_LAYOUT_STATEID_TYPE,
>  		NFS4_PNFS_DS_STATEID_TYPE,
> +		NFS4_REVOKED_STATEID_TYPE,
>  	} type;
>  };
>  
> 

--
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  http://vger.kernel.org/majordomo-info.html
Trond Myklebust Sept. 21, 2016, 4:39 p.m. UTC | #2
> On Sep 21, 2016, at 12:03, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:

> 

> Hi Trond,

> 

> On 09/20/2016 12:55 PM, Trond Myklebust wrote:

>> In some cases (e.g. when the SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED sequence

>> flag is set) we may already know that the stateid was revoked and that the

>> only valid operation we can call is FREE_STATEID. In those cases, allow

>> the stateid to carry the information in the type field, so that we skip

>> the redundant call to TEST_STATEID.

> 

> Would you like to add a Signed-off-by to this patch? :)


Umm…. Yes… Sorry...

Patch
diff mbox

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 97a614370604..3c1b8cb7dd95 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2422,18 +2422,29 @@  static int nfs41_test_and_free_expired_stateid(struct nfs_server *server,
 {
 	int status;
 
-	status = nfs41_test_stateid(server, stateid, cred);
+	switch (stateid->type) {
+	default:
+		break;
+	case NFS4_INVALID_STATEID_TYPE:
+	case NFS4_SPECIAL_STATEID_TYPE:
+		return -NFS4ERR_BAD_STATEID;
+	case NFS4_REVOKED_STATEID_TYPE:
+		goto out_free;
+	}
 
+	status = nfs41_test_stateid(server, stateid, cred);
 	switch (status) {
 	case -NFS4ERR_EXPIRED:
 	case -NFS4ERR_ADMIN_REVOKED:
 	case -NFS4ERR_DELEG_REVOKED:
-		/* Ack the revoked state to the server */
-		nfs41_free_stateid(server, stateid, cred);
-	case -NFS4ERR_BAD_STATEID:
+		break;
+	default:
 		return status;
 	}
-	return NFS_OK;
+out_free:
+	/* Ack the revoked state to the server */
+	nfs41_free_stateid(server, stateid, cred);
+	return -NFS4ERR_EXPIRED;
 }
 
 static void nfs41_check_delegation_stateid(struct nfs4_state *state)
@@ -2468,7 +2479,7 @@  static void nfs41_check_delegation_stateid(struct nfs4_state *state)
 	rcu_read_unlock();
 	status = nfs41_test_and_free_expired_stateid(server, &stateid, cred);
 	trace_nfs4_test_delegation_stateid(state, NULL, status);
-	if (status != NFS_OK)
+	if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID)
 		nfs_finish_clear_delegation_stateid(state, &stateid);
 
 	put_rpccred(cred);
@@ -2497,7 +2508,7 @@  static int nfs41_check_open_stateid(struct nfs4_state *state)
 
 	status = nfs41_test_and_free_expired_stateid(server, stateid, cred);
 	trace_nfs4_test_open_stateid(state, NULL, status);
-	if (status != NFS_OK) {
+	if (status == -NFS4ERR_EXPIRED || status == -NFS4ERR_BAD_STATEID) {
 		clear_bit(NFS_O_RDONLY_STATE, &state->flags);
 		clear_bit(NFS_O_WRONLY_STATE, &state->flags);
 		clear_bit(NFS_O_RDWR_STATE, &state->flags);
@@ -6124,7 +6135,7 @@  out:
  */
 static int nfs41_check_expired_locks(struct nfs4_state *state)
 {
-	int status, ret = -NFS4ERR_BAD_STATEID;
+	int status, ret = NFS_OK;
 	struct nfs4_lock_state *lsp;
 	struct nfs_server *server = NFS_SERVER(state->inode);
 
@@ -6136,9 +6147,12 @@  static int nfs41_check_expired_locks(struct nfs4_state *state)
 					&lsp->ls_stateid,
 					cred);
 			trace_nfs4_test_lock_stateid(state, lsp, status);
-			if (status != NFS_OK) {
+			if (status == -NFS4ERR_EXPIRED ||
+			    status == -NFS4ERR_BAD_STATEID)
 				clear_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags);
+			else if (status != NFS_OK) {
 				ret = status;
+				break;
 			}
 		}
 	};
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index c6564ada9beb..9094faf0699d 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -67,6 +67,7 @@  struct nfs4_stateid_struct {
 		NFS4_DELEGATION_STATEID_TYPE,
 		NFS4_LAYOUT_STATEID_TYPE,
 		NFS4_PNFS_DS_STATEID_TYPE,
+		NFS4_REVOKED_STATEID_TYPE,
 	} type;
 };