* [PATCH v3 01/24] NFSv4/pnfs: Free the netid when draining a data-server address list
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 02/24] NFSv4/flexfiles: Use the full 64-bit stripe_unit Benjamin Coddington
` (22 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
nfs4_pnfs_ds_addr_free() releases all three of an address entry's
allocations, but the four sites that drain a decoded list before it
reaches a data server open-code the loop and free only da_remotestr and
the entry, leaking da_netid. Both layout drivers drain that list when
nfs4_pnfs_ds_add() returned a data server already in the cache, and
again on their error exits.
Lift the drain into nfs4_pnfs_ds_addr_list_free() so an address list has
one owner. A striping mount reaches the cached case on nearly every
GETDEVICEINFO, since many deviceIDs name the same data servers.
Fixes: 4be78d26810b ("NFSv4/pNFS: Store the transport type in struct nfs4_pnfs_ds_addr")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/filelayout/filelayoutdev.c | 17 ++---------------
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 17 ++---------------
fs/nfs/pnfs.h | 1 +
fs/nfs/pnfs_nfs.c | 21 +++++++++++++--------
4 files changed, 18 insertions(+), 38 deletions(-)
diff --git a/fs/nfs/filelayout/filelayoutdev.c b/fs/nfs/filelayout/filelayoutdev.c
index 88bc79ec3459..57e0654cd98c 100644
--- a/fs/nfs/filelayout/filelayoutdev.c
+++ b/fs/nfs/filelayout/filelayoutdev.c
@@ -177,27 +177,14 @@ nfs4_fl_alloc_deviceid_node(struct nfs_server *server, struct pnfs_device *pdev,
trace_fl_getdevinfo(server, &pdev->dev_id, dsaddr->ds_list[i]->ds_remotestr);
/* If DS was already in cache, free ds addrs */
- while (!list_empty(&dsaddrs)) {
- da = list_first_entry(&dsaddrs,
- struct nfs4_pnfs_ds_addr,
- da_node);
- list_del_init(&da->da_node);
- kfree(da->da_remotestr);
- kfree(da);
- }
+ nfs4_pnfs_ds_addr_list_free(&dsaddrs);
}
folio_put(scratch);
return dsaddr;
out_err_drain_dsaddrs:
- while (!list_empty(&dsaddrs)) {
- da = list_first_entry(&dsaddrs, struct nfs4_pnfs_ds_addr,
- da_node);
- list_del_init(&da->da_node);
- kfree(da->da_remotestr);
- kfree(da);
- }
+ nfs4_pnfs_ds_addr_list_free(&dsaddrs);
out_err_free_deviceid:
nfs4_fl_free_deviceid(dsaddr);
/* stripe_indicies was part of dsaddr */
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index 5c0216bd5fce..6cd3859d4a24 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -159,26 +159,13 @@ nfs4_ff_alloc_deviceid_node(struct nfs_server *server, struct pnfs_device *pdev,
goto out_err_drain_dsaddrs;
/* If DS was already in cache, free ds addrs */
- while (!list_empty(&dsaddrs)) {
- da = list_first_entry(&dsaddrs,
- struct nfs4_pnfs_ds_addr,
- da_node);
- list_del_init(&da->da_node);
- kfree(da->da_remotestr);
- kfree(da);
- }
+ nfs4_pnfs_ds_addr_list_free(&dsaddrs);
folio_put(scratch);
return new_ds;
out_err_drain_dsaddrs:
- while (!list_empty(&dsaddrs)) {
- da = list_first_entry(&dsaddrs, struct nfs4_pnfs_ds_addr,
- da_node);
- list_del_init(&da->da_node);
- kfree(da->da_remotestr);
- kfree(da);
- }
+ nfs4_pnfs_ds_addr_list_free(&dsaddrs);
kfree(ds_versions);
out_scratch:
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 70d20f779678..5725ea06efc2 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -427,6 +427,7 @@ int nfs4_pnfs_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds,
struct nfs4_pnfs_ds_addr *nfs4_decode_mp_ds_addr(struct net *net,
struct xdr_stream *xdr,
gfp_t gfp_flags);
+void nfs4_pnfs_ds_addr_list_free(struct list_head *dsaddrs);
void pnfs_layout_mark_request_commit(struct nfs_page *req,
struct pnfs_layout_segment *lseg,
struct nfs_commit_info *cinfo,
diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index 93d63f75a355..5bd014b981bf 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -633,23 +633,28 @@ static void nfs4_pnfs_ds_addr_free(struct nfs4_pnfs_ds_addr *da)
kfree(da);
}
-static void destroy_ds(struct nfs4_pnfs_ds *ds)
+void nfs4_pnfs_ds_addr_list_free(struct list_head *dsaddrs)
{
struct nfs4_pnfs_ds_addr *da;
+ while (!list_empty(dsaddrs)) {
+ da = list_first_entry(dsaddrs, struct nfs4_pnfs_ds_addr,
+ da_node);
+ list_del_init(&da->da_node);
+ nfs4_pnfs_ds_addr_free(da);
+ }
+}
+EXPORT_SYMBOL_GPL(nfs4_pnfs_ds_addr_list_free);
+
+static void destroy_ds(struct nfs4_pnfs_ds *ds)
+{
dprintk("--> %s\n", __func__);
ifdebug(FACILITY)
print_ds(ds);
nfs_put_client(ds->ds_clp);
- while (!list_empty(&ds->ds_addrs)) {
- da = list_first_entry(&ds->ds_addrs,
- struct nfs4_pnfs_ds_addr,
- da_node);
- list_del_init(&da->da_node);
- nfs4_pnfs_ds_addr_free(da);
- }
+ nfs4_pnfs_ds_addr_list_free(&ds->ds_addrs);
kfree(ds->ds_remotestr);
kfree(ds);
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 02/24] NFSv4/flexfiles: Use the full 64-bit stripe_unit
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 01/24] NFSv4/pnfs: Free the netid when draining a data-server address list Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 03/24] NFSv4/pnfs: bound the CB_NOTIFY_DEVICEID array count before allocating Benjamin Coddington
` (21 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
ff_layout_alloc_lseg() decodes stripe_unit as the 64-bit value the
protocol defines and struct nfs4_ff_layout_segment stores it as one, but
both consumers narrow it back to 32 bits: nfs4_ff_layout_calc_dss_id()
divides with do_div(), which casts the divisor, and ff_layout_pg_test()
copies it into a u32 first. A stripe_unit that does not fit is silently
truncated, so the client stripes on a unit the server did not specify --
or divides by zero, if the low 32 bits happen to be clear.
Divide by the full value, with div64_u64() and div64_u64_rem().
Fixes: 20b1d75fb840 ("NFSv4/flexfiles: Add support for striped layouts")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 12 ++++++------
fs/nfs/flexfilelayout/flexfilelayout.h | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 7fe8b91fa47c..54ee8f6051ee 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -995,9 +995,9 @@ ff_layout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
{
unsigned int size;
u64 p_stripe, r_stripe;
- u32 stripe_offset;
+ u64 stripe_offset;
u64 segment_offset = pgio->pg_lseg->pls_range.offset;
- u32 stripe_unit = FF_LAYOUT_LSEG(pgio->pg_lseg)->stripe_unit;
+ u64 stripe_unit = FF_LAYOUT_LSEG(pgio->pg_lseg)->stripe_unit;
/* calls nfs_generic_pg_test */
size = pnfs_generic_pg_test(pgio, prev, req);
@@ -1010,21 +1010,21 @@ ff_layout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
if (prev) {
p_stripe = (u64)req_offset(prev) - segment_offset;
r_stripe = (u64)req_offset(req) - segment_offset;
- do_div(p_stripe, stripe_unit);
- do_div(r_stripe, stripe_unit);
+ p_stripe = div64_u64(p_stripe, stripe_unit);
+ r_stripe = div64_u64(r_stripe, stripe_unit);
if (p_stripe != r_stripe)
return 0;
}
/* calculate remaining bytes in the current stripe */
- div_u64_rem((u64)req_offset(req) - segment_offset,
+ div64_u64_rem((u64)req_offset(req) - segment_offset,
stripe_unit,
&stripe_offset);
WARN_ON_ONCE(stripe_offset > stripe_unit);
if (stripe_offset >= stripe_unit)
return 0;
- return min(stripe_unit - (unsigned int)stripe_offset, size);
+ return min_t(u64, stripe_unit - stripe_offset, size);
}
static void
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index a5bd00f69e82..ec69cd1c3ae9 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -220,7 +220,7 @@ nfs4_ff_layout_calc_dss_id(const u64 stripe_unit, const u32 dss_count, const lof
if (dss_count == 1 || stripe_unit == 0)
return 0;
- do_div(tmp, stripe_unit);
+ tmp = div64_u64(tmp, stripe_unit);
return do_div(tmp, dss_count);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 03/24] NFSv4/pnfs: bound the CB_NOTIFY_DEVICEID array count before allocating
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 01/24] NFSv4/pnfs: Free the netid when draining a data-server address list Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 02/24] NFSv4/flexfiles: Use the full 64-bit stripe_unit Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 04/24] pNFS: Fix CB_NOTIFY_DEVICEID CHANGE to consume ndc_immediate Benjamin Coddington
` (20 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
decode_devicenotify_args() hands the server's notification count
straight to kmalloc_objs() as the array length, without checking it
against the message that carried it. Bound it the way nfs4xdr.c bounds
an attribute length, against xdr_stream_remaining().
Fixes: 1be5683b03a7 ("pnfs: CB_NOTIFY_DEVICEID")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/callback_xdr.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index 88e1af0fda01..b1bc2c02b5a6 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -271,6 +271,13 @@ __be32 decode_devicenotify_args(struct svc_rqst *rqstp,
if (n == 0)
goto out;
+ /* sanity check the count against the remaining stream */
+ if (n > xdr_stream_remaining(xdr) /
+ ((4 * sizeof(uint32_t)) + NFS4_DEVICEID4_SIZE)) {
+ status = htonl(NFS4ERR_BADXDR);
+ goto out;
+ }
+
args->devs = kmalloc_objs(*args->devs, n);
if (!args->devs) {
status = htonl(NFS4ERR_DELAY);
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 04/24] pNFS: Fix CB_NOTIFY_DEVICEID CHANGE to consume ndc_immediate
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (2 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 03/24] NFSv4/pnfs: bound the CB_NOTIFY_DEVICEID array count before allocating Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 05/24] NFSv4/flexfiles: Use the full 64-bit offset for read DS selection Benjamin Coddington
` (19 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
decode_devicenotify_args() gated the trailing ndc_immediate on
cbd_layout_type rather than cbd_notify_type, so the flag was never
consumed and a multi-item cnda_changes<> misaligned after the first
entry.
Fixes: 1be5683b03a7 ("pnfs: CB_NOTIFY_DEVICEID")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/callback_xdr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index b1bc2c02b5a6..5a1f05469268 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -319,7 +319,7 @@ __be32 decode_devicenotify_args(struct svc_rqst *rqstp,
memcpy(dev->cbd_dev_id.data, p, NFS4_DEVICEID4_SIZE);
p += XDR_QUADLEN(NFS4_DEVICEID4_SIZE);
- if (dev->cbd_layout_type == NOTIFY_DEVICEID4_CHANGE) {
+ if (dev->cbd_notify_type == NOTIFY_DEVICEID4_CHANGE) {
p = xdr_inline_decode(xdr, sizeof(uint32_t));
if (unlikely(p == NULL)) {
status = htonl(NFS4ERR_BADXDR);
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 05/24] NFSv4/flexfiles: Use the full 64-bit offset for read DS selection
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (3 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 04/24] pNFS: Fix CB_NOTIFY_DEVICEID CHANGE to consume ndc_immediate Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 06/24] NFSv4/flexfiles: Bound page coalescing on the absolute stripe offset Benjamin Coddington
` (18 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
The read data-server selection helpers declare their offset parameter
as u32, truncating the file offset before nfs4_ff_layout_calc_dss_id()
computes the stripe index. The write path already passes the full
offset; make the read path match.
Fixes: 4934ccbeaed3 ("NFSv4/flexfiles: Read path updates for striped layouts")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 54ee8f6051ee..4078da49a93a 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -878,7 +878,7 @@ ff_layout_mark_ds_reachable(struct pnfs_layout_segment *lseg, u32 idx, u32 dss_i
static struct nfs4_pnfs_ds *
ff_layout_choose_ds_for_read(struct pnfs_layout_segment *lseg,
u32 start_idx, u32 *best_idx,
- u32 offset, u32 *dss_id,
+ u64 offset, u32 *dss_id,
bool check_device)
{
struct nfs4_ff_layout_segment *fls = FF_LAYOUT_LSEG(lseg);
@@ -914,7 +914,7 @@ ff_layout_choose_ds_for_read(struct pnfs_layout_segment *lseg,
static struct nfs4_pnfs_ds *
ff_layout_choose_any_ds_for_read(struct pnfs_layout_segment *lseg,
u32 start_idx, u32 *best_idx,
- u32 offset, u32 *dss_id)
+ u64 offset, u32 *dss_id)
{
return ff_layout_choose_ds_for_read(lseg, start_idx, best_idx,
offset, dss_id, false);
@@ -923,7 +923,7 @@ ff_layout_choose_any_ds_for_read(struct pnfs_layout_segment *lseg,
static struct nfs4_pnfs_ds *
ff_layout_choose_valid_ds_for_read(struct pnfs_layout_segment *lseg,
u32 start_idx, u32 *best_idx,
- u32 offset, u32 *dss_id)
+ u64 offset, u32 *dss_id)
{
return ff_layout_choose_ds_for_read(lseg, start_idx, best_idx,
offset, dss_id, true);
@@ -932,7 +932,7 @@ ff_layout_choose_valid_ds_for_read(struct pnfs_layout_segment *lseg,
static struct nfs4_pnfs_ds *
ff_layout_choose_best_ds_for_read(struct pnfs_layout_segment *lseg,
u32 start_idx, u32 *best_idx,
- u32 offset, u32 *dss_id)
+ u64 offset, u32 *dss_id)
{
struct nfs4_pnfs_ds *ds;
@@ -947,7 +947,7 @@ ff_layout_choose_best_ds_for_read(struct pnfs_layout_segment *lseg,
static struct nfs4_pnfs_ds *
ff_layout_get_ds_for_read(struct nfs_pageio_descriptor *pgio,
u32 *best_idx,
- u32 offset,
+ u64 offset,
u32 *dss_id)
{
struct pnfs_layout_segment *lseg = pgio->pg_lseg;
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 06/24] NFSv4/flexfiles: Bound page coalescing on the absolute stripe offset
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (4 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 05/24] NFSv4/flexfiles: Use the full 64-bit offset for read DS selection Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 07/24] NFSv4/filelayout: Anchor page coalescing on pattern_offset Benjamin Coddington
` (17 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
ff_layout_pg_test() was copied from the files layout, which partitions
data servers relative to pattern_offset and bounds coalescing with a
segment-relative offset. Flexfiles has no pattern_offset:
nfs4_ff_layout_calc_dss_id() partitions on the absolute file offset
(RFC 8435 Section 6). So when a layout segment's offset is not
stripe-unit-aligned, the segment-relative coalescing window is shifted
off the absolute stripe grid and a coalesced I/O can straddle a stripe
boundary, sending the bytes past it to the wrong data server.
Bound coalescing on the absolute offset to match calc_dss_id(). Aligned
segments, including all whole-file layouts, are unchanged.
Fixes: 4934ccbeaed3 ("NFSv4/flexfiles: Read path updates for striped layouts")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 4078da49a93a..2d254e5e5b94 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -996,7 +996,6 @@ ff_layout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
unsigned int size;
u64 p_stripe, r_stripe;
u64 stripe_offset;
- u64 segment_offset = pgio->pg_lseg->pls_range.offset;
u64 stripe_unit = FF_LAYOUT_LSEG(pgio->pg_lseg)->stripe_unit;
/* calls nfs_generic_pg_test */
@@ -1008,8 +1007,8 @@ ff_layout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
/* see if req and prev are in the same stripe */
if (prev) {
- p_stripe = (u64)req_offset(prev) - segment_offset;
- r_stripe = (u64)req_offset(req) - segment_offset;
+ p_stripe = (u64)req_offset(prev);
+ r_stripe = (u64)req_offset(req);
p_stripe = div64_u64(p_stripe, stripe_unit);
r_stripe = div64_u64(r_stripe, stripe_unit);
@@ -1018,7 +1017,7 @@ ff_layout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
}
/* calculate remaining bytes in the current stripe */
- div64_u64_rem((u64)req_offset(req) - segment_offset,
+ div64_u64_rem((u64)req_offset(req),
stripe_unit,
&stripe_offset);
WARN_ON_ONCE(stripe_offset > stripe_unit);
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 07/24] NFSv4/filelayout: Anchor page coalescing on pattern_offset
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (5 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 06/24] NFSv4/flexfiles: Bound page coalescing on the absolute stripe offset Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 08/24] NFSv4/flexfiles: Reference the device node across DS setup Benjamin Coddington
` (16 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
filelayout_pg_test() bounds page coalescing to a single stripe unit
using an offset relative to pls_range.offset, but the data server is
selected by nfs4_fl_calc_j_index() using an offset relative to
pattern_offset. When a segment's pattern_offset and range offset are
not congruent modulo the stripe unit, the coalescing window is shifted
off the DS-selection grid, so a coalesced I/O can straddle a stripe
boundary and send the bytes past it to the wrong data server.
Anchor coalescing on pattern_offset to match nfs4_fl_calc_j_index().
Stripe-congruent segments, including the common whole-file pattern_offset
0 case, are unchanged.
Fixes: c6194271f94b ("pnfs: filelayout: support non page aligned layouts")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/filelayout/filelayout.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
index 72e20b56fbc7..0d53277f972e 100644
--- a/fs/nfs/filelayout/filelayout.c
+++ b/fs/nfs/filelayout/filelayout.c
@@ -796,7 +796,7 @@ filelayout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
unsigned int size;
u64 p_stripe, r_stripe;
u32 stripe_offset;
- u64 segment_offset = pgio->pg_lseg->pls_range.offset;
+ u64 pattern_offset = FILELAYOUT_LSEG(pgio->pg_lseg)->pattern_offset;
u32 stripe_unit = FILELAYOUT_LSEG(pgio->pg_lseg)->stripe_unit;
/* calls nfs_generic_pg_test */
@@ -808,8 +808,8 @@ filelayout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
/* see if req and prev are in the same stripe */
if (prev) {
- p_stripe = (u64)req_offset(prev) - segment_offset;
- r_stripe = (u64)req_offset(req) - segment_offset;
+ p_stripe = (u64)req_offset(prev) - pattern_offset;
+ r_stripe = (u64)req_offset(req) - pattern_offset;
do_div(p_stripe, stripe_unit);
do_div(r_stripe, stripe_unit);
@@ -818,7 +818,7 @@ filelayout_pg_test(struct nfs_pageio_descriptor *pgio, struct nfs_page *prev,
}
/* calculate remaining bytes in the current stripe */
- div_u64_rem((u64)req_offset(req) - segment_offset,
+ div_u64_rem((u64)req_offset(req) - pattern_offset,
stripe_unit,
&stripe_offset);
WARN_ON_ONCE(stripe_offset > stripe_unit);
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 08/24] NFSv4/flexfiles: Reference the device node across DS setup
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (6 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 07/24] NFSv4/filelayout: Anchor page coalescing on pattern_offset Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 09/24] NFSv4/flexfiles: Carry the device node reference across each I/O Benjamin Coddington
` (15 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
The flexfiles I/O setup paths read the mirror's pinned per-stripe device
node (mirror->dss[dss_id].mirror_ds) repeatedly and locklessly, relying
on the mirror's lifetime pin. To prepare for re-resolving that pointer
in place on CB_NOTIFY_DEVICEID CHANGE, readers must hold their own
reference on the node they are using rather than trusting the pin.
Replace ff_layout_init_mirror_ds() with ff_layout_get_mirror_ds(), which
resolves on first use as before but returns the node with its own
reference held. Thread the referenced node through
nfs4_ff_layout_prepare_ds() and the DS selection helpers so each setup
path snapshots the node once, and drop the reference when setup is done.
No functional change: the pointer is still resolved once and pinned for
the life of the mirror. Each reference this adds is released on every
exit from the path that took it, error paths included.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 140 ++++++++++++++--------
fs/nfs/flexfilelayout/flexfilelayout.h | 16 ++-
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 74 +++++++-----
3 files changed, 141 insertions(+), 89 deletions(-)
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 2d254e5e5b94..11fc27a062f6 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -875,7 +875,7 @@ ff_layout_mark_ds_reachable(struct pnfs_layout_segment *lseg, u32 idx, u32 dss_i
nfs4_mark_deviceid_available(devid);
}
-static struct nfs4_pnfs_ds *
+static struct nfs4_ff_layout_ds *
ff_layout_choose_ds_for_read(struct pnfs_layout_segment *lseg,
u32 start_idx, u32 *best_idx,
u64 offset, u32 *dss_id,
@@ -883,7 +883,9 @@ ff_layout_choose_ds_for_read(struct pnfs_layout_segment *lseg,
{
struct nfs4_ff_layout_segment *fls = FF_LAYOUT_LSEG(lseg);
struct nfs4_ff_layout_mirror *mirror;
- struct nfs4_pnfs_ds *ds = ERR_PTR(-EAGAIN);
+ struct nfs4_ff_layout_ds *mirror_ds;
+ struct nfs4_ff_layout_ds *ret = ERR_PTR(-EAGAIN);
+ struct nfs4_pnfs_ds *ds;
u32 idx;
/* mirrors are initially sorted by efficiency */
@@ -893,25 +895,32 @@ ff_layout_choose_ds_for_read(struct pnfs_layout_segment *lseg,
fls->stripe_unit,
fls->mirror_array[idx]->dss_count,
offset);
- ds = nfs4_ff_layout_prepare_ds(lseg, mirror, *dss_id, false);
- if (IS_ERR(ds))
+ mirror_ds = ff_layout_get_mirror_ds(lseg->pls_layout, mirror,
+ *dss_id);
+ ds = nfs4_ff_layout_prepare_ds(lseg, mirror, mirror_ds,
+ *dss_id, false);
+ if (IS_ERR(ds)) {
+ nfs4_ff_layout_put_deviceid(mirror_ds);
+ ret = ERR_CAST(ds);
continue;
+ }
if (check_device &&
- nfs4_test_deviceid_unavailable(&mirror->dss[*dss_id].mirror_ds->id_node)) {
+ nfs4_test_deviceid_unavailable(&mirror_ds->id_node)) {
+ nfs4_ff_layout_put_deviceid(mirror_ds);
// reinitialize the error state in case if this is the last iteration
- ds = ERR_PTR(-EINVAL);
+ ret = ERR_PTR(-EINVAL);
continue;
}
*best_idx = idx;
- break;
+ return mirror_ds;
}
- return ds;
+ return ret;
}
-static struct nfs4_pnfs_ds *
+static struct nfs4_ff_layout_ds *
ff_layout_choose_any_ds_for_read(struct pnfs_layout_segment *lseg,
u32 start_idx, u32 *best_idx,
u64 offset, u32 *dss_id)
@@ -920,7 +929,7 @@ ff_layout_choose_any_ds_for_read(struct pnfs_layout_segment *lseg,
offset, dss_id, false);
}
-static struct nfs4_pnfs_ds *
+static struct nfs4_ff_layout_ds *
ff_layout_choose_valid_ds_for_read(struct pnfs_layout_segment *lseg,
u32 start_idx, u32 *best_idx,
u64 offset, u32 *dss_id)
@@ -929,34 +938,36 @@ ff_layout_choose_valid_ds_for_read(struct pnfs_layout_segment *lseg,
offset, dss_id, true);
}
-static struct nfs4_pnfs_ds *
+static struct nfs4_ff_layout_ds *
ff_layout_choose_best_ds_for_read(struct pnfs_layout_segment *lseg,
u32 start_idx, u32 *best_idx,
u64 offset, u32 *dss_id)
{
- struct nfs4_pnfs_ds *ds;
+ struct nfs4_ff_layout_ds *mirror_ds;
- ds = ff_layout_choose_valid_ds_for_read(lseg, start_idx, best_idx,
- offset, dss_id);
- if (!IS_ERR(ds))
- return ds;
+ mirror_ds = ff_layout_choose_valid_ds_for_read(lseg, start_idx,
+ best_idx, offset,
+ dss_id);
+ if (!IS_ERR(mirror_ds))
+ return mirror_ds;
return ff_layout_choose_any_ds_for_read(lseg, start_idx, best_idx,
offset, dss_id);
}
-static struct nfs4_pnfs_ds *
+static struct nfs4_ff_layout_ds *
ff_layout_get_ds_for_read(struct nfs_pageio_descriptor *pgio,
u32 *best_idx,
u64 offset,
u32 *dss_id)
{
struct pnfs_layout_segment *lseg = pgio->pg_lseg;
- struct nfs4_pnfs_ds *ds;
+ struct nfs4_ff_layout_ds *mirror_ds;
- ds = ff_layout_choose_best_ds_for_read(lseg, pgio->pg_mirror_idx,
- best_idx, offset, dss_id);
- if (!IS_ERR(ds) || !pgio->pg_mirror_idx)
- return ds;
+ mirror_ds = ff_layout_choose_best_ds_for_read(lseg,
+ pgio->pg_mirror_idx,
+ best_idx, offset, dss_id);
+ if (!IS_ERR(mirror_ds) || !pgio->pg_mirror_idx)
+ return mirror_ds;
return ff_layout_choose_best_ds_for_read(lseg, 0, best_idx,
offset, dss_id);
}
@@ -1031,8 +1042,7 @@ ff_layout_pg_init_read(struct nfs_pageio_descriptor *pgio,
struct nfs_page *req)
{
struct nfs_pgio_mirror *pgm;
- struct nfs4_ff_layout_mirror *mirror;
- struct nfs4_pnfs_ds *ds;
+ struct nfs4_ff_layout_ds *mirror_ds;
u32 ds_idx, dss_id;
if (NFS_SERVER(pgio->pg_inode)->flags &
@@ -1054,9 +1064,9 @@ ff_layout_pg_init_read(struct nfs_pageio_descriptor *pgio,
/* Reset wb_nio, since getting layout segment was successful */
req->wb_nio = 0;
- ds = ff_layout_get_ds_for_read(pgio, &ds_idx,
- req_offset(req), &dss_id);
- if (IS_ERR(ds)) {
+ mirror_ds = ff_layout_get_ds_for_read(pgio, &ds_idx,
+ req_offset(req), &dss_id);
+ if (IS_ERR(mirror_ds)) {
if (!ff_layout_no_fallback_to_mds(pgio->pg_lseg))
goto out_mds;
pnfs_generic_pg_cleanup(pgio);
@@ -1065,9 +1075,9 @@ ff_layout_pg_init_read(struct nfs_pageio_descriptor *pgio,
goto retry;
}
- mirror = FF_LAYOUT_COMP(pgio->pg_lseg, ds_idx);
pgm = &pgio->pg_mirrors[0];
- pgm->pg_bsize = mirror->dss[dss_id].mirror_ds->ds_versions[0].rsize;
+ pgm->pg_bsize = mirror_ds->ds_versions[0].rsize;
+ nfs4_ff_layout_put_deviceid(mirror_ds);
pgio->pg_mirror_idx = ds_idx;
return;
@@ -1102,6 +1112,7 @@ ff_layout_pg_init_write(struct nfs_pageio_descriptor *pgio,
struct nfs_page *req)
{
struct nfs4_ff_layout_mirror *mirror;
+ struct nfs4_ff_layout_ds *mirror_ds;
struct nfs_pgio_mirror *pgm;
struct nfs4_pnfs_ds *ds;
u32 i, dss_id;
@@ -1133,9 +1144,12 @@ ff_layout_pg_init_write(struct nfs_pageio_descriptor *pgio,
FF_LAYOUT_LSEG(pgio->pg_lseg)->stripe_unit,
mirror->dss_count,
req_offset(req));
+ mirror_ds = ff_layout_get_mirror_ds(pgio->pg_lseg->pls_layout,
+ mirror, dss_id);
ds = nfs4_ff_layout_prepare_ds(pgio->pg_lseg, mirror,
- dss_id, true);
+ mirror_ds, dss_id, true);
if (IS_ERR(ds)) {
+ nfs4_ff_layout_put_deviceid(mirror_ds);
if (!ff_layout_no_fallback_to_mds(pgio->pg_lseg))
goto out_mds;
pnfs_generic_pg_cleanup(pgio);
@@ -1144,7 +1158,8 @@ ff_layout_pg_init_write(struct nfs_pageio_descriptor *pgio,
goto retry;
}
pgm = &pgio->pg_mirrors[i];
- pgm->pg_bsize = mirror->dss[dss_id].mirror_ds->ds_versions[0].wsize;
+ pgm->pg_bsize = mirror_ds->ds_versions[0].wsize;
+ nfs4_ff_layout_put_deviceid(mirror_ds);
}
if (NFS_SERVER(pgio->pg_inode)->flags &
@@ -1276,14 +1291,16 @@ static void ff_layout_resend_pnfs_read(struct nfs_pgio_header *hdr)
u32 idx = hdr->pgio_mirror_idx + 1;
u32 new_idx = 0;
u32 dss_id = 0;
- struct nfs4_pnfs_ds *ds;
+ struct nfs4_ff_layout_ds *mirror_ds;
- ds = ff_layout_choose_any_ds_for_read(hdr->lseg, idx, &new_idx,
- hdr->args.offset, &dss_id);
- if (IS_ERR(ds))
+ mirror_ds = ff_layout_choose_any_ds_for_read(hdr->lseg, idx, &new_idx,
+ hdr->args.offset, &dss_id);
+ if (IS_ERR(mirror_ds)) {
pnfs_error_mark_layout_for_return(hdr->inode, hdr->lseg);
- else
+ } else {
+ nfs4_ff_layout_put_deviceid(mirror_ds);
ff_layout_send_layouterror(hdr->lseg);
+ }
pnfs_read_resend_pnfs(hdr, new_idx);
}
@@ -2166,6 +2183,7 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
struct rpc_clnt *ds_clnt;
struct nfsd_file *localio;
struct nfs4_ff_layout_mirror *mirror;
+ struct nfs4_ff_layout_ds *mirror_ds;
const struct cred *ds_cred;
loff_t offset = hdr->args.offset;
u32 idx = hdr->pgio_mirror_idx;
@@ -2183,22 +2201,24 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
FF_LAYOUT_LSEG(lseg)->stripe_unit,
mirror->dss_count,
offset);
- ds = nfs4_ff_layout_prepare_ds(lseg, mirror, dss_id, false);
+ mirror_ds = ff_layout_get_mirror_ds(lseg->pls_layout, mirror, dss_id);
+ ds = nfs4_ff_layout_prepare_ds(lseg, mirror, mirror_ds, dss_id, false);
if (IS_ERR(ds)) {
ds_fatal_error = nfs_error_is_fatal(PTR_ERR(ds));
goto out_failed;
}
- ds_clnt = nfs4_ff_find_or_create_ds_client(mirror, ds->ds_clp,
- hdr->inode, dss_id);
+ ds_clnt = nfs4_ff_find_or_create_ds_client(mirror_ds, ds->ds_clp,
+ hdr->inode);
if (IS_ERR(ds_clnt))
goto out_failed;
- ds_cred = ff_layout_get_ds_cred(mirror, &lseg->pls_range, hdr->cred, dss_id);
+ ds_cred = ff_layout_get_ds_cred(mirror, &lseg->pls_range, hdr->cred,
+ mirror_ds, dss_id);
if (!ds_cred)
goto out_failed;
- vers = nfs4_ff_layout_ds_version(mirror, dss_id);
+ vers = nfs4_ff_layout_ds_version(mirror_ds);
dprintk("%s USE DS: %s cl_count %d vers %d\n", __func__,
ds->ds_remotestr, refcount_read(&ds->ds_clp->cl_count), vers);
@@ -2210,7 +2230,8 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
if (fh)
hdr->args.fh = fh;
- nfs4_ff_layout_select_ds_stateid(mirror, dss_id, &hdr->args.stateid);
+ nfs4_ff_layout_select_ds_stateid(mirror, mirror_ds, dss_id,
+ &hdr->args.stateid);
/*
* Note that if we ever decide to split across DSes,
@@ -2233,9 +2254,11 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
&ff_layout_read_call_ops_v4,
0, RPC_TASK_SOFTCONN, localio);
put_cred(ds_cred);
+ nfs4_ff_layout_put_deviceid(mirror_ds);
return PNFS_ATTEMPTED;
out_failed:
+ nfs4_ff_layout_put_deviceid(mirror_ds);
if (ff_layout_avoid_mds_available_ds(lseg) && !ds_fatal_error)
return PNFS_TRY_AGAIN;
if (ff_layout_no_fallback_to_mds(lseg)) {
@@ -2261,6 +2284,7 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
struct rpc_clnt *ds_clnt;
struct nfsd_file *localio;
struct nfs4_ff_layout_mirror *mirror;
+ struct nfs4_ff_layout_ds *mirror_ds;
const struct cred *ds_cred;
loff_t offset = hdr->args.offset;
int vers;
@@ -2274,22 +2298,24 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
FF_LAYOUT_LSEG(lseg)->stripe_unit,
mirror->dss_count,
offset);
- ds = nfs4_ff_layout_prepare_ds(lseg, mirror, dss_id, true);
+ mirror_ds = ff_layout_get_mirror_ds(lseg->pls_layout, mirror, dss_id);
+ ds = nfs4_ff_layout_prepare_ds(lseg, mirror, mirror_ds, dss_id, true);
if (IS_ERR(ds)) {
ds_fatal_error = nfs_error_is_fatal(PTR_ERR(ds));
goto out_failed;
}
- ds_clnt = nfs4_ff_find_or_create_ds_client(mirror, ds->ds_clp,
- hdr->inode, dss_id);
+ ds_clnt = nfs4_ff_find_or_create_ds_client(mirror_ds, ds->ds_clp,
+ hdr->inode);
if (IS_ERR(ds_clnt))
goto out_failed;
- ds_cred = ff_layout_get_ds_cred(mirror, &lseg->pls_range, hdr->cred, dss_id);
+ ds_cred = ff_layout_get_ds_cred(mirror, &lseg->pls_range, hdr->cred,
+ mirror_ds, dss_id);
if (!ds_cred)
goto out_failed;
- vers = nfs4_ff_layout_ds_version(mirror, dss_id);
+ vers = nfs4_ff_layout_ds_version(mirror_ds);
dprintk("%s ino %llu sync %d req %zu@%llu DS: %s cl_count %d vers %d\n",
__func__, hdr->inode->i_ino, sync, (size_t) hdr->args.count,
@@ -2304,7 +2330,8 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
if (fh)
hdr->args.fh = fh;
- nfs4_ff_layout_select_ds_stateid(mirror, dss_id, &hdr->args.stateid);
+ nfs4_ff_layout_select_ds_stateid(mirror, mirror_ds, dss_id,
+ &hdr->args.stateid);
/*
* Note that if we ever decide to split across DSes,
@@ -2326,9 +2353,11 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
&ff_layout_write_call_ops_v4,
sync, RPC_TASK_SOFTCONN, localio);
put_cred(ds_cred);
+ nfs4_ff_layout_put_deviceid(mirror_ds);
return PNFS_ATTEMPTED;
out_failed:
+ nfs4_ff_layout_put_deviceid(mirror_ds);
if (ff_layout_avoid_mds_available_ds(lseg) && !ds_fatal_error)
return PNFS_TRY_AGAIN;
if (ff_layout_no_fallback_to_mds(lseg)) {
@@ -2363,6 +2392,7 @@ static int ff_layout_initiate_commit(struct nfs_commit_data *data, int how)
struct rpc_clnt *ds_clnt;
struct nfsd_file *localio;
struct nfs4_ff_layout_mirror *mirror;
+ struct nfs4_ff_layout_ds *mirror_ds = NULL;
const struct cred *ds_cred;
u32 idx, dss_id;
int vers, ret;
@@ -2375,20 +2405,22 @@ static int ff_layout_initiate_commit(struct nfs_commit_data *data, int how)
idx = calc_mirror_idx_from_commit(lseg, data->ds_commit_index);
mirror = FF_LAYOUT_COMP(lseg, idx);
dss_id = calc_dss_id_from_commit(lseg, data->ds_commit_index);
- ds = nfs4_ff_layout_prepare_ds(lseg, mirror, dss_id, true);
+ mirror_ds = ff_layout_get_mirror_ds(lseg->pls_layout, mirror, dss_id);
+ ds = nfs4_ff_layout_prepare_ds(lseg, mirror, mirror_ds, dss_id, true);
if (IS_ERR(ds))
goto out_err;
- ds_clnt = nfs4_ff_find_or_create_ds_client(mirror, ds->ds_clp,
- data->inode, dss_id);
+ ds_clnt = nfs4_ff_find_or_create_ds_client(mirror_ds, ds->ds_clp,
+ data->inode);
if (IS_ERR(ds_clnt))
goto out_err;
- ds_cred = ff_layout_get_ds_cred(mirror, &lseg->pls_range, data->cred, dss_id);
+ ds_cred = ff_layout_get_ds_cred(mirror, &lseg->pls_range, data->cred,
+ mirror_ds, dss_id);
if (!ds_cred)
goto out_err;
- vers = nfs4_ff_layout_ds_version(mirror, dss_id);
+ vers = nfs4_ff_layout_ds_version(mirror_ds);
dprintk("%s ino %llu, how %d cl_count %d vers %d\n", __func__,
data->inode->i_ino, how, refcount_read(&ds->ds_clp->cl_count),
@@ -2414,8 +2446,10 @@ static int ff_layout_initiate_commit(struct nfs_commit_data *data, int how)
&ff_layout_commit_call_ops_v4,
how, RPC_TASK_SOFTCONN, localio);
put_cred(ds_cred);
+ nfs4_ff_layout_put_deviceid(mirror_ds);
return ret;
out_err:
+ nfs4_ff_layout_put_deviceid(mirror_ds);
pnfs_generic_prepare_to_resend_writes(data);
pnfs_generic_commit_release(data);
return -EAGAIN;
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index ec69cd1c3ae9..f9e491a0347e 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -207,9 +207,9 @@ ff_layout_no_read_on_rw(struct pnfs_layout_segment *lseg)
}
static inline int
-nfs4_ff_layout_ds_version(const struct nfs4_ff_layout_mirror *mirror, u32 dss_id)
+nfs4_ff_layout_ds_version(const struct nfs4_ff_layout_ds *mirror_ds)
{
- return mirror->dss[dss_id].mirror_ds->ds_versions[0].version;
+ return mirror_ds->ds_versions[0].version;
}
static inline u32
@@ -245,23 +245,29 @@ struct nfs_fh *
nfs4_ff_layout_select_ds_fh(struct nfs4_ff_layout_mirror *mirror, u32 dss_id);
void
nfs4_ff_layout_select_ds_stateid(const struct nfs4_ff_layout_mirror *mirror,
+ const struct nfs4_ff_layout_ds *mirror_ds,
u32 dss_id,
nfs4_stateid *stateid);
+struct nfs4_ff_layout_ds *
+ff_layout_get_mirror_ds(struct pnfs_layout_hdr *lo,
+ struct nfs4_ff_layout_mirror *mirror,
+ u32 dss_id);
struct nfs4_pnfs_ds *
nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg,
struct nfs4_ff_layout_mirror *mirror,
+ struct nfs4_ff_layout_ds *mirror_ds,
u32 dss_id,
bool fail_return);
struct rpc_clnt *
-nfs4_ff_find_or_create_ds_client(struct nfs4_ff_layout_mirror *mirror,
+nfs4_ff_find_or_create_ds_client(const struct nfs4_ff_layout_ds *mirror_ds,
struct nfs_client *ds_clp,
- struct inode *inode,
- u32 dss_id);
+ struct inode *inode);
const struct cred *ff_layout_get_ds_cred(struct nfs4_ff_layout_mirror *mirror,
const struct pnfs_layout_range *range,
const struct cred *mdscred,
+ const struct nfs4_ff_layout_ds *mirror_ds,
u32 dss_id);
bool ff_layout_avoid_mds_available_ds(struct pnfs_layout_segment *lseg);
bool ff_layout_avoid_read_on_rw(struct pnfs_layout_segment *lseg);
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index 6cd3859d4a24..3712a47b20d0 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -304,24 +304,33 @@ nfs4_ff_layout_select_ds_fh(struct nfs4_ff_layout_mirror *mirror, u32 dss_id)
void
nfs4_ff_layout_select_ds_stateid(const struct nfs4_ff_layout_mirror *mirror,
+ const struct nfs4_ff_layout_ds *mirror_ds,
u32 dss_id,
nfs4_stateid *stateid)
{
- if (nfs4_ff_layout_ds_version(mirror, dss_id) == 4)
+ if (nfs4_ff_layout_ds_version(mirror_ds) == 4)
nfs4_stateid_copy(stateid, &mirror->dss[dss_id].stateid);
}
-static bool
-ff_layout_init_mirror_ds(struct pnfs_layout_hdr *lo,
- struct nfs4_ff_layout_mirror *mirror,
- u32 dss_id)
+/*
+ * Resolve the stripe's deviceid on first use and pin the node on the
+ * mirror. Returns a node the caller must put, or an ERR_PTR.
+ */
+struct nfs4_ff_layout_ds *
+ff_layout_get_mirror_ds(struct pnfs_layout_hdr *lo,
+ struct nfs4_ff_layout_mirror *mirror,
+ u32 dss_id)
{
+ struct nfs4_ff_layout_ds *mirror_ds;
+
if (mirror == NULL)
- goto outerr;
- if (mirror->dss[dss_id].mirror_ds == NULL) {
+ return ERR_PTR(-ENODEV);
+
+ mirror_ds = mirror->dss[dss_id].mirror_ds;
+ if (mirror_ds == NULL) {
struct nfs4_deviceid_node *node;
- struct nfs4_ff_layout_ds *mirror_ds = ERR_PTR(-ENODEV);
+ mirror_ds = ERR_PTR(-ENODEV);
node = nfs4_find_get_deviceid(NFS_SERVER(lo->plh_inode),
&mirror->dss[dss_id].devid, lo->plh_lc_cred,
GFP_KERNEL);
@@ -332,20 +341,23 @@ ff_layout_init_mirror_ds(struct pnfs_layout_hdr *lo,
if (cmpxchg(&mirror->dss[dss_id].mirror_ds, NULL, mirror_ds) &&
mirror_ds != ERR_PTR(-ENODEV))
nfs4_put_deviceid_node(node);
- }
- if (IS_ERR(mirror->dss[dss_id].mirror_ds))
- goto outerr;
+ mirror_ds = mirror->dss[dss_id].mirror_ds;
+ }
- return true;
-outerr:
- return false;
+ if (IS_ERR(mirror_ds))
+ return mirror_ds;
+ if (!atomic_inc_not_zero(&mirror_ds->id_node.ref))
+ return ERR_PTR(-ENODEV);
+ return mirror_ds;
}
/**
* nfs4_ff_layout_prepare_ds - prepare a DS connection for an RPC call
* @lseg: the layout segment we're operating on
* @mirror: layout mirror describing the DS to use
+ * @mirror_ds: referenced device node for the stripe, from
+ * ff_layout_get_mirror_ds() (may be an ERR_PTR)
* @dss_id: DS stripe id to select stripe to use
* @fail_return: return layout on connect failure?
*
@@ -363,6 +375,7 @@ ff_layout_init_mirror_ds(struct pnfs_layout_hdr *lo,
struct nfs4_pnfs_ds *
nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg,
struct nfs4_ff_layout_mirror *mirror,
+ struct nfs4_ff_layout_ds *mirror_ds,
u32 dss_id,
bool fail_return)
{
@@ -372,10 +385,10 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg,
unsigned int max_payload;
int status = -EAGAIN;
- if (!ff_layout_init_mirror_ds(lseg->pls_layout, mirror, dss_id))
+ if (IS_ERR_OR_NULL(mirror_ds))
goto noconnect;
- ds = mirror->dss[dss_id].mirror_ds->ds;
+ ds = mirror_ds->ds;
if (READ_ONCE(ds->ds_clp))
goto out;
/* matching smp_wmb() in _nfs4_pnfs_v3/4_ds_connect */
@@ -384,11 +397,11 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg,
/* FIXME: For now we assume the server sent only one version of NFS
* to use for the DS.
*/
- status = nfs4_pnfs_ds_connect(s, ds, &mirror->dss[dss_id].mirror_ds->id_node,
+ status = nfs4_pnfs_ds_connect(s, ds, &mirror_ds->id_node,
dataserver_timeo, dataserver_retrans,
- mirror->dss[dss_id].mirror_ds->ds_versions[0].version,
- mirror->dss[dss_id].mirror_ds->ds_versions[0].minor_version,
- mirror->dss[dss_id].mirror_ds->ds_versions[0].tightly_coupled);
+ mirror_ds->ds_versions[0].version,
+ mirror_ds->ds_versions[0].minor_version,
+ mirror_ds->ds_versions[0].tightly_coupled);
/* connect success, check rsize/wsize limit */
if (!status) {
@@ -401,10 +414,10 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg,
max_payload =
nfs_block_size(rpc_max_payload(ds->ds_clp->cl_rpcclient),
NULL);
- if (mirror->dss[dss_id].mirror_ds->ds_versions[0].rsize > max_payload)
- mirror->dss[dss_id].mirror_ds->ds_versions[0].rsize = max_payload;
- if (mirror->dss[dss_id].mirror_ds->ds_versions[0].wsize > max_payload)
- mirror->dss[dss_id].mirror_ds->ds_versions[0].wsize = max_payload;
+ if (mirror_ds->ds_versions[0].rsize > max_payload)
+ mirror_ds->ds_versions[0].rsize = max_payload;
+ if (mirror_ds->ds_versions[0].wsize > max_payload)
+ mirror_ds->ds_versions[0].wsize = max_payload;
goto out;
}
noconnect:
@@ -424,11 +437,12 @@ const struct cred *
ff_layout_get_ds_cred(struct nfs4_ff_layout_mirror *mirror,
const struct pnfs_layout_range *range,
const struct cred *mdscred,
+ const struct nfs4_ff_layout_ds *mirror_ds,
u32 dss_id)
{
const struct cred *cred;
- if (mirror && !mirror->dss[dss_id].mirror_ds->ds_versions[0].tightly_coupled) {
+ if (mirror && !mirror_ds->ds_versions[0].tightly_coupled) {
cred = ff_layout_get_mirror_cred(mirror, range->iomode, dss_id);
if (!cred)
cred = get_cred(mdscred);
@@ -440,20 +454,18 @@ ff_layout_get_ds_cred(struct nfs4_ff_layout_mirror *mirror,
/**
* nfs4_ff_find_or_create_ds_client - Find or create a DS rpc client
- * @mirror: pointer to the mirror
+ * @mirror_ds: device node for the stripe
* @ds_clp: nfs_client for the DS
* @inode: pointer to inode
- * @dss_id: DS stripe id
*
* Find or create a DS rpc client with th MDS server rpc client auth flavor
* in the nfs_client cl_ds_clients list.
*/
struct rpc_clnt *
-nfs4_ff_find_or_create_ds_client(struct nfs4_ff_layout_mirror *mirror,
- struct nfs_client *ds_clp, struct inode *inode,
- u32 dss_id)
+nfs4_ff_find_or_create_ds_client(const struct nfs4_ff_layout_ds *mirror_ds,
+ struct nfs_client *ds_clp, struct inode *inode)
{
- switch (mirror->dss[dss_id].mirror_ds->ds_versions[0].version) {
+ switch (mirror_ds->ds_versions[0].version) {
case 3:
/* For NFSv3 DS, flavor is set when creating DS connections */
return ds_clp->cl_rpcclient;
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 09/24] NFSv4/flexfiles: Carry the device node reference across each I/O
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (7 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 08/24] NFSv4/flexfiles: Reference the device node across DS setup Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 10/24] NFSv4/flexfiles: Hold a device node reference for layoutstats encoding Benjamin Coddington
` (14 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
Rather than dropping the device node reference when DS setup completes,
transfer it to the in-flight I/O: carry it on nfs_pgio_header and
nfs_commit_data as ds_dev, and release it when the header or commit
data is released, alongside the lseg reference.
Convert the completion paths to use the carried node instead of
re-reading the mirror's pinned pointer: DS error tracking, marking the
deviceid available/unavailable, and deleting the deviceid on connection
errors now act on the node the I/O was actually sent to. Once a CHANGE
notification can re-point the pinned pointer mid-flight, this keeps
error attribution on the old device rather than its replacement (and
removes a NULL dereference had the pointer been reset to NULL).
FF_LAYOUT_DEVID_NODE() is now unused; remove it.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 82 ++++++++++-------------
fs/nfs/flexfilelayout/flexfilelayout.h | 15 +----
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 11 +--
fs/nfs/pnfs.c | 2 +
fs/nfs/pnfs.h | 8 +++
fs/nfs/pnfs_nfs.c | 1 +
include/linux/nfs_xdr.h | 2 +
7 files changed, 58 insertions(+), 63 deletions(-)
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 11fc27a062f6..d55f2759df98 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -857,24 +857,6 @@ nfs4_ff_layout_stat_io_end_write(struct rpc_task *task,
spin_unlock(&mirror->lock);
}
-static void
-ff_layout_mark_ds_unreachable(struct pnfs_layout_segment *lseg, u32 idx, u32 dss_id)
-{
- struct nfs4_deviceid_node *devid = FF_LAYOUT_DEVID_NODE(lseg, idx, dss_id);
-
- if (devid)
- nfs4_mark_deviceid_unavailable(devid);
-}
-
-static void
-ff_layout_mark_ds_reachable(struct pnfs_layout_segment *lseg, u32 idx, u32 dss_id)
-{
- struct nfs4_deviceid_node *devid = FF_LAYOUT_DEVID_NODE(lseg, idx, dss_id);
-
- if (devid)
- nfs4_mark_deviceid_available(devid);
-}
-
static struct nfs4_ff_layout_ds *
ff_layout_choose_ds_for_read(struct pnfs_layout_segment *lseg,
u32 start_idx, u32 *best_idx,
@@ -1333,11 +1315,10 @@ static int ff_layout_async_handle_error_v4(struct rpc_task *task,
struct nfs4_state *state,
struct nfs_client *clp,
struct pnfs_layout_segment *lseg,
- u32 idx, u32 dss_id)
+ struct nfs4_deviceid_node *devid)
{
struct pnfs_layout_hdr *lo = lseg->pls_layout;
struct inode *inode = lo->plh_inode;
- struct nfs4_deviceid_node *devid = FF_LAYOUT_DEVID_NODE(lseg, idx, dss_id);
struct nfs4_slot_table *tbl = nfs4_has_session(clp) ?
&clp->cl_session->fc_slot_table : clp->cl_slot_tbl;
@@ -1410,8 +1391,9 @@ static int ff_layout_async_handle_error_v4(struct rpc_task *task,
case -ENODEV:
dprintk("%s DS connection error %d\n", __func__,
task->tk_status);
- nfs4_delete_deviceid(devid->ld, devid->nfs_client,
- &devid->deviceid);
+ if (devid)
+ nfs4_delete_deviceid(devid->ld, devid->nfs_client,
+ &devid->deviceid);
rpc_wake_up(&tbl->slot_tbl_waitq);
break;
default:
@@ -1435,9 +1417,8 @@ static int ff_layout_async_handle_error_v3(struct rpc_task *task,
u32 op_status,
struct nfs_client *clp,
struct pnfs_layout_segment *lseg,
- u32 idx, u32 dss_id)
+ struct nfs4_deviceid_node *devid)
{
- struct nfs4_deviceid_node *devid = FF_LAYOUT_DEVID_NODE(lseg, idx, dss_id);
switch (op_status) {
case NFS_OK:
@@ -1483,8 +1464,9 @@ static int ff_layout_async_handle_error_v3(struct rpc_task *task,
default:
dprintk("%s DS connection error %d\n", __func__,
task->tk_status);
- nfs4_delete_deviceid(devid->ld, devid->nfs_client,
- &devid->deviceid);
+ if (devid)
+ nfs4_delete_deviceid(devid->ld, devid->nfs_client,
+ &devid->deviceid);
}
out_reset_to_pnfs:
/* FIXME: Need to prevent infinite looping here. */
@@ -1501,12 +1483,13 @@ static int ff_layout_async_handle_error(struct rpc_task *task,
struct nfs4_state *state,
struct nfs_client *clp,
struct pnfs_layout_segment *lseg,
- u32 idx, u32 dss_id)
+ struct nfs4_deviceid_node *devid)
{
int vers = clp->cl_nfs_mod->rpc_vers->number;
if (task->tk_status >= 0) {
- ff_layout_mark_ds_reachable(lseg, idx, dss_id);
+ if (devid)
+ nfs4_mark_deviceid_available(devid);
return 0;
}
@@ -1517,10 +1500,10 @@ static int ff_layout_async_handle_error(struct rpc_task *task,
switch (vers) {
case 3:
return ff_layout_async_handle_error_v3(task, op_status, clp,
- lseg, idx, dss_id);
+ lseg, devid);
case 4:
return ff_layout_async_handle_error_v4(task, op_status, state,
- clp, lseg, idx, dss_id);
+ clp, lseg, devid);
default:
/* should never happen */
WARN_ON_ONCE(1);
@@ -1529,6 +1512,7 @@ static int ff_layout_async_handle_error(struct rpc_task *task,
}
static void ff_layout_io_track_ds_error(struct pnfs_layout_segment *lseg,
+ struct nfs4_deviceid_node *devid,
u32 idx, u32 dss_id, u64 offset, u64 length,
u32 *op_status, int opnum, int error)
{
@@ -1578,8 +1562,8 @@ static void ff_layout_io_track_ds_error(struct pnfs_layout_segment *lseg,
mirror = FF_LAYOUT_COMP(lseg, idx);
err = ff_layout_track_ds_error(FF_LAYOUT_FROM_HDR(lseg->pls_layout),
- mirror, dss_id, offset, length, status, opnum,
- nfs_io_gfp_mask());
+ mirror, devid, dss_id, offset, length,
+ status, opnum, nfs_io_gfp_mask());
/*
* I/O we cancelled ourselves to return a recalled or revoked layout
@@ -1596,7 +1580,8 @@ static void ff_layout_io_track_ds_error(struct pnfs_layout_segment *lseg,
case NFS4ERR_PERM:
break;
case NFS4ERR_NXIO:
- ff_layout_mark_ds_unreachable(lseg, idx, dss_id);
+ if (devid)
+ nfs4_mark_deviceid_unavailable(devid);
/*
* Don't return the layout if this is a read and we still
* have layouts to try
@@ -1625,7 +1610,7 @@ static int ff_layout_read_done_cb(struct rpc_task *task,
int err;
if (task->tk_status < 0) {
- ff_layout_io_track_ds_error(hdr->lseg,
+ ff_layout_io_track_ds_error(hdr->lseg, hdr->ds_dev,
hdr->pgio_mirror_idx, dss_id,
hdr->args.offset, hdr->args.count,
&hdr->res.op_status, OP_READ,
@@ -1636,8 +1621,7 @@ static int ff_layout_read_done_cb(struct rpc_task *task,
err = ff_layout_async_handle_error(task, hdr->res.op_status,
hdr->args.context->state,
hdr->ds_clp, hdr->lseg,
- hdr->pgio_mirror_idx,
- dss_id);
+ hdr->ds_dev);
trace_nfs4_pnfs_read(hdr, err);
clear_bit(NFS_IOHDR_RESEND_PNFS, &hdr->flags);
@@ -1830,7 +1814,7 @@ static int ff_layout_write_done_cb(struct rpc_task *task,
int err;
if (task->tk_status < 0) {
- ff_layout_io_track_ds_error(hdr->lseg,
+ ff_layout_io_track_ds_error(hdr->lseg, hdr->ds_dev,
hdr->pgio_mirror_idx, dss_id,
hdr->args.offset, hdr->args.count,
&hdr->res.op_status, OP_WRITE,
@@ -1841,8 +1825,7 @@ static int ff_layout_write_done_cb(struct rpc_task *task,
err = ff_layout_async_handle_error(task, hdr->res.op_status,
hdr->args.context->state,
hdr->ds_clp, hdr->lseg,
- hdr->pgio_mirror_idx,
- dss_id);
+ hdr->ds_dev);
trace_nfs4_pnfs_write(hdr, err);
clear_bit(NFS_IOHDR_RESEND_PNFS, &hdr->flags);
@@ -1884,7 +1867,7 @@ static int ff_layout_commit_done_cb(struct rpc_task *task,
u32 dss_id = calc_dss_id_from_commit(data->lseg, data->ds_commit_index);
if (task->tk_status < 0) {
- ff_layout_io_track_ds_error(data->lseg, idx, dss_id,
+ ff_layout_io_track_ds_error(data->lseg, data->ds_dev, idx, dss_id,
data->args.offset, data->args.count,
&data->res.op_status, OP_COMMIT,
task->tk_status);
@@ -1892,8 +1875,8 @@ static int ff_layout_commit_done_cb(struct rpc_task *task,
}
err = ff_layout_async_handle_error(task, data->res.op_status,
- NULL, data->ds_clp, data->lseg, idx,
- dss_id);
+ NULL, data->ds_clp, data->lseg,
+ data->ds_dev);
trace_nfs4_pnfs_commit_ds(data, err);
switch (err) {
@@ -2248,13 +2231,16 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
ff_layout_read_record_layoutstats_start(&hdr->task, hdr);
}
+ /* Transfer the device node reference to the I/O; put on release */
+ pnfs_put_ds_dev(hdr->ds_dev);
+ hdr->ds_dev = &mirror_ds->id_node;
+
/* Perform an asynchronous read to ds */
nfs_initiate_pgio(ds_clnt, hdr, ds_cred, ds->ds_clp->rpc_ops,
vers == 3 ? &ff_layout_read_call_ops_v3 :
&ff_layout_read_call_ops_v4,
0, RPC_TASK_SOFTCONN, localio);
put_cred(ds_cred);
- nfs4_ff_layout_put_deviceid(mirror_ds);
return PNFS_ATTEMPTED;
out_failed:
@@ -2347,13 +2333,16 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
ff_layout_write_record_layoutstats_start(&hdr->task, hdr);
}
+ /* Transfer the device node reference to the I/O; put on release */
+ pnfs_put_ds_dev(hdr->ds_dev);
+ hdr->ds_dev = &mirror_ds->id_node;
+
/* Perform an asynchronous write */
nfs_initiate_pgio(ds_clnt, hdr, ds_cred, ds->ds_clp->rpc_ops,
vers == 3 ? &ff_layout_write_call_ops_v3 :
&ff_layout_write_call_ops_v4,
sync, RPC_TASK_SOFTCONN, localio);
put_cred(ds_cred);
- nfs4_ff_layout_put_deviceid(mirror_ds);
return PNFS_ATTEMPTED;
out_failed:
@@ -2441,12 +2430,15 @@ static int ff_layout_initiate_commit(struct nfs_commit_data *data, int how)
ff_layout_commit_record_layoutstats_start(&data->task, data);
}
+ /* Transfer the device node reference to the commit; put on release */
+ pnfs_put_ds_dev(data->ds_dev);
+ data->ds_dev = &mirror_ds->id_node;
+
ret = nfs_initiate_commit(ds_clnt, data, ds->ds_clp->rpc_ops,
vers == 3 ? &ff_layout_commit_call_ops_v3 :
&ff_layout_commit_call_ops_v4,
how, RPC_TASK_SOFTCONN, localio);
put_cred(ds_cred);
- nfs4_ff_layout_put_deviceid(mirror_ds);
return ret;
out_err:
nfs4_ff_layout_put_deviceid(mirror_ds);
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index f9e491a0347e..7eb47ce09442 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -162,20 +162,6 @@ FF_LAYOUT_COMP(struct pnfs_layout_segment *lseg, u32 idx)
return NULL;
}
-static inline struct nfs4_deviceid_node *
-FF_LAYOUT_DEVID_NODE(struct pnfs_layout_segment *lseg, u32 idx, u32 dss_id)
-{
- struct nfs4_ff_layout_mirror *mirror = FF_LAYOUT_COMP(lseg, idx);
-
- if (mirror != NULL) {
- struct nfs4_ff_layout_ds *mirror_ds = mirror->dss[dss_id].mirror_ds;
-
- if (!IS_ERR_OR_NULL(mirror_ds))
- return &mirror_ds->id_node;
- }
- return NULL;
-}
-
static inline u32
FF_LAYOUT_MIRROR_COUNT(struct pnfs_layout_segment *lseg)
{
@@ -232,6 +218,7 @@ void nfs4_ff_layout_put_deviceid(struct nfs4_ff_layout_ds *mirror_ds);
void nfs4_ff_layout_free_deviceid(struct nfs4_ff_layout_ds *mirror_ds);
int ff_layout_track_ds_error(struct nfs4_flexfile_layout *flo,
struct nfs4_ff_layout_mirror *mirror,
+ const struct nfs4_deviceid_node *devid,
u32 dss_id, u64 offset, u64 length, int status,
enum nfs_opnum4 opnum, gfp_t gfp_flags);
void ff_layout_send_layouterror(struct pnfs_layout_segment *lseg);
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index 3712a47b20d0..ebcfba447879 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -243,6 +243,7 @@ ff_layout_add_ds_error_locked(struct nfs4_flexfile_layout *flo,
int ff_layout_track_ds_error(struct nfs4_flexfile_layout *flo,
struct nfs4_ff_layout_mirror *mirror,
+ const struct nfs4_deviceid_node *devid,
u32 dss_id, u64 offset, u64 length, int status,
enum nfs_opnum4 opnum, gfp_t gfp_flags)
{
@@ -251,7 +252,7 @@ int ff_layout_track_ds_error(struct nfs4_flexfile_layout *flo,
if (status == 0)
return 0;
- if (IS_ERR_OR_NULL(mirror->dss[dss_id].mirror_ds))
+ if (devid == NULL)
return -EINVAL;
dserr = kmalloc_obj(*dserr, gfp_flags);
@@ -264,8 +265,7 @@ int ff_layout_track_ds_error(struct nfs4_flexfile_layout *flo,
dserr->status = status;
dserr->opnum = opnum;
nfs4_stateid_copy(&dserr->stateid, &mirror->dss[dss_id].stateid);
- memcpy(&dserr->deviceid, &mirror->dss[dss_id].mirror_ds->id_node.deviceid,
- NFS4_DEVICEID4_SIZE);
+ memcpy(&dserr->deviceid, &devid->deviceid, NFS4_DEVICEID4_SIZE);
spin_lock(&flo->generic_hdr.plh_inode->i_lock);
ff_layout_add_ds_error_locked(flo, dserr);
@@ -422,7 +422,10 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg,
}
noconnect:
ff_layout_track_ds_error(FF_LAYOUT_FROM_HDR(lseg->pls_layout),
- mirror, dss_id, lseg->pls_range.offset,
+ mirror,
+ IS_ERR_OR_NULL(mirror_ds) ?
+ NULL : &mirror_ds->id_node,
+ dss_id, lseg->pls_range.offset,
lseg->pls_range.length, NFS4ERR_NXIO,
OP_ILLEGAL, GFP_NOIO);
ff_layout_send_layouterror(lseg);
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 4f9c0f639014..b44274ab8b09 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -3103,6 +3103,7 @@ pnfs_do_write(struct nfs_pageio_descriptor *desc,
static void pnfs_writehdr_free(struct nfs_pgio_header *hdr)
{
+ pnfs_put_ds_dev(hdr->ds_dev);
pnfs_put_lseg(hdr->lseg);
nfs_pgio_header_free(hdr);
}
@@ -3248,6 +3249,7 @@ pnfs_do_read(struct nfs_pageio_descriptor *desc, struct nfs_pgio_header *hdr)
static void pnfs_readhdr_free(struct nfs_pgio_header *hdr)
{
+ pnfs_put_ds_dev(hdr->ds_dev);
pnfs_put_lseg(hdr->lseg);
nfs_pgio_header_free(hdr);
}
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 5725ea06efc2..a2c70573dda5 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -385,6 +385,14 @@ void nfs4_init_deviceid_node(struct nfs4_deviceid_node *, struct nfs_server *,
const struct nfs4_deviceid *);
bool nfs4_put_deviceid_node(struct nfs4_deviceid_node *);
void nfs4_mark_deviceid_available(struct nfs4_deviceid_node *node);
+
+/* Put the device node reference carried by an in-flight I/O, if any */
+static inline void pnfs_put_ds_dev(struct nfs4_deviceid_node *dev)
+{
+ if (dev)
+ nfs4_put_deviceid_node(dev);
+}
+
void nfs4_mark_deviceid_unavailable(struct nfs4_deviceid_node *node);
bool nfs4_test_deviceid_unavailable(struct nfs4_deviceid_node *node);
void nfs4_deviceid_purge_client(const struct nfs_client *);
diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index 5bd014b981bf..e0e3fc7414e6 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -55,6 +55,7 @@ void pnfs_generic_commit_release(void *calldata)
struct nfs_commit_data *data = calldata;
data->completion_ops->completion(data);
+ pnfs_put_ds_dev(data->ds_dev);
pnfs_put_lseg(data->lseg);
nfs_put_client(data->ds_clp);
nfs_commitdata_release(data);
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 7ed8fdb930d6..c0e29b4dfa62 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1693,6 +1693,7 @@ struct nfs_pgio_header {
struct nfs_client *ds_clp; /* pNFS data server */
u32 ds_commit_idx; /* ds index if ds_clp is set */
u32 pgio_mirror_idx;/* mirror index in pgio layer */
+ struct nfs4_deviceid_node *ds_dev; /* device node ref held across the I/O */
};
struct nfs_mds_commit_info {
@@ -1731,6 +1732,7 @@ struct nfs_commit_data {
struct nfs_open_context *context;
struct pnfs_layout_segment *lseg;
struct nfs_client *ds_clp; /* pNFS data server */
+ struct nfs4_deviceid_node *ds_dev; /* device node ref held across the commit */
int ds_commit_index;
loff_t lwb;
const struct rpc_call_ops *mds_ops;
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 10/24] NFSv4/flexfiles: Hold a device node reference for layoutstats encoding
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (8 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 09/24] NFSv4/flexfiles: Carry the device node reference across each I/O Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 11/24] NFSv4/flexfiles: Make the pinned device node pointer RCU-managed Benjamin Coddington
` (13 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
ff_layout_mirror_prepare_stats() records a stripe's device under i_lock,
but the layoutstats/layoutreturn XDR encode runs later and dereferences
the mirror's pinned device node (for the DS netaddr) with no reference
of its own. Once a CHANGE notification can re-point the pinned node,
that read becomes use-after-free.
Take a node reference at prepare time and carry it with the stripe in a
per-devinfo nfs4_ff_layoutstat_priv, released in
ff_layout_free_layoutstats(). The priv entries live alongside the
devinfo array: co-allocated for LAYOUTSTATS, an additional member of
nfs4_flexfile_layoutreturn_args for LAYOUTRETURN.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 53 +++++++++++++++++---------
fs/nfs/flexfilelayout/flexfilelayout.h | 11 ++++++
2 files changed, 47 insertions(+), 17 deletions(-)
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index d55f2759df98..015428fd6937 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -44,10 +44,11 @@ static void ff_layout_read_record_layoutstats_done(struct rpc_task *task,
static int
ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
struct nfs42_layoutstat_devinfo *devinfo,
+ struct nfs4_ff_layoutstat_priv *priv,
int dev_limit, enum nfs4_ff_op_type type);
static void ff_layout_encode_ff_layoutupdate(struct xdr_stream *xdr,
const struct nfs42_layoutstat_devinfo *devinfo,
- struct nfs4_ff_layout_ds_stripe *dss_info);
+ struct nfs4_ff_layoutstat_priv *priv);
static struct pnfs_layout_hdr *
ff_layout_alloc_layout_hdr(struct inode *inode, gfp_t gfp_flags)
@@ -2725,7 +2726,7 @@ ff_layout_prepare_layoutreturn(struct nfs4_layoutreturn_args *args)
spin_lock(&args->inode->i_lock);
ff_args->num_dev = ff_layout_mirror_prepare_stats(
- &ff_layout->generic_hdr, &ff_args->devinfo[0],
+ &ff_layout->generic_hdr, &ff_args->devinfo[0], &ff_args->priv[0],
ARRAY_SIZE(ff_args->devinfo), NFS4_FF_OP_LAYOUTRETURN);
spin_unlock(&args->inode->i_lock);
@@ -2900,10 +2901,11 @@ ff_layout_encode_io_latency(struct xdr_stream *xdr,
static void
ff_layout_encode_ff_layoutupdate(struct xdr_stream *xdr,
const struct nfs42_layoutstat_devinfo *devinfo,
- struct nfs4_ff_layout_ds_stripe *dss_info)
+ struct nfs4_ff_layoutstat_priv *priv)
{
+ struct nfs4_ff_layout_ds_stripe *dss_info = priv->dss_info;
struct nfs4_pnfs_ds_addr *da;
- struct nfs4_pnfs_ds *ds = dss_info->mirror_ds->ds;
+ struct nfs4_pnfs_ds *ds = priv->mirror_ds->ds;
struct nfs_fh *fh = &dss_info->fh_versions[0];
__be32 *p;
@@ -2950,10 +2952,10 @@ ff_layout_encode_layoutstats(struct xdr_stream *xdr, const void *args,
static void
ff_layout_free_layoutstats(struct nfs4_xdr_opaque_data *opaque)
{
- struct nfs4_ff_layout_ds_stripe *dss_info = opaque->data;
- struct nfs4_ff_layout_mirror *mirror = dss_info->mirror;
+ struct nfs4_ff_layoutstat_priv *priv = opaque->data;
- ff_layout_put_mirror(mirror);
+ nfs4_ff_layout_put_deviceid(priv->mirror_ds);
+ ff_layout_put_mirror(priv->dss_info->mirror);
}
static const struct nfs4_xdr_opaque_ops layoutstat_ops = {
@@ -2964,12 +2966,13 @@ static const struct nfs4_xdr_opaque_ops layoutstat_ops = {
static int
ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
struct nfs42_layoutstat_devinfo *devinfo,
+ struct nfs4_ff_layoutstat_priv *priv,
int dev_limit, enum nfs4_ff_op_type type)
{
struct nfs4_flexfile_layout *ff_layout = FF_LAYOUT_FROM_HDR(lo);
struct nfs4_ff_layout_mirror *mirror;
struct nfs4_ff_layout_ds_stripe *dss_info;
- struct nfs4_deviceid_node *dev;
+ struct nfs4_ff_layout_ds *mirror_ds;
int i = 0, dss_id;
list_for_each_entry(mirror, &ff_layout->mirrors, mirrors) {
@@ -2977,7 +2980,8 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
dss_info = &mirror->dss[dss_id];
if (i >= dev_limit)
break;
- if (IS_ERR_OR_NULL(dss_info->mirror_ds))
+ mirror_ds = dss_info->mirror_ds;
+ if (IS_ERR_OR_NULL(mirror_ds))
continue;
if (!test_and_clear_bit(NFS4_FF_MIRROR_STAT_AVAIL,
&mirror->flags) &&
@@ -2986,9 +2990,14 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
/* mirror refcount put in cleanup_layoutstats */
if (!refcount_inc_not_zero(&mirror->ref))
continue;
- dev = &dss_info->mirror_ds->id_node;
+ /*
+ * The mirror's pin holds the node while we're under
+ * i_lock; take a reference for the encode, put in
+ * ff_layout_free_layoutstats().
+ */
+ atomic_inc(&mirror_ds->id_node.ref);
memcpy(&devinfo->dev_id,
- &dev->deviceid,
+ &mirror_ds->id_node.deviceid,
NFS4_DEVICEID4_SIZE);
devinfo->offset = 0;
devinfo->length = NFS4_MAX_UINT64;
@@ -3004,9 +3013,12 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
spin_unlock(&mirror->lock);
devinfo->layout_type = LAYOUT_FLEX_FILES;
devinfo->ld_private.ops = &layoutstat_ops;
- devinfo->ld_private.data = &mirror->dss[dss_id];
+ priv->dss_info = dss_info;
+ priv->mirror_ds = mirror_ds;
+ devinfo->ld_private.data = priv;
devinfo++;
+ priv++;
i++;
}
}
@@ -3017,21 +3029,28 @@ static int ff_layout_prepare_layoutstats(struct nfs42_layoutstat_args *args)
{
struct pnfs_layout_hdr *lo;
struct nfs4_flexfile_layout *ff_layout;
+ struct nfs4_ff_layoutstat_priv *priv;
const int dev_count = PNFS_LAYOUTSTATS_MAXDEV;
- /* For now, send at most PNFS_LAYOUTSTATS_MAXDEV statistics */
- args->devinfo = kmalloc_objs(*args->devinfo, dev_count,
- nfs_io_gfp_mask());
+ /*
+ * For now, send at most PNFS_LAYOUTSTATS_MAXDEV statistics.
+ * The per-devinfo private entries are co-allocated after the
+ * devinfo array and freed along with it.
+ */
+ args->devinfo = kmalloc(dev_count * (sizeof(*args->devinfo) +
+ sizeof(*priv)),
+ nfs_io_gfp_mask());
if (!args->devinfo)
return -ENOMEM;
+ priv = (struct nfs4_ff_layoutstat_priv *)&args->devinfo[dev_count];
spin_lock(&args->inode->i_lock);
lo = NFS_I(args->inode)->layout;
if (lo && pnfs_layout_is_valid(lo)) {
ff_layout = FF_LAYOUT_FROM_HDR(lo);
args->num_dev = ff_layout_mirror_prepare_stats(
- &ff_layout->generic_hdr, &args->devinfo[0], dev_count,
- NFS4_FF_OP_LAYOUTSTATS);
+ &ff_layout->generic_hdr, &args->devinfo[0], priv,
+ dev_count, NFS4_FF_OP_LAYOUTSTATS);
} else
args->num_dev = 0;
spin_unlock(&args->inode->i_lock);
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 7eb47ce09442..8b42a98a2c4c 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -124,9 +124,20 @@ struct nfs4_flexfile_layout {
unsigned long flags;
};
+/*
+ * Per-devinfo private data for a layoutstats/layoutreturn encode: the
+ * stripe the stats describe plus a reference on its device node so the
+ * node (and its DS addresses) stay valid until the XDR encode runs.
+ */
+struct nfs4_ff_layoutstat_priv {
+ struct nfs4_ff_layout_ds_stripe *dss_info;
+ struct nfs4_ff_layout_ds *mirror_ds;
+};
+
struct nfs4_flexfile_layoutreturn_args {
struct list_head errors;
struct nfs42_layoutstat_devinfo devinfo[FF_LAYOUTSTATS_MAXDEV];
+ struct nfs4_ff_layoutstat_priv priv[FF_LAYOUTSTATS_MAXDEV];
unsigned int num_errors;
unsigned int num_dev;
struct page *pages[1];
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 11/24] NFSv4/flexfiles: Make the pinned device node pointer RCU-managed
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (9 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 10/24] NFSv4/flexfiles: Hold a device node reference for layoutstats encoding Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 12/24] pNFS: Add a reresolve_deviceid layout driver hook Benjamin Coddington
` (12 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
Annotate mirror->dss[dss_id].mirror_ds as __rcu and convert the
remaining readers, completing the preparation for re-pointing the pinned
node while I/O is in flight:
- ff_layout_get_mirror_ds() takes its reference under rcu_read_lock()
with atomic_inc_not_zero(), retrying if it races a reset. The
resolve path takes the caller's reference before publishing the node
with cmpxchg(), so a concurrent reset cannot drop the last reference
under the caller: one reference is held for the installed pointer and
one for the caller, and the pointer's reference is released elsewhere
by xchg + put.
- The availability scans hold rcu_read_lock() across the walk; they
only test flags on the RCU-freed node.
- ff_layout_cancel_io() holds a reference across the cancel and
disconnect calls. Both of its callers hold i_lock and nothing on
that path sleeps, so the reference is not about blocking: it is
what keeps the node alive between the RCU-protected read and the
use of mirror_ds->ds. The final put is never reached here, because
the mirror's own pin outlives the loop -- which matters, since that
put ends in nfs_put_client() and cannot run under a spinlock.
- ff_layout_mirror_prepare_stats() reads the pointer with
rcu_dereference() under rcu_read_lock(). i_lock, which both callers
hold, is what excludes the re-pointing walk added later in this
series; it does not exclude the resolve path's cmpxchg(), which runs
from I/O submission, so the read cannot claim i_lock as its update-
side lock. A non-NULL pointer read there is stable regardless: the
resolve path only ever installs over NULL.
- ff_layout_free_mirror() tears down the last reference; no concurrency.
The pointer is still only ever set once per mirror lifetime, so there is
no behavior change; this commit makes the subsequent in-place re-resolve
on CB_NOTIFY_DEVICEID CHANGE safe to introduce.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 35 ++++---
fs/nfs/flexfilelayout/flexfilelayout.h | 2 +-
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 110 ++++++++++++++--------
3 files changed, 93 insertions(+), 54 deletions(-)
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 015428fd6937..eeda084d20dd 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -313,7 +313,9 @@ static void ff_layout_free_mirror(struct nfs4_ff_layout_mirror *mirror)
cred = rcu_access_pointer(mirror->dss[dss_id].rw_cred);
put_cred(cred);
nfs_close_local_fh(&mirror->dss[dss_id].nfl);
- nfs4_ff_layout_put_deviceid(mirror->dss[dss_id].mirror_ds);
+ /* the last reference to the mirror is gone; no concurrency */
+ nfs4_ff_layout_put_deviceid(rcu_dereference_protected(
+ mirror->dss[dss_id].mirror_ds, 1));
}
kfree(mirror->dss);
@@ -2498,22 +2500,29 @@ static void ff_layout_cancel_io(struct pnfs_layout_segment *lseg)
for (idx = 0; idx < flseg->mirror_array_cnt; idx++) {
mirror = flseg->mirror_array[idx];
for (dss_id = 0; dss_id < mirror->dss_count; dss_id++) {
- mirror_ds = mirror->dss[dss_id].mirror_ds;
- if (IS_ERR_OR_NULL(mirror_ds))
+ rcu_read_lock();
+ mirror_ds = rcu_dereference(mirror->dss[dss_id].mirror_ds);
+ if (IS_ERR_OR_NULL(mirror_ds) ||
+ !atomic_inc_not_zero(&mirror_ds->id_node.ref)) {
+ rcu_read_unlock();
continue;
- ds = mirror->dss[dss_id].mirror_ds->ds;
+ }
+ rcu_read_unlock();
+ ds = mirror_ds->ds;
if (!ds)
- continue;
+ goto next;
ds_clp = ds->ds_clp;
if (!ds_clp)
- continue;
+ goto next;
clnt = ds_clp->cl_rpcclient;
if (!clnt)
- continue;
+ goto next;
if (!rpc_cancel_tasks(clnt, -ECANCELED,
ff_layout_match_io, lseg))
- continue;
+ goto next;
rpc_clnt_disconnect(clnt);
+next:
+ nfs4_ff_layout_put_deviceid(mirror_ds);
}
}
}
@@ -2975,12 +2984,13 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
struct nfs4_ff_layout_ds *mirror_ds;
int i = 0, dss_id;
+ rcu_read_lock();
list_for_each_entry(mirror, &ff_layout->mirrors, mirrors) {
for (dss_id = 0; dss_id < mirror->dss_count; ++dss_id) {
dss_info = &mirror->dss[dss_id];
if (i >= dev_limit)
break;
- mirror_ds = dss_info->mirror_ds;
+ mirror_ds = rcu_dereference(dss_info->mirror_ds);
if (IS_ERR_OR_NULL(mirror_ds))
continue;
if (!test_and_clear_bit(NFS4_FF_MIRROR_STAT_AVAIL,
@@ -2990,10 +3000,8 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
/* mirror refcount put in cleanup_layoutstats */
if (!refcount_inc_not_zero(&mirror->ref))
continue;
- /*
- * The mirror's pin holds the node while we're under
- * i_lock; take a reference for the encode, put in
- * ff_layout_free_layoutstats().
+ /* The pin holds a reference; it is exchanged out only
+ * under i_lock. Put in ff_layout_free_layoutstats().
*/
atomic_inc(&mirror_ds->id_node.ref);
memcpy(&devinfo->dev_id,
@@ -3022,6 +3030,7 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
i++;
}
}
+ rcu_read_unlock();
return i;
}
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 8b42a98a2c4c..72b11034851a 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -79,7 +79,7 @@ struct nfs4_ff_layout_ds_stripe {
struct nfs4_ff_layout_mirror *mirror;
struct nfs4_deviceid devid;
u32 efficiency;
- struct nfs4_ff_layout_ds *mirror_ds;
+ struct nfs4_ff_layout_ds __rcu *mirror_ds;
u32 fh_versions_cnt;
struct nfs_fh *fh_versions;
nfs4_stateid stateid;
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index ebcfba447879..5cb09e5e2138 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -321,35 +321,54 @@ ff_layout_get_mirror_ds(struct pnfs_layout_hdr *lo,
struct nfs4_ff_layout_mirror *mirror,
u32 dss_id)
{
- struct nfs4_ff_layout_ds *mirror_ds;
+ struct nfs4_ff_layout_ds *mirror_ds, *old;
+ struct nfs4_deviceid_node *node;
if (mirror == NULL)
return ERR_PTR(-ENODEV);
- mirror_ds = mirror->dss[dss_id].mirror_ds;
- if (mirror_ds == NULL) {
- struct nfs4_deviceid_node *node;
-
+retry:
+ rcu_read_lock();
+ mirror_ds = rcu_dereference(mirror->dss[dss_id].mirror_ds);
+ if (mirror_ds && !IS_ERR(mirror_ds) &&
+ atomic_inc_not_zero(&mirror_ds->id_node.ref)) {
+ rcu_read_unlock();
+ return mirror_ds;
+ }
+ rcu_read_unlock();
+ if (IS_ERR(mirror_ds))
+ return mirror_ds;
+ if (mirror_ds != NULL)
+ /* raced with a reset; the field is being re-pointed */
+ goto retry;
+
+ node = nfs4_find_get_deviceid(NFS_SERVER(lo->plh_inode),
+ &mirror->dss[dss_id].devid, lo->plh_lc_cred,
+ GFP_KERNEL);
+ if (node) {
+ mirror_ds = FF_LAYOUT_MIRROR_DS(node);
+ /*
+ * Take the caller's reference before the pointer becomes
+ * visible below, so a concurrent reset of the installed
+ * pointer cannot drop the last reference under us.
+ */
+ atomic_inc(&node->ref);
+ } else {
mirror_ds = ERR_PTR(-ENODEV);
- node = nfs4_find_get_deviceid(NFS_SERVER(lo->plh_inode),
- &mirror->dss[dss_id].devid, lo->plh_lc_cred,
- GFP_KERNEL);
- if (node)
- mirror_ds = FF_LAYOUT_MIRROR_DS(node);
-
- /* check for race with another call to this function */
- if (cmpxchg(&mirror->dss[dss_id].mirror_ds, NULL, mirror_ds) &&
- mirror_ds != ERR_PTR(-ENODEV))
- nfs4_put_deviceid_node(node);
-
- mirror_ds = mirror->dss[dss_id].mirror_ds;
}
- if (IS_ERR(mirror_ds))
+ /* check for race with another call to this function */
+ old = unrcu_pointer(cmpxchg(&mirror->dss[dss_id].mirror_ds,
+ NULL, RCU_INITIALIZER(mirror_ds)));
+ if (old == NULL)
return mirror_ds;
- if (!atomic_inc_not_zero(&mirror_ds->id_node.ref))
- return ERR_PTR(-ENODEV);
- return mirror_ds;
+
+ /* lost the race; use the winner's node instead */
+ if (node) {
+ nfs4_put_deviceid_node(node);
+ nfs4_put_deviceid_node(node);
+ }
+ goto retry;
}
/**
@@ -573,49 +592,60 @@ unsigned int ff_layout_fetch_ds_ioerr(struct pnfs_layout_hdr *lo,
static bool ff_read_layout_has_available_ds(struct pnfs_layout_segment *lseg)
{
struct nfs4_ff_layout_mirror *mirror;
- struct nfs4_deviceid_node *devid;
+ struct nfs4_ff_layout_ds *mirror_ds;
+ bool ret = false;
u32 idx, dss_id;
+ rcu_read_lock();
for (idx = 0; idx < FF_LAYOUT_MIRROR_COUNT(lseg); idx++) {
mirror = FF_LAYOUT_COMP(lseg, idx);
if (!mirror)
continue;
for (dss_id = 0; dss_id < mirror->dss_count; dss_id++) {
- if (!mirror->dss[dss_id].mirror_ds)
- return true;
- if (IS_ERR(mirror->dss[dss_id].mirror_ds))
+ mirror_ds = rcu_dereference(mirror->dss[dss_id].mirror_ds);
+ if (!mirror_ds) {
+ ret = true;
+ goto out;
+ }
+ if (IS_ERR(mirror_ds))
continue;
- devid = &mirror->dss[dss_id].mirror_ds->id_node;
- if (!nfs4_test_deviceid_unavailable(devid))
- return true;
+ if (!nfs4_test_deviceid_unavailable(&mirror_ds->id_node)) {
+ ret = true;
+ goto out;
+ }
}
}
-
- return false;
+out:
+ rcu_read_unlock();
+ return ret;
}
static bool ff_rw_layout_has_available_ds(struct pnfs_layout_segment *lseg)
{
struct nfs4_ff_layout_mirror *mirror;
- struct nfs4_deviceid_node *devid;
+ struct nfs4_ff_layout_ds *mirror_ds;
+ bool ret = false;
u32 idx, dss_id;
+ rcu_read_lock();
for (idx = 0; idx < FF_LAYOUT_MIRROR_COUNT(lseg); idx++) {
mirror = FF_LAYOUT_COMP(lseg, idx);
if (!mirror)
- return false;
+ goto out;
for (dss_id = 0; dss_id < mirror->dss_count; dss_id++) {
- if (IS_ERR(mirror->dss[dss_id].mirror_ds))
- return false;
- if (!mirror->dss[dss_id].mirror_ds)
+ mirror_ds = rcu_dereference(mirror->dss[dss_id].mirror_ds);
+ if (IS_ERR(mirror_ds))
+ goto out;
+ if (!mirror_ds)
continue;
- devid = &mirror->dss[dss_id].mirror_ds->id_node;
- if (nfs4_test_deviceid_unavailable(devid))
- return false;
+ if (nfs4_test_deviceid_unavailable(&mirror_ds->id_node))
+ goto out;
}
}
-
- return FF_LAYOUT_MIRROR_COUNT(lseg) != 0;
+ ret = FF_LAYOUT_MIRROR_COUNT(lseg) != 0;
+out:
+ rcu_read_unlock();
+ return ret;
}
static bool ff_layout_has_available_ds(struct pnfs_layout_segment *lseg)
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 12/24] pNFS: Add a reresolve_deviceid layout driver hook
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (10 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 11/24] NFSv4/flexfiles: Make the pinned device node pointer RCU-managed Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 13/24] NFSv4/flexfiles: Implement in-place device re-resolve on CHANGE Benjamin Coddington
` (11 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
RFC 8881 Section 12.2.10 lets a server change a deviceid's mapping under
live layouts by sending CB_NOTIFY_DEVICEID CHANGE instead of recalling
the layouts, but the client's only response today is to unhash the
cached device, which never reaches references pinned inside a layout
driver's segments.
Add a reresolve_deviceid hook to pnfs_layoutdriver_type and a generic
driver, pnfs_layout_reresolve_deviceid_byclid(), that walks every layout
on every server of the client using that driver and invokes the hook
under the layout inode's i_lock. Because the final put of a device node
can sleep (it may tear down the DS nfs_client), the hook must not drop
references itself: for each node it un-pins it allocates an
nfs4_deviceid_put entry and queues it on a list, and the generic driver
puts the node and frees the entry once all locks are dropped. A
deviceid node is a shared, refcounted object, so one re-resolve pass can
unpin the same node more than once (multiple stripes, or multiple
layouts over a common data server); a per-reference entry expresses
that, where a single list_head embedded in the node could not.
The walk is deliberately not gated on pnfs_layout_is_valid(). A header
with NFS_LAYOUT_INVALID_STID set can still carry lsegs whose mirrors pin
the stale node -- pnfs_mark_layout_stateid_invalid() sets the bit and
reports whether segments were left behind -- and by the time the walk
runs the cached device has already been unhashed, so nothing would
re-resolve that pin later. Worse, a subsequent LAYOUTGET on the same
header can pick the surviving mirror back up (the driver dedups mirrors
by deviceid and filehandle) and carry the old mapping into a fresh
layout. Un-pinning a device node does not touch the layout stateid, so
the hook has no need of a valid one; it walks only the driver's mirror
list, which i_lock protects, and the header cannot be freed under the
walk because the driver frees it with kfree_rcu().
Note this differs from the reference-collection walker added later in
the series, which does take a layout header reference and therefore
does depend on the validity check for its refcount argument.
No driver implements the hook yet, and nothing calls the walker: no
behavior change.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/pnfs.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
fs/nfs/pnfs.h | 25 +++++++++++++++++++++
2 files changed, 87 insertions(+)
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index b44274ab8b09..2f54b22d0e21 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -2875,6 +2875,68 @@ pnfs_layout_return_unused_byclid(struct nfs_client *clp,
&range);
}
+struct pnfs_reresolve_deviceid_args {
+ const struct pnfs_layoutdriver_type *ld;
+ const struct nfs4_deviceid *id;
+ bool immediate;
+ struct list_head put_list;
+};
+
+static int pnfs_layout_reresolve_deviceid_byserver(struct nfs_server *server,
+ void *data)
+{
+ struct pnfs_reresolve_deviceid_args *args = data;
+ struct pnfs_layout_hdr *lo;
+ struct inode *inode;
+
+ if (server->pnfs_curr_ld != args->ld)
+ return 0;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(lo, &server->layouts, plh_layouts) {
+ inode = lo->plh_inode;
+ if (!inode)
+ continue;
+ spin_lock(&inode->i_lock);
+ args->ld->reresolve_deviceid(lo, args->id, args->immediate,
+ &args->put_list);
+ spin_unlock(&inode->i_lock);
+ }
+ rcu_read_unlock();
+ return 0;
+}
+
+/*
+ * Invoke @ld's reresolve_deviceid hook for @id on every layout of @clp's
+ * servers, then drain the put_list once the locks are dropped.
+ */
+void
+pnfs_layout_reresolve_deviceid_byclid(struct nfs_client *clp,
+ const struct pnfs_layoutdriver_type *ld,
+ const struct nfs4_deviceid *id,
+ bool immediate)
+{
+ struct pnfs_reresolve_deviceid_args args = {
+ .ld = ld,
+ .id = id,
+ .immediate = immediate,
+ .put_list = LIST_HEAD_INIT(args.put_list),
+ };
+ struct nfs4_deviceid_put *put, *tmp;
+
+ if (!ld->reresolve_deviceid)
+ return;
+
+ nfs_client_for_each_server(clp,
+ pnfs_layout_reresolve_deviceid_byserver, &args);
+
+ list_for_each_entry_safe(put, tmp, &args.put_list, node) {
+ list_del(&put->node);
+ nfs4_put_deviceid_node(put->dev);
+ kfree(put);
+ }
+}
+
/* Check if we have we have a valid layout but if there isn't an intersection
* between the request and the pgio->pg_lseg, put this pgio->pg_lseg away.
*/
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index a2c70573dda5..08bea2c4186e 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -171,6 +171,19 @@ struct pnfs_layoutdriver_type {
struct nfs4_deviceid_node * (*alloc_deviceid_node)
(struct nfs_server *server, struct pnfs_device *pdev,
gfp_t gfp_flags);
+ /*
+ * Re-resolve @lo's references to the changed deviceid @id. Called
+ * under @lo's inode i_lock inside an RCU read-side critical section:
+ * must not sleep, allocations are GFP_ATOMIC. Rather than put the
+ * references it gives up (the final put can sleep), the hook
+ * allocates an nfs4_deviceid_put per reference and queues it on
+ * @put_list for the caller to put and free. On allocation failure
+ * it must leave the reference in place.
+ */
+ void (*reresolve_deviceid)(struct pnfs_layout_hdr *lo,
+ const struct nfs4_deviceid *id,
+ bool immediate,
+ struct list_head *put_list);
int (*prepare_layoutreturn) (struct nfs4_layoutreturn_args *);
@@ -354,6 +367,10 @@ void pnfs_error_mark_layout_for_return(struct inode *inode,
struct pnfs_layout_segment *lseg);
void pnfs_layout_return_unused_byclid(struct nfs_client *clp,
enum pnfs_iomode iomode);
+void pnfs_layout_reresolve_deviceid_byclid(struct nfs_client *clp,
+ const struct pnfs_layoutdriver_type *ld,
+ const struct nfs4_deviceid *id,
+ bool immediate);
int pnfs_layout_handle_reboot(struct nfs_client *clp);
/* nfs4_deviceid_flags */
@@ -376,6 +393,14 @@ struct nfs4_deviceid_node {
atomic_t ref;
};
+/* One reference given up by reresolve_deviceid; nodes are shared, so a
+ * single pass can unpin the same node more than once.
+ */
+struct nfs4_deviceid_put {
+ struct list_head node;
+ struct nfs4_deviceid_node *dev;
+};
+
struct nfs4_deviceid_node *
nfs4_find_get_deviceid(struct nfs_server *server,
const struct nfs4_deviceid *id, const struct cred *cred,
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 13/24] NFSv4/flexfiles: Implement in-place device re-resolve on CHANGE
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (11 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 12/24] pNFS: Add a reresolve_deviceid layout driver hook Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 14/24] NFSv4: Dispatch CB_NOTIFY_DEVICEID CHANGE to an in-place refresh Benjamin Coddington
` (10 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
Implement the reresolve_deviceid hook: walk the layout's mirrors and,
for every stripe whose raw deviceid matches the changed one, exchange
the pinned device node out for NULL. In-flight I/O completes on the old
node through its own reference; the next I/O to the stripe re-resolves
via ff_layout_get_mirror_ds() and picks up the server's new mapping
with a fresh GETDEVICEINFO. The un-pinned references are handed back
on the walker's list to be put outside the locks.
Matching uses the raw deviceid decoded from the layout (dss[].devid), so
a stripe that was never resolved, or already exchanged, is left alone.
A stripe whose resolution previously failed holds an ERR_PTR sentinel
rather than a node; that is cleared too, so the stripe retries
GETDEVICEINFO on its next I/O. Nothing dispatches CHANGE to the walker
yet: no behavior change.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 40 ++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index eeda084d20dd..a7fda4422c5f 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -2527,6 +2527,45 @@ static void ff_layout_cancel_io(struct pnfs_layout_segment *lseg)
}
}
+/*
+ * Un-pin every stripe node resolved from @id: in-flight I/O drains on the
+ * old node through its own reference, the next I/O re-resolves.
+ */
+static void ff_layout_reresolve_deviceid(struct pnfs_layout_hdr *lo,
+ const struct nfs4_deviceid *id,
+ bool immediate,
+ struct list_head *head)
+{
+ struct nfs4_flexfile_layout *flo = FF_LAYOUT_FROM_HDR(lo);
+ struct nfs4_ff_layout_mirror *mirror;
+ struct nfs4_ff_layout_ds *old;
+ struct nfs4_deviceid_put *put;
+ u32 dss_id;
+
+ list_for_each_entry(mirror, &flo->mirrors, mirrors) {
+ for (dss_id = 0; dss_id < mirror->dss_count; dss_id++) {
+ if (memcmp(&mirror->dss[dss_id].devid, id,
+ sizeof(*id)) != 0)
+ continue;
+ /* Allocate before un-pinning: on failure the reference
+ * stays put rather than being dropped here, where the
+ * final put may not sleep.
+ */
+ put = kzalloc_obj(*put, GFP_ATOMIC);
+ if (!put)
+ continue;
+ old = unrcu_pointer(
+ xchg(&mirror->dss[dss_id].mirror_ds, NULL));
+ if (IS_ERR_OR_NULL(old)) {
+ kfree(put);
+ continue;
+ }
+ put->dev = &old->id_node;
+ list_add(&put->node, head);
+ }
+ }
+}
+
static struct pnfs_ds_commit_info *
ff_layout_get_ds_info(struct inode *inode)
{
@@ -3108,6 +3147,7 @@ static struct pnfs_layoutdriver_type flexfilelayout_type = {
.pg_write_ops = &ff_layout_pg_write_ops,
.get_ds_info = ff_layout_get_ds_info,
.free_deviceid_node = ff_layout_free_deviceid_node,
+ .reresolve_deviceid = ff_layout_reresolve_deviceid,
.read_pagelist = ff_layout_read_pagelist,
.write_pagelist = ff_layout_write_pagelist,
.alloc_deviceid_node = ff_layout_alloc_deviceid_node,
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 14/24] NFSv4: Dispatch CB_NOTIFY_DEVICEID CHANGE to an in-place refresh
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (12 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 13/24] NFSv4/flexfiles: Implement in-place device re-resolve on CHANGE Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 15/24] NFSv4/flexfiles: Honor ndc_immediate on CB_NOTIFY_DEVICEID CHANGE Benjamin Coddington
` (9 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
nfs4_callback_devicenotify() treated CHANGE identically to DELETE: both
only unhashed the cached device, so references pinned under live
layouts kept sending I/O to the old mapping until the layouts were
freed. Per RFC 8881 Section 12.2.10, CHANGE exists precisely so a
server can change a mapping without recalling the layouts.
For CHANGE, after unhashing the stale cache entry (so re-resolution
cannot re-pin it), invoke the layout driver's re-resolve walker. The
walker does not install the new mapping itself: it un-pins the stale
node so the next I/O to that stripe re-resolves and fetches the new
one, while I/O already in flight completes on the old node through its
own reference. DELETE is unchanged, and drivers without a
reresolve_deviceid hook see no change.
Re-resolution is best effort. A lookup that hit the cache just before
the unhash can still install that node after the walk has passed the
stripe, and the walk skips nothing else; such a stripe keeps the old
mapping until the next notification. Nothing is left inconsistent by
that -- the old mapping is a valid address the server published -- so
the race is tolerated rather than serialised against.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/callback_proc.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 3fb10c8e4271..01372d8548e1 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -391,7 +391,16 @@ __be32 nfs4_callback_devicenotify(void *argp, void *resp,
if (!ld)
continue;
}
+ /*
+ * Unhash the cached device first so re-resolution cannot
+ * re-pin the stale node, then re-point any references
+ * pinned under live layouts (RFC 8881 Section 12.2.10).
+ */
nfs4_delete_deviceid(ld, cps->clp, &dev->cbd_dev_id);
+ if (dev->cbd_notify_type == NOTIFY_DEVICEID4_CHANGE)
+ pnfs_layout_reresolve_deviceid_byclid(cps->clp, ld,
+ &dev->cbd_dev_id,
+ dev->cbd_immediate);
}
pnfs_put_layoutdriver(ld);
out:
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 15/24] NFSv4/flexfiles: Honor ndc_immediate on CB_NOTIFY_DEVICEID CHANGE
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (13 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 14/24] NFSv4: Dispatch CB_NOTIFY_DEVICEID CHANGE to an in-place refresh Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 16/24] pNFS: Discard a GETDEVICEINFO reply that raced a CHANGE notification Benjamin Coddington
` (8 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
When a CHANGE notification carries ndc_immediate, RFC 8881 Section
20.12 says the change is enforced immediately and the client might not
be able to complete pending I/O. In addition to un-pinning the stripe's
device node, mark the old node unavailable.
Marking does not recall the references already handed out. A write
whose DS connection is already up keeps using the old node until that
I/O errors: nfs4_ff_layout_prepare_ds() returns early on a live
ds_clp, and the unavailable flag is only consulted when a connection
is being established. What the mark does change is that a read skips
the node while another mirror is usable, and that an IOMODE_RW segment
still pinning it stops counting as fully available -- so I/O the
server rejects falls back to the MDS rather than being retried against
a mapping the server has already withdrawn.
The walk can also exchange out a node that already carries the new
mapping: a re-resolve that completed between the unhash and the walk
reaching the stripe, raced by this walk or by the walk of a later
CHANGE for the same deviceid. Ripping such a node out is harmless (it
is still hashed, so the next I/O re-pins it from the cache), but it
must not be marked unavailable. A superseded node is distinguishable
by hashed-ness: it was unhashed before its walk began and is never
re-inserted, while a fresh node is inserted before it is installed.
Only mark nodes that are no longer hashed.
That test is hlist_unhashed_lockless(): the hook runs under the layout
inode's i_lock and rcu_read_lock(), but not under nfs4_deviceid_lock,
which is what serializes the writers of node.pprev -- and __hlist_del()
stores a neighbour's pprev with WRITE_ONCE(), so removing any other
entry in the same bucket can write the field this test reads.
Without ndc_immediate, pending I/O drains on the old mapping and only
new I/O re-resolves, as before.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index a7fda4422c5f..9caf4b3b45ae 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -2560,6 +2560,13 @@ static void ff_layout_reresolve_deviceid(struct pnfs_layout_hdr *lo,
kfree(put);
continue;
}
+ /* A node still hashed was fetched after the unhash
+ * and carries the new mapping; mark only the
+ * superseded ones.
+ */
+ if (immediate &&
+ hlist_unhashed_lockless(&old->id_node.node))
+ nfs4_mark_deviceid_unavailable(&old->id_node);
put->dev = &old->id_node;
list_add(&put->node, head);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 16/24] pNFS: Discard a GETDEVICEINFO reply that raced a CHANGE notification
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (14 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 15/24] NFSv4/flexfiles: Honor ndc_immediate on CB_NOTIFY_DEVICEID CHANGE Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 17/24] pNFS: Add deviceid reference query and collection walkers Benjamin Coddington
` (7 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
RFC 8881 Section 18.40.4: a GETDEVICEINFO reply in flight while the
server changes the device mapping may carry the pre-change mapping; if
it is inserted into the cache after the CHANGE notification unhashed
the stale entry, the client re-caches stale data.
Track a change epoch, bumped when a CHANGE notification is processed
before the stale entry is unhashed. nfs4_find_get_deviceid() snapshots
the epoch before issuing GETDEVICEINFO and, serialized against the
unhash by nfs4_deviceid_lock at insert time, discards the reply and
refetches if the epoch moved. A stale insert that instead precedes the
unhash is removed by the unhash itself, so the cache does not retain the
pre-change entry either way; a reference already handed to a caller in
that ordering is dropped by the re-resolve walk instead.
The refetch is bounded. The epoch is bumped once per CHANGE entry --
that is, at a rate the server chooses -- so an unbounded retry would let
a server drive GETDEVICEINFO traffic without limit, and each discarded
node can carry a DS client teardown and reconnect with it. After
NFS4_DEVICEID_FETCH_RETRIES attempts the reply is accepted. That is
safe because discarding is an optimisation rather than a correctness
requirement: it avoids caching a mapping already known to be superseded,
but before this patch the client cached whatever the reply carried, so
the bounded case is no worse than the previous behaviour and a mapping
that really is stale is corrected by the notification that follows.
The epoch lives on the nfs_client, so a CHANGE delivered on one server's
callback channel does not force an unrelated server's in-flight lookup
to discard its reply and refetch. Mounts that share an nfs_client do
share the counter; the deviceid cache is keyed per client ID, so that is
the granularity the race is defined at.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/callback_proc.c | 4 ++++
fs/nfs/pnfs.h | 1 +
fs/nfs/pnfs_dev.c | 25 +++++++++++++++++++++++++
include/linux/nfs_fs_sb.h | 2 ++
4 files changed, 32 insertions(+)
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 01372d8548e1..ea8c558b07b6 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -395,7 +395,11 @@ __be32 nfs4_callback_devicenotify(void *argp, void *resp,
* Unhash the cached device first so re-resolution cannot
* re-pin the stale node, then re-point any references
* pinned under live layouts (RFC 8881 Section 12.2.10).
+ * The epoch bump lets an in-flight GETDEVICEINFO detect
+ * that its reply may predate the change.
*/
+ if (dev->cbd_notify_type == NOTIFY_DEVICEID4_CHANGE)
+ nfs4_deviceid_bump_change_epoch(cps->clp);
nfs4_delete_deviceid(ld, cps->clp, &dev->cbd_dev_id);
if (dev->cbd_notify_type == NOTIFY_DEVICEID4_CHANGE)
pnfs_layout_reresolve_deviceid_byclid(cps->clp, ld,
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 08bea2c4186e..2c0f5d4d38ab 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -406,6 +406,7 @@ nfs4_find_get_deviceid(struct nfs_server *server,
const struct nfs4_deviceid *id, const struct cred *cred,
gfp_t gfp_mask);
void nfs4_delete_deviceid(const struct pnfs_layoutdriver_type *, const struct nfs_client *, const struct nfs4_deviceid *);
+void nfs4_deviceid_bump_change_epoch(struct nfs_client *clp);
void nfs4_init_deviceid_node(struct nfs4_deviceid_node *, struct nfs_server *,
const struct nfs4_deviceid *);
bool nfs4_put_deviceid_node(struct nfs4_deviceid_node *);
diff --git a/fs/nfs/pnfs_dev.c b/fs/nfs/pnfs_dev.c
index 274abdd6d5f3..a3b28409539a 100644
--- a/fs/nfs/pnfs_dev.c
+++ b/fs/nfs/pnfs_dev.c
@@ -181,6 +181,21 @@ __nfs4_find_get_deviceid(struct nfs_server *server,
return d;
}
+/*
+ * Bumped before the stale entry is unhashed, so an insert serialised
+ * after the unhash by nfs4_deviceid_lock observes the new epoch.
+ */
+void
+nfs4_deviceid_bump_change_epoch(struct nfs_client *clp)
+{
+ atomic_inc(&clp->cl_deviceid_change_epoch);
+}
+
+/* Discarding a raced reply is an optimisation, not a correctness
+ * requirement, and the epoch moves at the server's rate: bound it.
+ */
+#define NFS4_DEVICEID_FETCH_RETRIES 3
+
struct nfs4_deviceid_node *
nfs4_find_get_deviceid(struct nfs_server *server,
const struct nfs4_deviceid *id, const struct cred *cred,
@@ -188,11 +203,14 @@ nfs4_find_get_deviceid(struct nfs_server *server,
{
long hash = nfs4_deviceid_hash(id);
struct nfs4_deviceid_node *d, *new;
+ int epoch, tries = 0;
+retry:
d = __nfs4_find_get_deviceid(server, id, hash);
if (d)
goto found;
+ epoch = atomic_read(&server->nfs_client->cl_deviceid_change_epoch);
new = nfs4_get_device_info(server, id, cred, gfp_mask);
if (!new) {
trace_nfs4_find_deviceid(server, id, -ENOENT);
@@ -200,6 +218,13 @@ nfs4_find_get_deviceid(struct nfs_server *server,
}
spin_lock(&nfs4_deviceid_lock);
+ if (atomic_read(&server->nfs_client->cl_deviceid_change_epoch) != epoch &&
+ ++tries <= NFS4_DEVICEID_FETCH_RETRIES) {
+ /* a mapping changed while we fetched; ours may be stale */
+ spin_unlock(&nfs4_deviceid_lock);
+ server->pnfs_curr_ld->free_deviceid_node(new);
+ goto retry;
+ }
d = __nfs4_find_get_deviceid(server, id, hash);
if (d) {
spin_unlock(&nfs4_deviceid_lock);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 34d294774f8c..cd3ebca61dd1 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -74,6 +74,8 @@ struct nfs_client {
u64 cl_clientid; /* constant */
nfs4_verifier cl_confirm; /* Clientid verifier */
unsigned long cl_state;
+ /* bumped on each CB_NOTIFY_DEVICEID CHANGE for this client */
+ atomic_t cl_deviceid_change_epoch;
spinlock_t cl_lock;
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 17/24] pNFS: Add deviceid reference query and collection walkers
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (15 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 16/24] pNFS: Discard a GETDEVICEINFO reply that raced a CHANGE notification Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-10 16:58 ` Anna Schumaker
2026-09-04 16:53 ` [PATCH v3 18/24] NFSv4/pnfs: Recover revoked layouts on a deleted deviceID Benjamin Coddington
` (6 subsequent siblings)
23 siblings, 1 reply; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
The CB_NOTIFY_DEVICEID DELETE race recovery (RFC 8881 Section
18.40.4) needs to ask whether any live layout still references a
deviceID, and to enumerate those layouts for TEST_STATEID. Add a
layout_references_deviceid hook (sibling of reresolve_deviceid; the
flexfiles implementation memcmps each mirror stripe's decoded devid,
valid independent of the pinned device node) and two walkers over
the byserver pattern:
- pnfs_layout_deviceid_referenced_byclid(): boolean existence query,
early-stopping, entirely under i_lock.
- pnfs_layout_collect_deviceid_refs(): collects each matching layout
with the hdr pinned, the inode grabbed with its superblock active
(a pinned hdr does not hold its inode -- same discipline as the
bulk-destroy walker), and the layout stateid and cred snapshotted
under i_lock, so the caller can issue sleeping RPCs against the
collection.
The collection walker pins each matching header with a plain
pnfs_get_layout_hdr(), which cannot resurrect a dying header because
the walker holds i_lock and has checked NFS_I()->layout == lo.
pnfs_put_layout_hdr() drops the last reference under i_lock --
refcount_dec_and_lock() only decrements one to zero once it holds the
lock -- and clears NFS_I()->layout in that same critical section,
before it unlocks and frees. So a header still installed on its inode
cannot have reached a zero refcount.
That argument deliberately does not rest on NFS_LAYOUT_INVALID_STID.
A header can reach its final put while still valid: a full
LAYOUTRETURN ends in pnfs_layoutreturn_free_lsegs(), which resets the
layout stateid rather than invalidating it, and that is the ordinary
end of life for a return-on-close layout. The validity check the
walkers do apply is a policy filter, not a lifetime guarantee.
Because pnfs_put_layout_hdr() can send a layoutreturn and sleep, a
header pinned for a layout whose inode can no longer be grabbed is put
after the RCU read-side critical section, not within it.
Both ways the walk can end early report it. A GFP_ATOMIC allocation
failure aborts with -ENOMEM, and an inode that can no longer be
grabbed -- igrab() fails from I_FREEING on, while the layout may still
be valid and still name the deviceID -- aborts with -EAGAIN. Either
way the caller is told the collection is partial instead of receiving
a short list it would read as "no references".
No callers yet; no behavior change.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 17 +++
fs/nfs/pnfs.c | 164 +++++++++++++++++++++++++
fs/nfs/pnfs.h | 29 +++++
3 files changed, 210 insertions(+)
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 9caf4b3b45ae..12b7be95a9c3 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -2527,6 +2527,22 @@ static void ff_layout_cancel_io(struct pnfs_layout_segment *lseg)
}
}
+/* Called under @lo's inode i_lock. */
+static bool ff_layout_references_deviceid(struct pnfs_layout_hdr *lo,
+ const struct nfs4_deviceid *id)
+{
+ struct nfs4_flexfile_layout *flo = FF_LAYOUT_FROM_HDR(lo);
+ struct nfs4_ff_layout_mirror *mirror;
+ u32 dss_id;
+
+ list_for_each_entry(mirror, &flo->mirrors, mirrors)
+ for (dss_id = 0; dss_id < mirror->dss_count; dss_id++)
+ if (memcmp(&mirror->dss[dss_id].devid, id,
+ sizeof(*id)) == 0)
+ return true;
+ return false;
+}
+
/*
* Un-pin every stripe node resolved from @id: in-flight I/O drains on the
* old node through its own reference, the next I/O re-resolves.
@@ -3155,6 +3171,7 @@ static struct pnfs_layoutdriver_type flexfilelayout_type = {
.get_ds_info = ff_layout_get_ds_info,
.free_deviceid_node = ff_layout_free_deviceid_node,
.reresolve_deviceid = ff_layout_reresolve_deviceid,
+ .layout_references_deviceid = ff_layout_references_deviceid,
.read_pagelist = ff_layout_read_pagelist,
.write_pagelist = ff_layout_write_pagelist,
.alloc_deviceid_node = ff_layout_alloc_deviceid_node,
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 2f54b22d0e21..23f12eec99d7 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -2937,6 +2937,170 @@ pnfs_layout_reresolve_deviceid_byclid(struct nfs_client *clp,
}
}
+struct pnfs_deviceid_ref_args {
+ const struct pnfs_layoutdriver_type *ld;
+ const struct nfs4_deviceid *id;
+ struct list_head *result;
+ bool found;
+};
+
+static int pnfs_layout_deviceid_referenced_byserver(
+ struct nfs_server *server, void *data)
+{
+ struct pnfs_deviceid_ref_args *args = data;
+ struct pnfs_layout_hdr *lo;
+ struct inode *inode;
+
+ if (server->pnfs_curr_ld != args->ld)
+ return 0;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(lo, &server->layouts, plh_layouts) {
+ inode = lo->plh_inode;
+ if (!inode)
+ continue;
+ spin_lock(&inode->i_lock);
+ if (NFS_I(inode)->layout == lo && pnfs_layout_is_valid(lo) &&
+ args->ld->layout_references_deviceid(lo, args->id))
+ args->found = true;
+ spin_unlock(&inode->i_lock);
+ if (args->found)
+ break;
+ }
+ rcu_read_unlock();
+ return args->found;
+}
+
+/*
+ * pnfs_layout_deviceid_referenced_byclid - does any live layout of
+ * @clp's servers using @ld still reference deviceid @id?
+ */
+bool
+pnfs_layout_deviceid_referenced_byclid(struct nfs_client *clp,
+ const struct pnfs_layoutdriver_type *ld,
+ const struct nfs4_deviceid *id)
+{
+ struct pnfs_deviceid_ref_args args = {
+ .ld = ld,
+ .id = id,
+ };
+
+ if (!ld->layout_references_deviceid)
+ return false;
+
+ nfs_client_for_each_server(clp,
+ pnfs_layout_deviceid_referenced_byserver, &args);
+ return args.found;
+}
+
+static int pnfs_layout_collect_deviceid_refs_byserver(
+ struct nfs_server *server, void *data)
+{
+ struct pnfs_deviceid_ref_args *args = data;
+ struct nfs4_deviceid_ref *ref, *tmp;
+ struct pnfs_layout_hdr *lo;
+ struct inode *inode;
+ LIST_HEAD(putme);
+ bool matched;
+ int ret = 0;
+
+ if (server->pnfs_curr_ld != args->ld)
+ return 0;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(lo, &server->layouts, plh_layouts) {
+ inode = lo->plh_inode;
+ if (!inode)
+ continue;
+
+ spin_lock(&inode->i_lock);
+ matched = NFS_I(inode)->layout == lo &&
+ pnfs_layout_is_valid(lo) &&
+ args->ld->layout_references_deviceid(lo, args->id);
+ if (!matched) {
+ spin_unlock(&inode->i_lock);
+ continue;
+ }
+ ref = kzalloc_obj(*ref, GFP_ATOMIC);
+ if (!ref) {
+ spin_unlock(&inode->i_lock);
+ ret = -ENOMEM;
+ break;
+ }
+ /* NFS_I()->layout == lo under i_lock means the refcount has
+ * not reached zero: pnfs_put_layout_hdr() decrements to zero
+ * and detaches in the same critical section.
+ */
+ pnfs_get_layout_hdr(lo);
+ ref->lo = lo;
+ nfs4_stateid_copy(&ref->stateid, &lo->plh_stateid);
+ ref->cred = get_cred(lo->plh_lc_cred);
+ spin_unlock(&inode->i_lock);
+
+ /* the pinned hdr does not hold the inode: grab it (and
+ * keep the superblock active) for use across RPCs
+ */
+ ref->inode = nfs_igrab_and_active(inode);
+ if (!ref->inode) {
+ /* The layout may still name the deviceID, so report a
+ * partial list rather than silently shortening it.
+ * Defer the put: it can layoutreturn and sleep.
+ */
+ list_add(&ref->node, &putme);
+ ret = -EAGAIN;
+ break;
+ }
+ list_add_tail(&ref->node, args->result);
+ }
+ rcu_read_unlock();
+
+ list_for_each_entry_safe(ref, tmp, &putme, node) {
+ list_del(&ref->node);
+ pnfs_put_layout_hdr(ref->lo);
+ put_cred(ref->cred);
+ kfree(ref);
+ }
+ return ret;
+}
+
+/*
+ * Collect @clp's layouts referencing @id onto @result as entries usable
+ * across sleeping RPCs; release with pnfs_layout_put_deviceid_refs().
+ * A negative return means @result is only a partial set.
+ */
+int
+pnfs_layout_collect_deviceid_refs(struct nfs_client *clp,
+ const struct pnfs_layoutdriver_type *ld,
+ const struct nfs4_deviceid *id,
+ struct list_head *result)
+{
+ struct pnfs_deviceid_ref_args args = {
+ .ld = ld,
+ .id = id,
+ .result = result,
+ };
+
+ if (!ld->layout_references_deviceid)
+ return 0;
+
+ return nfs_client_for_each_server(clp,
+ pnfs_layout_collect_deviceid_refs_byserver, &args);
+}
+
+void
+pnfs_layout_put_deviceid_refs(struct list_head *result)
+{
+ struct nfs4_deviceid_ref *ref, *tmp;
+
+ list_for_each_entry_safe(ref, tmp, result, node) {
+ list_del(&ref->node);
+ put_cred(ref->cred);
+ pnfs_put_layout_hdr(ref->lo);
+ nfs_iput_and_deactive(ref->inode);
+ kfree(ref);
+ }
+}
+
/* Check if we have we have a valid layout but if there isn't an intersection
* between the request and the pgio->pg_lseg, put this pgio->pg_lseg away.
*/
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 2c0f5d4d38ab..d1bc4d1d8d4c 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -184,6 +184,12 @@ struct pnfs_layoutdriver_type {
const struct nfs4_deviceid *id,
bool immediate,
struct list_head *put_list);
+ /*
+ * Does @lo hold any reference to deviceid @id? Called under
+ * @lo's inode i_lock; must not sleep.
+ */
+ bool (*layout_references_deviceid)(struct pnfs_layout_hdr *lo,
+ const struct nfs4_deviceid *id);
int (*prepare_layoutreturn) (struct nfs4_layoutreturn_args *);
@@ -371,6 +377,29 @@ void pnfs_layout_reresolve_deviceid_byclid(struct nfs_client *clp,
const struct pnfs_layoutdriver_type *ld,
const struct nfs4_deviceid *id,
bool immediate);
+bool pnfs_layout_deviceid_referenced_byclid(struct nfs_client *clp,
+ const struct pnfs_layoutdriver_type *ld,
+ const struct nfs4_deviceid *id);
+
+/*
+ * One live layout referencing a deviceID, collected for the
+ * CB_NOTIFY_DEVICEID DELETE recovery: the hdr is pinned, the inode
+ * igrab'd with its superblock active, and the layout stateid and
+ * cred snapshotted for TEST_STATEID.
+ */
+struct nfs4_deviceid_ref {
+ struct list_head node;
+ struct pnfs_layout_hdr *lo;
+ struct inode *inode;
+ nfs4_stateid stateid;
+ const struct cred *cred;
+};
+
+int pnfs_layout_collect_deviceid_refs(struct nfs_client *clp,
+ const struct pnfs_layoutdriver_type *ld,
+ const struct nfs4_deviceid *id,
+ struct list_head *result);
+void pnfs_layout_put_deviceid_refs(struct list_head *result);
int pnfs_layout_handle_reboot(struct nfs_client *clp);
/* nfs4_deviceid_flags */
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v3 17/24] pNFS: Add deviceid reference query and collection walkers
2026-09-04 16:53 ` [PATCH v3 17/24] pNFS: Add deviceid reference query and collection walkers Benjamin Coddington
@ 2026-09-10 16:58 ` Anna Schumaker
2026-09-10 17:24 ` Benjamin Coddington
0 siblings, 1 reply; 28+ messages in thread
From: Anna Schumaker @ 2026-09-10 16:58 UTC (permalink / raw)
To: Benjamin Coddington, Trond Myklebust
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
Hi Ben,
On Fri, Sep 4, 2026, at 12:53 PM, Benjamin Coddington wrote:
> The CB_NOTIFY_DEVICEID DELETE race recovery (RFC 8881 Section
> 18.40.4) needs to ask whether any live layout still references a
> deviceID, and to enumerate those layouts for TEST_STATEID. Add a
> layout_references_deviceid hook (sibling of reresolve_deviceid; the
> flexfiles implementation memcmps each mirror stripe's decoded devid,
> valid independent of the pinned device node) and two walkers over
> the byserver pattern:
>
> - pnfs_layout_deviceid_referenced_byclid(): boolean existence query,
> early-stopping, entirely under i_lock.
> - pnfs_layout_collect_deviceid_refs(): collects each matching layout
> with the hdr pinned, the inode grabbed with its superblock active
> (a pinned hdr does not hold its inode -- same discipline as the
> bulk-destroy walker), and the layout stateid and cred snapshotted
> under i_lock, so the caller can issue sleeping RPCs against the
> collection.
>
> The collection walker pins each matching header with a plain
> pnfs_get_layout_hdr(), which cannot resurrect a dying header because
> the walker holds i_lock and has checked NFS_I()->layout == lo.
> pnfs_put_layout_hdr() drops the last reference under i_lock --
> refcount_dec_and_lock() only decrements one to zero once it holds the
> lock -- and clears NFS_I()->layout in that same critical section,
> before it unlocks and frees. So a header still installed on its inode
> cannot have reached a zero refcount.
>
> That argument deliberately does not rest on NFS_LAYOUT_INVALID_STID.
> A header can reach its final put while still valid: a full
> LAYOUTRETURN ends in pnfs_layoutreturn_free_lsegs(), which resets the
> layout stateid rather than invalidating it, and that is the ordinary
> end of life for a return-on-close layout. The validity check the
> walkers do apply is a policy filter, not a lifetime guarantee.
>
> Because pnfs_put_layout_hdr() can send a layoutreturn and sleep, a
> header pinned for a layout whose inode can no longer be grabbed is put
> after the RCU read-side critical section, not within it.
>
> Both ways the walk can end early report it. A GFP_ATOMIC allocation
> failure aborts with -ENOMEM, and an inode that can no longer be
> grabbed -- igrab() fails from I_FREEING on, while the layout may still
> be valid and still name the deviceID -- aborts with -EAGAIN. Either
> way the caller is told the collection is partial instead of receiving
> a short list it would read as "no references".
>
> No callers yet; no behavior change.
>
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
> ---
> fs/nfs/flexfilelayout/flexfilelayout.c | 17 +++
> fs/nfs/pnfs.c | 164 +++++++++++++++++++++++++
> fs/nfs/pnfs.h | 29 +++++
> 3 files changed, 210 insertions(+)
>
> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c
> b/fs/nfs/flexfilelayout/flexfilelayout.c
> index 9caf4b3b45ae..12b7be95a9c3 100644
> --- a/fs/nfs/flexfilelayout/flexfilelayout.c
> +++ b/fs/nfs/flexfilelayout/flexfilelayout.c
> @@ -2527,6 +2527,22 @@ static void ff_layout_cancel_io(struct
> pnfs_layout_segment *lseg)
> }
> }
>
> +/* Called under @lo's inode i_lock. */
> +static bool ff_layout_references_deviceid(struct pnfs_layout_hdr *lo,
> + const struct nfs4_deviceid *id)
> +{
> + struct nfs4_flexfile_layout *flo = FF_LAYOUT_FROM_HDR(lo);
> + struct nfs4_ff_layout_mirror *mirror;
> + u32 dss_id;
> +
> + list_for_each_entry(mirror, &flo->mirrors, mirrors)
> + for (dss_id = 0; dss_id < mirror->dss_count; dss_id++)
> + if (memcmp(&mirror->dss[dss_id].devid, id,
> + sizeof(*id)) == 0)
> + return true;
> + return false;
> +}
> +
> /*
> * Un-pin every stripe node resolved from @id: in-flight I/O drains on
> the
> * old node through its own reference, the next I/O re-resolves.
> @@ -3155,6 +3171,7 @@ static struct pnfs_layoutdriver_type
> flexfilelayout_type = {
> .get_ds_info = ff_layout_get_ds_info,
> .free_deviceid_node = ff_layout_free_deviceid_node,
> .reresolve_deviceid = ff_layout_reresolve_deviceid,
> + .layout_references_deviceid = ff_layout_references_deviceid,
> .read_pagelist = ff_layout_read_pagelist,
> .write_pagelist = ff_layout_write_pagelist,
> .alloc_deviceid_node = ff_layout_alloc_deviceid_node,
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 2f54b22d0e21..23f12eec99d7 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -2937,6 +2937,170 @@ pnfs_layout_reresolve_deviceid_byclid(struct
> nfs_client *clp,
> }
> }
>
> +struct pnfs_deviceid_ref_args {
> + const struct pnfs_layoutdriver_type *ld;
> + const struct nfs4_deviceid *id;
> + struct list_head *result;
> + bool found;
> +};
> +
> +static int pnfs_layout_deviceid_referenced_byserver(
> + struct nfs_server *server, void *data)
> +{
> + struct pnfs_deviceid_ref_args *args = data;
> + struct pnfs_layout_hdr *lo;
> + struct inode *inode;
> +
> + if (server->pnfs_curr_ld != args->ld)
> + return 0;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(lo, &server->layouts, plh_layouts) {
> + inode = lo->plh_inode;
> + if (!inode)
> + continue;
> + spin_lock(&inode->i_lock);
> + if (NFS_I(inode)->layout == lo && pnfs_layout_is_valid(lo) &&
> + args->ld->layout_references_deviceid(lo, args->id))
> + args->found = true;
> + spin_unlock(&inode->i_lock);
> + if (args->found)
> + break;
> + }
> + rcu_read_unlock();
> + return args->found;
> +}
> +
> +/*
> + * pnfs_layout_deviceid_referenced_byclid - does any live layout of
> + * @clp's servers using @ld still reference deviceid @id?
> + */
> +bool
> +pnfs_layout_deviceid_referenced_byclid(struct nfs_client *clp,
> + const struct pnfs_layoutdriver_type *ld,
> + const struct nfs4_deviceid *id)
> +{
> + struct pnfs_deviceid_ref_args args = {
> + .ld = ld,
> + .id = id,
> + };
I was wondering if we could go with different names for the members
of this struct? "ld" and "id" look very similar, and at first glance
I thought you were assigning the same values twice here.
Anna
> +
> + if (!ld->layout_references_deviceid)
> + return false;
> +
> + nfs_client_for_each_server(clp,
> + pnfs_layout_deviceid_referenced_byserver, &args);
> + return args.found;
> +}
> +
> +static int pnfs_layout_collect_deviceid_refs_byserver(
> + struct nfs_server *server, void *data)
> +{
> + struct pnfs_deviceid_ref_args *args = data;
> + struct nfs4_deviceid_ref *ref, *tmp;
> + struct pnfs_layout_hdr *lo;
> + struct inode *inode;
> + LIST_HEAD(putme);
> + bool matched;
> + int ret = 0;
> +
> + if (server->pnfs_curr_ld != args->ld)
> + return 0;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(lo, &server->layouts, plh_layouts) {
> + inode = lo->plh_inode;
> + if (!inode)
> + continue;
> +
> + spin_lock(&inode->i_lock);
> + matched = NFS_I(inode)->layout == lo &&
> + pnfs_layout_is_valid(lo) &&
> + args->ld->layout_references_deviceid(lo, args->id);
> + if (!matched) {
> + spin_unlock(&inode->i_lock);
> + continue;
> + }
> + ref = kzalloc_obj(*ref, GFP_ATOMIC);
> + if (!ref) {
> + spin_unlock(&inode->i_lock);
> + ret = -ENOMEM;
> + break;
> + }
> + /* NFS_I()->layout == lo under i_lock means the refcount has
> + * not reached zero: pnfs_put_layout_hdr() decrements to zero
> + * and detaches in the same critical section.
> + */
> + pnfs_get_layout_hdr(lo);
> + ref->lo = lo;
> + nfs4_stateid_copy(&ref->stateid, &lo->plh_stateid);
> + ref->cred = get_cred(lo->plh_lc_cred);
> + spin_unlock(&inode->i_lock);
> +
> + /* the pinned hdr does not hold the inode: grab it (and
> + * keep the superblock active) for use across RPCs
> + */
> + ref->inode = nfs_igrab_and_active(inode);
> + if (!ref->inode) {
> + /* The layout may still name the deviceID, so report a
> + * partial list rather than silently shortening it.
> + * Defer the put: it can layoutreturn and sleep.
> + */
> + list_add(&ref->node, &putme);
> + ret = -EAGAIN;
> + break;
> + }
> + list_add_tail(&ref->node, args->result);
> + }
> + rcu_read_unlock();
> +
> + list_for_each_entry_safe(ref, tmp, &putme, node) {
> + list_del(&ref->node);
> + pnfs_put_layout_hdr(ref->lo);
> + put_cred(ref->cred);
> + kfree(ref);
> + }
> + return ret;
> +}
> +
> +/*
> + * Collect @clp's layouts referencing @id onto @result as entries usable
> + * across sleeping RPCs; release with pnfs_layout_put_deviceid_refs().
> + * A negative return means @result is only a partial set.
> + */
> +int
> +pnfs_layout_collect_deviceid_refs(struct nfs_client *clp,
> + const struct pnfs_layoutdriver_type *ld,
> + const struct nfs4_deviceid *id,
> + struct list_head *result)
> +{
> + struct pnfs_deviceid_ref_args args = {
> + .ld = ld,
> + .id = id,
> + .result = result,
> + };
> +
> + if (!ld->layout_references_deviceid)
> + return 0;
> +
> + return nfs_client_for_each_server(clp,
> + pnfs_layout_collect_deviceid_refs_byserver, &args);
> +}
> +
> +void
> +pnfs_layout_put_deviceid_refs(struct list_head *result)
> +{
> + struct nfs4_deviceid_ref *ref, *tmp;
> +
> + list_for_each_entry_safe(ref, tmp, result, node) {
> + list_del(&ref->node);
> + put_cred(ref->cred);
> + pnfs_put_layout_hdr(ref->lo);
> + nfs_iput_and_deactive(ref->inode);
> + kfree(ref);
> + }
> +}
> +
> /* Check if we have we have a valid layout but if there isn't an intersection
> * between the request and the pgio->pg_lseg, put this pgio->pg_lseg away.
> */
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index 2c0f5d4d38ab..d1bc4d1d8d4c 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -184,6 +184,12 @@ struct pnfs_layoutdriver_type {
> const struct nfs4_deviceid *id,
> bool immediate,
> struct list_head *put_list);
> + /*
> + * Does @lo hold any reference to deviceid @id? Called under
> + * @lo's inode i_lock; must not sleep.
> + */
> + bool (*layout_references_deviceid)(struct pnfs_layout_hdr *lo,
> + const struct nfs4_deviceid *id);
>
> int (*prepare_layoutreturn) (struct nfs4_layoutreturn_args *);
>
> @@ -371,6 +377,29 @@ void pnfs_layout_reresolve_deviceid_byclid(struct
> nfs_client *clp,
> const struct pnfs_layoutdriver_type *ld,
> const struct nfs4_deviceid *id,
> bool immediate);
> +bool pnfs_layout_deviceid_referenced_byclid(struct nfs_client *clp,
> + const struct pnfs_layoutdriver_type *ld,
> + const struct nfs4_deviceid *id);
> +
> +/*
> + * One live layout referencing a deviceID, collected for the
> + * CB_NOTIFY_DEVICEID DELETE recovery: the hdr is pinned, the inode
> + * igrab'd with its superblock active, and the layout stateid and
> + * cred snapshotted for TEST_STATEID.
> + */
> +struct nfs4_deviceid_ref {
> + struct list_head node;
> + struct pnfs_layout_hdr *lo;
> + struct inode *inode;
> + nfs4_stateid stateid;
> + const struct cred *cred;
> +};
> +
> +int pnfs_layout_collect_deviceid_refs(struct nfs_client *clp,
> + const struct pnfs_layoutdriver_type *ld,
> + const struct nfs4_deviceid *id,
> + struct list_head *result);
> +void pnfs_layout_put_deviceid_refs(struct list_head *result);
> int pnfs_layout_handle_reboot(struct nfs_client *clp);
>
> /* nfs4_deviceid_flags */
> --
> 2.53.0
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v3 17/24] pNFS: Add deviceid reference query and collection walkers
2026-09-10 16:58 ` Anna Schumaker
@ 2026-09-10 17:24 ` Benjamin Coddington
2026-09-10 18:05 ` Anna Schumaker
0 siblings, 1 reply; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-10 17:24 UTC (permalink / raw)
To: Anna Schumaker
Cc: Benjamin Coddington, Trond Myklebust, linux-nfs, Jonathan Curley,
Mike Snitzer, Jeff Layton, Junrui Luo
On 10 Sep 2026, at 12:58, Anna Schumaker wrote:
> Hi Ben,
>
> On Fri, Sep 4, 2026, at 12:53 PM, Benjamin Coddington wrote:
>> The CB_NOTIFY_DEVICEID DELETE race recovery (RFC 8881 Section
>> 18.40.4) needs to ask whether any live layout still references a
>> deviceID, and to enumerate those layouts for TEST_STATEID. Add a
>> layout_references_deviceid hook (sibling of reresolve_deviceid; the
>> flexfiles implementation memcmps each mirror stripe's decoded devid,
>> valid independent of the pinned device node) and two walkers over
>> the byserver pattern:
>>
>> - pnfs_layout_deviceid_referenced_byclid(): boolean existence query,
>> early-stopping, entirely under i_lock.
>> - pnfs_layout_collect_deviceid_refs(): collects each matching layout
>> with the hdr pinned, the inode grabbed with its superblock active
>> (a pinned hdr does not hold its inode -- same discipline as the
>> bulk-destroy walker), and the layout stateid and cred snapshotted
>> under i_lock, so the caller can issue sleeping RPCs against the
>> collection.
>>
>> The collection walker pins each matching header with a plain
>> pnfs_get_layout_hdr(), which cannot resurrect a dying header because
>> the walker holds i_lock and has checked NFS_I()->layout == lo.
>> pnfs_put_layout_hdr() drops the last reference under i_lock --
>> refcount_dec_and_lock() only decrements one to zero once it holds the
>> lock -- and clears NFS_I()->layout in that same critical section,
>> before it unlocks and frees. So a header still installed on its inode
>> cannot have reached a zero refcount.
>>
>> That argument deliberately does not rest on NFS_LAYOUT_INVALID_STID.
>> A header can reach its final put while still valid: a full
>> LAYOUTRETURN ends in pnfs_layoutreturn_free_lsegs(), which resets the
>> layout stateid rather than invalidating it, and that is the ordinary
>> end of life for a return-on-close layout. The validity check the
>> walkers do apply is a policy filter, not a lifetime guarantee.
>>
>> Because pnfs_put_layout_hdr() can send a layoutreturn and sleep, a
>> header pinned for a layout whose inode can no longer be grabbed is put
>> after the RCU read-side critical section, not within it.
>>
>> Both ways the walk can end early report it. A GFP_ATOMIC allocation
>> failure aborts with -ENOMEM, and an inode that can no longer be
>> grabbed -- igrab() fails from I_FREEING on, while the layout may still
>> be valid and still name the deviceID -- aborts with -EAGAIN. Either
>> way the caller is told the collection is partial instead of receiving
>> a short list it would read as "no references".
>>
>> No callers yet; no behavior change.
>>
>> Assisted-by: Claude:claude-fable-5
>> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
>> ---
>> fs/nfs/flexfilelayout/flexfilelayout.c | 17 +++
>> fs/nfs/pnfs.c | 164 +++++++++++++++++++++++++
>> fs/nfs/pnfs.h | 29 +++++
>> 3 files changed, 210 insertions(+)
>>
>> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c
>> b/fs/nfs/flexfilelayout/flexfilelayout.c
>> index 9caf4b3b45ae..12b7be95a9c3 100644
>> --- a/fs/nfs/flexfilelayout/flexfilelayout.c
>> +++ b/fs/nfs/flexfilelayout/flexfilelayout.c
>> @@ -2527,6 +2527,22 @@ static void ff_layout_cancel_io(struct
>> pnfs_layout_segment *lseg)
>> }
>> }
>>
>> +/* Called under @lo's inode i_lock. */
>> +static bool ff_layout_references_deviceid(struct pnfs_layout_hdr *lo,
>> + const struct nfs4_deviceid *id)
>> +{
>> + struct nfs4_flexfile_layout *flo = FF_LAYOUT_FROM_HDR(lo);
>> + struct nfs4_ff_layout_mirror *mirror;
>> + u32 dss_id;
>> +
>> + list_for_each_entry(mirror, &flo->mirrors, mirrors)
>> + for (dss_id = 0; dss_id < mirror->dss_count; dss_id++)
>> + if (memcmp(&mirror->dss[dss_id].devid, id,
>> + sizeof(*id)) == 0)
>> + return true;
>> + return false;
>> +}
>> +
>> /*
>> * Un-pin every stripe node resolved from @id: in-flight I/O drains on
>> the
>> * old node through its own reference, the next I/O re-resolves.
>> @@ -3155,6 +3171,7 @@ static struct pnfs_layoutdriver_type
>> flexfilelayout_type = {
>> .get_ds_info = ff_layout_get_ds_info,
>> .free_deviceid_node = ff_layout_free_deviceid_node,
>> .reresolve_deviceid = ff_layout_reresolve_deviceid,
>> + .layout_references_deviceid = ff_layout_references_deviceid,
>> .read_pagelist = ff_layout_read_pagelist,
>> .write_pagelist = ff_layout_write_pagelist,
>> .alloc_deviceid_node = ff_layout_alloc_deviceid_node,
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index 2f54b22d0e21..23f12eec99d7 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -2937,6 +2937,170 @@ pnfs_layout_reresolve_deviceid_byclid(struct
>> nfs_client *clp,
>> }
>> }
>>
>> +struct pnfs_deviceid_ref_args {
>> + const struct pnfs_layoutdriver_type *ld;
>> + const struct nfs4_deviceid *id;
>> + struct list_head *result;
>> + bool found;
>> +};
>> +
>> +static int pnfs_layout_deviceid_referenced_byserver(
>> + struct nfs_server *server, void *data)
>> +{
>> + struct pnfs_deviceid_ref_args *args = data;
>> + struct pnfs_layout_hdr *lo;
>> + struct inode *inode;
>> +
>> + if (server->pnfs_curr_ld != args->ld)
>> + return 0;
>> +
>> + rcu_read_lock();
>> + list_for_each_entry_rcu(lo, &server->layouts, plh_layouts) {
>> + inode = lo->plh_inode;
>> + if (!inode)
>> + continue;
>> + spin_lock(&inode->i_lock);
>> + if (NFS_I(inode)->layout == lo && pnfs_layout_is_valid(lo) &&
>> + args->ld->layout_references_deviceid(lo, args->id))
>> + args->found = true;
>> + spin_unlock(&inode->i_lock);
>> + if (args->found)
>> + break;
>> + }
>> + rcu_read_unlock();
>> + return args->found;
>> +}
>> +
>> +/*
>> + * pnfs_layout_deviceid_referenced_byclid - does any live layout of
>> + * @clp's servers using @ld still reference deviceid @id?
>> + */
>> +bool
>> +pnfs_layout_deviceid_referenced_byclid(struct nfs_client *clp,
>> + const struct pnfs_layoutdriver_type *ld,
>> + const struct nfs4_deviceid *id)
>> +{
>> + struct pnfs_deviceid_ref_args args = {
>> + .ld = ld,
>> + .id = id,
>> + };
>
> I was wondering if we could go with different names for the members
> of this struct? "ld" and "id" look very similar, and at first glance
> I thought you were assigning the same values twice here.
Sure we can.. there's precedent for "ld" being used to refer to
pnfs_layoutdriver_type. Should we change them all? Happy to reversion and
resend, or let you re-write it as well. Let me know what's most helpful.
Ben
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v3 17/24] pNFS: Add deviceid reference query and collection walkers
2026-09-10 17:24 ` Benjamin Coddington
@ 2026-09-10 18:05 ` Anna Schumaker
0 siblings, 0 replies; 28+ messages in thread
From: Anna Schumaker @ 2026-09-10 18:05 UTC (permalink / raw)
To: Benjamin Coddington
Cc: Trond Myklebust, linux-nfs, Jonathan Curley, Mike Snitzer,
Jeff Layton, Junrui Luo
On Thu, Sep 10, 2026, at 1:24 PM, Benjamin Coddington wrote:
> On 10 Sep 2026, at 12:58, Anna Schumaker wrote:
>
>> Hi Ben,
>>
>> On Fri, Sep 4, 2026, at 12:53 PM, Benjamin Coddington wrote:
>>> The CB_NOTIFY_DEVICEID DELETE race recovery (RFC 8881 Section
>>> 18.40.4) needs to ask whether any live layout still references a
>>> deviceID, and to enumerate those layouts for TEST_STATEID. Add a
>>> layout_references_deviceid hook (sibling of reresolve_deviceid; the
>>> flexfiles implementation memcmps each mirror stripe's decoded devid,
>>> valid independent of the pinned device node) and two walkers over
>>> the byserver pattern:
>>>
>>> - pnfs_layout_deviceid_referenced_byclid(): boolean existence query,
>>> early-stopping, entirely under i_lock.
>>> - pnfs_layout_collect_deviceid_refs(): collects each matching layout
>>> with the hdr pinned, the inode grabbed with its superblock active
>>> (a pinned hdr does not hold its inode -- same discipline as the
>>> bulk-destroy walker), and the layout stateid and cred snapshotted
>>> under i_lock, so the caller can issue sleeping RPCs against the
>>> collection.
>>>
>>> The collection walker pins each matching header with a plain
>>> pnfs_get_layout_hdr(), which cannot resurrect a dying header because
>>> the walker holds i_lock and has checked NFS_I()->layout == lo.
>>> pnfs_put_layout_hdr() drops the last reference under i_lock --
>>> refcount_dec_and_lock() only decrements one to zero once it holds the
>>> lock -- and clears NFS_I()->layout in that same critical section,
>>> before it unlocks and frees. So a header still installed on its inode
>>> cannot have reached a zero refcount.
>>>
>>> That argument deliberately does not rest on NFS_LAYOUT_INVALID_STID.
>>> A header can reach its final put while still valid: a full
>>> LAYOUTRETURN ends in pnfs_layoutreturn_free_lsegs(), which resets the
>>> layout stateid rather than invalidating it, and that is the ordinary
>>> end of life for a return-on-close layout. The validity check the
>>> walkers do apply is a policy filter, not a lifetime guarantee.
>>>
>>> Because pnfs_put_layout_hdr() can send a layoutreturn and sleep, a
>>> header pinned for a layout whose inode can no longer be grabbed is put
>>> after the RCU read-side critical section, not within it.
>>>
>>> Both ways the walk can end early report it. A GFP_ATOMIC allocation
>>> failure aborts with -ENOMEM, and an inode that can no longer be
>>> grabbed -- igrab() fails from I_FREEING on, while the layout may still
>>> be valid and still name the deviceID -- aborts with -EAGAIN. Either
>>> way the caller is told the collection is partial instead of receiving
>>> a short list it would read as "no references".
>>>
>>> No callers yet; no behavior change.
>>>
>>> Assisted-by: Claude:claude-fable-5
>>> Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
>>> ---
>>> fs/nfs/flexfilelayout/flexfilelayout.c | 17 +++
>>> fs/nfs/pnfs.c | 164 +++++++++++++++++++++++++
>>> fs/nfs/pnfs.h | 29 +++++
>>> 3 files changed, 210 insertions(+)
>>>
>>> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c
>>> b/fs/nfs/flexfilelayout/flexfilelayout.c
>>> index 9caf4b3b45ae..12b7be95a9c3 100644
>>> --- a/fs/nfs/flexfilelayout/flexfilelayout.c
>>> +++ b/fs/nfs/flexfilelayout/flexfilelayout.c
>>> @@ -2527,6 +2527,22 @@ static void ff_layout_cancel_io(struct
>>> pnfs_layout_segment *lseg)
>>> }
>>> }
>>>
>>> +/* Called under @lo's inode i_lock. */
>>> +static bool ff_layout_references_deviceid(struct pnfs_layout_hdr *lo,
>>> + const struct nfs4_deviceid *id)
>>> +{
>>> + struct nfs4_flexfile_layout *flo = FF_LAYOUT_FROM_HDR(lo);
>>> + struct nfs4_ff_layout_mirror *mirror;
>>> + u32 dss_id;
>>> +
>>> + list_for_each_entry(mirror, &flo->mirrors, mirrors)
>>> + for (dss_id = 0; dss_id < mirror->dss_count; dss_id++)
>>> + if (memcmp(&mirror->dss[dss_id].devid, id,
>>> + sizeof(*id)) == 0)
>>> + return true;
>>> + return false;
>>> +}
>>> +
>>> /*
>>> * Un-pin every stripe node resolved from @id: in-flight I/O drains on
>>> the
>>> * old node through its own reference, the next I/O re-resolves.
>>> @@ -3155,6 +3171,7 @@ static struct pnfs_layoutdriver_type
>>> flexfilelayout_type = {
>>> .get_ds_info = ff_layout_get_ds_info,
>>> .free_deviceid_node = ff_layout_free_deviceid_node,
>>> .reresolve_deviceid = ff_layout_reresolve_deviceid,
>>> + .layout_references_deviceid = ff_layout_references_deviceid,
>>> .read_pagelist = ff_layout_read_pagelist,
>>> .write_pagelist = ff_layout_write_pagelist,
>>> .alloc_deviceid_node = ff_layout_alloc_deviceid_node,
>>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>>> index 2f54b22d0e21..23f12eec99d7 100644
>>> --- a/fs/nfs/pnfs.c
>>> +++ b/fs/nfs/pnfs.c
>>> @@ -2937,6 +2937,170 @@ pnfs_layout_reresolve_deviceid_byclid(struct
>>> nfs_client *clp,
>>> }
>>> }
>>>
>>> +struct pnfs_deviceid_ref_args {
>>> + const struct pnfs_layoutdriver_type *ld;
>>> + const struct nfs4_deviceid *id;
>>> + struct list_head *result;
>>> + bool found;
>>> +};
>>> +
>>> +static int pnfs_layout_deviceid_referenced_byserver(
>>> + struct nfs_server *server, void *data)
>>> +{
>>> + struct pnfs_deviceid_ref_args *args = data;
>>> + struct pnfs_layout_hdr *lo;
>>> + struct inode *inode;
>>> +
>>> + if (server->pnfs_curr_ld != args->ld)
>>> + return 0;
>>> +
>>> + rcu_read_lock();
>>> + list_for_each_entry_rcu(lo, &server->layouts, plh_layouts) {
>>> + inode = lo->plh_inode;
>>> + if (!inode)
>>> + continue;
>>> + spin_lock(&inode->i_lock);
>>> + if (NFS_I(inode)->layout == lo && pnfs_layout_is_valid(lo) &&
>>> + args->ld->layout_references_deviceid(lo, args->id))
>>> + args->found = true;
>>> + spin_unlock(&inode->i_lock);
>>> + if (args->found)
>>> + break;
>>> + }
>>> + rcu_read_unlock();
>>> + return args->found;
>>> +}
>>> +
>>> +/*
>>> + * pnfs_layout_deviceid_referenced_byclid - does any live layout of
>>> + * @clp's servers using @ld still reference deviceid @id?
>>> + */
>>> +bool
>>> +pnfs_layout_deviceid_referenced_byclid(struct nfs_client *clp,
>>> + const struct pnfs_layoutdriver_type *ld,
>>> + const struct nfs4_deviceid *id)
>>> +{
>>> + struct pnfs_deviceid_ref_args args = {
>>> + .ld = ld,
>>> + .id = id,
>>> + };
>>
>> I was wondering if we could go with different names for the members
>> of this struct? "ld" and "id" look very similar, and at first glance
>> I thought you were assigning the same values twice here.
>
> Sure we can.. there's precedent for "ld" being used to refer to
> pnfs_layoutdriver_type. Should we change them all? Happy to reversion and
> resend, or let you re-write it as well. Let me know what's most helpful.
I think just adjusting the naming in this patch is enough. You could even
leave "ld" alone and change "id" to something like "devid" and I'd be happy.
Anna
>
> Ben
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 18/24] NFSv4/pnfs: Recover revoked layouts on a deleted deviceID
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (16 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 17/24] pNFS: Add deviceid reference query and collection walkers Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 19/24] NFSv4/pnfs: Confirm a deviceID delete via GETDEVICEINFO Benjamin Coddington
` (5 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
RFC 8881 Section 20.12 lets a server send CB_NOTIFY_DEVICEID DELETE for
a deviceID once it has revoked every layout referring to it. Revocation
is not announced, so the client can still be holding what it believes
are live layouts on that deviceID. Section 18.40.4 resolves that:
TEST_STATEID each referring layout and recover the ones that come back
revoked -- mark the layout stateid invalid, free the lsegs, FREE_STATEID
to acknowledge.
The callback thread cannot issue fore-channel RPCs, so suspects are
queued on the nfs_client (dedup'd, holding a layoutdriver reference) and
resolved by a new state-manager step keyed on NFS4CLNT_DEVICEID_DELETE.
The worker re-collects the referring layouts, so layouts returned or
recalled in the meantime are skipped.
Drop the cached device once the collected layouts account for the
delete: every one of them was revoked here. Section 18.48.3 defines
TEST_STATEID's answers, and NFS4ERR_OLD_STATEID says the layout exists
and was not revoked -- only that it moved on after this stateid was
snapshotted -- so it counts against the delete as NFS4_OK does. Any
other answer leaves the revocation unresolved and keeps the device
cached, as does a layout the server still considers valid (verifying
that one with GETDEVICEINFO comes next).
A layout counts as revoked only if it was invalidated here; a stateid
that no longer matches its layout is a stale snapshot. Invalidating one
is paired with nfs_commit_inode(), since pnfs_clear_lseg_state() drops
only the VALID and LAYOUTCOMMIT references, and an lseg still held by a
commit bucket would keep the layout -- and the device nodes this
recovery is trying to release -- alive.
If the walk collects no referring layouts, the device is unreferenced
and the delete is carried out directly. If the collection could not be
completed, recovery leaves the device cached for the next notification.
Nothing enqueues suspects yet, so no behavior change.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/nfs4_fs.h | 2 +
fs/nfs/nfs4client.c | 2 +
fs/nfs/nfs4proc.c | 83 +++++++++++++++++++++++++++++++++++++++
fs/nfs/nfs4state.c | 3 ++
fs/nfs/pnfs.c | 62 +++++++++++++++++++++++++++++
fs/nfs/pnfs.h | 19 +++++++++
include/linux/nfs_fs_sb.h | 2 +
7 files changed, 173 insertions(+)
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index b48e5b87cb2a..d642aca0adc3 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -52,6 +52,7 @@ enum nfs4_client_state {
NFS4CLNT_RECALL_ANY_LAYOUT_READ,
NFS4CLNT_RECALL_ANY_LAYOUT_RW,
NFS4CLNT_DELEGRETURN_DELAYED,
+ NFS4CLNT_DEVICEID_DELETE,
};
#define NFS4_RENEW_TIMEOUT 0x01
@@ -493,6 +494,7 @@ int nfs41_discover_server_trunking(struct nfs_client *clp,
struct nfs_client **, const struct cred *);
extern void nfs4_schedule_session_recovery(struct nfs4_session *, int);
extern void nfs41_notify_server(struct nfs_client *);
+extern void nfs4_deviceid_delete_recover_run(struct nfs_client *clp);
bool nfs4_check_serverowner_major_id(struct nfs41_server_owner *o1,
struct nfs41_server_owner *o2);
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index b661f446ea49..e6a589913666 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -217,6 +217,7 @@ struct nfs_client *nfs4_alloc_client(const struct nfs_client_initdata *cl_init)
clp->cl_last_renewal = jiffies;
init_waitqueue_head(&clp->cl_lock_waitq);
INIT_LIST_HEAD(&clp->pending_cb_stateids);
+ INIT_LIST_HEAD(&clp->cl_deviceid_deletes);
if (cl_init->minorversion != 0)
__set_bit(NFS_CS_INFINITE_SLOTS, &clp->cl_flags);
@@ -286,6 +287,7 @@ static void nfs4_shutdown_client(struct nfs_client *clp)
nfs4_kill_renewd(clp);
clp->cl_mvops->shutdown_client(clp);
nfs4_destroy_callback(clp);
+ pnfs_deviceid_delete_queue_free(clp);
if (__test_and_clear_bit(NFS_CS_IDMAP, &clp->cl_res_state))
nfs_idmap_delete(clp);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 04b1987115d5..02df5f4f5d84 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -10498,6 +10498,89 @@ static int nfs41_free_stateid(struct nfs_server *server,
return ret;
}
+/*
+ * A DELETE for a deviceID we still hold layouts on implies the server
+ * revoked them: run the RFC 8881 Section 18.40.4 recovery.
+ */
+static void nfs4_deviceid_delete_recover(struct nfs_client *clp,
+ const struct pnfs_layoutdriver_type *ld,
+ const struct nfs4_deviceid *id)
+{
+ LIST_HEAD(layouts);
+ struct nfs4_deviceid_ref *ref;
+ bool revoked = false;
+ bool referenced = false;
+ bool inconclusive = false;
+
+ if (pnfs_layout_collect_deviceid_refs(clp, ld, id, &layouts)) {
+ /* Only a partial list -- an allocation failed, or an inode is
+ * being evicted. Leave the device cached and recover on a
+ * later notification.
+ */
+ pnfs_layout_put_deviceid_refs(&layouts);
+ return;
+ }
+
+ if (list_empty(&layouts)) {
+ nfs4_delete_deviceid(ld, clp, id);
+ return;
+ }
+
+ list_for_each_entry(ref, &layouts, node) {
+ struct pnfs_layout_hdr *lo = ref->lo;
+ struct inode *inode = ref->inode;
+ bool invalidated = false;
+ LIST_HEAD(head);
+ int status;
+
+ status = nfs41_test_stateid(NFS_SERVER(inode), &ref->stateid,
+ ref->cred);
+ switch (status) {
+ case NFS_OK:
+ case -NFS4ERR_OLD_STATEID:
+ referenced = true;
+ break;
+ case -NFS4ERR_ADMIN_REVOKED:
+ case -NFS4ERR_DELEG_REVOKED:
+ case -NFS4ERR_EXPIRED:
+ case -NFS4ERR_BAD_STATEID:
+ spin_lock(&inode->i_lock);
+ if (pnfs_layout_is_valid(lo) &&
+ nfs4_stateid_match_other(&ref->stateid,
+ &lo->plh_stateid)) {
+ pnfs_mark_layout_stateid_invalid(lo, &head);
+ revoked = true;
+ invalidated = true;
+ }
+ spin_unlock(&inode->i_lock);
+ pnfs_free_lseg_list(&head);
+ if (invalidated)
+ nfs_commit_inode(inode, 0);
+ nfs41_free_stateid(NFS_SERVER(inode), &ref->stateid,
+ ref->cred, true);
+ break;
+ default:
+ inconclusive = true;
+ break;
+ }
+ }
+ pnfs_layout_put_deviceid_refs(&layouts);
+
+ if (revoked && !referenced && !inconclusive)
+ nfs4_delete_deviceid(ld, clp, id);
+}
+
+void nfs4_deviceid_delete_recover_run(struct nfs_client *clp)
+{
+ struct nfs4_deviceid_delete *dd;
+
+ while ((dd = pnfs_deviceid_delete_dequeue(clp)) != NULL) {
+ nfs4_deviceid_delete_recover(clp, dd->ld, &dd->id);
+ pnfs_put_layoutdriver(dd->ld);
+ kfree(dd);
+ }
+}
+
static void
nfs41_free_lock_state(struct nfs_server *server, struct nfs4_lock_state *lsp)
{
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index a5dec0473e22..1faf9dafd331 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2669,6 +2669,9 @@ static void nfs4_state_manager(struct nfs_client *clp)
set_bit(NFS4CLNT_RUN_MANAGER, &clp->cl_state);
}
nfs4_layoutreturn_any_run(clp);
+ if (test_and_clear_bit(NFS4CLNT_DEVICEID_DELETE,
+ &clp->cl_state))
+ nfs4_deviceid_delete_recover_run(clp);
clear_bit(NFS4CLNT_RECALL_RUNNING, &clp->cl_state);
}
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 23f12eec99d7..e577001b78f2 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -3101,6 +3101,68 @@ pnfs_layout_put_deviceid_refs(struct list_head *result)
}
}
+/*
+ * Queue @id for the state manager's Section 18.40.4 recovery,
+ * dropping duplicates of an already-queued suspect.
+ */
+void pnfs_deviceid_delete_mark(struct nfs_client *clp,
+ const struct pnfs_layoutdriver_type *ld,
+ const struct nfs4_deviceid *id)
+{
+ struct nfs4_deviceid_delete *dd, *new;
+
+ new = kzalloc_obj(*new, GFP_KERNEL);
+ if (!new)
+ return; /* lost notification; recovery waits for the next */
+ new->ld = pnfs_find_layoutdriver(ld->id);
+ if (!new->ld) {
+ kfree(new);
+ return;
+ }
+ memcpy(&new->id, id, sizeof(new->id));
+
+ spin_lock(&clp->cl_lock);
+ list_for_each_entry(dd, &clp->cl_deviceid_deletes, list) {
+ if (dd->ld == new->ld &&
+ !memcmp(&dd->id, &new->id, sizeof(dd->id))) {
+ spin_unlock(&clp->cl_lock);
+ pnfs_put_layoutdriver(new->ld);
+ kfree(new);
+ return;
+ }
+ }
+ list_add_tail(&new->list, &clp->cl_deviceid_deletes);
+ spin_unlock(&clp->cl_lock);
+
+ set_bit(NFS4CLNT_DEVICEID_DELETE, &clp->cl_state);
+ nfs4_schedule_state_manager(clp);
+}
+
+struct nfs4_deviceid_delete *pnfs_deviceid_delete_dequeue(
+ struct nfs_client *clp)
+{
+ struct nfs4_deviceid_delete *dd = NULL;
+
+ spin_lock(&clp->cl_lock);
+ if (!list_empty(&clp->cl_deviceid_deletes)) {
+ dd = list_first_entry(&clp->cl_deviceid_deletes,
+ struct nfs4_deviceid_delete, list);
+ list_del(&dd->list);
+ }
+ spin_unlock(&clp->cl_lock);
+ return dd;
+}
+
+void pnfs_deviceid_delete_queue_free(struct nfs_client *clp)
+{
+ struct nfs4_deviceid_delete *dd;
+
+ while ((dd = pnfs_deviceid_delete_dequeue(clp)) != NULL) {
+ pnfs_put_layoutdriver(dd->ld);
+ kfree(dd);
+ }
+}
+
/* Check if we have we have a valid layout but if there isn't an intersection
* between the request and the pgio->pg_lseg, put this pgio->pg_lseg away.
*/
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index d1bc4d1d8d4c..734f8531c819 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -400,6 +400,25 @@ int pnfs_layout_collect_deviceid_refs(struct nfs_client *clp,
const struct nfs4_deviceid *id,
struct list_head *result);
void pnfs_layout_put_deviceid_refs(struct list_head *result);
+
+/*
+ * A CB_NOTIFY_DEVICEID DELETE naming a deviceID that live layouts
+ * still reference (RFC 8881 Section 18.40.4). Queued on
+ * nfs_client.cl_deviceid_deletes under cl_lock for the state manager
+ * to resolve; holds a layoutdriver reference.
+ */
+struct nfs4_deviceid_delete {
+ struct list_head list;
+ const struct pnfs_layoutdriver_type *ld;
+ struct nfs4_deviceid id;
+};
+
+void pnfs_deviceid_delete_mark(struct nfs_client *clp,
+ const struct pnfs_layoutdriver_type *ld,
+ const struct nfs4_deviceid *id);
+struct nfs4_deviceid_delete *pnfs_deviceid_delete_dequeue(
+ struct nfs_client *clp);
+void pnfs_deviceid_delete_queue_free(struct nfs_client *clp);
int pnfs_layout_handle_reboot(struct nfs_client *clp);
/* nfs4_deviceid_flags */
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index cd3ebca61dd1..11b4a3f10c60 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -103,6 +103,8 @@ struct nfs_client {
/* The flags used for obtaining the clientid during EXCHANGE_ID */
u32 cl_exchange_flags;
struct nfs4_session *cl_session; /* shared session */
+ /* CB_NOTIFY_DEVICEID DELETE suspects, protected by cl_lock */
+ struct list_head cl_deviceid_deletes;
bool cl_preserve_clid;
struct nfs41_server_owner *cl_serverowner;
struct nfs41_server_scope *cl_serverscope;
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 19/24] NFSv4/pnfs: Confirm a deviceID delete via GETDEVICEINFO
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (17 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 18/24] NFSv4/pnfs: Recover revoked layouts on a deleted deviceID Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 20/24] NFSv4/pnfs: Dispatch CB_NOTIFY_DEVICEID DELETE to race recovery Benjamin Coddington
` (4 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
RFC 8881 Section 18.40.4: if TEST_STATEID says at least one layout
referring to the deleted deviceID is still valid, the delete cannot
be trusted -- verify it with GETDEVICEINFO. The device really being
gone while the server also considers a referring layout valid means
the server is faulty; recover by re-establishing the client ID and
drop the cached device. Any other answer -- including the device
existing, i.e. an erroneous DELETE -- keeps the cached device and
the layout intact.
Re-establishing the client ID is nfs4_reset_all_state(), which sets
NFS4CLNT_PURGE_STATE so the state manager runs nfs4_purge_lease():
a fresh EXCHANGE_ID, then state reclaim with no grace period. The
grace-less reclaim is the point -- the server has not rebooted, so
there is nothing to reclaim under CLAIM_PREVIOUS, and the new client
ID orphans the state held under the old one. The obvious-looking
nfs4_schedule_lease_recovery() is not the right call here: it sets
NFS4CLNT_CHECK_LEASE, which the state manager turns into a lease
renewal, and on a healthy session -- which this one is, the server
having just answered TEST_STATEID and GETDEVICEINFO on it -- that
renewal succeeds and no EXCHANGE_ID is ever sent.
This is the only path on which a device notification can escalate to
a full client-ID reset, and every open, lock and delegation on the
client is reclaimed as a result. From userspace that is
indistinguishable from a spontaneous lease expiry, so the escalation
is announced with a rate-limited warning naming the server.
The raw-status probe calls nfs4_proc_getdeviceinfo() directly
because nfs4_get_device_info() swallows the RPC status and cannot
distinguish NFS4ERR_NOENT from a transient failure. A one-page
reply buffer is enough: a device too large for it fails with
something other than -ENOENT, which still proves existence.
Still nothing enqueues suspects; no behavior change.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/nfs4_fs.h | 1 +
fs/nfs/nfs4proc.c | 59 ++++++++++++++++++++++++++++++++++++++++------
fs/nfs/nfs4state.c | 2 +-
3 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index d642aca0adc3..76dae699d4d7 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -511,6 +511,7 @@ extern void nfs_inode_find_state_and_recover(struct inode *inode,
const nfs4_stateid *stateid);
extern int nfs4_state_mark_reclaim_nograce(struct nfs_client *, struct nfs4_state *);
extern void nfs4_schedule_lease_recovery(struct nfs_client *);
+extern void nfs4_reset_all_state(struct nfs_client *);
extern int nfs4_wait_clnt_recover(struct nfs_client *clp);
extern int nfs4_client_recover_expired_lease(struct nfs_client *clp);
extern void nfs4_schedule_state_manager(struct nfs_client *);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 02df5f4f5d84..75545294ebb1 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -10498,19 +10498,50 @@ static int nfs41_free_stateid(struct nfs_server *server,
return ret;
}
+/*
+ * GETDEVICEINFO surfacing the raw status; nfs4_get_device_info()
+ * swallows it. A device too large for one page fails with something
+ * other than -ENOENT, which still proves existence.
+ */
+static int nfs4_deviceid_validate(struct nfs_server *server,
+ const struct pnfs_layoutdriver_type *ld,
+ const struct nfs4_deviceid *id, const struct cred *cred)
+{
+ struct pnfs_device pdev;
+ struct page *page;
+ int status;
+
+ page = alloc_page(GFP_KERNEL);
+ if (!page)
+ return -ENOMEM;
+
+ memset(&pdev, 0, sizeof(pdev));
+ memcpy(&pdev.dev_id, id, sizeof(pdev.dev_id));
+ pdev.layout_type = ld->id;
+ pdev.pages = &page;
+ pdev.pglen = PAGE_SIZE;
+ pdev.maxcount = PAGE_SIZE - nfs41_maxgetdevinfo_overhead;
+
+ status = nfs4_proc_getdeviceinfo(server, &pdev, cred);
+ __free_page(page);
+ return status;
+}
+
/*
* A DELETE for a deviceID we still hold layouts on implies the server
- * revoked them: run the RFC 8881 Section 18.40.4 recovery.
+ * revoked them: run the RFC 8881 Section 18.40.4 recovery. A layout the
+ * server still calls valid leaves the revocations unable to confirm the
+ * delete, so verify it with GETDEVICEINFO.
*/
static void nfs4_deviceid_delete_recover(struct nfs_client *clp,
const struct pnfs_layoutdriver_type *ld,
const struct nfs4_deviceid *id)
{
LIST_HEAD(layouts);
- struct nfs4_deviceid_ref *ref;
+ struct nfs4_deviceid_ref *ref, *confirm = NULL;
bool revoked = false;
- bool referenced = false;
bool inconclusive = false;
+ int status;
if (pnfs_layout_collect_deviceid_refs(clp, ld, id, &layouts)) {
/* Only a partial list -- an allocation failed, or an inode is
@@ -10531,14 +10562,14 @@ static void nfs4_deviceid_delete_recover(struct nfs_client *clp,
struct inode *inode = ref->inode;
bool invalidated = false;
LIST_HEAD(head);
- int status;
status = nfs41_test_stateid(NFS_SERVER(inode), &ref->stateid,
ref->cred);
switch (status) {
case NFS_OK:
case -NFS4ERR_OLD_STATEID:
- referenced = true;
+ if (!confirm)
+ confirm = ref;
break;
case -NFS4ERR_ADMIN_REVOKED:
case -NFS4ERR_DELEG_REVOKED:
@@ -10564,10 +10595,24 @@ static void nfs4_deviceid_delete_recover(struct nfs_client *clp,
break;
}
}
- pnfs_layout_put_deviceid_refs(&layouts);
- if (revoked && !referenced && !inconclusive)
+ if (confirm) {
+ status = nfs4_deviceid_validate(NFS_SERVER(confirm->inode),
+ ld, id, confirm->cred);
+ if (status == -ENOENT) {
+ /* Section 18.40.4 prescribes EXCHANGE_ID here;
+ * nfs4_schedule_lease_recovery() would only renew
+ * the existing lease.
+ */
+ pr_warn_ratelimited("NFS: server %s deleted a deviceID referred to by a layout it still considers valid; re-establishing the client ID\n",
+ clp->cl_hostname);
+ nfs4_reset_all_state(clp);
+ nfs4_delete_deviceid(ld, clp, id);
+ }
+ } else if (revoked && !inconclusive) {
nfs4_delete_deviceid(ld, clp, id);
+ }
+ pnfs_layout_put_deviceid_refs(&layouts);
}
void nfs4_deviceid_delete_recover_run(struct nfs_client *clp)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 1faf9dafd331..4c085c00abb1 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2354,7 +2354,7 @@ void nfs41_notify_server(struct nfs_client *clp)
nfs4_schedule_state_manager(clp);
}
-static void nfs4_reset_all_state(struct nfs_client *clp)
+void nfs4_reset_all_state(struct nfs_client *clp)
{
if (test_and_set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) == 0) {
set_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 20/24] NFSv4/pnfs: Dispatch CB_NOTIFY_DEVICEID DELETE to race recovery
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (18 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 19/24] NFSv4/pnfs: Confirm a deviceID delete via GETDEVICEINFO Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 21/24] NFSv4/pnfs: Grow the deviceid cache hash table Benjamin Coddington
` (3 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
Turn on the RFC 8881 Section 18.40.4 DELETE handling: a DELETE for a
deviceID that no live layout references keeps today's cheap behavior
(drop the cached device, no state-manager wake). A DELETE for a
deviceID that live layouts still reference is deferred to the state
manager, which TEST_STATEIDs the referring layouts, recovers revoked
ones, and confirms or refutes the delete with GETDEVICEINFO.
The deferred case no longer unhashes the device immediately: if the
recovery concludes the DELETE was erroneous (the deviceID still
exists and a referring layout is still valid), the client keeps
using the cached device.
Re-arm the state manager afterwards, as the delegation return above it
does. The recovery issues synchronous RPCs, and a manager thread that
starts and exits while it runs clears NFS4CLNT_RUN_MANAGER on its way
out.
Bump the deviceid change epoch for both notification types rather
than only for CHANGE. A GETDEVICEINFO whose reply is already in
flight can otherwise re-cache a device the notification has just
invalidated; that is as true of a delete as of a change, and Section
18.40.4 opens by describing the race for the delete case.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/callback_proc.c | 24 +++++++++++++++---------
fs/nfs/nfs4state.c | 4 +++-
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index ea8c558b07b6..c9b71dbae9ea 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -392,19 +392,25 @@ __be32 nfs4_callback_devicenotify(void *argp, void *resp,
continue;
}
/*
- * Unhash the cached device first so re-resolution cannot
- * re-pin the stale node, then re-point any references
- * pinned under live layouts (RFC 8881 Section 12.2.10).
- * The epoch bump lets an in-flight GETDEVICEINFO detect
- * that its reply may predate the change.
+ * Bump the epoch before touching the cache so a
+ * GETDEVICEINFO already in flight can detect that it
+ * predates the notification. A referenced DELETE may be
+ * racing revocation, so defer it to the state manager --
+ * this thread cannot issue fore-channel RPCs.
*/
- if (dev->cbd_notify_type == NOTIFY_DEVICEID4_CHANGE)
- nfs4_deviceid_bump_change_epoch(cps->clp);
- nfs4_delete_deviceid(ld, cps->clp, &dev->cbd_dev_id);
- if (dev->cbd_notify_type == NOTIFY_DEVICEID4_CHANGE)
+ nfs4_deviceid_bump_change_epoch(cps->clp);
+ if (dev->cbd_notify_type == NOTIFY_DEVICEID4_CHANGE) {
+ nfs4_delete_deviceid(ld, cps->clp, &dev->cbd_dev_id);
pnfs_layout_reresolve_deviceid_byclid(cps->clp, ld,
&dev->cbd_dev_id,
dev->cbd_immediate);
+ } else if (pnfs_layout_deviceid_referenced_byclid(cps->clp,
+ ld, &dev->cbd_dev_id)) {
+ pnfs_deviceid_delete_mark(cps->clp, ld,
+ &dev->cbd_dev_id);
+ } else {
+ nfs4_delete_deviceid(ld, cps->clp, &dev->cbd_dev_id);
+ }
}
pnfs_put_layoutdriver(ld);
out:
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 4c085c00abb1..b5d6daa2c416 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2670,8 +2670,10 @@ static void nfs4_state_manager(struct nfs_client *clp)
}
nfs4_layoutreturn_any_run(clp);
if (test_and_clear_bit(NFS4CLNT_DEVICEID_DELETE,
- &clp->cl_state))
+ &clp->cl_state)) {
nfs4_deviceid_delete_recover_run(clp);
+ set_bit(NFS4CLNT_RUN_MANAGER, &clp->cl_state);
+ }
clear_bit(NFS4CLNT_RECALL_RUNNING, &clp->cl_state);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 21/24] NFSv4/pnfs: Grow the deviceid cache hash table
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (19 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 20/24] NFSv4/pnfs: Dispatch CB_NOTIFY_DEVICEID DELETE to race recovery Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 22/24] NFSv4/pnfs: Re-home the data-server cache onto hash buckets Benjamin Coddington
` (2 subsequent siblings)
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
The global deviceid cache has 32 buckets shared by every server and
layout type. Striping deployments initially anticipate device counts
scaling to 1024 or more devices, which leaves those chains 32 entries
deep for every resolution to walk.
Grow to 256 buckets, four deep at that scale, for 2KB of BSS on
64-bit.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/pnfs_dev.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/pnfs_dev.c b/fs/nfs/pnfs_dev.c
index a3b28409539a..e8143977965c 100644
--- a/fs/nfs/pnfs_dev.c
+++ b/fs/nfs/pnfs_dev.c
@@ -40,8 +40,11 @@
/*
* Device ID RCU cache. A device ID is unique per server and layout type.
+ *
+ * 256 buckets keeps the chains short at the 1024-or-more devices a
+ * striping deployment expects, for 2KB of BSS on 64-bit.
*/
-#define NFS4_DEVICE_ID_HASH_BITS 5
+#define NFS4_DEVICE_ID_HASH_BITS 8
#define NFS4_DEVICE_ID_HASH_SIZE (1 << NFS4_DEVICE_ID_HASH_BITS)
#define NFS4_DEVICE_ID_HASH_MASK (NFS4_DEVICE_ID_HASH_SIZE - 1)
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 22/24] NFSv4/pnfs: Re-home the data-server cache onto hash buckets
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (20 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 21/24] NFSv4/pnfs: Grow the deviceid cache hash table Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 23/24] NFSv4/pnfs: Key the data-server cache by its address set and version Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 24/24] NFSv4/flexfiles: Add a dataserver_nconnect cap Benjamin Coddington
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
The per-net data-server cache is a single list, and every
GETDEVICEINFO decode walks all of it looking for a match, so filling
the cache costs O(n^2) in the number of data servers -- which a
striping mount does in one burst, at the same scale the deviceid
cache was just sized for. Key it by the DS address set instead.
This patch is the mechanical half: nfs4_pnfs_ds.ds_node becomes an
hlist_node, netns init and teardown cover every bucket, and removal
uses hlist_del_init (which needs no bucket reference). Insertion
still targets bucket 0 and lookup still scans every entry, so
behavior is unchanged; the key comes next. Splitting it this way
keeps a bisect able to tell a list-conversion bug from a hash-key
bug.
The buckets live in struct nfs_net, so this costs 2KB per network
namespace on 64-bit, paid once nfs.ko is loaded whether or not that
namespace ever mounts NFS.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/client.c | 6 ++++--
fs/nfs/netns.h | 5 ++++-
fs/nfs/pnfs.h | 2 +-
fs/nfs/pnfs_nfs.c | 19 +++++++++++--------
4 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 60386330aeec..dbb5131375f8 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1295,7 +1295,8 @@ void nfs_clients_init(struct net *net)
INIT_LIST_HEAD(&nn->nfs_volume_list);
#if IS_ENABLED(CONFIG_NFS_V4)
idr_init(&nn->cb_ident_idr);
- INIT_LIST_HEAD(&nn->nfs4_data_server_cache);
+ for (int i = 0; i < NFS4_DS_CACHE_HASH_SIZE; i++)
+ INIT_HLIST_HEAD(&nn->nfs4_data_server_cache[i]);
spin_lock_init(&nn->nfs4_data_server_lock);
#endif /* CONFIG_NFS_V4 */
spin_lock_init(&nn->nfs_client_lock);
@@ -1315,7 +1316,8 @@ void nfs_clients_exit(struct net *net)
WARN_ON_ONCE(!list_empty(&nn->nfs_client_list));
WARN_ON_ONCE(!list_empty(&nn->nfs_volume_list));
#if IS_ENABLED(CONFIG_NFS_V4)
- WARN_ON_ONCE(!list_empty(&nn->nfs4_data_server_cache));
+ for (int i = 0; i < NFS4_DS_CACHE_HASH_SIZE; i++)
+ WARN_ON_ONCE(!hlist_empty(&nn->nfs4_data_server_cache[i]));
#endif /* CONFIG_NFS_V4 */
}
diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
index 36658579100d..560fa95726b0 100644
--- a/fs/nfs/netns.h
+++ b/fs/nfs/netns.h
@@ -31,7 +31,10 @@ struct nfs_net {
unsigned short nfs_callback_tcpport;
unsigned short nfs_callback_tcpport6;
int cb_users[NFS4_MAX_MINOR_VERSION + 1];
- struct list_head nfs4_data_server_cache;
+#define NFS4_DS_CACHE_HASH_BITS 8
+#define NFS4_DS_CACHE_HASH_SIZE (1 << NFS4_DS_CACHE_HASH_BITS)
+ /* every entry is still in bucket 0 until the key is added */
+ struct hlist_head nfs4_data_server_cache[NFS4_DS_CACHE_HASH_SIZE];
spinlock_t nfs4_data_server_lock;
#endif /* CONFIG_NFS_V4 */
struct nfs_netns_client *nfs_client;
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 734f8531c819..8b612f3679a3 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -57,7 +57,7 @@ struct nfs4_pnfs_ds_addr {
};
struct nfs4_pnfs_ds {
- struct list_head ds_node; /* nfs4_pnfs_dev_hlist dev_dslist */
+ struct hlist_node ds_node; /* nfs_net nfs4_data_server_cache */
char *ds_remotestr; /* comma sep list of addrs */
struct list_head ds_addrs;
const struct net *ds_net;
diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index e0e3fc7414e6..f88a9784988a 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -604,7 +604,7 @@ _same_data_server_addrs_locked(const struct list_head *dsaddrs1,
}
/*
- * Lookup DS by addresses and NFS version. nfs4_ds_cache_lock is held
+ * Lookup DS by addresses and NFS version. nfs4_data_server_lock is held
*/
static struct nfs4_pnfs_ds *
_data_server_lookup_locked(const struct nfs_net *nn,
@@ -612,10 +612,13 @@ _data_server_lookup_locked(const struct nfs_net *nn,
{
struct nfs4_pnfs_ds *ds;
- list_for_each_entry(ds, &nn->nfs4_data_server_cache, ds_node)
- if (ds->ds_version == version &&
- _same_data_server_addrs_locked(&ds->ds_addrs, dsaddrs))
- return ds;
+ for (int i = 0; i < NFS4_DS_CACHE_HASH_SIZE; i++)
+ hlist_for_each_entry(ds, &nn->nfs4_data_server_cache[i],
+ ds_node)
+ if (ds->ds_version == version &&
+ _same_data_server_addrs_locked(&ds->ds_addrs,
+ dsaddrs))
+ return ds;
return NULL;
}
@@ -666,7 +669,7 @@ void nfs4_pnfs_ds_put(struct nfs4_pnfs_ds *ds)
struct nfs_net *nn = net_generic(ds->ds_net, nfs_net_id);
if (refcount_dec_and_lock(&ds->ds_count, &nn->nfs4_data_server_lock)) {
- list_del_init(&ds->ds_node);
+ hlist_del_init(&ds->ds_node);
spin_unlock(&nn->nfs4_data_server_lock);
destroy_ds(ds);
}
@@ -753,11 +756,11 @@ nfs4_pnfs_ds_add(const struct net *net, struct list_head *dsaddrs, u32 version,
list_splice_init(dsaddrs, &ds->ds_addrs);
ds->ds_remotestr = remotestr;
refcount_set(&ds->ds_count, 1);
- INIT_LIST_HEAD(&ds->ds_node);
+ INIT_HLIST_NODE(&ds->ds_node);
ds->ds_net = net;
ds->ds_clp = NULL;
ds->ds_version = version;
- list_add(&ds->ds_node, &nn->nfs4_data_server_cache);
+ hlist_add_head(&ds->ds_node, &nn->nfs4_data_server_cache[0]);
dprintk("%s add new data server %s\n", __func__,
ds->ds_remotestr);
} else {
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 23/24] NFSv4/pnfs: Key the data-server cache by its address set and version
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (21 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 22/24] NFSv4/pnfs: Re-home the data-server cache onto hash buckets Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
2026-09-04 16:53 ` [PATCH v3 24/24] NFSv4/flexfiles: Add a dataserver_nconnect cap Benjamin Coddington
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
Populating the data-server cache was quadratic: every GETDEVICEINFO
decode scanned the whole per-net cache under one spinlock. At the
anticipated scale of 1024 data servers a striping mount pays that in a
burst at first access, and again on notification-driven re-resolution.
Hash each DS to a bucket keyed on the whole cache key -- per-address
jhash over exactly the fields same_sockaddr() compares, combined by
addition so multipath ordering cannot change the bucket, then the NFS
version folded in -- with the existing comparator as the in-bucket
tiebreaker. Lookup and insert now touch one bucket; teardown is
unchanged (hlist_del_init needs no bucket).
Keying on the whole set requires the comparator to test set equality,
so it is tightened from the subset test it did before -- a subset
match would hash to a different bucket and simply never be found.
That test was also wrong in a way worth naming. It answered "is
dsaddrs1 a subset of dsaddrs2", and the caller passes the cached list
first, so a cached data server whose address set was contained in an
incoming one was returned for that incoming set. The aliasing was
therefore one-directional: cache {A,B} first and an incoming {A} did
not match, but cache {A} first and an incoming {A,B} did.
The consequence was mild, which is why it went unnoticed: every
address on one device's multipath list names the same data server, so
a merged entry's addresses are all paths that server also advertised.
The effect is lost path diversity and a truncated ds_remotestr (and
the netaddr flexfiles reports in layoutstats), not I/O sent to the
wrong server.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/pnfs_nfs.c | 68 ++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 58 insertions(+), 10 deletions(-)
diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index f88a9784988a..c9bbcb765f4a 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -15,6 +15,8 @@
#include "nfs4session.h"
#include "internal.h"
+#include <linux/hash.h>
+#include <linux/jhash.h>
#include "pnfs.h"
#include "netns.h"
#include "nfs4trace.h"
@@ -577,8 +579,8 @@ same_sockaddr(struct sockaddr *addr1, struct sockaddr *addr2)
}
/*
- * Checks if 'dsaddrs1' contains a subset of 'dsaddrs2'. If it does,
- * declare a match.
+ * Checks if 'dsaddrs1' and 'dsaddrs2' hold the same set of addresses.
+ * If they do, declare a match.
*/
static bool
_same_data_server_addrs_locked(const struct list_head *dsaddrs1,
@@ -588,6 +590,10 @@ _same_data_server_addrs_locked(const struct list_head *dsaddrs1,
struct sockaddr *sa1, *sa2;
bool match = false;
+ if (list_count_nodes((struct list_head *)dsaddrs1) !=
+ list_count_nodes((struct list_head *)dsaddrs2))
+ return false;
+
list_for_each_entry(da1, dsaddrs1, da_node) {
sa1 = (struct sockaddr *)&da1->da_addr;
match = false;
@@ -603,6 +609,47 @@ _same_data_server_addrs_locked(const struct list_head *dsaddrs1,
return match;
}
+/* Hash family, address bytes, and port - as same_sockaddr() */
+static u32
+nfs4_ds_addr_hash(const struct sockaddr *sa)
+{
+ u32 h = sa->sa_family;
+
+ switch (sa->sa_family) {
+ case AF_INET: {
+ const struct sockaddr_in *a = (const struct sockaddr_in *)sa;
+
+ h = jhash(&a->sin_addr.s_addr, sizeof(a->sin_addr.s_addr), h);
+ h = jhash(&a->sin_port, sizeof(a->sin_port), h);
+ break;
+ }
+ case AF_INET6: {
+ const struct sockaddr_in6 *a = (const struct sockaddr_in6 *)sa;
+
+ h = jhash(&a->sin6_addr, sizeof(a->sin6_addr), h);
+ h = jhash(&a->sin6_port, sizeof(a->sin6_port), h);
+ break;
+ }
+ }
+ return h;
+}
+
+/*
+ * Bucket index for a DS cache key. Per-address hashes combine by
+ * addition so the multipath list order cannot change the bucket,
+ * matching the order-independent set comparison above.
+ */
+static u32
+nfs4_ds_cache_hash(const struct list_head *dsaddrs, u32 version)
+{
+ const struct nfs4_pnfs_ds_addr *da;
+ u32 h = 0;
+
+ list_for_each_entry(da, dsaddrs, da_node)
+ h += nfs4_ds_addr_hash((const struct sockaddr *)&da->da_addr);
+ return hash_32(jhash_1word(version, h), NFS4_DS_CACHE_HASH_BITS);
+}
+
/*
* Lookup DS by addresses and NFS version. nfs4_data_server_lock is held
*/
@@ -611,14 +658,12 @@ _data_server_lookup_locked(const struct nfs_net *nn,
const struct list_head *dsaddrs, u32 version)
{
struct nfs4_pnfs_ds *ds;
+ u32 bucket = nfs4_ds_cache_hash(dsaddrs, version);
- for (int i = 0; i < NFS4_DS_CACHE_HASH_SIZE; i++)
- hlist_for_each_entry(ds, &nn->nfs4_data_server_cache[i],
- ds_node)
- if (ds->ds_version == version &&
- _same_data_server_addrs_locked(&ds->ds_addrs,
- dsaddrs))
- return ds;
+ hlist_for_each_entry(ds, &nn->nfs4_data_server_cache[bucket], ds_node)
+ if (ds->ds_version == version &&
+ _same_data_server_addrs_locked(&ds->ds_addrs, dsaddrs))
+ return ds;
return NULL;
}
@@ -735,6 +780,7 @@ nfs4_pnfs_ds_add(const struct net *net, struct list_head *dsaddrs, u32 version,
{
struct nfs_net *nn = net_generic(net, nfs_net_id);
struct nfs4_pnfs_ds *tmp_ds, *ds = NULL;
+ struct hlist_head *bucket;
char *remotestr;
if (list_empty(dsaddrs)) {
@@ -748,6 +794,8 @@ nfs4_pnfs_ds_add(const struct net *net, struct list_head *dsaddrs, u32 version,
/* this is only used for debugging, so it's ok if its NULL */
remotestr = nfs4_pnfs_remotestr(dsaddrs, gfp_flags);
+ /* @dsaddrs is empty after the splice below. */
+ bucket = &nn->nfs4_data_server_cache[nfs4_ds_cache_hash(dsaddrs, version)];
spin_lock(&nn->nfs4_data_server_lock);
tmp_ds = _data_server_lookup_locked(nn, dsaddrs, version);
@@ -760,7 +808,7 @@ nfs4_pnfs_ds_add(const struct net *net, struct list_head *dsaddrs, u32 version,
ds->ds_net = net;
ds->ds_clp = NULL;
ds->ds_version = version;
- hlist_add_head(&ds->ds_node, &nn->nfs4_data_server_cache[0]);
+ hlist_add_head(&ds->ds_node, bucket);
dprintk("%s add new data server %s\n", __func__,
ds->ds_remotestr);
} else {
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 24/24] NFSv4/flexfiles: Add a dataserver_nconnect cap
2026-09-04 16:52 [PATCH v3 00/24] NFS: flexfiles device notifications and caching for wide striped layouts Benjamin Coddington
` (22 preceding siblings ...)
2026-09-04 16:53 ` [PATCH v3 23/24] NFSv4/pnfs: Key the data-server cache by its address set and version Benjamin Coddington
@ 2026-09-04 16:53 ` Benjamin Coddington
23 siblings, 0 replies; 28+ messages in thread
From: Benjamin Coddington @ 2026-09-04 16:53 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, Jonathan Curley, Mike Snitzer, Jeff Layton, Junrui Luo
Data-server clients inherit the MDS nconnect setting. A striping
mount multiplies that by every distinct data server: at the
anticipated scale of 1024 and nconnect=16 the client opens 16k
sockets plus their sunrpc slot tables, and striping already spreads
I/O across the data servers, so a high per-DS transport count buys
little for that workload.
Add a dataserver_nconnect module parameter to the flexfiles layout
driver alongside its existing dataserver_timeo and dataserver_retrans
knobs, and thread the value through nfs4_pnfs_ds_connect() to both
the v3 and v4 data-server client setup paths. The default of 0
preserves today's inherit-from-MDS behavior, so the cap is opt-in
and non-regressing. The files layout passes 0, unchanged.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/filelayout/filelayoutdev.c | 2 +-
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 7 +++++++
fs/nfs/internal.h | 3 ++-
fs/nfs/nfs3client.c | 9 +++++++--
fs/nfs/nfs4client.c | 6 +++++-
fs/nfs/pnfs.h | 3 ++-
fs/nfs/pnfs_nfs.c | 21 ++++++++++++++-------
7 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/fs/nfs/filelayout/filelayoutdev.c b/fs/nfs/filelayout/filelayoutdev.c
index 57e0654cd98c..5995f32f07ec 100644
--- a/fs/nfs/filelayout/filelayoutdev.c
+++ b/fs/nfs/filelayout/filelayoutdev.c
@@ -267,7 +267,7 @@ nfs4_fl_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx)
goto out_test_devid;
status = nfs4_pnfs_ds_connect(s, ds, devid, dataserver_timeo,
- dataserver_retrans, 4,
+ dataserver_retrans, 0, 4,
s->nfs_client->cl_minorversion, true);
if (status) {
nfs4_mark_deviceid_unavailable(devid);
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index 5cb09e5e2138..d52f485d0650 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -20,6 +20,7 @@
static unsigned int dataserver_timeo = NFS_DEF_TCP_TIMEO;
static unsigned int dataserver_retrans;
+static unsigned int dataserver_nconnect;
static bool ff_layout_has_available_ds(struct pnfs_layout_segment *lseg);
@@ -418,6 +419,7 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg,
*/
status = nfs4_pnfs_ds_connect(s, ds, &mirror_ds->id_node,
dataserver_timeo, dataserver_retrans,
+ dataserver_nconnect,
mirror_ds->ds_versions[0].version,
mirror_ds->ds_versions[0].minor_version,
mirror_ds->ds_versions[0].tightly_coupled);
@@ -676,3 +678,8 @@ module_param(dataserver_timeo, uint, 0644);
MODULE_PARM_DESC(dataserver_timeo, "The time (in tenths of a second) the "
"NFSv4.1 client waits for a response from a "
" data server before it retries an NFS request.");
+module_param(dataserver_nconnect, uint, 0644);
+MODULE_PARM_DESC(dataserver_nconnect, "The maximum number of connections "
+ "the NFSv4.1 client opens to each data server, "
+ "capping the value inherited from the MDS nconnect "
+ "mount option. 0 (default) applies no cap.");
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index abc81f5ae578..48f7c0e25da1 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -251,6 +251,7 @@ extern struct nfs_client *nfs4_set_ds_client(struct nfs_server *mds_srv,
int ds_addrlen, int ds_proto,
unsigned int ds_timeo,
unsigned int ds_retrans,
+ unsigned int ds_nconnect,
u32 minor_version,
bool tightly_coupled);
extern struct rpc_clnt *nfs4_find_or_create_ds_client(struct nfs_client *,
@@ -260,7 +261,7 @@ extern void nfs4_session_limit_xasize(struct nfs_server *server);
extern struct nfs_client *nfs3_set_ds_client(struct nfs_server *mds_srv,
const struct sockaddr_storage *ds_addr, int ds_addrlen,
int ds_proto, unsigned int ds_timeo,
- unsigned int ds_retrans);
+ unsigned int ds_retrans, unsigned int ds_nconnect);
#ifdef CONFIG_PROC_FS
extern int __init nfs_fs_proc_init(void);
extern void nfs_fs_proc_exit(void);
diff --git a/fs/nfs/nfs3client.c b/fs/nfs/nfs3client.c
index 5d97c1d38bb6..cf2f7be4b435 100644
--- a/fs/nfs/nfs3client.c
+++ b/fs/nfs/nfs3client.c
@@ -84,7 +84,8 @@ struct nfs_server *nfs3_clone_server(struct nfs_server *source,
*/
struct nfs_client *nfs3_set_ds_client(struct nfs_server *mds_srv,
const struct sockaddr_storage *ds_addr, int ds_addrlen,
- int ds_proto, unsigned int ds_timeo, unsigned int ds_retrans)
+ int ds_proto, unsigned int ds_timeo, unsigned int ds_retrans,
+ unsigned int ds_nconnect)
{
struct rpc_timeout ds_timeout;
unsigned long connect_timeout = ds_timeo * (ds_retrans + 1) * HZ / 10;
@@ -124,8 +125,12 @@ struct nfs_client *nfs3_set_ds_client(struct nfs_server *mds_srv,
fallthrough;
case XPRT_TRANSPORT_RDMA:
case XPRT_TRANSPORT_TCP:
- if (mds_clp->cl_nconnect > 1)
+ if (mds_clp->cl_nconnect > 1) {
cl_init.nconnect = mds_clp->cl_nconnect;
+ if (ds_nconnect)
+ cl_init.nconnect = min(cl_init.nconnect,
+ ds_nconnect);
+ }
}
if (mds_srv->flags & NFS_MOUNT_NORESVPORT)
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index e6a589913666..fe779fb2ec72 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -794,7 +794,8 @@ static int nfs4_set_client(struct nfs_server *server,
struct nfs_client *nfs4_set_ds_client(struct nfs_server *mds_srv,
const struct sockaddr_storage *ds_addr, int ds_addrlen,
int ds_proto, unsigned int ds_timeo, unsigned int ds_retrans,
- u32 minor_version, bool tightly_coupled)
+ unsigned int ds_nconnect, u32 minor_version,
+ bool tightly_coupled)
{
struct rpc_timeout ds_timeout;
struct nfs_client *mds_clp = mds_srv->nfs_client;
@@ -832,6 +833,9 @@ struct nfs_client *nfs4_set_ds_client(struct nfs_server *mds_srv,
case XPRT_TRANSPORT_TCP:
if (mds_clp->cl_nconnect > 1) {
cl_init.nconnect = mds_clp->cl_nconnect;
+ if (ds_nconnect)
+ cl_init.nconnect = min(cl_init.nconnect,
+ ds_nconnect);
cl_init.max_connect = NFS_MAX_TRANSPORTS;
}
}
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 8b612f3679a3..719657f3f4e1 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -504,7 +504,8 @@ struct nfs4_pnfs_ds *nfs4_pnfs_ds_add(const struct net *net,
void nfs4_pnfs_v3_ds_connect_unload(void);
int nfs4_pnfs_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds,
struct nfs4_deviceid_node *devid, unsigned int timeo,
- unsigned int retrans, u32 version, u32 minor_version,
+ unsigned int retrans, unsigned int nconnect,
+ u32 version, u32 minor_version,
bool tightly_coupled);
struct nfs4_pnfs_ds_addr *nfs4_decode_mp_ds_addr(struct net *net,
struct xdr_stream *xdr,
diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
index c9bbcb765f4a..7adb6f941cf2 100644
--- a/fs/nfs/pnfs_nfs.c
+++ b/fs/nfs/pnfs_nfs.c
@@ -844,7 +844,8 @@ static struct nfs_client *(*get_v3_ds_connect)(
int ds_addrlen,
int ds_proto,
unsigned int ds_timeo,
- unsigned int ds_retrans);
+ unsigned int ds_retrans,
+ unsigned int ds_nconnect);
static bool load_v3_ds_connect(void)
{
@@ -867,7 +868,8 @@ void nfs4_pnfs_v3_ds_connect_unload(void)
static int _nfs4_pnfs_v3_ds_connect(struct nfs_server *mds_srv,
struct nfs4_pnfs_ds *ds,
unsigned int timeo,
- unsigned int retrans)
+ unsigned int retrans,
+ unsigned int nconnect)
{
struct nfs_client *clp = ERR_PTR(-EIO);
struct nfs_client *mds_clp = mds_srv->nfs_client;
@@ -919,7 +921,7 @@ static int _nfs4_pnfs_v3_ds_connect(struct nfs_server *mds_srv,
ds_proto = XPRT_TRANSPORT_TCP_TLS;
clp = get_v3_ds_connect(mds_srv, &da->da_addr, da->da_addrlen,
- ds_proto, timeo, retrans);
+ ds_proto, timeo, retrans, nconnect);
if (IS_ERR(clp))
continue;
clp->cl_rpcclient->cl_softerr = 0;
@@ -942,6 +944,7 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
struct nfs4_pnfs_ds *ds,
unsigned int timeo,
unsigned int retrans,
+ unsigned int nconnect,
u32 minor_version,
bool tightly_coupled)
{
@@ -1033,7 +1036,8 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
clp = nfs4_set_ds_client(mds_srv, &da->da_addr,
da->da_addrlen, ds_proto,
- timeo, retrans, minor_version,
+ timeo, retrans, nconnect,
+ minor_version,
tightly_coupled);
if (IS_ERR(clp))
continue;
@@ -1068,7 +1072,8 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server *mds_srv,
*/
int nfs4_pnfs_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds,
struct nfs4_deviceid_node *devid, unsigned int timeo,
- unsigned int retrans, u32 version, u32 minor_version,
+ unsigned int retrans, unsigned int nconnect,
+ u32 version, u32 minor_version,
bool tightly_coupled)
{
int err;
@@ -1088,11 +1093,13 @@ int nfs4_pnfs_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds,
switch (version) {
case 3:
- err = _nfs4_pnfs_v3_ds_connect(mds_srv, ds, timeo, retrans);
+ err = _nfs4_pnfs_v3_ds_connect(mds_srv, ds, timeo, retrans,
+ nconnect);
break;
case 4:
err = _nfs4_pnfs_v4_ds_connect(mds_srv, ds, timeo, retrans,
- minor_version, tightly_coupled);
+ nconnect, minor_version,
+ tightly_coupled);
break;
default:
dprintk("%s: unsupported DS version %d\n", __func__, version);
--
2.53.0
^ permalink raw reply related [flat|nested] 28+ messages in thread