Linux NFS development
 help / color / mirror / Atom feed
* delayed delegation return handling fix
@ 2026-01-28  4:46 Christoph Hellwig
  2026-01-28  4:46 ` [PATCH 1/7] NFS: return void from nfs4_inode_make_writeable Christoph Hellwig
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Christoph Hellwig @ 2026-01-28  4:46 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: Chris Mason, linux-nfs

Hi all,

Chris Mason reported issues with the handling of delayed delegation
returns in the resent "add a LRU for delegations" series.

This series fixes that by not only doing the proper unlock, but also
by adding a new list dedicated to the delayed returned delegations.

Note that I could not trigger the delayed delegation handling naturally
and had to add crude error injection to force it.

Diffstat:
 fs/nfs/client.c           |    1 
 fs/nfs/delegation.c       |   94 ++++++++++++++++++----------------------------
 fs/nfs/delegation.h       |    5 --
 fs/nfs/nfs3proc.c         |    3 -
 fs/nfs/nfs4trace.h        |    3 -
 fs/nfs/proc.c             |    3 -
 include/linux/nfs_fs_sb.h |    2 
 include/linux/nfs_xdr.h   |    2 
 8 files changed, 45 insertions(+), 68 deletions(-)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/7] NFS: return void from nfs4_inode_make_writeable
  2026-01-28  4:46 delayed delegation return handling fix Christoph Hellwig
@ 2026-01-28  4:46 ` Christoph Hellwig
  2026-01-28  4:46 ` [PATCH 2/7] NFS: return void from ->return_delegation Christoph Hellwig
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2026-01-28  4:46 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: Chris Mason, linux-nfs

None of the callers checks the return value, so drop it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/delegation.c | 10 +++-------
 fs/nfs/delegation.h |  2 +-
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 4d5f1f3162b0..c77c7b2d5877 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -912,23 +912,19 @@ void nfs4_inode_return_delegation_on_close(struct inode *inode)
  * @inode: pointer to inode
  *
  * Make the inode writeable by returning the delegation if necessary
- *
- * Returns zero on success, or a negative errno value.
  */
-int nfs4_inode_make_writeable(struct inode *inode)
+void nfs4_inode_make_writeable(struct inode *inode)
 {
 	struct nfs_delegation *delegation;
-	int error = 0;
 
 	delegation = nfs4_get_valid_delegation(inode);
 	if (!delegation)
-		return 0;
+		return;
 
 	if (!nfs4_has_session(NFS_SERVER(inode)->nfs_client) ||
 	    !(delegation->type & FMODE_WRITE))
-		error = nfs4_inode_return_delegation(inode);
+		nfs4_inode_return_delegation(inode);
 	nfs_put_delegation(delegation);
-	return error;
 }
 
 static void
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index a6733f034442..d30f19a28077 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -87,7 +87,7 @@ int nfs4_check_delegation(struct inode *inode, fmode_t type);
 bool nfs4_delegation_flush_on_close(const struct inode *inode);
 void nfs_inode_find_delegation_state_and_recover(struct inode *inode,
 		const nfs4_stateid *stateid);
-int nfs4_inode_make_writeable(struct inode *inode);
+void nfs4_inode_make_writeable(struct inode *inode);
 
 #endif
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/7] NFS: return void from ->return_delegation
  2026-01-28  4:46 delayed delegation return handling fix Christoph Hellwig
  2026-01-28  4:46 ` [PATCH 1/7] NFS: return void from nfs4_inode_make_writeable Christoph Hellwig
@ 2026-01-28  4:46 ` Christoph Hellwig
  2026-01-28  4:46 ` [PATCH 3/7] NFS: use bool for the issync argument to nfs_end_delegation_return Christoph Hellwig
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2026-01-28  4:46 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: Chris Mason, linux-nfs

The caller doesn't check the return value, so drop it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/delegation.c     | 8 +++-----
 fs/nfs/delegation.h     | 2 +-
 fs/nfs/nfs3proc.c       | 3 +--
 fs/nfs/proc.c           | 3 +--
 include/linux/nfs_xdr.h | 2 +-
 5 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index c77c7b2d5877..fe1f57ec326c 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -814,23 +814,21 @@ void nfs_inode_evict_delegation(struct inode *inode)
  *
  * Returns zero on success, or a negative errno value.
  */
-int nfs4_inode_return_delegation(struct inode *inode)
+void nfs4_inode_return_delegation(struct inode *inode)
 {
 	struct nfs_inode *nfsi = NFS_I(inode);
 	struct nfs_delegation *delegation;
-	int err;
 
 	delegation = nfs_start_delegation_return(nfsi);
 	if (!delegation)
-		return 0;
+		return;
 
 	/* Synchronous recall of any application leases */
 	break_lease(inode, O_WRONLY | O_RDWR);
 	if (S_ISREG(inode->i_mode))
 		nfs_wb_all(inode);
-	err = nfs_end_delegation_return(inode, delegation, 1);
+	nfs_end_delegation_return(inode, delegation, 1);
 	nfs_put_delegation(delegation);
-	return err;
 }
 
 /**
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index d30f19a28077..eda39fcb032b 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -47,7 +47,7 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred,
 void nfs_inode_reclaim_delegation(struct inode *inode, const struct cred *cred,
 				  fmode_t type, const nfs4_stateid *stateid,
 				  unsigned long pagemod_limit, u32 deleg_type);
-int nfs4_inode_return_delegation(struct inode *inode);
+void nfs4_inode_return_delegation(struct inode *inode);
 void nfs4_inode_return_delegation_on_close(struct inode *inode);
 void nfs4_inode_set_return_delegation_on_close(struct inode *inode);
 int nfs_async_inode_return_delegation(struct inode *inode, const nfs4_stateid *stateid);
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index 1181f9cc6dbd..d3d2fbeba89d 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -1027,11 +1027,10 @@ static int nfs3_have_delegation(struct inode *inode, fmode_t type, int flags)
 	return 0;
 }
 
-static int nfs3_return_delegation(struct inode *inode)
+static void nfs3_return_delegation(struct inode *inode)
 {
 	if (S_ISREG(inode->i_mode))
 		nfs_wb_all(inode);
-	return 0;
 }
 
 static const struct inode_operations nfs3_dir_inode_operations = {
diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
index 39df80e4ae6f..0e440ebf5335 100644
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -697,11 +697,10 @@ static int nfs_have_delegation(struct inode *inode, fmode_t type, int flags)
 	return 0;
 }
 
-static int nfs_return_delegation(struct inode *inode)
+static void nfs_return_delegation(struct inode *inode)
 {
 	if (S_ISREG(inode->i_mode))
 		nfs_wb_all(inode);
-	return 0;
 }
 
 static const struct inode_operations nfs_dir_inode_operations = {
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 79fe2dfb470f..1c121f6f64c3 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1849,7 +1849,7 @@ struct nfs_rpc_ops {
 				struct iattr *iattr,
 				int *);
 	int (*have_delegation)(struct inode *, fmode_t, int);
-	int (*return_delegation)(struct inode *);
+	void (*return_delegation)(struct inode *);
 	struct nfs_client *(*alloc_client) (const struct nfs_client_initdata *);
 	struct nfs_client *(*init_client) (struct nfs_client *,
 				const struct nfs_client_initdata *);
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/7] NFS: use bool for the issync argument to nfs_end_delegation_return
  2026-01-28  4:46 delayed delegation return handling fix Christoph Hellwig
  2026-01-28  4:46 ` [PATCH 1/7] NFS: return void from nfs4_inode_make_writeable Christoph Hellwig
  2026-01-28  4:46 ` [PATCH 2/7] NFS: return void from ->return_delegation Christoph Hellwig
@ 2026-01-28  4:46 ` Christoph Hellwig
  2026-01-28  4:46 ` [PATCH 4/7] NFS: remove the delegation == NULL check in nfs_end_delegation_return Christoph Hellwig
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2026-01-28  4:46 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: Chris Mason, linux-nfs

Replace the integer used as boolean with a bool type, and tidy up
the prototype and top of function comment.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/delegation.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index fe1f57ec326c..d95a6e9876f1 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -560,9 +560,12 @@ int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred,
 }
 
 /*
- * Basic procedure for returning a delegation to the server
+ * Basic procedure for returning a delegation to the server.
+ * If @issync is set, wait until state recovery has finished.  Otherwise
+ * return -EAGAIN to the caller if we need more time.
  */
