NFSv4: Fix a potential state reclaim deadlock
authorTrond Myklebust <trond.myklebust@hammerspace.com>
Sun, 16 Oct 2022 18:44:31 +0000 (14:44 -0400)
committerAnna Schumaker <Anna.Schumaker@Netapp.com>
Thu, 27 Oct 2022 19:52:10 +0000 (15:52 -0400)
If the server reboots while we are engaged in a delegation return, and
there is a pNFS layout with return-on-close set, then the current code
can end up deadlocking in pnfs_roc() when nfs_inode_set_delegation()
tries to return the old delegation.
Now that delegreturn actually uses its own copy of the stateid, it
should be safe to just always update the delegation stateid in place.

Fixes: 078000d02d57 ("pNFS: We want return-on-close to complete when evicting the inode")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
fs/nfs/delegation.c

index 5c97cad741a73f4e328cb4a277c05e3a1aec26fa..ead8a0e06abf9abdc179601f61ee3a3fe2c4736c 100644 (file)
@@ -228,8 +228,7 @@ again:
  *
  */
 void nfs_inode_reclaim_delegation(struct inode *inode, const struct cred *cred,
-                                 fmode_t type,
-                                 const nfs4_stateid *stateid,
+                                 fmode_t type, const nfs4_stateid *stateid,
                                  unsigned long pagemod_limit)
 {
        struct nfs_delegation *delegation;
@@ -239,25 +238,24 @@ void nfs_inode_reclaim_delegation(struct inode *inode, const struct cred *cred,
        delegation = rcu_dereference(NFS_I(inode)->delegation);
        if (delegation != NULL) {
                spin_lock(&delegation->lock);
-               if (nfs4_is_valid_delegation(delegation, 0)) {
-                       nfs4_stateid_copy(&delegation->stateid, stateid);
-                       delegation->type = type;
-                       delegation->pagemod_limit = pagemod_limit;
-                       oldcred = delegation->cred;
-                       delegation->cred = get_cred(cred);
-                       clear_bit(NFS_DELEGATION_NEED_RECLAIM,
-                                 &delegation->flags);
-                       spin_unlock(&delegation->lock);
-                       rcu_read_unlock();
-                       put_cred(oldcred);
-                       trace_nfs4_reclaim_delegation(inode, type);
-                       return;
-               }
-               /* We appear to have raced with a delegation return. */
+               nfs4_stateid_copy(&delegation->stateid, stateid);
+               delegation->type = type;
+               delegation->pagemod_limit = pagemod_limit;
+               oldcred = delegation->cred;
+               delegation->cred = get_cred(cred);
+               clear_bit(NFS_DELEGATION_NEED_RECLAIM, &delegation->flags);
+               if (test_and_clear_bit(NFS_DELEGATION_REVOKED,
+                                      &delegation->flags))
+                       atomic_long_inc(&nfs_active_delegations);
                spin_unlock(&delegation->lock);
+               rcu_read_unlock();
+               put_cred(oldcred);
+               trace_nfs4_reclaim_delegation(inode, type);
+       } else {
+               rcu_read_unlock();
+               nfs_inode_set_delegation(inode, cred, type, stateid,
+                                        pagemod_limit);
        }
-       rcu_read_unlock();
-       nfs_inode_set_delegation(inode, cred, type, stateid, pagemod_limit);
 }
 
 static int nfs_do_return_delegation(struct inode *inode, struct nfs_delegation *delegation, int issync)