* [PATCH v2 1/2] pnfs/blocklayout: don't mark extents committed after a failed LAYOUTCOMMIT
2026-10-26 23:27 [PATCH v2 0/2] pnfs/blocklayout: report LAYOUTCOMMIT failures Benjamin Coddington
@ 2026-10-26 23:27 ` Benjamin Coddington
2026-10-28 16:18 ` Christoph Hellwig
2026-10-26 23:27 ` [PATCH v2 2/2] pnfs/blocklayout: report data lost to " Benjamin Coddington
1 sibling, 1 reply; 5+ messages in thread
From: Benjamin Coddington @ 2026-10-26 23:27 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker; +Cc: linux-nfs, Daejun Park, Christoph Hellwig
From: Benjamin Coddington <bcodding@hammerspace.com>
bl_cleanup_layoutcommit() decides whether the server converted the
extents from res.status, which only decode_layoutcommit() sets. When the
compound fails before the LAYOUTCOMMIT op is decoded, as with a soft
timeout or an error on PUTFH, res.status is still zero and
ext_tree_mark_committed() turns the extents into
PNFS_BLOCK_READWRITE_DATA, although the server may not have converted
them.
Fall back to the task status when res.status is zero.
Fixes: b2be7811dd94 ("pnfsblock: cleanup_layoutcommit")
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/blocklayout/blocklayout.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index d54a141a89b3..aafcb8e30057 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -794,7 +794,8 @@ bl_prepare_layoutcommit(struct nfs4_layoutcommit_args *arg)
static void
bl_cleanup_layoutcommit(struct nfs4_layoutcommit_data *lcdata)
{
- ext_tree_mark_committed(&lcdata->args, lcdata->res.status);
+ ext_tree_mark_committed(&lcdata->args,
+ lcdata->res.status ?: lcdata->task.tk_status);
}
static int
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 2/2] pnfs/blocklayout: report data lost to a failed LAYOUTCOMMIT
2026-10-26 23:27 [PATCH v2 0/2] pnfs/blocklayout: report LAYOUTCOMMIT failures Benjamin Coddington
2026-10-26 23:27 ` [PATCH v2 1/2] pnfs/blocklayout: don't mark extents committed after a failed LAYOUTCOMMIT Benjamin Coddington
@ 2026-10-26 23:27 ` Benjamin Coddington
2026-10-28 16:18 ` Christoph Hellwig
1 sibling, 1 reply; 5+ messages in thread
From: Benjamin Coddington @ 2026-10-26 23:27 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker; +Cc: linux-nfs, Daejun Park, Christoph Hellwig
From: Benjamin Coddington <bcodding@hammerspace.com>
Data written through a block or SCSI layout into PNFS_BLOCK_INVALID_DATA
extents stays unwritten on the server until a LAYOUTCOMMIT converts
those extents. When the LAYOUTCOMMIT fails, ext_tree_mark_committed()
tags the extents written again so that a later LAYOUTCOMMIT can cover
them, and reports nothing. nfs4_layoutcommit_done() also zeroes the
task status for NFS4ERR_BADLAYOUT, NFS4ERR_BADIOMODE,
NFS4ERR_DELEG_REVOKED and NFS4ERR_GRACE, so a synchronous LAYOUTCOMMIT
returns 0 for those. A server that has revoked the layout answers
NFS4ERR_BADLAYOUT; once the client drops that layout its extent tree
goes with it, and fsync() and close() have returned 0 for data that
reads back as zeroes.
When ext_tree_mark_committed() puts extents back, record -EIO in the
mapping's writeback error and the superblock's, and have the block and
SCSI ->sync return an error recorded during its LAYOUTCOMMIT. ->sync
also runs from inode revalidation, so compare against the raw errseq
value rather than a sample, which would return an older unseen error
from every stat(). The block and SCSI layouts are the only users of
pnfs_generic_sync(), so replace it with bl_sync() in the block layout
driver. The files and flexfiles layouts, whose LAYOUTCOMMIT does not
decide what a read of the file returns, are unchanged.
Fixes: b2be7811dd94 ("pnfsblock: cleanup_layoutcommit")
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
---
fs/nfs/blocklayout/blocklayout.c | 17 +++++++++++++++--
fs/nfs/blocklayout/extent_tree.c | 14 +++++++++++---
fs/nfs/pnfs.c | 7 -------
fs/nfs/pnfs.h | 1 -
4 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index aafcb8e30057..b41fc7a6f65c 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -798,6 +798,19 @@ bl_cleanup_layoutcommit(struct nfs4_layoutcommit_data *lcdata)
lcdata->res.status ?: lcdata->task.tk_status);
}
+static int
+bl_sync(struct inode *inode, bool datasync)
+{
+ /* An error nobody has seen yet is not this LAYOUTCOMMIT's to return */
+ errseq_t since = READ_ONCE(inode->i_mapping->wb_err);
+ int ret;
+
+ ret = pnfs_layoutcommit_inode(inode, true);
+ if (ret)
+ return ret;
+ return filemap_check_wb_err(inode->i_mapping, since);
+}
+
static int
bl_set_layoutdriver(struct nfs_server *server, const struct nfs_fh *fh)
{
@@ -976,7 +989,7 @@ static struct pnfs_layoutdriver_type blocklayout_type = {
.free_deviceid_node = bl_free_deviceid_node,
.pg_read_ops = &bl_pg_read_ops,
.pg_write_ops = &bl_pg_write_ops,
- .sync = pnfs_generic_sync,
+ .sync = bl_sync,
};
static struct pnfs_layoutdriver_type scsilayout_type = {
@@ -1000,7 +1013,7 @@ static struct pnfs_layoutdriver_type scsilayout_type = {
.free_deviceid_node = bl_free_deviceid_node,
.pg_read_ops = &bl_pg_read_ops,
.pg_write_ops = &bl_pg_write_ops,
- .sync = pnfs_generic_sync,
+ .sync = bl_sync,
};
diff --git a/fs/nfs/blocklayout/extent_tree.c b/fs/nfs/blocklayout/extent_tree.c
index 7393118c42a2..e408dba57401 100644
--- a/fs/nfs/blocklayout/extent_tree.c
+++ b/fs/nfs/blocklayout/extent_tree.c
@@ -696,6 +696,7 @@ ext_tree_mark_committed(struct nfs4_layoutcommit_args *arg, int status)
struct pnfs_block_layout *bl = BLK_LO2EXT(NFS_I(arg->inode)->layout);
struct rb_root *root = &bl->bl_ext_rw;
struct pnfs_block_extent *be;
+ bool uncommitted = false;
dprintk("%s status %d\n", __func__, status);
@@ -709,11 +710,13 @@ ext_tree_mark_committed(struct nfs4_layoutcommit_args *arg, int status)
if (status) {
/*
- * Mark as written and try again.
- *
- * XXX: some real error handling here wouldn't hurt..
+ * The server may not have converted these blocks, so
+ * what we wrote to them may not be part of the file.
+ * Keep them for a later LAYOUTCOMMIT, but tell fsync
+ * and close.
*/
be->be_tag = EXTENT_WRITTEN;
+ uncommitted = true;
} else {
be->be_state = PNFS_BLOCK_READWRITE_DATA;
be->be_tag = 0;
@@ -723,4 +726,9 @@ ext_tree_mark_committed(struct nfs4_layoutcommit_args *arg, int status)
be = ext_try_to_merge_right(root, be);
}
spin_unlock(&bl->bl_ext_lock);
+
+ if (uncommitted) {
+ filemap_set_wb_err(arg->inode->i_mapping, -EIO);
+ errseq_set(&arg->inode->i_sb->s_wb_err, -EIO);
+ }
}
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 4f9c0f639014..99e33a69c087 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -3441,13 +3441,6 @@ pnfs_layoutcommit_inode(struct inode *inode, bool sync)
}
EXPORT_SYMBOL_GPL(pnfs_layoutcommit_inode);
-int
-pnfs_generic_sync(struct inode *inode, bool datasync)
-{
- return pnfs_layoutcommit_inode(inode, true);
-}
-EXPORT_SYMBOL_GPL(pnfs_generic_sync);
-
struct nfs4_threshold *pnfs_mdsthreshold_alloc(void)
{
struct nfs4_threshold *thp;
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 70d20f779678..85b2ba557b11 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -316,7 +316,6 @@ bool pnfs_wait_on_layoutreturn(struct inode *ino, struct rpc_task *task);
void pnfs_set_layoutcommit(struct inode *, struct pnfs_layout_segment *, loff_t);
void pnfs_cleanup_layoutcommit(struct nfs4_layoutcommit_data *data);
int pnfs_layoutcommit_inode(struct inode *inode, bool sync);
-int pnfs_generic_sync(struct inode *inode, bool datasync);
int pnfs_nfs_generic_sync(struct inode *inode, bool datasync);
int _pnfs_return_layout(struct inode *);
int pnfs_commit_and_return_layout(struct inode *);
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread