From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8CCB51FC0FC; Tue, 28 Oct 2025 00:40:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1761612024; cv=none; b=dtnb1IdE5/+/xE/MpuaiOs8PzeSEnPjc6Gmk/D2758BrIFGQlxE06RlFwLiSeY0COJFfAgzTW4F++F/HVO6fyj4u4BhJJU088Dbg9w3OVm1S0kjKi3w8pbNPHl52E6Wg7mCN8PCNWWZj3QF7MjOf6PYopzx1MCBC88h3MuxCLG0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1761612024; c=relaxed/simple; bh=dvgAA+ZxPB4+pUIkRpKcb0srDm0jX/Sc0B5IdALDge0=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=Fme0X1HAf/lpI6V3ftcs8vV1Z6PJmxyF2dseXoLgmt1shF4GcZeCLs3PyyEh0e+Y4vumbqE0TfhwoW4gprctB4rwaGosW70b3XJy9bSactt49s894qdjWk7BvJAdOCrcJ4+X5/tlEm433nColSAeo3SHD+SjaARWjxTzStCDVRo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=l1HTZmW/; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="l1HTZmW/" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6D928C116B1; Tue, 28 Oct 2025 00:40:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1761612024; bh=dvgAA+ZxPB4+pUIkRpKcb0srDm0jX/Sc0B5IdALDge0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=l1HTZmW/bXHKuetlne6DBWZ3VnFF2ujb5AbvBeesPyQSSsaF8a1/5LCZP7KV3GJme ro6peLCcJvuSAqT1OZ5OGjrZYzEaEl3GdMmTcy9Y5Je0Skw6VeAlFlITViHfAvpK5b /9+JCx0Jt418tllh9KoJDo5q56nWbPZnKhT2Ju4mRwMATwVhuHN7IVqL18Fd0nR+PT t7uN8hvVgryWU0tEIp1aAHBeMyD1V2l8DrPB1E4zKhRo82eUvsJbZSHCG5r8JI/wq4 9Dyx4/Bg4Jhes8JglbRQ5Wb8skbS6Ry/KaRKifFECpkXAcMbXzOomFPTTg0CU2ZvA6 pWU9IX/sb9Gag== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Joshua Watt , Benjamin Coddington , Anna Schumaker , Sasha Levin , trondmy@kernel.org, anna@kernel.org, linux-nfs@vger.kernel.org Subject: [PATCH AUTOSEL 6.17-6.12] NFS4: Apply delay_retrans to async operations Date: Mon, 27 Oct 2025 20:39:03 -0400 Message-ID: <20251028003940.884625-19-sashal@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20251028003940.884625-1-sashal@kernel.org> References: <20251028003940.884625-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: patches@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.17.5 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From: Joshua Watt [ Upstream commit 7a84394f02ab1985ebbe0a8d6f6d69bd040de4b3 ] The setting of delay_retrans is applied to synchronous RPC operations because the retransmit count is stored in same struct nfs4_exception that is passed each time an error is checked. However, for asynchronous operations (READ, WRITE, LOCKU, CLOSE, DELEGRETURN), a new struct nfs4_exception is made on the stack each time the task callback is invoked. This means that the retransmit count is always zero and thus delay_retrans never takes effect. Apply delay_retrans to these operations by tracking and updating their retransmit count. Change-Id: Ieb33e046c2b277cb979caa3faca7f52faf0568c9 Signed-off-by: Joshua Watt Reviewed-by: Benjamin Coddington Signed-off-by: Anna Schumaker Signed-off-by: Sasha Levin --- LLM Generated explanations, may be completely bogus: ## BACKPORT RECOMMENDATION: **YES** ## Extensive Analysis ### 1. Code Changes Analysis The commit adds a `retrans` field to 5 data structures and propagates it through async operation callbacks: **Data structures modified:** - `struct nfs4_closedata` (fs/nfs/nfs4proc.c:3639) - `struct nfs4_delegreturndata` (fs/nfs/nfs4proc.c:6735) - `struct nfs4_unlockdata` (fs/nfs/nfs4proc.c:7105) - `struct nfs_pgio_header` (include/linux/nfs_xdr.h:1661) **Functions modified:** - `nfs4_close_done()` - CLOSE operation callback - `nfs4_delegreturn_done()` - DELEGRETURN operation callback - `nfs4_locku_done()` - LOCKU operation callback - `nfs4_read_done_cb()` - READ operation callback - `nfs4_write_done_cb()` - WRITE operation callback Each modification follows the same pattern: 1. Initialize `exception.retrans` from persistent storage (e.g., `calldata->retrans`) 2. Call `nfs4_async_handle_exception()` which increments retrans via `nfs4_exception_should_retrans()` 3. Save updated retrans back to persistent storage ### 2. Semantic Analysis Tools Used **mcp__semcode__find_function**: Located all 5 modified async callback functions and examined their implementations to understand the callback pattern. **mcp__semcode__find_type**: Examined `struct nfs4_exception` (fs/nfs/nfs4_fs.h:206) confirming it already contains the `retrans` field in v6.10+. **mcp__semcode__find_callers**: Verified that: - `nfs4_read_done_cb` is called by `nfs4_read_done` (fs/nfs/nfs4proc.c:5638) - `nfs4_write_done_cb` is called by `nfs4_write_done` (fs/nfs/nfs4proc.c:5740) - Other callbacks are registered via `rpc_call_ops` structures (e.g., `nfs4_close_ops`) **mcp__semcode__grep_functions**: Found `nfs4_exception_should_retrans()` (fs/nfs/nfs4proc.c:628-636) which implements the delay_retrans logic: ```c if (server->flags & NFS_MOUNT_SOFTERR && nfs_delay_retrans >= 0) { if (exception->retrans++ >= (unsigned short)nfs_delay_retrans) return -EAGAIN; } ``` ### 3. Impact Scope Assessment **User-space reachability**: CRITICAL - All affected operations are directly triggered by userspace: - **READ/WRITE**: Every file read/write operation (most common NFS operations) - **CLOSE**: Every file close operation - **LOCKU**: Every file unlock operation - **DELEGRETURN**: Delegation returns during file operations **Call graph analysis**: The async operations form the core I/O path: - User calls `read()`/`write()` → VFS → NFS client → `nfs4_read_done_cb()`/`nfs4_write_done_cb()` - User calls `close()` → VFS → NFS client → `nfs4_close_done()` **Impact severity**: HIGH - Without this fix, the `delay_retrans` parameter (introduced in v6.10 via commit 5b9d31ae1c92) is **completely non-functional** for async operations - Systems using 'softerr' mounts with `nfs.delay_retrans` configured experience infinite retry loops on NFS4ERR_DELAY - This causes knfsd server thread busy-waiting and client hangs - The retrans counter is reset to 0 on every callback invocation because a new `struct nfs4_exception` is allocated on the stack ### 4. Dependencies and Backport Risk **Dependencies**: - Requires commit 5b9d31ae1c92 "NFSv4: Add a parameter to limit the number of retries after NFS4ERR_DELAY" (merged in v6.10) - All required struct fields (`nfs4_exception.retrans`) exist in v6.10+ **Backport applicability**: - **v6.10 through v6.17**: YES - this is the bug window, fix is highly applicable - **v6.9 and earlier**: NO - delay_retrans feature doesn't exist **Risk assessment**: VERY LOW - Changes are purely additive (new field, value propagation) - No behavioral changes to existing code paths - No ABI/API changes visible to userspace - Semantic diff analysis shows no complex interactions - The fix enables intended functionality that was documented but broken ### 5. Stable Tree Compliance ✅ **Bug fix**: Yes - fixes broken delay_retrans for async operations ✅ **Important impact**: Yes - affects common I/O operations (read/write/close) ✅ **Small and contained**: Yes - adds one field to 5 structs, simple value propagation ✅ **No new features**: Correct - just makes existing feature work properly ✅ **No architectural changes**: Correct - preserves existing design ✅ **Clear rationale**: Commit message clearly explains the bug and fix **Reviewed-by tag**: Present (Benjamin Coddington from Red Hat) ### 6. Historical Context - delay_retrans introduced: v6.10 (Sept 2023, commit 5b9d31ae1c92) - Bug existed: v6.10 through v6.17 (8 kernel versions, ~15 months) - Fix merged: v6.18-rc2 (Oct 2025, commit 7a84394f02ab1) - Module parameter: `nfs.delay_retrans` (configurable, default -1 = disabled) ### Conclusion This is a **clear backport candidate** for stable kernels v6.10 through v6.17. The commit fixes a real bug that completely breaks the delay_retrans feature for the most common NFS operations (READ, WRITE, CLOSE). Without this fix, users configuring `nfs.delay_retrans` on 'softerr' mounts get no protection against infinite NFS4ERR_DELAY retry loops for async operations, defeating the entire purpose of the feature and potentially causing production issues (server thread starvation, client hangs). The fix is minimal risk, well-contained, and makes an existing documented feature work as intended. fs/nfs/nfs4proc.c | 13 +++++++++++++ include/linux/nfs_xdr.h | 1 + 2 files changed, 14 insertions(+) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 611e6283c194f..6875215de9a44 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3634,6 +3634,7 @@ struct nfs4_closedata { } lr; struct nfs_fattr fattr; unsigned long timestamp; + unsigned short retrans; }; static void nfs4_free_closedata(void *data) @@ -3662,6 +3663,7 @@ static void nfs4_close_done(struct rpc_task *task, void *data) .state = state, .inode = calldata->inode, .stateid = &calldata->arg.stateid, + .retrans = calldata->retrans, }; if (!nfs4_sequence_done(task, &calldata->res.seq_res)) @@ -3709,6 +3711,7 @@ static void nfs4_close_done(struct rpc_task *task, void *data) default: task->tk_status = nfs4_async_handle_exception(task, server, task->tk_status, &exception); + calldata->retrans = exception.retrans; if (exception.retry) goto out_restart; } @@ -5591,9 +5594,11 @@ static int nfs4_read_done_cb(struct rpc_task *task, struct nfs_pgio_header *hdr) .inode = hdr->inode, .state = hdr->args.context->state, .stateid = &hdr->args.stateid, + .retrans = hdr->retrans, }; task->tk_status = nfs4_async_handle_exception(task, server, task->tk_status, &exception); + hdr->retrans = exception.retrans; if (exception.retry) { rpc_restart_call_prepare(task); return -EAGAIN; @@ -5707,10 +5712,12 @@ static int nfs4_write_done_cb(struct rpc_task *task, .inode = hdr->inode, .state = hdr->args.context->state, .stateid = &hdr->args.stateid, + .retrans = hdr->retrans, }; task->tk_status = nfs4_async_handle_exception(task, NFS_SERVER(inode), task->tk_status, &exception); + hdr->retrans = exception.retrans; if (exception.retry) { rpc_restart_call_prepare(task); return -EAGAIN; @@ -6724,6 +6731,7 @@ struct nfs4_delegreturndata { struct nfs_fh fh; nfs4_stateid stateid; unsigned long timestamp; + unsigned short retrans; struct { struct nfs4_layoutreturn_args arg; struct nfs4_layoutreturn_res res; @@ -6744,6 +6752,7 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata) .inode = data->inode, .stateid = &data->stateid, .task_is_privileged = data->args.seq_args.sa_privileged, + .retrans = data->retrans, }; if (!nfs4_sequence_done(task, &data->res.seq_res)) @@ -6815,6 +6824,7 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata) task->tk_status = nfs4_async_handle_exception(task, data->res.server, task->tk_status, &exception); + data->retrans = exception.retrans; if (exception.retry) goto out_restart; } @@ -7091,6 +7101,7 @@ struct nfs4_unlockdata { struct file_lock fl; struct nfs_server *server; unsigned long timestamp; + unsigned short retrans; }; static struct nfs4_unlockdata *nfs4_alloc_unlockdata(struct file_lock *fl, @@ -7145,6 +7156,7 @@ static void nfs4_locku_done(struct rpc_task *task, void *data) struct nfs4_exception exception = { .inode = calldata->lsp->ls_state->inode, .stateid = &calldata->arg.stateid, + .retrans = calldata->retrans, }; if (!nfs4_sequence_done(task, &calldata->res.seq_res)) @@ -7178,6 +7190,7 @@ static void nfs4_locku_done(struct rpc_task *task, void *data) task->tk_status = nfs4_async_handle_exception(task, calldata->server, task->tk_status, &exception); + calldata->retrans = exception.retrans; if (exception.retry) rpc_restart_call_prepare(task); } diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index ac4bff6e99135..ea437e468a91c 100644 --- a/include/linux/nfs_xdr.h +++ b/include/linux/nfs_xdr.h @@ -1659,6 +1659,7 @@ struct nfs_pgio_header { void *netfs; #endif + unsigned short retrans; int pnfs_error; int error; /* merge with pnfs_error */ unsigned int good_bytes; /* boundary of good data */ -- 2.51.0