[PATCH v2 0/3] pNFS/flexfiles: mirror instance error handling

[PATCH v2 0/3] pNFS/flexfiles: mirror instance error handling

[Benjamin Coddington]

When a flexfiles data server fails, the client marks the layout for return
and cancels the layout segment's in-flight I/O.  ff_layout_cancel_io()
cancels I/O to every mirror instance rather than just the failed one, and
cancelling a request that has already been transmitted does not stop the
data server from executing it.  Those writes may complete after the client
has sent its LAYOUTRETURN, and the metadata server will see writes to a file
for which no write layout is outstanding.

Patch 1 reports the operation the client was preparing to send when a
data server connection fails, instead of OP_ILLEGAL.

Patches 2 and 3 let a layout driver cancel I/O to a single device, and
pass the failed device down from the flexfiles error paths, so that I/O
to the remaining mirrors drains before the LAYOUTRETURN is sent.

Layout recalls that set clora_changed, bulk returns and revocations are
unchanged: they continue to cancel I/O to every device.

This version applies on top of the flexfiles device-notification and
caching series [1], which rewrites the data-server setup and
I/O-cancellation paths these patches touch.

v1 posting:
  https://lore.kernel.org/linux-nfs/cover.1786636529.git.bcodding@hammerspace.com/

Changes since v1:

 - Rebase onto [1].  No functional change from v1; every conflict was
   in context that series rewrites:

   - nfs4_ff_layout_prepare_ds() now takes the device node the caller
     has already resolved and referenced, so patch 1 replaces the
     trailing fail_return argument of a five-argument signature, and
     three of the five call sites wrap to stay inside 80 columns.

   - ff_layout_cancel_io() now walks mirror->dss[].mirror_ds under RCU
     and takes a reference on the node, so patch 2 applies its
     device-ID filter ahead of the RCU dereference rather than in
     place of a plain pointer read.  The filter still compares the raw
     device ID from the layout, which is immutable across the in-place
     device re-resolve that series adds.

   - ff_layout_resend_pnfs_read() now yields a device node rather than
     a data server and puts it on the success path, so patch 2's NULL
     device-ID argument moves into the braced error arm.

[1] https://lore.kernel.org/linux-nfs/cover.1788530385.git.bcodding@hammerspace.com/

Benjamin Coddington (3):
  NFSv4/flexfiles: report the intended opnum when DS connection setup
    fails
  pNFS: allow layout drivers to cancel I/O to a single device
  NFSv4/flexfiles: only cancel I/O to a failed mirror instance

 fs/nfs/blocklayout/blocklayout.c          |  6 +++--
 fs/nfs/callback_proc.c                    |  2 +-
 fs/nfs/filelayout/filelayout.c            |  4 +--
 fs/nfs/flexfilelayout/flexfilelayout.c    | 32 +++++++++++++++--------
 fs/nfs/flexfilelayout/flexfilelayout.h    |  2 +-
 fs/nfs/flexfilelayout/flexfilelayoutdev.c | 17 +++++++-----
 fs/nfs/pnfs.c                             | 32 ++++++++++++++---------
 fs/nfs/pnfs.h                             | 14 ++++++----
 8 files changed, 67 insertions(+), 42 deletions(-)

-- 
2.53.0

[PATCH v2 1/3] NFSv4/flexfiles: report the intended opnum when DS connection setup fails

[Benjamin Coddington]

When nfs4_ff_layout_prepare_ds() cannot establish a connection to a
data server, it records a device error with opnum OP_ILLEGAL, since no
operation ever made it to the wire.  But servers can use the ff_ioerr4
opnum to distinguish failed read-class operations from write-class
operations when deciding how to recover the affected mirror -- a mirror
that may have missed writes needs to be brought back in sync, while one
that merely failed to serve a read does not.  OP_ILLEGAL gives the
server nothing to act on.

Every caller of nfs4_ff_layout_prepare_ds() knows which operation it
was preparing to send, and the existing fail_return argument already
divides the callers along the same boundary (false for READ, true for
WRITE and COMMIT).  Replace the boolean with the intended opnum, derive
the layout return decision from it, and report it in the tracked device
error instead of OP_ILLEGAL.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
Reviewed-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 fs/nfs/flexfilelayout/flexfilelayout.c    | 13 ++++++++-----
 fs/nfs/flexfilelayout/flexfilelayout.h    |  2 +-
 fs/nfs/flexfilelayout/flexfilelayoutdev.c | 14 ++++++++------
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 12b7be95a9c3..8b43d642969b 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -883,7 +883,7 @@ ff_layout_choose_ds_for_read(struct pnfs_layout_segment *lseg,
 		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);
