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 fa1a14d..4ba612e 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 581698f..cf17f4b 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 206b460..e9c4f7b 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 9a5d911..8486230 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;
                }