NFS: fix delayed delegation return handling

Rework this code that was totally busted at least as of my most
recent changes.  Introduce a separate list for delayed delegations
so that they can't get lost and don't clutter up the returns list.
Add a missing spin_unlock in the helper marking it as a regular
pending return.

Fixes: 0ebe655bd033 ("NFS: add a separate delegation return list")
Reported-by: Chris Mason <clm@meta.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anna Schumaker <anna.schumaker@oracle.com>

5 files changed, 15 insertions, 22 deletions

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 6b9a65615a51..fd15731cf361 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1063,6 +1063,7 @@ struct nfs_server *nfs_alloc_server(void)
 	spin_lock_init(&server->delegations_lock);
 	INIT_LIST_HEAD(&server->delegations_return);
 	INIT_LIST_HEAD(&server->delegations_lru);
+	INIT_LIST_HEAD(&server->delegations_delayed);
 	INIT_LIST_HEAD(&server->layouts);
 	INIT_LIST_HEAD(&server->state_owners_lru);
 	INIT_LIST_HEAD(&server->ss_copies);
diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index cff49a934c9e..94103f8d3f21 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -336,10 +336,8 @@ nfs_start_delegation_return(struct nfs_inode *nfsi)
 
 	spin_lock(&delegation->lock);
 	if (delegation->inode &&
-	    !test_and_set_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) {
-		clear_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags);
+	    !test_and_set_bit(NFS_DELEGATION_RETURNING, &delegation->flags))
 		return_now = true;
-	}
 	spin_unlock(&delegation->lock);
 
 	if (!return_now) {
@@ -586,8 +584,11 @@ static int nfs_end_delegation_return(struct inode *inode,
 out_return:
 	return nfs_do_return_delegation(inode, delegation, issync);
 delay:
-	set_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags);
-	set_bit(NFS4SERV_DELEGRETURN_DELAYED, &server->delegation_flags);
+	spin_lock(&server->delegations_lock);
+	if (list_empty(&delegation->entry))
+		refcount_inc(&delegation->refcount);
+	list_move_tail(&delegation->entry, &server->delegations_return);
+	spin_unlock(&server->delegations_lock);
 	set_bit(NFS4CLNT_DELEGRETURN_DELAYED, &server->nfs_client->cl_state);
 abort:
 	clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags);
@@ -616,22 +617,16 @@ static int nfs_return_one_delegation(struct nfs_server *server)
 		spin_unlock(&delegation->lock);
 		goto out_put_delegation;
 	}
-	if (test_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags) ||
-	    test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) ||
+	if (test_bit(NFS_DELEGATION_REVOKED, &delegation->flags) ||
 	    test_and_set_bit(NFS_DELEGATION_RETURNING, &delegation->flags)) {
 		spin_unlock(&delegation->lock);
 		goto out_put_inode;
 	}
-	clear_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags);
 	spin_unlock(&delegation->lock);
 
 	nfs_clear_verifier_delegated(inode);
 
 	err = nfs_end_delegation_return(inode, delegation, false);
-	if (err) {
-		nfs_mark_return_delegation(server, delegation);
-		goto out_put_inode;
-	}
 
 out_put_inode:
 	iput(inode);
@@ -708,19 +703,18 @@ static void nfs_delegation_add_lru(struct nfs_server *server,
 
 static bool nfs_server_clear_delayed_delegations(struct nfs_server *server)
 {
-	struct nfs_delegation *d;
 	bool ret = false;
 
-	if (!test_and_clear_bit(NFS4SERV_DELEGRETURN_DELAYED,
-				&server->delegation_flags))
+	if (list_empty_careful(&server->delegations_delayed))
 		return false;
 
 	spin_lock(&server->delegations_lock);
-	list_for_each_entry_rcu(d, &server->delegations_return, entry) {
-		if (test_bit(NFS_DELEGATION_RETURN_DELAYED, &d->flags))
-			clear_bit(NFS_DELEGATION_RETURN_DELAYED, &d->flags);
+	if (!list_empty(&server->delegations_delayed)) {
+		list_splice_tail_init(&server->delegations_delayed,
+				      &server->delegations_return);
 		ret = true;
 	}
+	spin_unlock(&server->delegations_lock);
 
 	return ret;
 }
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index eda39fcb032b..fba4699952b8 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -37,7 +37,6 @@ enum {
 	NFS_DELEGATION_RETURNING,
 	NFS_DELEGATION_REVOKED,
 	NFS_DELEGATION_TEST_EXPIRED,
-	NFS_DELEGATION_RETURN_DELAYED,
 	NFS_DELEGATION_DELEGTIME,
 };
 
diff --git a/fs/nfs/nfs4trace.h b/fs/nfs/nfs4trace.h
index a598d94d4536..c939533b9881 100644
--- a/fs/nfs/nfs4trace.h
+++ b/fs/nfs/nfs4trace.h
@@ -991,8 +991,7 @@ DEFINE_NFS4_SET_DELEGATION_EVENT(nfs4_detach_delegation);
 		{ BIT(NFS_DELEGATION_REFERENCED), "REFERENCED" }, \
 		{ BIT(NFS_DELEGATION_RETURNING), "RETURNING" }, \
 		{ BIT(NFS_DELEGATION_REVOKED), "REVOKED" }, \
-		{ BIT(NFS_DELEGATION_TEST_EXPIRED), "TEST_EXPIRED" }, \
-		{ BIT(NFS_DELEGATION_RETURN_DELAYED), "RETURN_DELAYED" })
+		{ BIT(NFS_DELEGATION_TEST_EXPIRED), "TEST_EXPIRED" })
 
 DECLARE_EVENT_CLASS(nfs4_delegation_event,
 		TP_PROTO(
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 89826c3e15a2..4daee27fa5eb 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -260,6 +260,7 @@ struct nfs_server {
 	spinlock_t		delegations_lock;
 	struct list_head	delegations_return;
 	struct list_head	delegations_lru;
+	struct list_head	delegations_delayed;
 	atomic_long_t		nr_active_delegations;
 	unsigned int		delegation_hash_mask;
 	struct hlist_head	*delegation_hash_table;
@@ -268,7 +269,6 @@ struct nfs_server {
 
 	unsigned long		delegation_flags;
 #define NFS4SERV_DELEGATION_EXPIRED	(1)
-#define NFS4SERV_DELEGRETURN_DELAYED	(2)
 	unsigned long		delegation_gen;
 	unsigned long		mig_gen;
 	unsigned long		mig_status;