+					       *dss_id, OP_READ);
 		if (IS_ERR(ds)) {
 			nfs4_ff_layout_put_deviceid(mirror_ds);
 			ret = ERR_CAST(ds);
@@ -1132,7 +1132,7 @@ ff_layout_pg_init_write(struct nfs_pageio_descriptor *pgio,
 		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,
-					       mirror_ds, dss_id, true);
+					       mirror_ds, dss_id, OP_WRITE);
 		if (IS_ERR(ds)) {
 			nfs4_ff_layout_put_deviceid(mirror_ds);
 			if (!ff_layout_no_fallback_to_mds(pgio->pg_lseg))
@@ -2188,7 +2188,8 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
 		mirror->dss_count,
 		offset);
 	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);
+	ds = nfs4_ff_layout_prepare_ds(lseg, mirror, mirror_ds, dss_id,
+				       OP_READ);
 	if (IS_ERR(ds)) {
 		ds_fatal_error = nfs_error_is_fatal(PTR_ERR(ds));
 		goto out_failed;
@@ -2288,7 +2289,8 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
 		mirror->dss_count,
 		offset);
 	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);
+	ds = nfs4_ff_layout_prepare_ds(lseg, mirror, mirror_ds, dss_id,
+				       OP_WRITE);
 	if (IS_ERR(ds)) {
 		ds_fatal_error = nfs_error_is_fatal(PTR_ERR(ds));
 		goto out_failed;
@@ -2398,7 +2400,8 @@ static int ff_layout_initiate_commit(struct nfs_commit_data *data, int how)
 	mirror = FF_LAYOUT_COMP(lseg, idx);
 	dss_id = calc_dss_id_from_commit(lseg, data->ds_commit_index);
 	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);
+	ds = nfs4_ff_layout_prepare_ds(lseg, mirror, mirror_ds, dss_id,
+				       OP_COMMIT);
 	if (IS_ERR(ds))
 		goto out_err;
 
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 72b11034851a..09c3cd6964fd 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -256,7 +256,7 @@ 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);
+			  enum nfs_opnum4 opnum);
 
 struct rpc_clnt *
 nfs4_ff_find_or_create_ds_client(const struct nfs4_ff_layout_ds *mirror_ds,
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index d52f485d0650..bc95628a811f 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -379,7 +379,7 @@ ff_layout_get_mirror_ds(struct pnfs_layout_hdr *lo,
  * @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?
+ * @opnum: operation this connection is being prepared for
  *
  * Try to prepare a DS connection to accept an RPC call. This involves
  * selecting a mirror to use and connecting the client to it if it's not
@@ -387,8 +387,10 @@ ff_layout_get_mirror_ds(struct pnfs_layout_hdr *lo,
  *
  * Since we only need a single functioning mirror to satisfy a read, we don't
  * want to return the layout if there is one. For writes though, any down
- * mirror should result in a LAYOUTRETURN. @fail_return is how we distinguish
- * between the two cases.
+ * mirror should result in a LAYOUTRETURN. @opnum is how we distinguish
+ * between the two cases. On failure, @opnum is also reported in the tracked
+ * device error so that the server can tell which class of I/O the client
+ * was unable to send to the mirror.
  *
  * Returns a pointer to a connected DS object on success or NULL on failure.
  */
@@ -397,7 +399,7 @@ 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)
+			  enum nfs_opnum4 opnum)
 {
 	struct nfs4_pnfs_ds *ds;
 	struct inode *ino = lseg->pls_layout->plh_inode;
@@ -448,9 +450,9 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg,
 					NULL : &mirror_ds->id_node,
 				 dss_id, lseg->pls_range.offset,
 				 lseg->pls_range.length, NFS4ERR_NXIO,
-				 OP_ILLEGAL, GFP_NOIO);
+				 opnum, GFP_NOIO);
 	ff_layout_send_layouterror(lseg);
-	if (fail_return || !ff_layout_has_available_ds(lseg))
+	if (opnum != OP_READ || !ff_layout_has_available_ds(lseg))
 		pnfs_error_mark_layout_for_return(ino, lseg);
 	ds = ERR_PTR(status);
 out:
