[PATCH] nfs: Fix open state losing after delegation return

[PATCH] nfs: Fix open state losing after delegation return

[Kinglong Mee]

After delegation return caused by setting file mode, 
client lost the open state,  DESTROY_CLIENTID will 
get error NFS4ERR_CLIENTID_BUSY from server.

The delegation_type passed to nfs4_open_recover_helper
with NFS4_OPEN_CLAIM_DELEG_CUR_FH is never set.

Reproduce steps,
# mount -t nfs nfs-server:/ /mnt/
# ./delegation_test /mnt/
# umount /mnt    <--- costs 10 seconds

The delegation_test.c is,
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <unistd.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <error.h>

int main(int argc, char **argv)
{
        int fd1, fd2;
        char fname1[1024], fname2[1024];
        struct stat sb;

        if (argc < 2)
                return -1;

        sprintf(fname1, "%s/test1", argv[1]);
        sprintf(fname2, "%s/test2", argv[1]);

        fd1 = open(fname1, O_RDONLY);
        fd2 = open(fname2, O_RDONLY);

        fstat(fd1, &sb);
        fchmod(fd1, sb.st_mode + 1);

        close(fd1);
        close(fd2);

        return 0;
}

Fixes: 39f897fdbd ("NFSv4: When returning a delegation, don't reclaim an incompatible open mode.")
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
 fs/nfs/nfs4proc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 693b903..472a52f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1576,8 +1576,7 @@ static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmod
 	struct nfs4_state *newstate;
 	int ret;
 
-	if ((opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEGATE_CUR ||
-	     opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEG_CUR_FH) &&
+	if (opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEGATE_CUR &&
 	    (opendata->o_arg.u.delegation_type & fmode) != fmode)
 		/* This mode can't have been delegated, so we must have
 		 * a valid open_stateid to cover it - not need to reclaim.
-- 
2.5.0

Re: [PATCH] nfs: Fix open state losing after delegation return

[Neil Brown]

[-- Attachment #1: Type: text/plain, Size: 3572 bytes --]

Kinglong Mee <kinglongmee@gmail.com> writes:

> After delegation return caused by setting file mode, 
> client lost the open state,  DESTROY_CLIENTID will 
> get error NFS4ERR_CLIENTID_BUSY from server.
>
> The delegation_type passed to nfs4_open_recover_helper
> with NFS4_OPEN_CLAIM_DELEG_CUR_FH is never set.
>
> Reproduce steps,
> # mount -t nfs nfs-server:/ /mnt/
> # ./delegation_test /mnt/
> # umount /mnt    <--- costs 10 seconds
>
> The delegation_test.c is,
> #include <stdio.h>
> #include <stdlib.h>
> #include <sys/types.h>
> #include <unistd.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <error.h>
>
> int main(int argc, char **argv)
> {
>         int fd1, fd2;
>         char fname1[1024], fname2[1024];
>         struct stat sb;
>
>         if (argc < 2)
>                 return -1;
>
>         sprintf(fname1, "%s/test1", argv[1]);
>         sprintf(fname2, "%s/test2", argv[1]);
>
>         fd1 = open(fname1, O_RDONLY);
>         fd2 = open(fname2, O_RDONLY);
>
>         fstat(fd1, &sb);
>         fchmod(fd1, sb.st_mode + 1);
>
>         close(fd1);
>         close(fd2);
>
>         return 0;
> }
>
> Fixes: 39f897fdbd ("NFSv4: When returning a delegation, don't reclaim an incompatible open mode.")
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
>  fs/nfs/nfs4proc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 693b903..472a52f 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -1576,8 +1576,7 @@ static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmod
>  	struct nfs4_state *newstate;
>  	int ret;
>  
> -	if ((opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEGATE_CUR ||
> -	     opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEG_CUR_FH) &&
> +	if (opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEGATE_CUR &&
>  	    (opendata->o_arg.u.delegation_type & fmode) != fmode)
>  		/* This mode can't have been delegated, so we must have
>  		 * a valid open_stateid to cover it - not need to reclaim.
> -- 
> 2.5.0

This patches doesn't look right to me.  It isn't at all clear why the
change given addresses the symptoms described.

However looking back at my patch (39f897fdbd) it looks really wrong and
I cannot imagine how I missed the problem when I submitted it.

nfs4_open_recover assumes that if nfs4_open_recover_helper returns zero,
the 'newstate' has been given a value and if that doesn't match 'state'
it returns -ESTALE.

However with my patch, nfs4_open_recovery_helper can return zero and
leave 'newstate' uniitialised.  I cannot even see how that passed
testing :-(

You could maybe fix with

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 693b903b48bd..6899197ff1c3 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1604,7 +1604,7 @@ static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmod
 
 static int nfs4_open_recover(struct nfs4_opendata *opendata, struct nfs4_state *state)
 {
-	struct nfs4_state *newstate;
+	struct nfs4_state *newstate = state;
 	int ret;
 
 	/* Don't trigger recovery in nfs_test_and_clear_all_open_stateid */

but I'm not at all sure I like that.  I'll try to find time to look at
the code properly and make a more concrete proposal - and to test with
your test case too.  However you didn't give details on the mount: I
assume it was a 4.1 mount?  Was it against the Linux nfs server or
something else?

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH] nfs: Fix open state losing after delegation return

