NFSv4: nfs4_do_open() is incorrectly triggering state recovery

We're seeing spurious calls to nfs4_schedule_stateid_recovery() from
nfs4_do_open() in situations where there is no trigger coming from the
server.
In theory the code path being triggered is supposed to notice that state
recovery happened while we were processing the open call result from the
server, before the open stateid is published. However in the years since
that code was added, we've also added the 'session draining' mechanism,
which ensures that the state recovery will wait until all the session
slots have been returned. In nfs4_do_open() the session slot is only
returned on exit of the function, so we don't need the legacy mechanism.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index fa1a14def45cea2fc485b598831becc9e7497992..4ba612e78da87b29182349184db2532cc0fe5d0d 100644 (file)
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -181,7 +181,6 @@ static int nfs_delegation_claim_opens(struct inode *inode,
        struct nfs_open_context *ctx;
        struct nfs4_state_owner *sp;
        struct nfs4_state *state;
-       unsigned int seq;
        int err;
 
 again:
@@ -202,12 +201,9 @@ again:
                sp = state->owner;
                /* 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);
                if (!err)
                        err = nfs_delegation_claim_locks(state, stateid);
-               if (!err && read_seqcount_retry(&sp->so_reclaim_seqcount, seq))
-                       err = -EAGAIN;
                mutex_unlock(&sp->so_delegreturn_mutex);
                put_nfs_open_context(ctx);
                if (err != 0)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 581698f1b7b2441025d5421b3b00b4fba42b2ab8..cf17f4bc9815fdcb7362f9c616d3fbba40064ca5 100644 (file)
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -120,7 +120,6 @@ struct nfs4_state_owner {
        unsigned long        so_flags;
        struct list_head     so_states;
        struct nfs_seqid_counter so_seqid;
-       seqcount_spinlock_t  so_reclaim_seqcount;
        struct mutex         so_delegreturn_mutex;
 };
 

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 206b4607b29ad5c18c0eb5627235d791cffefeb1..e9c4f7b9d44e97b04eaa3dab4b7abc3e842dbfa7 100644 (file)
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3069,10 +3069,8 @@ static int _nfs4_open_and_get_state(struct nfs4_opendata *opendata,
        fmode_t acc_mode = _nfs4_ctx_to_accessmode(ctx);
        struct inode *dir = d_inode(opendata->dir);
        unsigned long dir_verifier;
-       unsigned int seq;
        int ret;
 
-       seq = raw_seqcount_begin(&sp->so_reclaim_seqcount);
        dir_verifier = nfs_save_change_attribute(dir);
 
        ret = _nfs4_proc_open(opendata, ctx);
@@ -3125,11 +3123,8 @@ static int _nfs4_open_and_get_state(struct nfs4_opendata *opendata,
        if (ret != 0)
                goto out;
 
-       if (d_inode(dentry) == state->inode) {
+       if (d_inode(dentry) == state->inode)
                nfs_inode_attach_open_context(ctx);
-               if (read_seqcount_retry(&sp->so_reclaim_seqcount, seq))
-                       nfs4_schedule_stateid_recovery(server, state);
-       }
 
 out:
        if (!opendata->cancelled) {

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 9a5d911a7edc77cae53cac45aa43f196b32caf20..8486230d99e12e865dd0d7c823a59466a607b16f 100644 (file)
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -513,7 +513,6 @@ nfs4_alloc_state_owner(struct nfs_server *server,
        nfs4_init_seqid_counter(&sp->so_seqid);
        atomic_set(&sp->so_count, 1);
        INIT_LIST_HEAD(&sp->so_lru);
-       seqcount_spinlock_init(&sp->so_reclaim_seqcount, &sp->so_lock);
        mutex_init(&sp->so_delegreturn_mutex);
        return sp;
 }
@@ -1667,7 +1666,6 @@ static int nfs4_reclaim_open_state(struct nfs4_state_owner *sp,
         * server that doesn't support a grace period.
         */
        spin_lock(&sp->so_lock);
-       raw_write_seqcount_begin(&sp->so_reclaim_seqcount);
 restart:
        list_for_each_entry(state, &sp->so_states, open_states) {
                if (!test_and_clear_bit(ops->state_flag_bit, &state->flags))
@@ -1735,7 +1733,6 @@ restart:
                spin_lock(&sp->so_lock);
                goto restart;
        }
-       raw_write_seqcount_end(&sp->so_reclaim_seqcount);
        spin_unlock(&sp->so_lock);
 #ifdef CONFIG_NFS_V4_2
        if (found_ssc_copy_state)
@@ -1745,7 +1742,6 @@ restart:
 out_err:
        nfs4_put_open_state(state);
        spin_lock(&sp->so_lock);
-       raw_write_seqcount_end(&sp->so_reclaim_seqcount);
        spin_unlock(&sp->so_lock);
        return status;
 }
@@ -1928,9 +1924,12 @@ static int nfs4_do_reclaim(struct nfs_client *clp, const struct nfs4_state_recov
        struct nfs_server *server;
        struct rb_node *pos;
        LIST_HEAD(freeme);
-       int status = 0;
        int lost_locks = 0;
+       int status;
 
+       status = nfs4_begin_drain_session(clp);
+       if (status < 0)
+               return status;
 restart:
        rcu_read_lock();
        list_for_each_entry_rcu(server, &clp->cl_superblocks, client_link) {
@@ -2694,6 +2693,9 @@ static void nfs4_state_manager(struct nfs_client *clp)
                /* Detect expired delegations... */
                if (test_and_clear_bit(NFS4CLNT_DELEGATION_EXPIRED, &clp->cl_state)) {
                        section = "detect expired delegations";
+                       status = nfs4_begin_drain_session(clp);
+                       if (status < 0)
+                               goto out_error;
                        nfs_reap_expired_delegations(clp);
                        continue;
                }