-- 
2.53.0

[PATCH v2 2/3] pNFS: allow layout drivers to cancel I/O to a single device

[Benjamin Coddington]

The ->cancel_io() layout operation cancels all in-flight I/O for a
layout segment that is being returned.  When the return is triggered
by the failure of a single mirror instance, cancelling I/O to the
healthy mirrors is both unnecessary and harmful: requests that were
already transmitted cannot be un-sent, and will complete on the data
servers after the layout has been returned.

Give ->cancel_io() a device ID argument identifying the failed device,
and thread it through pnfs_error_mark_layout_for_return() and
pnfs_mark_matching_lsegs_return().  The flexfiles implementation
compares it against the raw device ID from the layout, so that it can
identify the failed mirror instance even when its device ID node was
never instantiated.

A NULL device ID preserves the existing cancel-everything behavior,
and all callers pass NULL for now, so this patch makes no change in
behavior.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
Reviewed-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 fs/nfs/blocklayout/blocklayout.c          |  6 +++--
 fs/nfs/callback_proc.c                    |  2 +-
 fs/nfs/filelayout/filelayout.c            |  4 +--
 fs/nfs/flexfilelayout/flexfilelayout.c    | 16 +++++++-----
 fs/nfs/flexfilelayout/flexfilelayoutdev.c |  2 +-
 fs/nfs/pnfs.c                             | 32 ++++++++++++++---------
 fs/nfs/pnfs.h                             | 14 ++++++----
 7 files changed, 46 insertions(+), 30 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index d54a141a89b3..d86702e604f9 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -859,7 +859,8 @@ bl_pg_init_read(struct nfs_pageio_descriptor *pgio, struct nfs_page *req)
 
 	if (pgio->pg_lseg &&
 		test_bit(NFS_LSEG_UNAVAILABLE, &pgio->pg_lseg->pls_flags)) {
-		pnfs_error_mark_layout_for_return(pgio->pg_inode, pgio->pg_lseg);
+		pnfs_error_mark_layout_for_return(pgio->pg_inode, pgio->pg_lseg,
+						  NULL);
 		pnfs_set_lo_fail(pgio->pg_lseg);
 		nfs_pageio_reset_read_mds(pgio);
 	}