[Kinglong Mee]

On 9/20/2015 13:05, Neil Brown wrote:
> Kinglong Mee <kinglongmee@gmail.com> writes:
> 
>> After delegation return caused by setting file mode, 
>> client lost the open state,  DESTROY_CLIENTID will 
>> get error NFS4ERR_CLIENTID_BUSY from server.
>>
>> The delegation_type passed to nfs4_open_recover_helper
>> with NFS4_OPEN_CLAIM_DELEG_CUR_FH is never set.
>>
>> Reproduce steps,
>> # mount -t nfs nfs-server:/ /mnt/

Missing a step, 
# touch /mnt/test1 /mnt/test2

>> # ./delegation_test /mnt/
>> # umount /mnt    <--- costs 10 seconds
>>
>> The delegation_test.c is,
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <sys/types.h>
>> #include <unistd.h>
>> #include <sys/stat.h>
>> #include <fcntl.h>
>> #include <error.h>
>>
>> int main(int argc, char **argv)
>> {
>>         int fd1, fd2;
>>         char fname1[1024], fname2[1024];
>>         struct stat sb;
>>
>>         if (argc < 2)
>>                 return -1;
>>
>>         sprintf(fname1, "%s/test1", argv[1]);
>>         sprintf(fname2, "%s/test2", argv[1]);
>>
>>         fd1 = open(fname1, O_RDONLY);
>>         fd2 = open(fname2, O_RDONLY);
>>
>>         fstat(fd1, &sb);
>>         fchmod(fd1, sb.st_mode + 1);
>>
>>         close(fd1);
>>         close(fd2);
>>
>>         return 0;
>> }
>>
>> Fixes: 39f897fdbd ("NFSv4: When returning a delegation, don't reclaim an incompatible open mode.")
>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>> ---
>>  fs/nfs/nfs4proc.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 693b903..472a52f 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -1576,8 +1576,7 @@ static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmod
>>  	struct nfs4_state *newstate;
>>  	int ret;
>>  
>> -	if ((opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEGATE_CUR ||
>> -	     opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEG_CUR_FH) &&
>> +	if (opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEGATE_CUR &&
>>  	    (opendata->o_arg.u.delegation_type & fmode) != fmode)
>>  		/* This mode can't have been delegated, so we must have
>>  		 * a valid open_stateid to cover it - not need to reclaim.
>> -- 
>> 2.5.0
> 
> This patches doesn't look right to me.  It isn't at all clear why the
> change given addresses the symptoms described.

There are three places calling nfs4_open_recover(), those open_claim_type
are, NFS4_OPEN_CLAIM_PREVIOUS, NFS4_OPEN_CLAIM_DELEG_CUR_FH and 
NFS4_OPEN_CLAIM_FH.

Only set delegation_type with NFS4_OPEN_CLAIM_PREVIOUS.

So the checking seems wrong here.

> 
> However looking back at my patch (39f897fdbd) it looks really wrong and
> I cannot imagine how I missed the problem when I submitted it.
> 
> nfs4_open_recover assumes that if nfs4_open_recover_helper returns zero,
> the 'newstate' has been given a value and if that doesn't match 'state'
> it returns -ESTALE.
> 
> However with my patch, nfs4_open_recovery_helper can return zero and
> leave 'newstate' uniitialised.  I cannot even see how that passed
> testing :-(

Yes, it's strange.
But it does not affect the problem I reported.
You can send it as a separate patch.

> 
> You could maybe fix with
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 693b903b48bd..6899197ff1c3 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -1604,7 +1604,7 @@ static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmod
>  
>  static int nfs4_open_recover(struct nfs4_opendata *opendata, struct nfs4_state *state)
>  {
> -	struct nfs4_state *newstate;
> +	struct nfs4_state *newstate = state;
>  	int ret;
>  
>  	/* Don't trigger recovery in nfs_test_and_clear_all_open_stateid */
> 
> but I'm not at all sure I like that.  I'll try to find time to look at
> the code properly and make a more concrete proposal - and to test with
> your test case too.  However you didn't give details on the mount: I
> assume it was a 4.1 mount?  Was it against the Linux nfs server or
> something else?

My default nfs mount is 4.2, 
nfs-server:/ /mnt nfs4 rw,seclabel,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.8.128,local_lock=none,addr=192.168.8.128 0 0

The nfs server is a linux with latest kernel 4.3.0-rc1.

thanks,
Kinglong Mee

[PATCH] NFSv4: Recovery of recalled read delegations is broken

[Trond Myklebust]

When a read delegation is being recalled, and we're reclaiming the
cached opens, we need to make sure that we only reclaim read-only
modes.
A previous attempt to do this, relied on retrieving the delegation
type from the nfs4_opendata structure. Unfortunately, as Kinglong
pointed out, this field can only be set when performing reboot recovery.

Furthermore, if we call nfs4_open_recover(), then we end up clobbering
the state->flags for all modes that we're not recovering...

The fix is to have the delegation recall code pass this information
to the recovery call, and then refactor the recovery code so that
nfs4_open_delegation_recall() does not need to call nfs4_open_recover().

Reported-by: Kinglong Mee <kinglongmee@gmail.com>
Fixes: 39f897fdbd46 ("NFSv4: When returning a delegation, don't...")
Cc: NeilBrown <neilb@suse.com>
Cc: stable@vger.kernel.org # v4.2+
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/delegation.c |  8 ++++--
 fs/nfs/delegation.h |  2 +-
 fs/nfs/nfs4proc.c   | 81 +++++++++++++++++++++++++++++++----------------------
 3 files changed, 53 insertions(+), 38 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 2714ef835bdd..be806ead7f4d 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -113,7 +113,8 @@ out:
 	return status;
 }
 