-static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation *delegation, int issync)
+static int nfs_end_delegation_return(struct inode *inode,
+		struct nfs_delegation *delegation, bool issync)
 {
 	struct nfs_server *server = NFS_SERVER(inode);
 	unsigned int mode = O_WRONLY | O_RDWR;
@@ -635,7 +638,7 @@ static int nfs_return_one_delegation(struct nfs_server *server)
 
 	nfs_clear_verifier_delegated(inode);
 
-	err = nfs_end_delegation_return(inode, delegation, 0);
+	err = nfs_end_delegation_return(inode, delegation, false);
 	if (err) {
 		nfs_mark_return_delegation(server, delegation);
 		goto out_put_inode;
@@ -827,7 +830,7 @@ void nfs4_inode_return_delegation(struct inode *inode)
 	break_lease(inode, O_WRONLY | O_RDWR);
 	if (S_ISREG(inode->i_mode))
 		nfs_wb_all(inode);
-	nfs_end_delegation_return(inode, delegation, 1);
+	nfs_end_delegation_return(inode, delegation, true);
 	nfs_put_delegation(delegation);
 }
 
@@ -863,7 +866,7 @@ void nfs4_inode_set_return_delegation_on_close(struct inode *inode)
 	spin_unlock(&delegation->lock);
 	if (return_now) {
 		nfs_clear_verifier_delegated(inode);
-		nfs_end_delegation_return(inode, delegation, 0);
+		nfs_end_delegation_return(inode, delegation, false);
 	}
 	nfs_put_delegation(delegation);
 }
@@ -898,7 +901,7 @@ void nfs4_inode_return_delegation_on_close(struct inode *inode)
 
 	if (return_now) {
 		nfs_clear_verifier_delegated(inode);
-		nfs_end_delegation_return(inode, delegation, 0);
+		nfs_end_delegation_return(inode, delegation, false);
 	} else {
 		nfs_delegation_add_lru(server, delegation);
 	}
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/7] NFS: remove the delegation == NULL check in nfs_end_delegation_return
  2026-01-28  4:46 delayed delegation return handling fix Christoph Hellwig
                   ` (2 preceding siblings ...)
  2026-01-28  4:46 ` [PATCH 3/7] NFS: use bool for the issync argument to nfs_end_delegation_return Christoph Hellwig
@ 2026-01-28  4:46 ` Christoph Hellwig
  2026-01-28  4:46 ` [PATCH 5/7] NFS: fold nfs_abort_delegation_return into nfs_end_delegation_return Christoph Hellwig
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2026-01-28  4:46 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: Chris Mason, linux-nfs

All callers now pass a non-NULL delegation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/delegation.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index d95a6e9876f1..32803963b5d7 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -571,9 +571,6 @@ static int nfs_end_delegation_return(struct inode *inode,
 	unsigned int mode = O_WRONLY | O_RDWR;
 	int err = 0;
 
-	if (delegation == NULL)
-		return 0;
-
 	/* Directory delegations don't require any state recovery */
 	if (!S_ISREG(inode->i_mode))
 		goto out_return;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/7] NFS: fold nfs_abort_delegation_return into nfs_end_delegation_return
  2026-01-28  4:46 delayed delegation return handling fix Christoph Hellwig
                   ` (3 preceding siblings ...)
  2026-01-28  4:46 ` [PATCH 4/7] NFS: remove the delegation == NULL check in nfs_end_delegation_return Christoph Hellwig