@@ -921,7 +922,8 @@ bl_pg_init_write(struct nfs_pageio_descriptor *pgio, struct nfs_page *req)
 	if (pgio->pg_lseg &&
 		test_bit(NFS_LSEG_UNAVAILABLE, &pgio->pg_lseg->pls_flags)) {
 
-		pnfs_error_mark_layout_for_return(pgio->pg_inode, pgio->pg_lseg);
+		pnfs_error_mark_layout_for_return(pgio->pg_inode, pgio->pg_lseg,
+						  NULL);
 		pnfs_set_lo_fail(pgio->pg_lseg);
 		nfs_pageio_reset_write_mds(pgio);
 	}
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index c9b71dbae9ea..9d836cb72e78 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -292,7 +292,7 @@ static u32 initiate_file_draining(struct nfs_client *clp,
 	switch (pnfs_mark_matching_lsegs_return(lo, &free_me_list,
 				&args->cbl_range,
 				be32_to_cpu(args->cbl_stateid.seqid),
-				args->cbl_layoutchanged)) {
+				args->cbl_layoutchanged, NULL)) {
 	case 0:
 	case -EBUSY:
 		/* There are layout segments that need to be returned */
diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayout.c
index 0d53277f972e..d1c08d529e02 100644
--- a/fs/nfs/filelayout/filelayout.c
+++ b/fs/nfs/filelayout/filelayout.c
@@ -186,7 +186,7 @@ static int filelayout_async_handle_error(struct rpc_task *task,
 		dprintk("%s DS connection error %d\n", __func__,
 			task->tk_status);
 		nfs4_mark_deviceid_unavailable(devid);
-		pnfs_error_mark_layout_for_return(inode, lseg);
+		pnfs_error_mark_layout_for_return(inode, lseg, NULL);
 		pnfs_set_lo_fail(lseg);
 		rpc_wake_up(&tbl->slot_tbl_waitq);
 		fallthrough;
@@ -856,7 +856,7 @@ fl_pnfs_update_layout(struct inode *ino,
 
 	status = filelayout_check_deviceid(lo, fl, gfp_flags);
 	if (status) {
-		pnfs_error_mark_layout_for_return(ino, lseg);
+		pnfs_error_mark_layout_for_return(ino, lseg, NULL);
 		pnfs_set_lo_fail(lseg);
 		pnfs_put_lseg(lseg);
 		lseg = NULL;
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 8b43d642969b..fb7222a04559 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -1281,7 +1281,7 @@ static void ff_layout_resend_pnfs_read(struct nfs_pgio_header *hdr)
 	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);
+		pnfs_error_mark_layout_for_return(hdr->inode, hdr->lseg, NULL);
 	} else {
 		nfs4_ff_layout_put_deviceid(mirror_ds);
 		ff_layout_send_layouterror(hdr->lseg);
@@ -1294,7 +1294,7 @@ static void ff_layout_reset_read(struct nfs_pgio_header *hdr)
 	struct rpc_task *task = &hdr->task;
 
 	pnfs_layoutcommit_inode(hdr->inode, false);
-	pnfs_error_mark_layout_for_return(hdr->inode, hdr->lseg);
+	pnfs_error_mark_layout_for_return(hdr->inode, hdr->lseg, NULL);
 
 	if (!test_and_set_bit(NFS_IOHDR_REDO, &hdr->flags)) {
 		dprintk("%s Reset task %5u for i/o through MDS "
@@ -1594,7 +1594,7 @@ static void ff_layout_io_track_ds_error(struct pnfs_layout_segment *lseg,
 		fallthrough;
 	default:
 		pnfs_error_mark_layout_for_return(lseg->pls_layout->plh_inode,
-						  lseg);
+						  lseg, NULL);
 	}
 
 out:
@@ -2256,7 +2256,7 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
 		 * FF_FLAGS_NO_IO_THRU_MDS: force fresh LAYOUTGET,
 		 * never fall through to MDS I/O.
 		 */
-		pnfs_error_mark_layout_for_return(hdr->inode, lseg);
+		pnfs_error_mark_layout_for_return(hdr->inode, lseg, NULL);
 		return PNFS_TRY_AGAIN;
 	}
 	trace_pnfs_mds_fallback_read_pagelist(hdr->inode,
@@ -2359,7 +2359,7 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
 		 * FF_FLAGS_NO_IO_THRU_MDS: force fresh LAYOUTGET,
 		 * never fall through to MDS I/O.
 		 */
-		pnfs_error_mark_layout_for_return(hdr->inode, lseg);
+		pnfs_error_mark_layout_for_return(hdr->inode, lseg, NULL);
 		return PNFS_TRY_AGAIN;
 	}
 	trace_pnfs_mds_fallback_write_pagelist(hdr->inode,
@@ -2490,7 +2490,8 @@ static bool ff_layout_match_io(const struct rpc_task *task, const void *data)
 	return false;
 }
 
-static void ff_layout_cancel_io(struct pnfs_layout_segment *lseg)
+static void ff_layout_cancel_io(struct pnfs_layout_segment *lseg,
+				const struct nfs4_deviceid *devid)
 {
 	struct nfs4_ff_layout_segment *flseg = FF_LAYOUT_LSEG(lseg);
 	struct nfs4_ff_layout_mirror *mirror;
@@ -2503,6 +2504,9 @@ 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++) {
+			if (devid && memcmp(&mirror->dss[dss_id].devid, devid,
+					    sizeof(*devid)) != 0)
+				continue;
 			rcu_read_lock();
 			mirror_ds = rcu_dereference(mirror->dss[dss_id].mirror_ds);
 			if (IS_ERR_OR_NULL(mirror_ds) ||
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index bc95628a811f..7bb0f2094e9d 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -453,7 +453,7 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg,
 				 opnum, GFP_NOIO);
 	ff_layout_send_layouterror(lseg);
 	if (opnum != OP_READ || !ff_layout_has_available_ds(lseg))
-		pnfs_error_mark_layout_for_return(ino, lseg);
+		pnfs_error_mark_layout_for_return(ino, lseg, NULL);
 	ds = ERR_PTR(status);
 out:
 	return ds;
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index e577001b78f2..937c09c4c6f9 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -433,7 +433,7 @@ bool nfs4_layout_refresh_old_stateid(nfs4_stateid *dst,
 		}
 		/* Try to update the seqid to the most recent */
 		err = pnfs_mark_matching_lsegs_return(lo, &head, &range, 0,
-						      true);
+						      true, NULL);
 		if (err != -EBUSY) {
 			dst->seqid = lo->plh_stateid.seqid;
 			*dst_range = range;
@@ -487,7 +487,8 @@ static int pnfs_mark_layout_stateid_return(struct pnfs_layout_hdr *lo,
 		.length = NFS4_MAX_UINT64,
 	};
 
-	return pnfs_mark_matching_lsegs_return(lo, lseg_list, &range, seq, true);
+	return pnfs_mark_matching_lsegs_return(lo, lseg_list, &range, seq, true,
+					       NULL);
 }
 
 static int
@@ -525,7 +526,7 @@ pnfs_layout_io_set_failed(struct pnfs_layout_hdr *lo, u32 iomode)
 
 	spin_lock(&inode->i_lock);
 	pnfs_layout_set_fail_bit(lo, pnfs_iomode_to_fail_bit(iomode));
-	pnfs_mark_matching_lsegs_return(lo, &head, &range, 0, true);
+	pnfs_mark_matching_lsegs_return(lo, &head, &range, 0, true, NULL);
 	spin_unlock(&inode->i_lock);
 	pnfs_free_lseg_list(&head);
 	dprintk("%s Setting layout IOMODE_%s fail bit\n", __func__,
@@ -740,7 +741,7 @@ pnfs_mark_matching_lsegs_invalid(struct pnfs_layout_hdr *lo,
 			if (mark_lseg_invalid(lseg, tmp_list))
 				continue;
 			remaining++;
-			pnfs_lseg_cancel_io(server, lseg);
+			pnfs_lseg_cancel_io(server, lseg, NULL);
 		}
 	dprintk("%s:Return %i\n", __func__, remaining);
 	return remaining;
@@ -1462,7 +1463,7 @@ _pnfs_return_layout(struct inode *ino)
 	}
 	valid_layout = pnfs_layout_is_valid(lo);
 	pnfs_clear_layoutcommit(ino, &tmp_list);
-	pnfs_mark_matching_lsegs_return(lo, &tmp_list, &range, 0, true);
+	pnfs_mark_matching_lsegs_return(lo, &tmp_list, &range, 0, true, NULL);
 
 
 	/* Don't send a LAYOUTRETURN if list was initially empty */
@@ -2616,7 +2617,8 @@ pnfs_layout_process(struct nfs4_layoutget *lgp)
 			.iomode = IOMODE_ANY,
 			.length = NFS4_MAX_UINT64,
 		};
-		pnfs_mark_matching_lsegs_return(lo, &free_me, &range, 0, true);
+		pnfs_mark_matching_lsegs_return(lo, &free_me, &range, 0, true,
+						NULL);
 		goto out_forget;
 	} else {
 		/* We have a completely new layout */
@@ -2649,6 +2651,7 @@ pnfs_layout_process(struct nfs4_layoutget *lgp)
  * @return_range: describe layout segment ranges to be returned
  * @seq: stateid seqid to match
  * @cancel_io: signal io be cancelled
+ * @devid: only cancel io directed at this device (all devices if NULL)
  *
  * This function is mainly intended for use by layoutrecall. It attempts
  * to free the layout segment immediately, or else to mark it for return
@@ -2663,7 +2666,8 @@ int
 pnfs_mark_matching_lsegs_return(struct pnfs_layout_hdr *lo,
 				struct list_head *tmp_list,
 				const struct pnfs_layout_range *return_range,
-				u32 seq, bool cancel_io)
+				u32 seq, bool cancel_io,
+				const struct nfs4_deviceid *devid)
 {
 	struct pnfs_layout_segment *lseg, *next;
 	struct nfs_server *server = NFS_SERVER(lo->plh_inode);
@@ -2690,7 +2694,7 @@ pnfs_mark_matching_lsegs_return(struct pnfs_layout_hdr *lo,
 			remaining++;
 			set_bit(NFS_LSEG_LAYOUTRETURN, &lseg->pls_flags);
 			if (cancel_io)
-				pnfs_lseg_cancel_io(server, lseg);
+				pnfs_lseg_cancel_io(server, lseg, devid);
 		}
 
 	if (remaining) {
@@ -2708,7 +2712,8 @@ pnfs_mark_matching_lsegs_return(struct pnfs_layout_hdr *lo,
 
 static void
 pnfs_mark_layout_for_return(struct inode *inode,
-			    const struct pnfs_layout_range *range)
+			    const struct pnfs_layout_range *range,
+			    const struct nfs4_deviceid *devid)
 {
 	struct pnfs_layout_hdr *lo;
 	bool return_now = false;
@@ -2726,7 +2731,7 @@ pnfs_mark_layout_for_return(struct inode *inode,
 	 * for how it works.
 	 */
 	if (pnfs_mark_matching_lsegs_return(lo, &lo->plh_return_segs, range, 0,
-					    true) != -EBUSY) {
+					    true, devid) != -EBUSY) {
 		const struct cred *cred;
 		nfs4_stateid stateid;
 		enum pnfs_iomode iomode;
@@ -2743,7 +2748,8 @@ pnfs_mark_layout_for_return(struct inode *inode,
 }
 
 void pnfs_error_mark_layout_for_return(struct inode *inode,
-				       struct pnfs_layout_segment *lseg)
+				       struct pnfs_layout_segment *lseg,
+				       const struct nfs4_deviceid *devid)
 {
 	struct pnfs_layout_range range = {
 		.iomode = lseg->pls_range.iomode,
@@ -2751,7 +2757,7 @@ void pnfs_error_mark_layout_for_return(struct inode *inode,
 		.length = NFS4_MAX_UINT64,
 	};
 
-	pnfs_mark_layout_for_return(inode, &range);
+	pnfs_mark_layout_for_return(inode, &range, devid);
 }
 EXPORT_SYMBOL_GPL(pnfs_error_mark_layout_for_return);
 
@@ -2841,7 +2847,7 @@ static int pnfs_layout_return_unused_byserver(struct nfs_server *server,
 		pnfs_get_layout_hdr(lo);
 		pnfs_set_plh_return_info(lo, range->iomode, 0);
 		if (pnfs_mark_matching_lsegs_return(lo, &lo->plh_return_segs,
-						    range, 0, true) != 0 ||
+						    range, 0, true, NULL) != 0 ||
 		    !pnfs_prepare_layoutreturn(lo, &stateid, &cred, &iomode)) {
 			spin_unlock(&inode->i_lock);
 			rcu_read_unlock();
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 719657f3f4e1..9136ba64da94 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -197,7 +197,8 @@ struct pnfs_layoutdriver_type {
 	int (*prepare_layoutcommit) (struct nfs4_layoutcommit_args *args);
 	int (*prepare_layoutstats) (struct nfs42_layoutstat_args *args);
 
-	void (*cancel_io)(struct pnfs_layout_segment *lseg);
+	void (*cancel_io)(struct pnfs_layout_segment *lseg,
+			  const struct nfs4_deviceid *devid);
 };
 
 struct pnfs_commit_ops {
@@ -320,7 +321,8 @@ int pnfs_mark_matching_lsegs_invalid(struct pnfs_layout_hdr *lo,
 int pnfs_mark_matching_lsegs_return(struct pnfs_layout_hdr *lo,
 				struct list_head *tmp_list,
 				const struct pnfs_layout_range *recall_range,
-				u32 seq, bool cancel_io);
+				u32 seq, bool cancel_io,
+				const struct nfs4_deviceid *devid);
 int pnfs_mark_layout_stateid_invalid(struct pnfs_layout_hdr *lo,
 		struct list_head *lseg_list);
 bool pnfs_roc(struct inode *ino, struct nfs4_layoutreturn_args *args,
@@ -370,7 +372,8 @@ int pnfs_read_done_resend_to_mds(struct nfs_pgio_header *);
 int pnfs_write_done_resend_to_mds(struct nfs_pgio_header *);
 struct nfs4_threshold *pnfs_mdsthreshold_alloc(void);
 void pnfs_error_mark_layout_for_return(struct inode *inode,
-				       struct pnfs_layout_segment *lseg);
+				       struct pnfs_layout_segment *lseg,
+				       const struct nfs4_deviceid *devid);
 void pnfs_layout_return_unused_byclid(struct nfs_client *clp,
 				      enum pnfs_iomode iomode);
 void pnfs_layout_reresolve_deviceid_byclid(struct nfs_client *clp,
@@ -774,10 +777,11 @@ pnfs_lseg_request_intersecting(struct pnfs_layout_segment *lseg, struct nfs_page
 }
 
 static inline void pnfs_lseg_cancel_io(struct nfs_server *server,
-				       struct pnfs_layout_segment *lseg)
+				       struct pnfs_layout_segment *lseg,
+				       const struct nfs4_deviceid *devid)
 {
 	if (server->pnfs_curr_ld->cancel_io)
-		server->pnfs_curr_ld->cancel_io(lseg);
+		server->pnfs_curr_ld->cancel_io(lseg, devid);
 }
 
 extern unsigned int layoutstats_timer;
-- 
2.53.0

[PATCH v2 3/3] NFSv4/flexfiles: only cancel I/O to a failed mirror instance

[Benjamin Coddington]

When an error causes the flexfiles driver to return a layout,
ff_layout_cancel_io() kills every in-flight RPC for the layout
segment, across all mirror instances.  Cancelled requests that had
already been transmitted to a healthy data server cannot be un-sent:
they complete on the data server after the client has sent its
LAYOUTRETURN, and the metadata server then observes writes to a file
for which no write layout is outstanding.

RFC 8881 Section 20.3.4 recommends that the client wait for the
response from in-process or in-flight READ, WRITE, or COMMIT
operations before returning the layout, and the machinery for that
wait already exists: the LAYOUTRETURN is deferred until every request
drops its layout segment reference, and requests that have not yet
been transmitted exit at RPC prepare time once the segment has been
invalidated.  Cancellation is only needed to avoid waiting forever on
a device that will never answer.

Pass the failed instance's device ID when marking the layout for
return, so that ff_layout_cancel_io() cancels only I/O directed at
the device we have given up on.  In-flight I/O to the remaining
healthy instances drains normally -- typically within a round trip --
before the LAYOUTRETURN is sent.  If a spared instance turns out to
be unresponsive, its requests fail with their own device error, and
the resulting layout return cancels its I/O in turn.

Layout recalls with clora_changed set, bulk returns, and layout
revocations continue to cancel I/O to every device, as do error
paths where no single failed device can be identified.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
Reviewed-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 fs/nfs/flexfilelayout/flexfilelayout.c    | 9 ++++++---
 fs/nfs/flexfilelayout/flexfilelayoutdev.c | 3 ++-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index fb7222a04559..3e51e9640819 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -1594,7 +1594,8 @@ static void ff_layout_io_track_ds_error(struct pnfs_layout_segment *lseg,
 		fallthrough;
 	default:
 		pnfs_error_mark_layout_for_return(lseg->pls_layout->plh_inode,
-						  lseg, NULL);
+						  lseg,
+						  &mirror->dss[dss_id].devid);
 	}
 
 out:
@@ -2256,7 +2257,8 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
 		 * FF_FLAGS_NO_IO_THRU_MDS: force fresh LAYOUTGET,
 		 * never fall through to MDS I/O.
 		 */
-		pnfs_error_mark_layout_for_return(hdr->inode, lseg, NULL);
+		pnfs_error_mark_layout_for_return(hdr->inode, lseg,
+						  &mirror->dss[dss_id].devid);
 		return PNFS_TRY_AGAIN;
 	}
 	trace_pnfs_mds_fallback_read_pagelist(hdr->inode,
@@ -2359,7 +2361,8 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
 		 * FF_FLAGS_NO_IO_THRU_MDS: force fresh LAYOUTGET,
 		 * never fall through to MDS I/O.
 		 */
-		pnfs_error_mark_layout_for_return(hdr->inode, lseg, NULL);
+		pnfs_error_mark_layout_for_return(hdr->inode, lseg,
+						  &mirror->dss[dss_id].devid);
 		return PNFS_TRY_AGAIN;
 	}
 	trace_pnfs_mds_fallback_write_pagelist(hdr->inode,
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index 7bb0f2094e9d..6165c41fcf62 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -453,7 +453,8 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg,
 				 opnum, GFP_NOIO);
 	ff_layout_send_layouterror(lseg);
 	if (opnum != OP_READ || !ff_layout_has_available_ds(lseg))
-		pnfs_error_mark_layout_for_return(ino, lseg, NULL);
+		pnfs_error_mark_layout_for_return(ino, lseg,
+						  &mirror->dss[dss_id].devid);
 	ds = ERR_PTR(status);
 out:
 	return ds;
-- 
2.53.0