NFS: Fix up TEST_STATEID and FREE_STATEID return code handling

The TEST_STATEID and FREE_STATEID operations can return
-NFS4ERR_BAD_STATEID, -NFS4ERR_OLD_STATEID, or -NFS4ERR_DEADSESSION.

nfs41_{test,free}_stateid() should not pass these errors to
nfs4_handle_exception() during state recovery, since that will
recursively kick off state recovery again, resulting in a deadlock.

In particular, when the TEST_STATEID operation returns NFS4_OK,
res.status can contain one of these errors.  _nfs41_test_stateid()
replaces NFS4_OK with the value in res.status, which is then returned
to callers.

But res.status is not passed through nfs4_stat_to_errno(), and thus is
a positive NFS4ERR value.  Currently callers are only interested in
!NFS4_OK, and nfs4_handle_exception() ignores positive values.

Thus the res.status values are currently ignored by
nfs4_handle_exception() and won't cause the deadlock above.  Thanks to
this missing negative, it is only when these operations fail (which
is very rare) that a deadlock can occur.

Bryan agrees the original intent was to return res.status as a
negative NFS4ERR value to callers of nfs41_test_stateid().

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 006e98da730a6691225d21e4ae73183699235fce..af3abf957f2caad66d51d6cd256cb971f308478a 100644 (file)
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6578,10 +6578,9 @@ static int _nfs41_test_stateid(struct nfs_server *server, nfs4_stateid *stateid)
 
        nfs41_init_sequence(&args.seq_args, &res.seq_res, 0);
        status = nfs4_call_sync_sequence(server->client, server, &msg, &args.seq_args, &res.seq_res, 1);
-
-       if (status == NFS_OK)
-               return res.status;
-       return status;
+       if (status != NFS_OK)
+               return status;
+       return -res.status;
 }
 
 static int nfs41_test_stateid(struct nfs_server *server, nfs4_stateid *stateid)
@@ -6589,9 +6588,10 @@ static int nfs41_test_stateid(struct nfs_server *server, nfs4_stateid *stateid)
        struct nfs4_exception exception = { };
        int err;
        do {
-               err = nfs4_handle_exception(server,
-                               _nfs41_test_stateid(server, stateid),
-                               &exception);
+               err = _nfs41_test_stateid(server, stateid);
+               if (err != -NFS4ERR_DELAY)
+                       break;
+               nfs4_handle_exception(server, err, &exception);
        } while (exception.retry);
        return err;
 }
@@ -6609,7 +6609,8 @@ static int _nfs4_free_stateid(struct nfs_server *server, nfs4_stateid *stateid)
        };
 
        nfs41_init_sequence(&args.seq_args, &res.seq_res, 0);
-       return nfs4_call_sync_sequence(server->client, server, &msg, &args.seq_args, &res.seq_res, 1);
+       return nfs4_call_sync_sequence(server->client, server, &msg,
+                                        &args.seq_args, &res.seq_res, 1);
 }
 
 static int nfs41_free_stateid(struct nfs_server *server, nfs4_stateid *stateid)
@@ -6617,9 +6618,10 @@ static int nfs41_free_stateid(struct nfs_server *server, nfs4_stateid *stateid)
        struct nfs4_exception exception = { };
        int err;
        do {
-               err = nfs4_handle_exception(server,
-                               _nfs4_free_stateid(server, stateid),
-                               &exception);
+               err = _nfs4_free_stateid(server, stateid);
+               if (err != -NFS4ERR_DELAY)
+                       break;
+               nfs4_handle_exception(server, err, &exception);
        } while (exception.retry);
        return err;
 }