@ 2026-01-28  4:46 ` Christoph Hellwig
  2026-01-28  4:46 ` [PATCH 6/7] NFS: simplify error handling in nfs_end_delegation_return Christoph Hellwig
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2026-01-28  4:46 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: Chris Mason, linux-nfs

This will allow to simplify the error handling flow.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/delegation.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 32803963b5d7..cf90aa7f922a 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -350,21 +350,6 @@ nfs_start_delegation_return(struct nfs_inode *nfsi)
 	return delegation;
 }
 
-static void nfs_abort_delegation_return(struct nfs_delegation *delegation,
-					struct nfs_server *server, int err)
-{
-	spin_lock(&delegation->lock);
-	clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags);
-	if (err == -EAGAIN) {
-		set_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags);
-		set_bit(NFS4SERV_DELEGRETURN_DELAYED,
-			&server->delegation_flags);
-		set_bit(NFS4CLNT_DELEGRETURN_DELAYED,
-			&server->nfs_client->cl_state);
-	}
-	spin_unlock(&delegation->lock);
-}
-
 static bool
 nfs_detach_delegations_locked(struct nfs_inode *nfsi,
 		struct nfs_delegation *delegation,
@@ -593,13 +578,23 @@ static int nfs_end_delegation_return(struct inode *inode,
 		err = nfs4_wait_clnt_recover(server->nfs_client);
 	}
 
-	if (err) {
-		nfs_abort_delegation_return(delegation, server, err);
-		return err;
-	}
+	if (err)
+		goto abort;
 
 out_return:
 	return nfs_do_return_delegation(inode, delegation, issync);
+abort:
+	spin_lock(&delegation->lock);
+	clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags);
+	if (err == -EAGAIN) {
+		set_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags);
+		set_bit(NFS4SERV_DELEGRETURN_DELAYED,
+			&server->delegation_flags);
+		set_bit(NFS4CLNT_DELEGRETURN_DELAYED,
+			&server->nfs_client->cl_state);
+	}
+	spin_unlock(&delegation->lock);
+	return err;
 }
 
 static int nfs_return_one_delegation(struct nfs_server *server)
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/7] NFS: simplify error handling in nfs_end_delegation_return
  2026-01-28  4:46 delayed delegation return handling fix Christoph Hellwig
                   ` (4 preceding siblings ...)
  2026-01-28  4:46 ` [PATCH 5/7] NFS: fold nfs_abort_delegation_return into nfs_end_delegation_return Christoph Hellwig