-static int nfs_delegation_claim_opens(struct inode *inode, const nfs4_stateid *stateid)
+static int nfs_delegation_claim_opens(struct inode *inode,
+		const nfs4_stateid *stateid, fmode_t type)
 {
 	struct nfs_inode *nfsi = NFS_I(inode);
 	struct nfs_open_context *ctx;
@@ -140,7 +141,7 @@ again:
 		/* Block nfs4_proc_unlck */
 		mutex_lock(&sp->so_delegreturn_mutex);
 		seq = raw_seqcount_begin(&sp->so_reclaim_seqcount);
-		err = nfs4_open_delegation_recall(ctx, state, stateid);
+		err = nfs4_open_delegation_recall(ctx, state, stateid, type);
 		if (!err)
 			err = nfs_delegation_claim_locks(ctx, state, stateid);
 		if (!err && read_seqcount_retry(&sp->so_reclaim_seqcount, seq))
@@ -411,7 +412,8 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation
 	do {
 		if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags))
 			break;
-		err = nfs_delegation_claim_opens(inode, &delegation->stateid);
+		err = nfs_delegation_claim_opens(inode, &delegation->stateid,
+				delegation->type);
 		if (!issync || err != -EAGAIN)
 			break;
 		/*
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index a44829173e57..333063e032f0 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -54,7 +54,7 @@ void nfs_delegation_reap_unclaimed(struct nfs_client *clp);
 
 /* NFSv4 delegation-related procedures */
 int nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, const nfs4_stateid *stateid, int issync);
-int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid);
+int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid, fmode_t type);
 int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state, const nfs4_stateid *stateid);
 bool nfs4_copy_delegation_stateid(nfs4_stateid *dst, struct inode *inode, fmode_t flags);
 
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 693b903b48bd..099a0a83ee8d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1127,6 +1127,21 @@ static int nfs4_wait_for_completion_rpc_task(struct rpc_task *task)
 	return ret;
 }
 
+static bool nfs4_mode_match_open_stateid(struct nfs4_state *state,
+		fmode_t fmode)
+{
+	switch(fmode & (FMODE_READ|FMODE_WRITE)) {
+	case FMODE_READ|FMODE_WRITE:
+		return state->n_rdwr != 0;
+	case FMODE_WRITE:
+		return state->n_wronly != 0;
+	case FMODE_READ:
+		return state->n_rdonly != 0;
+	}
+	WARN_ON_ONCE(1);
+	return false;
+}
+
 static int can_open_cached(struct nfs4_state *state, fmode_t mode, int open_mode)
 {
 	int ret = 0;
@@ -1571,17 +1586,13 @@ static struct nfs4_opendata *nfs4_open_recoverdata_alloc(struct nfs_open_context
 	return opendata;
 }
 
-static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmode, struct nfs4_state **res)
+static int nfs4_open_recover_helper(struct nfs4_opendata *opendata,
+		fmode_t fmode)
 {
 	struct nfs4_state *newstate;
 	int ret;
 
-	if ((opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEGATE_CUR ||
-	     opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEG_CUR_FH) &&
-	    (opendata->o_arg.u.delegation_type & fmode) != fmode)
-		/* This mode can't have been delegated, so we must have
-		 * a valid open_stateid to cover it - not need to reclaim.
-		 */
+	if (!nfs4_mode_match_open_stateid(opendata->state, fmode))
 		return 0;
 	opendata->o_arg.open_flags = 0;
 	opendata->o_arg.fmode = fmode;
@@ -1597,14 +1608,14 @@ static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmod
 	newstate = nfs4_opendata_to_nfs4_state(opendata);
 	if (IS_ERR(newstate))
 		return PTR_ERR(newstate);
+	if (newstate != opendata->state)
+		ret = -ESTALE;
 	nfs4_close_state(newstate, fmode);
-	*res = newstate;
-	return 0;
+	return ret;
 }
 
 static int nfs4_open_recover(struct nfs4_opendata *opendata, struct nfs4_state *state)
 {
-	struct nfs4_state *newstate;
 	int ret;
 
 	/* Don't trigger recovery in nfs_test_and_clear_all_open_stateid */
@@ -1615,27 +1626,15 @@ static int nfs4_open_recover(struct nfs4_opendata *opendata, struct nfs4_state *
 	clear_bit(NFS_DELEGATED_STATE, &state->flags);
 	clear_bit(NFS_OPEN_STATE, &state->flags);
 	smp_rmb();
-	if (state->n_rdwr != 0) {
-		ret = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE, &newstate);
-		if (ret != 0)
-			return ret;
-		if (newstate != state)
-			return -ESTALE;
-	}
-	if (state->n_wronly != 0) {
-		ret = nfs4_open_recover_helper(opendata, FMODE_WRITE, &newstate);
-		if (ret != 0)
-			return ret;
-		if (newstate != state)
-			return -ESTALE;
-	}
-	if (state->n_rdonly != 0) {
-		ret = nfs4_open_recover_helper(opendata, FMODE_READ, &newstate);
-		if (ret != 0)
-			return ret;
-		if (newstate != state)
-			return -ESTALE;
-	}
+	ret = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE);
+	if (ret != 0)
+		return ret;
+	ret = nfs4_open_recover_helper(opendata, FMODE_WRITE);
+	if (ret != 0)
+		return ret;
+	ret = nfs4_open_recover_helper(opendata, FMODE_READ);
+	if (ret != 0)
+		return ret;
 	/*
 	 * We may have performed cached opens for all three recoveries.
 	 * Check if we need to update the current stateid.
@@ -1759,18 +1758,32 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
 	return err;
 }
 
-int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid)
+int nfs4_open_delegation_recall(struct nfs_open_context *ctx,
+		struct nfs4_state *state, const nfs4_stateid *stateid,
+		fmode_t type)
 {
 	struct nfs_server *server = NFS_SERVER(state->inode);
 	struct nfs4_opendata *opendata;
-	int err;
+	int err = 0;
 
 	opendata = nfs4_open_recoverdata_alloc(ctx, state,
 			NFS4_OPEN_CLAIM_DELEG_CUR_FH);
 	if (IS_ERR(opendata))
 		return PTR_ERR(opendata);
 	nfs4_stateid_copy(&opendata->o_arg.u.delegation, stateid);
-	err = nfs4_open_recover(opendata, state);
+	clear_bit(NFS_DELEGATED_STATE, &state->flags);
+	switch (type & (FMODE_READ|FMODE_WRITE)) {
+	case FMODE_READ|FMODE_WRITE:
+	case FMODE_WRITE:
+		err = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE);
+		if (err)
+			break;
+		err = nfs4_open_recover_helper(opendata, FMODE_WRITE);
+		if (err)
+			break;
+	case FMODE_READ:
+		err = nfs4_open_recover_helper(opendata, FMODE_READ);
+	}
 	nfs4_opendata_put(opendata);
 	return nfs4_handle_delegation_recall_error(server, state, stateid, err);
 }
-- 
2.4.3

Re: [PATCH] NFSv4: Recovery of recalled read delegations is broken

[Kinglong Mee]

On 9/21/2015 00:26, Trond Myklebust wrote:
> When a read delegation is being recalled, and we're reclaiming the
> cached opens, we need to make sure that we only reclaim read-only
> modes.
> A previous attempt to do this, relied on retrieving the delegation
> type from the nfs4_opendata structure. Unfortunately, as Kinglong
> pointed out, this field can only be set when performing reboot recovery.
> 
> Furthermore, if we call nfs4_open_recover(), then we end up clobbering
> the state->flags for all modes that we're not recovering...
> 
> The fix is to have the delegation recall code pass this information
> to the recovery call, and then refactor the recovery code so that
> nfs4_open_delegation_recall() does not need to call nfs4_open_recover().
> 
> Reported-by: Kinglong Mee <kinglongmee@gmail.com>
> Fixes: 39f897fdbd46 ("NFSv4: When returning a delegation, don't...")
> Cc: NeilBrown <neilb@suse.com>
> Cc: stable@vger.kernel.org # v4.2+
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>

Tested-by: Kinglong Mee <kinglongmee@gmail.com>

thanks,
Kinglong Mee

> ---
>  fs/nfs/delegation.c |  8 ++++--
>  fs/nfs/delegation.h |  2 +-
>  fs/nfs/nfs4proc.c   | 81 +++++++++++++++++++++++++++++++----------------------
>  3 files changed, 53 insertions(+), 38 deletions(-)
> 
> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
> index 2714ef835bdd..be806ead7f4d 100644
> --- a/fs/nfs/delegation.c
> +++ b/fs/nfs/delegation.c
> @@ -113,7 +113,8 @@ out:
>  	return status;
>  }
>  
> -static int nfs_delegation_claim_opens(struct inode *inode, const nfs4_stateid *stateid)
> +static int nfs_delegation_claim_opens(struct inode *inode,
> +		const nfs4_stateid *stateid, fmode_t type)
>  {
>  	struct nfs_inode *nfsi = NFS_I(inode);
>  	struct nfs_open_context *ctx;
> @@ -140,7 +141,7 @@ again:
>  		/* Block nfs4_proc_unlck */
>  		mutex_lock(&sp->so_delegreturn_mutex);
>  		seq = raw_seqcount_begin(&sp->so_reclaim_seqcount);
> -		err = nfs4_open_delegation_recall(ctx, state, stateid);
> +		err = nfs4_open_delegation_recall(ctx, state, stateid, type);
>  		if (!err)
>  			err = nfs_delegation_claim_locks(ctx, state, stateid);
>  		if (!err && read_seqcount_retry(&sp->so_reclaim_seqcount, seq))
> @@ -411,7 +412,8 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation
>  	do {
>  		if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags))
>  			break;
> -		err = nfs_delegation_claim_opens(inode, &delegation->stateid);
> +		err = nfs_delegation_claim_opens(inode, &delegation->stateid,
> +				delegation->type);
>  		if (!issync || err != -EAGAIN)
>  			break;
>  		/*
> diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
> index a44829173e57..333063e032f0 100644
> --- a/fs/nfs/delegation.h
> +++ b/fs/nfs/delegation.h
> @@ -54,7 +54,7 @@ void nfs_delegation_reap_unclaimed(struct nfs_client *clp);
>  
>  /* NFSv4 delegation-related procedures */
>  int nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, const nfs4_stateid *stateid, int issync);
> -int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid);
> +int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid, fmode_t type);
>  int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state, const nfs4_stateid *stateid);
>  bool nfs4_copy_delegation_stateid(nfs4_stateid *dst, struct inode *inode, fmode_t flags);
>  
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 693b903b48bd..099a0a83ee8d 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -1127,6 +1127,21 @@ static int nfs4_wait_for_completion_rpc_task(struct rpc_task *task)
>  	return ret;
>  }
>  
> +static bool nfs4_mode_match_open_stateid(struct nfs4_state *state,
> +		fmode_t fmode)
> +{
> +	switch(fmode & (FMODE_READ|FMODE_WRITE)) {
> +	case FMODE_READ|FMODE_WRITE:
> +		return state->n_rdwr != 0;
> +	case FMODE_WRITE:
> +		return state->n_wronly != 0;
> +	case FMODE_READ:
> +		return state->n_rdonly != 0;
> +	}
> +	WARN_ON_ONCE(1);
> +	return false;
> +}
> +
>  static int can_open_cached(struct nfs4_state *state, fmode_t mode, int open_mode)
>  {
>  	int ret = 0;
> @@ -1571,17 +1586,13 @@ static struct nfs4_opendata *nfs4_open_recoverdata_alloc(struct nfs_open_context
>  	return opendata;
>  }
>  
> -static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmode, struct nfs4_state **res)
> +static int nfs4_open_recover_helper(struct nfs4_opendata *opendata,
> +		fmode_t fmode)
>  {
>  	struct nfs4_state *newstate;
>  	int ret;
>  
> -	if ((opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEGATE_CUR ||
> -	     opendata->o_arg.claim == NFS4_OPEN_CLAIM_DELEG_CUR_FH) &&
> -	    (opendata->o_arg.u.delegation_type & fmode) != fmode)
> -		/* This mode can't have been delegated, so we must have
> -		 * a valid open_stateid to cover it - not need to reclaim.
> -		 */
> +	if (!nfs4_mode_match_open_stateid(opendata->state, fmode))
>  		return 0;
>  	opendata->o_arg.open_flags = 0;
>  	opendata->o_arg.fmode = fmode;
> @@ -1597,14 +1608,14 @@ static int nfs4_open_recover_helper(struct nfs4_opendata *opendata, fmode_t fmod
>  	newstate = nfs4_opendata_to_nfs4_state(opendata);
>  	if (IS_ERR(newstate))
>  		return PTR_ERR(newstate);
> +	if (newstate != opendata->state)
> +		ret = -ESTALE;
>  	nfs4_close_state(newstate, fmode);
> -	*res = newstate;
> -	return 0;
> +	return ret;
>  }
>  
>  static int nfs4_open_recover(struct nfs4_opendata *opendata, struct nfs4_state *state)
>  {
> -	struct nfs4_state *newstate;
>  	int ret;
>  
>  	/* Don't trigger recovery in nfs_test_and_clear_all_open_stateid */
> @@ -1615,27 +1626,15 @@ static int nfs4_open_recover(struct nfs4_opendata *opendata, struct nfs4_state *
>  	clear_bit(NFS_DELEGATED_STATE, &state->flags);
>  	clear_bit(NFS_OPEN_STATE, &state->flags);
>  	smp_rmb();
> -	if (state->n_rdwr != 0) {
> -		ret = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE, &newstate);
> -		if (ret != 0)
> -			return ret;
> -		if (newstate != state)
> -			return -ESTALE;
> -	}
> -	if (state->n_wronly != 0) {
> -		ret = nfs4_open_recover_helper(opendata, FMODE_WRITE, &newstate);
> -		if (ret != 0)
> -			return ret;
> -		if (newstate != state)
> -			return -ESTALE;
> -	}
> -	if (state->n_rdonly != 0) {
> -		ret = nfs4_open_recover_helper(opendata, FMODE_READ, &newstate);
> -		if (ret != 0)
> -			return ret;
> -		if (newstate != state)
> -			return -ESTALE;
> -	}
> +	ret = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE);
> +	if (ret != 0)
> +		return ret;
> +	ret = nfs4_open_recover_helper(opendata, FMODE_WRITE);
> +	if (ret != 0)
> +		return ret;
> +	ret = nfs4_open_recover_helper(opendata, FMODE_READ);
> +	if (ret != 0)
> +		return ret;
>  	/*
>  	 * We may have performed cached opens for all three recoveries.
>  	 * Check if we need to update the current stateid.
> @@ -1759,18 +1758,32 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
>  	return err;
>  }
>  
> -int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid)
> +int nfs4_open_delegation_recall(struct nfs_open_context *ctx,
> +		struct nfs4_state *state, const nfs4_stateid *stateid,
> +		fmode_t type)
>  {
>  	struct nfs_server *server = NFS_SERVER(state->inode);
>  	struct nfs4_opendata *opendata;
> -	int err;
> +	int err = 0;
>  
>  	opendata = nfs4_open_recoverdata_alloc(ctx, state,
>  			NFS4_OPEN_CLAIM_DELEG_CUR_FH);
>  	if (IS_ERR(opendata))
>  		return PTR_ERR(opendata);
>  	nfs4_stateid_copy(&opendata->o_arg.u.delegation, stateid);
> -	err = nfs4_open_recover(opendata, state);
> +	clear_bit(NFS_DELEGATED_STATE, &state->flags);
> +	switch (type & (FMODE_READ|FMODE_WRITE)) {
> +	case FMODE_READ|FMODE_WRITE:
> +	case FMODE_WRITE:
> +		err = nfs4_open_recover_helper(opendata, FMODE_READ|FMODE_WRITE);
> +		if (err)
> +			break;
> +		err = nfs4_open_recover_helper(opendata, FMODE_WRITE);
> +		if (err)
> +			break;
> +	case FMODE_READ:
> +		err = nfs4_open_recover_helper(opendata, FMODE_READ);
> +	}
>  	nfs4_opendata_put(opendata);
>  	return nfs4_handle_delegation_recall_error(server, state, stateid, err);
>  }
>