@ 2026-01-28  4:46 ` Christoph Hellwig
  2026-01-28  4:46 ` [PATCH 7/7] NFS: fix delayed delegation return handling Christoph Hellwig
  2026-01-28 19:15 ` delayed delegation return handling fix Chris Mason
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2026-01-28  4:46 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: Chris Mason, linux-nfs

Drop the pointless delegation->lock held over setting multiple
atomic bits in different structures, and use separate labels
for the delay vs abort cases.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/delegation.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index cf90aa7f922a..cff49a934c9e 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -570,30 +570,27 @@ static int nfs_end_delegation_return(struct inode *inode,
 			break;
 		err = nfs_delegation_claim_opens(inode, &delegation->stateid,
 				delegation->type);
-		if (!issync || err != -EAGAIN)
+		if (!err)
 			break;
+		if (err != -EAGAIN)
+			goto abort;
+		if (!issync)
+			goto delay;
+
 		/*
 		 * Guard against state recovery
 		 */
 		err = nfs4_wait_clnt_recover(server->nfs_client);
 	}
 
-	if (err)
-		goto abort;
-
 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);
+	set_bit(NFS4CLNT_DELEGRETURN_DELAYED, &server->nfs_client->cl_state);
 abort:
-	spin_lock(&delegation->lock);
 	clear_bit(NFS_DELEGATION_RETURNING, &delegation->flags);
-	if (err == -EAGAIN) {
-		set_bit(NFS_DELEGATION_RETURN_DELAYED, &delegation->flags);
-		set_bit(NFS4SERV_DELEGRETURN_DELAYED,
-			&server->delegation_flags);
-		set_bit(NFS4CLNT_DELEGRETURN_DELAYED,
-			&server->nfs_client->cl_state);
-	}
-	spin_unlock(&delegation->lock);
 	return err;
 }
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 7/7] NFS: fix delayed delegation return handling
  2026-01-28  4:46 delayed delegation return handling fix Christoph Hellwig
                   ` (5 preceding siblings ...)
  2026-01-28  4:46 ` [PATCH 6/7] NFS: simplify error handling in nfs_end_delegation_return Christoph Hellwig
@ 2026-01-28  4:46 ` Christoph Hellwig
  2026-01-28 19:15 ` delayed delegation return handling fix Chris Mason
  7 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2026-01-28  4:46 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: Chris Mason, linux-nfs

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>
---
 fs/nfs/client.c           |  1 +
 fs/nfs/delegation.c       | 30 ++++++++++++------------------
 fs/nfs/delegation.h       |  1 -
 fs/nfs/nfs4trace.h        |  3 +--
 include/linux/nfs_fs_sb.h |  2 +-
 5 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 62aece00f810..4c0bba1488cc 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 8ff6396bc206..bf44aabb844c 100644
--- a/fs/nfs/nfs4trace.h
+++ b/fs/nfs/nfs4trace.h
@@ -994,8 +994,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 bb13a294b69e..cfda0ff0174d 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -262,6 +262,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;
@@ -270,7 +271,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;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: delayed delegation return handling fix
  2026-01-28  4:46 delayed delegation return handling fix Christoph Hellwig
                   ` (6 preceding siblings ...)
  2026-01-28  4:46 ` [PATCH 7/7] NFS: fix delayed delegation return handling Christoph Hellwig
@ 2026-01-28 19:15 ` Chris Mason
  7 siblings, 0 replies; 9+ messages in thread
From: Chris Mason @ 2026-01-28 19:15 UTC (permalink / raw)
  To: Christoph Hellwig, Trond Myklebust, Anna Schumaker; +Cc: linux-nfs



On 1/27/26 11:46 PM, Christoph Hellwig wrote:
> Hi all,
> 
> Chris Mason reported issues with the handling of delayed delegation
> returns in the resent "add a LRU for delegations" series.
> 
> This series fixes that by not only doing the proper unlock, but also
> by adding a new list dedicated to the delayed returned delegations.
> 
> Note that I could not trigger the delayed delegation handling naturally
> and had to add crude error injection to force it.
> 

Thanks Christoph, I reran this through the prompts and didn't find any
new issues.

-chris


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-01-28 19:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-28  4:46 delayed delegation return handling fix Christoph Hellwig
2026-01-28  4:46 ` [PATCH 1/7] NFS: return void from nfs4_inode_make_writeable Christoph Hellwig
2026-01-28  4:46 ` [PATCH 2/7] NFS: return void from ->return_delegation Christoph Hellwig
2026-01-28  4:46 ` [PATCH 3/7] NFS: use bool for the issync argument to nfs_end_delegation_return Christoph Hellwig
2026-01-28  4:46 ` [PATCH 4/7] NFS: remove the delegation == NULL check in nfs_end_delegation_return Christoph Hellwig
2026-01-28  4:46 ` [PATCH 5/7] NFS: fold nfs_abort_delegation_return into nfs_end_delegation_return Christoph Hellwig
2026-01-28  4:46 ` [PATCH 6/7] NFS: simplify error handling in nfs_end_delegation_return Christoph Hellwig
2026-01-28  4:46 ` [PATCH 7/7] NFS: fix delayed delegation return handling Christoph Hellwig
2026-01-28 19:15 ` delayed delegation return handling fix Chris Mason

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox