[PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O

[PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O

[Mike Snitzer]

While qualifying NFSD's NFSD_IO_DIRECT write path with byte-level data
verification, it was found that the ITER_BVEC payloads nfsd submits 
expose silent data corruption in two bio-based block drivers and
two defects in nfsd itself. Example problematic payloads is the first
fragment starts mid-page because the RPC header precedes it in the
receive buffer, and fragment lengths need not be sector multiples.
bio_iov_bvec_set() passes such an array to the queue as-is; nothing
below it validates per-bvec sector alignment.

Patches 1-2 fix silent corruption (write completes successfully, data
lands wrong) and are stable candidates:

- brd re-derives each segment's device position from
  bio->bi_iter.bi_sector, which bio_advance_iter_single() advances by
  whole sectors only, so a sub-sector segment length skews everything
  that follows.

- zram hardwires is_partial_io() to false on 4K-page kernels, sending
  sub-page bvecs down a whole-page path that ignores bv_offset/bv_len
  entirely, and has the same sector-cursor skew.

Both are verified with a synthetic-bio reproducer (stamped pattern,
write, read back, compare) across mid-page and page-aligned
geometries.

Patches 3-4 fix nfsd: the filecache never fetches DIO alignment
attributes on the supplied-file acquire branch, so every WRITE to a
file created via NFSv4 OPEN(CREATE) is refused direct I/O for the
file's cached lifetime; and nfsd's statx-based DIO gate is weaker than
bio_split_io_at()'s split-time checks, so an admitted iterator can
still be rejected by the block layer -- retry the segment buffered
instead of failing a valid WRITE with NFS4ERR_INVAL.

One open question for the iomap/block maintainers: should ITER_BVEC
direct I/O with sub-sector bvec boundaries be validated or bounced
centrally rather than trusted to every driver's iteration?  An audit
of in-tree bio-based drivers found the same bi_sector-derived position
pattern in dm-io, dm-log-writes, dm-writecache (pmem path) and
dm-integrity -- unreachable through nfsd today only because dm queues
advertise dma_alignment >= 511, which nfsd's alignment gate refuses.

Tested with the reproducer matrix on brd, zram and nvme-loop at 4K and
16K page size (aarch64) and 4K (x86_64), plus 30-connection NFS write
rigs comparing source against export byte-for-byte: clean with the
fixes, corrupting or erroring without them.

Mike Snitzer (3):
  brd: iterate the bio by byte position, not bi_sector
  zram: handle sub-page bvec segments without corrupting data
  nfsd: fall back to buffered I/O when a direct write gets -EINVAL

David Flynn (1):
  nfsd: fetch direct I/O alignment for files handed to the filecache

 drivers/block/brd.c           | 29 +++++++++++++++++++++++------
 drivers/block/zram/zram_drv.c | 36 +++++++++++++++++------------------
 fs/nfsd/filecache.c           |  4 ++--
 fs/nfsd/vfs.c                 | 29 +++++++++++++++++++++++++++++
 4 files changed, 77 insertions(+), 31 deletions(-)

--
2.52.0

[PATCH 1/4] brd: iterate the bio by byte position, not bi_sector

[Mike Snitzer]

brd_rw_bvec() takes the device position from bio->bi_iter.bi_sector,
which bio_advance_iter_single() advances by bytes >> SECTOR_SHIFT. For
a bvec whose length is not a multiple of the sector size the sector
cursor silently loses the sub-sector residue while the data cursor
(bi_bvec_done/bi_size) consumes the full length -- from that segment
on, data is written at a device offset short of where it belongs, and
every subsequent byte lands shifted with no error reported anywhere.

Such bvec geometry is legal at the submitter: ITER_BVEC direct I/O
passes the caller's bio_vec array through as-is (bio_iov_bvec_set()),
so e.g. NFSD's NFSD_IO_DIRECT write path hands XFS/iomap a payload
whose first fragment starts mid-page (the RPC header precedes it in
the receive buffer) and whose fragment lengths are not sector
multiples. A 1 MiB write arriving as bv0=(160,16224) + 63x(0,16384) +
(0,160) reproduces on brd as: first 15872 = ALIGN_DOWN(16224, 512)
bytes correct, everything after shifted forward by 352 = 16224 - 15872
bytes -- while the write completes successfully. Any NFSD_IO_DIRECT
(or other kernel bvec direct I/O) write to a brd-backed filesystem is
exposed; request-based drivers are unaffected because nothing in the
request path does per-bvec sector arithmetic.

Track the device position as a byte offset owned by the submit loop
and advanced by the number of bytes each segment actually processed,
instead of re-deriving it from the skewed bi_sector. Verified with a
synthetic-bio reproducer over brd directly and through nvme-loop:
mid-page-offset geometries and the page-aligned control now all read
back byte-identical, and 20 fresh NFS connections x 16 MiB of O_DIRECT
writes over an XFS-on-nvme-loop-on-brd export complete with zero data
mismatches (previously most connections corrupted).

Fixes: 3185444f0504 ("brd: split I/O at page boundaries")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 drivers/block/brd.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 00cc8122068f..4011538cecaf 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -134,12 +134,24 @@ static void brd_free_pages(struct brd_device *brd)
 /*
  * Process a single segment.  The segment is capped to not cross page boundaries
  * in both the bio and the brd backing memory.
+ *
+ * The device position is @pos, a byte offset maintained by the caller --
+ * not bio->bi_iter.bi_sector: bio_advance_iter_single() advances bi_sector
+ * by bytes >> SECTOR_SHIFT, so a bvec whose length is not a multiple of the
+ * sector size silently skews bi_sector against the bytes actually consumed
+ * and corrupts everything that follows.  Byte-granular bvec boundaries
+ * reach us from ITER_BVEC direct I/O submitters whose caller's bio_vec
+ * array is passed through as-is (bio_iov_bvec_set()).
+ *
+ * Returns the number of bytes processed, or 0 on error (the bio has then
+ * been completed).
  */
-static bool brd_rw_bvec(struct brd_device *brd, struct bio *bio)
+static unsigned int brd_rw_bvec(struct brd_device *brd, struct bio *bio,
+				loff_t pos)
 {
 	struct bio_vec bv = bio_iter_iovec(bio, bio->bi_iter);
-	sector_t sector = bio->bi_iter.bi_sector;
-	u32 offset = (sector & (PAGE_SECTORS - 1)) << SECTOR_SHIFT;
+	sector_t sector = pos >> SECTOR_SHIFT;
+	u32 offset = pos & (PAGE_SIZE - 1);
 	blk_opf_t opf = bio->bi_opf;
 	struct page *page;
 	void *kaddr;
@@ -167,14 +179,14 @@ static bool brd_rw_bvec(struct brd_device *brd, struct bio *bio)
 	bio_advance_iter_single(bio, &bio->bi_iter, bv.bv_len);
 	if (page)
 		put_page(page);
-	return true;
+	return bv.bv_len;
 
 out_error:
 	if (PTR_ERR(page) == -ENOMEM && (opf & REQ_NOWAIT))
 		bio_wouldblock_error(bio);
 	else
 		bio_io_error(bio);
-	return false;
+	return 0;
 }
 
 static void brd_do_discard(struct brd_device *brd, sector_t sector, u32 size)
@@ -202,6 +214,7 @@ static void brd_do_discard(struct brd_device *brd, sector_t sector, u32 size)
 static void brd_submit_bio(struct bio *bio)
 {
 	struct brd_device *brd = bio->bi_bdev->bd_disk->private_data;
+	loff_t pos;
 
 	if (unlikely(op_is_discard(bio->bi_opf))) {
 		brd_do_discard(brd, bio->bi_iter.bi_sector,
@@ -210,9 +223,13 @@ static void brd_submit_bio(struct bio *bio)
 		return;
 	}
 
+	pos = (loff_t)bio->bi_iter.bi_sector << SECTOR_SHIFT;
 	do {
-		if (!brd_rw_bvec(brd, bio))
+		unsigned int len = brd_rw_bvec(brd, bio, pos);
+
+		if (!len)
 			return;
+		pos += len;
 	} while (bio->bi_iter.bi_size);
 
 	bio_endio(bio);
-- 
2.52.0

[PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O

[Mike Snitzer]

While qualifying NFSD's NFSD_IO_DIRECT write path with byte-level data
verification, it was found that the ITER_BVEC payloads nfsd submits 
expose silent data corruption in two bio-based block drivers and
two defects in nfsd itself. Example problematic payloads is the first
fragment starts mid-page because the RPC header precedes it in the
receive buffer, and fragment lengths need not be sector multiples.
bio_iov_bvec_set() passes such an array to the queue as-is; nothing
below it validates per-bvec sector alignment.

Patches 1-2 fix silent corruption (write completes successfully, data
lands wrong) and are stable candidates:

- brd re-derives each segment's device position from
  bio->bi_iter.bi_sector, which bio_advance_iter_single() advances by
  whole sectors only, so a sub-sector segment length skews everything
  that follows.

- zram hardwires is_partial_io() to false on 4K-page kernels, sending
  sub-page bvecs down a whole-page path that ignores bv_offset/bv_len
  entirely, and has the same sector-cursor skew.

Both are verified with a synthetic-bio reproducer (stamped pattern,
write, read back, compare) across mid-page and page-aligned
geometries.

Patches 3-4 fix nfsd: the filecache never fetches DIO alignment
attributes on the supplied-file acquire branch, so every WRITE to a
file created via NFSv4 OPEN(CREATE) is refused direct I/O for the
file's cached lifetime; and nfsd's statx-based DIO gate is weaker than
bio_split_io_at()'s split-time checks, so an admitted iterator can
still be rejected by the block layer -- retry the segment buffered
instead of failing a valid WRITE with NFS4ERR_INVAL.

One open question for the iomap/block maintainers: should ITER_BVEC
direct I/O with sub-sector bvec boundaries be validated or bounced
centrally rather than trusted to every driver's iteration?  An audit
of in-tree bio-based drivers found the same bi_sector-derived position
pattern in dm-io, dm-log-writes, dm-writecache (pmem path) and
dm-integrity -- unreachable through nfsd today only because dm queues
advertise dma_alignment >= 511, which nfsd's alignment gate refuses.

Tested with the reproducer matrix on brd, zram and nvme-loop at 4K and
16K page size (aarch64) and 4K (x86_64), plus 30-connection NFS write
rigs comparing source against export byte-for-byte: clean with the
fixes, corrupting or erroring without them.

Mike Snitzer (3):
  brd: iterate the bio by byte position, not bi_sector
  zram: handle sub-page bvec segments without corrupting data
  nfsd: fall back to buffered I/O when a direct write gets -EINVAL

David Flynn (1):
  nfsd: fetch direct I/O alignment for files handed to the filecache

 drivers/block/brd.c           | 29 +++++++++++++++++++++++------
 drivers/block/zram/zram_drv.c | 36 +++++++++++++++++------------------
 fs/nfsd/filecache.c           |  4 ++--
 fs/nfsd/vfs.c                 | 29 +++++++++++++++++++++++++++++
 4 files changed, 77 insertions(+), 31 deletions(-)

--
2.52.0

[PATCH 1/4] brd: iterate the bio by byte position, not bi_sector

[Mike Snitzer]

brd_rw_bvec() takes the device position from bio->bi_iter.bi_sector,
which bio_advance_iter_single() advances by bytes >> SECTOR_SHIFT. For
a bvec whose length is not a multiple of the sector size the sector
cursor silently loses the sub-sector residue while the data cursor
(bi_bvec_done/bi_size) consumes the full length -- from that segment
on, data is written at a device offset short of where it belongs, and
every subsequent byte lands shifted with no error reported anywhere.

Such bvec geometry is legal at the submitter: ITER_BVEC direct I/O
passes the caller's bio_vec array through as-is (bio_iov_bvec_set()),
so e.g. NFSD's NFSD_IO_DIRECT write path hands XFS/iomap a payload
whose first fragment starts mid-page (the RPC header precedes it in
the receive buffer) and whose fragment lengths are not sector
multiples. A 1 MiB write arriving as bv0=(160,16224) + 63x(0,16384) +
(0,160) reproduces on brd as: first 15872 = ALIGN_DOWN(16224, 512)
bytes correct, everything after shifted forward by 352 = 16224 - 15872
bytes -- while the write completes successfully. Any NFSD_IO_DIRECT
(or other kernel bvec direct I/O) write to a brd-backed filesystem is
exposed; request-based drivers are unaffected because nothing in the
request path does per-bvec sector arithmetic.

Track the device position as a byte offset owned by the submit loop
and advanced by the number of bytes each segment actually processed,
instead of re-deriving it from the skewed bi_sector. Verified with a
synthetic-bio reproducer over brd directly and through nvme-loop:
mid-page-offset geometries and the page-aligned control now all read
back byte-identical, and 20 fresh NFS connections x 16 MiB of O_DIRECT
writes over an XFS-on-nvme-loop-on-brd export complete with zero data
mismatches (previously most connections corrupted).

Fixes: 3185444f0504 ("brd: split I/O at page boundaries")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 drivers/block/brd.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 00cc8122068f..4011538cecaf 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -134,12 +134,24 @@ static void brd_free_pages(struct brd_device *brd)
 /*
  * Process a single segment.  The segment is capped to not cross page boundaries
  * in both the bio and the brd backing memory.
+ *
+ * The device position is @pos, a byte offset maintained by the caller --
+ * not bio->bi_iter.bi_sector: bio_advance_iter_single() advances bi_sector
+ * by bytes >> SECTOR_SHIFT, so a bvec whose length is not a multiple of the
+ * sector size silently skews bi_sector against the bytes actually consumed
+ * and corrupts everything that follows.  Byte-granular bvec boundaries
+ * reach us from ITER_BVEC direct I/O submitters whose caller's bio_vec
+ * array is passed through as-is (bio_iov_bvec_set()).
+ *
+ * Returns the number of bytes processed, or 0 on error (the bio has then
+ * been completed).
  */
-static bool brd_rw_bvec(struct brd_device *brd, struct bio *bio)
+static unsigned int brd_rw_bvec(struct brd_device *brd, struct bio *bio,
+				loff_t pos)
 {
 	struct bio_vec bv = bio_iter_iovec(bio, bio->bi_iter);
-	sector_t sector = bio->bi_iter.bi_sector;
-	u32 offset = (sector & (PAGE_SECTORS - 1)) << SECTOR_SHIFT;
+	sector_t sector = pos >> SECTOR_SHIFT;
+	u32 offset = pos & (PAGE_SIZE - 1);
 	blk_opf_t opf = bio->bi_opf;
 	struct page *page;
 	void *kaddr;
@@ -167,14 +179,14 @@ static bool brd_rw_bvec(struct brd_device *brd, struct bio *bio)
 	bio_advance_iter_single(bio, &bio->bi_iter, bv.bv_len);
 	if (page)
 		put_page(page);
-	return true;
+	return bv.bv_len;
 
 out_error:
 	if (PTR_ERR(page) == -ENOMEM && (opf & REQ_NOWAIT))
 		bio_wouldblock_error(bio);
 	else
 		bio_io_error(bio);
-	return false;
+	return 0;
 }
 
 static void brd_do_discard(struct brd_device *brd, sector_t sector, u32 size)
@@ -202,6 +214,7 @@ static void brd_do_discard(struct brd_device *brd, sector_t sector, u32 size)
 static void brd_submit_bio(struct bio *bio)
 {
 	struct brd_device *brd = bio->bi_bdev->bd_disk->private_data;
+	loff_t pos;
 
 	if (unlikely(op_is_discard(bio->bi_opf))) {
 		brd_do_discard(brd, bio->bi_iter.bi_sector,
@@ -210,9 +223,13 @@ static void brd_submit_bio(struct bio *bio)
 		return;
 	}
 
+	pos = (loff_t)bio->bi_iter.bi_sector << SECTOR_SHIFT;
 	do {
-		if (!brd_rw_bvec(brd, bio))
+		unsigned int len = brd_rw_bvec(brd, bio, pos);
+
+		if (!len)
 			return;
+		pos += len;
 	} while (bio->bi_iter.bi_size);
 
 	bio_endio(bio);
-- 
2.52.0

[PATCH 2/4] zram: handle sub-page bvec segments without corrupting data

[Mike Snitzer]

zram's submit loops have two independent defects for a bvec whose
length is not PAGE_SIZE, and on a 4K-page kernel they compound into
silent full-page corruption for any ITER_BVEC direct I/O submitter
whose bio_vec array carries sub-page segments (bio_iov_bvec_set()
passes the caller's array through as-is).

First, on PAGE_SIZE == 4096 is_partial_io() is hardwired to false, on
the reasoning that logical_block_size == PAGE_SIZE guarantees
whole-page bvecs. It guarantees no such thing: queue limits constrain
a bio's starting sector and total size, not individual bvec lengths.
Every sub-page segment then takes the full-page fast path --
zram_write_page(zram, bvec->bv_page, index) consumes the entire page
and ignores bv_offset/bv_len completely -- so consecutive segments
that map to the same page index each rewrite the whole slot, and a
1 MiB write arriving as bv0=(684,3412) + 255x(0,4096) + (0,684) reads
back with every byte wrong while the write reports success. The read
side is equally exposed: zram_read_page() lands a full page in
bvec->bv_page, clobbering reader memory outside the bvec. Make
is_partial_io() honest on every page size; the partial-IO helpers it
routes to already exist and honor bv_offset/bv_len.

Second, the loops re-derive each segment's target page index and
in-page offset from iter.bi_sector, which bio_advance_iter_single()
advances by bytes >> SECTOR_SHIFT -- a sub-sector residue in any
segment length skews every subsequent segment's position while the
data cursor consumes the full length (the same defect just fixed in
brd). With partial detection made honest this would still misplace
data through the read-modify-write path. Track the device position as
a byte offset owned by the submit loop and advanced by the bytes each
segment actually processed.

Verified with a synthetic-bio reproducer: mid-page-offset geometries
(684, 160, 512) and the page-aligned control now all read back
byte-identical on a 4K-page kernel; before the fix the unaligned
geometries corrupted all 1 MiB silently.

With partial IO possible on every page size, the ZRAM_PARTIAL_IO
guard in read_from_bdev() is dead code -- drop it along with the
define.

Fixes: 1f7319c74275 ("zram: partial IO refactoring")
Fixes: 82ca875d2549 ("zram: refactor highlevel read and write handling")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 drivers/block/zram/zram_drv.c | 36 +++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 789c76dc8613..6c5ba814ba5b 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -217,18 +217,19 @@ static bool zram_can_store_page(struct zram *zram)
 	return !zram->limit_pages || alloced_pages <= zram->limit_pages;
 }
 
-#if PAGE_SIZE != 4096
+/*
+ * A whole-page bvec is required for the full-page fast paths, which
+ * consume bv_page outright and ignore bv_offset/bv_len.  The queue's
+ * logical_block_size == PAGE_SIZE only constrains a bio's starting
+ * sector and total size -- individual bvec lengths are not constrained
+ * by any queue limit, and ITER_BVEC direct I/O submitters pass the
+ * caller's bio_vec array through as-is (bio_iov_bvec_set()), so
+ * sub-page segments reach us on every PAGE_SIZE.
+ */
 static inline bool is_partial_io(struct bio_vec *bvec)
 {
 	return bvec->bv_len != PAGE_SIZE;
 }
-#define ZRAM_PARTIAL_IO		1
-#else
-static inline bool is_partial_io(struct bio_vec *bvec)
-{
-	return false;
-}
-#endif
 
 #if defined CONFIG_ZRAM_WRITEBACK || defined CONFIG_ZRAM_MULTI_COMP
 struct zram_pp_slot {
@@ -1510,11 +1511,8 @@ static int read_from_bdev(struct zram *zram, struct page *page,
 			  struct bio *parent)
 {
 	atomic64_inc(&zram->stats.bd_reads);
-	if (!parent) {
-		if (WARN_ON_ONCE(!IS_ENABLED(ZRAM_PARTIAL_IO)))
-			return -EIO;
+	if (!parent)
 		return read_from_bdev_sync(zram, page, index, blk_idx);
-	}
 	return read_from_bdev_async(zram, page, index, blk_idx, parent);
 }
 #else
@@ -2718,11 +2716,11 @@ static void zram_bio_read(struct zram *zram, struct bio *bio)
 {
 	unsigned long start_time = bio_start_io_acct(bio);
 	struct bvec_iter iter = bio->bi_iter;
+	loff_t pos = (loff_t)iter.bi_sector << SECTOR_SHIFT;
 
 	do {
-		unsigned long index = iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
-		u32 offset = (iter.bi_sector & (SECTORS_PER_PAGE - 1)) <<
-				SECTOR_SHIFT;
+		unsigned long index = pos >> PAGE_SHIFT;
+		u32 offset = pos & (PAGE_SIZE - 1);
 		struct bio_vec bv = bio_iter_iovec(bio, iter);
 
 		bv.bv_len = min_t(u32, bv.bv_len, PAGE_SIZE - offset);
@@ -2738,6 +2736,7 @@ static void zram_bio_read(struct zram *zram, struct bio *bio)
 		mark_slot_accessed(zram, index);
 		slot_unlock(zram, index);
 
+		pos += bv.bv_len;
 		bio_advance_iter_single(bio, &iter, bv.bv_len);
 	} while (iter.bi_size);
 
@@ -2749,11 +2748,11 @@ static void zram_bio_write(struct zram *zram, struct bio *bio)
 {
 	unsigned long start_time = bio_start_io_acct(bio);
 	struct bvec_iter iter = bio->bi_iter;
+	loff_t pos = (loff_t)iter.bi_sector << SECTOR_SHIFT;
 
 	do {
-		unsigned long index = iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
-		u32 offset = (iter.bi_sector & (SECTORS_PER_PAGE - 1)) <<
-				SECTOR_SHIFT;
+		unsigned long index = pos >> PAGE_SHIFT;
+		u32 offset = pos & (PAGE_SIZE - 1);
 		struct bio_vec bv = bio_iter_iovec(bio, iter);
 
 		bv.bv_len = min_t(u32, bv.bv_len, PAGE_SIZE - offset);
@@ -2768,6 +2767,7 @@ static void zram_bio_write(struct zram *zram, struct bio *bio)
 		mark_slot_accessed(zram, index);
 		slot_unlock(zram, index);
 
+		pos += bv.bv_len;
 		bio_advance_iter_single(bio, &iter, bv.bv_len);
 	} while (iter.bi_size);
 
-- 
2.52.0

[PATCH 3/4] nfsd: fetch direct I/O alignment for files handed to the filecache

[Mike Snitzer]

From: David Flynn <david.flynn@hammerspace.com>

nfsd_file_do_acquire() fetches a file's direct I/O alignment
attributes (nf_dio_mem_align, nf_dio_offset_align and
nf_dio_read_offset_align) only on the branch where NFSD opens the
file itself. The other branch, taken when the caller supplies an
already-open struct file, stores that file in the nfsd_file and
leaves the three alignment fields zero.

NFSv4 OPEN with CREATE takes the supplied-file branch:
nfsd4_create_file() creates the file with dentry_create() and passes
the resulting struct file as open->op_filp, which nfs4_get_vfs_file()
hands to nfsd_file_acquire_opened(). The cached nfsd_file therefore
keeps zero alignment for its whole life, and
nfsd_write_dio_iters_init() refuses direct I/O for every WRITE to a
file the client has just created.

Observed on an export whose backing device reports a dio_mem_align of
4: all 16 of 16 1 MiB NFSv4.2 writes that followed an OPEN(CREATE)
took the buffered path, while 16 of 16 writes to the same file
reopened without CREATE went direct.

Hoist the nfsd_file_get_dio_attrs() call out of the open branch so
that it runs once, for both branches, whenever the acquire has
otherwise succeeded. A getattr failure now fails the acquire on the
supplied-file branch exactly as it already does on the open branch.

Fixes: d11f6cd1bb4a ("NFSD: filecache: add STATX_DIOALIGN and STATX_DIO_READ_ALIGN support")
Assisted-by: Claude:claude-fable-5.1
Signed-off-by: David Flynn <david.flynn@hammerspace.com>
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfsd/filecache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index b9548eb17c77..e80f88787ef7 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -1285,9 +1285,9 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
 			}
 			status = nfserrno(ret);
 			trace_nfsd_file_open(nf, status);
-			if (status == nfs_ok)
-				status = nfsd_file_get_dio_attrs(fhp, nf);
 		}
+		if (status == nfs_ok)
+			status = nfsd_file_get_dio_attrs(fhp, nf);
 	} else
 		status = nfserr_jukebox;
 	/*
-- 
2.52.0

[PATCH 4/4] nfsd: fall back to buffered I/O when a direct write gets -EINVAL

[Mike Snitzer]

nfsd_dio_iter_is_aligned() approves a write iterator against the
file's STATX_DIOALIGN attributes (a whole-iterator iov_iter_alignment()
test against dio_mem_align), but the block stack applies stricter
geometry tests at bio split time: bio_split_io_at() checks each bvec's
offset and length against the queue's dma_alignment and may find no
valid block-size-aligned split at all. An ITER_BVEC WRITE payload can
pass the former and fail the latter: bio_iov_bvec_set() hands nfsd's
bvec array to the queue as-is, and the payload's first fragment starts
mid-page (the RPC header precedes it in the receive buffer), so the
iterator's interior page boundaries need not be logical-block aligned
and a bio the queue must split may have no valid split point. When
that happens, nfsd_direct_write() returned the -EINVAL to the client
as a failed WRITE (NFS4ERR_INVAL) -- for a perfectly valid request.

Observed against a brd-backed nvme-loop XFS export (dio_mem_align=4)
with 1 MiB WRITEs, e.g. arriving as 65 bvecs with bv0=(408,15976):
the gate admits the iterator, the block layer rejects it, and every
large write on the affected connection errors out (dd: Invalid
argument).

Treat -EINVAL from the direct attempt as "not direct-able": restore the
segment's iterator and retry it as (uncached when FOP_DONTCACHE)
buffered I/O, the same fallback nfsd_write_dio_iters_init() picks for
geometries it rejects itself.

The failed attempt must be assumed to have left the iterator advanced:
 ->write_iter() advances it while building and submitting bios before
the split-time rejection can fire, and vfs_iocb_iter_write() does not
revert on error. That is why the restore is a struct copy taken before
the attempt -- it snapshots the complete cursor by value (iov_offset,
count, bvec, nr_segs; the underlying bio_vec array is never mutated by
iteration), where iov_iter_revert() would need a byte count that an
error return does not provide. ki_pos is only advanced on success
(iomap_dio_complete() bumps it under ret > 0), so the retry lands at
the original offset, and any sectors a partially-split attempt already
reached are rewritten with the same data. The retry emits
nfsd_write_vector after the original nfsd_write_direct, so a fallback
is visible in tracing as the pair.

With this fix the same rig survives 30 fresh connections x 16 MiB of
page-aligned O_DIRECT client writes with zero client-visible errors
(fallback observed on 20 of 30 connections).

Fixes: 06c5c97293e3 ("NFSD: Implement NFSD_IO_DIRECT for NFS WRITE")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfsd/vfs.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 1af4f77f82fc..f43bbd0ae731 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1456,6 +1456,8 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
 
 	*cnt = 0;
 	for (i = 0; i < nsegs; i++) {
+		struct iov_iter saved_iter = segments[i].iter;
+
 		kiocb->ki_flags = segments[i].flags;
 		if (kiocb->ki_flags & IOCB_DIRECT)
 			trace_nfsd_write_direct(rqstp, fhp, kiocb->ki_pos,
@@ -1467,6 +1469,33 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		expected = iov_iter_count(&segments[i].iter);
 
 		host_err = vfs_iocb_iter_write(file, kiocb, &segments[i].iter);
+		if (unlikely(host_err == -EINVAL &&
+			     (kiocb->ki_flags & IOCB_DIRECT))) {
+			/*
+			 * nfsd_dio_iter_is_aligned() approves the iterator
+			 * against the file's STATX_DIOALIGN attributes, but
+			 * the block stack applies stricter geometry tests at
+			 * split time (bio_split_io_at() checks each bvec's
+			 * offset and length against the queue's dma_alignment
+			 * and may find no valid block-size-aligned split).
+			 * A receive-buffer iterator can pass the former and
+			 * still fail the latter at split time, so treat
+			 * -EINVAL from the direct attempt as "not direct-able"
+			 * and retry the segment as (uncached) buffered I/O
+			 * rather than failing the WRITE.  ki_pos is not
+			 * advanced on error, and any sectors the failed
+			 * attempt already reached are rewritten with the
+			 * same data.
+			 */
+			segments[i].iter = saved_iter;
+			kiocb->ki_flags &= ~IOCB_DIRECT;
+			if (file->f_op->fop_flags & FOP_DONTCACHE)
+				kiocb->ki_flags |= IOCB_DONTCACHE;
+			trace_nfsd_write_vector(rqstp, fhp, kiocb->ki_pos,
+						segments[i].iter.count);
+			host_err = vfs_iocb_iter_write(file, kiocb,
+						       &segments[i].iter);
+		}
 		if (host_err < 0)
 			return host_err;
 		*cnt += host_err;
-- 
2.52.0

[PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O

[Mike Snitzer]

While qualifying NFSD's NFSD_IO_DIRECT write path with byte-level data
verification, it was found that the ITER_BVEC payloads nfsd submits 
expose silent data corruption in two bio-based block drivers and
two defects in nfsd itself. Example problematic payloads is the first
fragment starts mid-page because the RPC header precedes it in the
receive buffer, and fragment lengths need not be sector multiples.
bio_iov_bvec_set() passes such an array to the queue as-is; nothing
below it validates per-bvec sector alignment.

Patches 1-2 fix silent corruption (write completes successfully, data
lands wrong) and are stable candidates:

- brd re-derives each segment's device position from
  bio->bi_iter.bi_sector, which bio_advance_iter_single() advances by
  whole sectors only, so a sub-sector segment length skews everything
  that follows.

- zram hardwires is_partial_io() to false on 4K-page kernels, sending
  sub-page bvecs down a whole-page path that ignores bv_offset/bv_len
  entirely, and has the same sector-cursor skew.

Both are verified with a synthetic-bio reproducer (stamped pattern,
write, read back, compare) across mid-page and page-aligned
geometries.

Patches 3-4 fix nfsd: the filecache never fetches DIO alignment
attributes on the supplied-file acquire branch, so every WRITE to a
file created via NFSv4 OPEN(CREATE) is refused direct I/O for the
file's cached lifetime; and nfsd's statx-based DIO gate is weaker than
bio_split_io_at()'s split-time checks, so an admitted iterator can
still be rejected by the block layer -- retry the segment buffered
instead of failing a valid WRITE with NFS4ERR_INVAL.

One open question for the iomap/block maintainers: should ITER_BVEC
direct I/O with sub-sector bvec boundaries be validated or bounced
centrally rather than trusted to every driver's iteration?  An audit
of in-tree bio-based drivers found the same bi_sector-derived position
pattern in dm-io, dm-log-writes, dm-writecache (pmem path) and
dm-integrity -- unreachable through nfsd today only because dm queues
advertise dma_alignment >= 511, which nfsd's alignment gate refuses.

Tested with the reproducer matrix on brd, zram and nvme-loop at 4K and
16K page size (aarch64) and 4K (x86_64), plus 30-connection NFS write
rigs comparing source against export byte-for-byte: clean with the
fixes, corrupting or erroring without them.

Mike Snitzer (3):
  brd: iterate the bio by byte position, not bi_sector
  zram: handle sub-page bvec segments without corrupting data
  nfsd: fall back to buffered I/O when a direct write gets -EINVAL

David Flynn (1):
  nfsd: fetch direct I/O alignment for files handed to the filecache

 drivers/block/brd.c           | 29 +++++++++++++++++++++++------
 drivers/block/zram/zram_drv.c | 36 +++++++++++++++++------------------
 fs/nfsd/filecache.c           |  4 ++--
 fs/nfsd/vfs.c                 | 29 +++++++++++++++++++++++++++++
 4 files changed, 77 insertions(+), 31 deletions(-)

--
2.52.0

[PATCH 1/4] brd: iterate the bio by byte position, not bi_sector

[Mike Snitzer]

brd_rw_bvec() takes the device position from bio->bi_iter.bi_sector,
which bio_advance_iter_single() advances by bytes >> SECTOR_SHIFT. For
a bvec whose length is not a multiple of the sector size the sector
cursor silently loses the sub-sector residue while the data cursor
(bi_bvec_done/bi_size) consumes the full length -- from that segment
on, data is written at a device offset short of where it belongs, and
every subsequent byte lands shifted with no error reported anywhere.

Such bvec geometry is legal at the submitter: ITER_BVEC direct I/O
passes the caller's bio_vec array through as-is (bio_iov_bvec_set()),
so e.g. NFSD's NFSD_IO_DIRECT write path hands XFS/iomap a payload
whose first fragment starts mid-page (the RPC header precedes it in
the receive buffer) and whose fragment lengths are not sector
multiples. A 1 MiB write arriving as bv0=(160,16224) + 63x(0,16384) +
(0,160) reproduces on brd as: first 15872 = ALIGN_DOWN(16224, 512)
bytes correct, everything after shifted forward by 352 = 16224 - 15872
bytes -- while the write completes successfully. Any NFSD_IO_DIRECT
(or other kernel bvec direct I/O) write to a brd-backed filesystem is
exposed; request-based drivers are unaffected because nothing in the
request path does per-bvec sector arithmetic.

Track the device position as a byte offset owned by the submit loop
and advanced by the number of bytes each segment actually processed,
instead of re-deriving it from the skewed bi_sector. Verified with a
synthetic-bio reproducer over brd directly and through nvme-loop:
mid-page-offset geometries and the page-aligned control now all read
back byte-identical, and 20 fresh NFS connections x 16 MiB of O_DIRECT
writes over an XFS-on-nvme-loop-on-brd export complete with zero data
mismatches (previously most connections corrupted).

Fixes: 3185444f0504 ("brd: split I/O at page boundaries")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 drivers/block/brd.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 00cc8122068f..4011538cecaf 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -134,12 +134,24 @@ static void brd_free_pages(struct brd_device *brd)
 /*
  * Process a single segment.  The segment is capped to not cross page boundaries
  * in both the bio and the brd backing memory.
+ *
+ * The device position is @pos, a byte offset maintained by the caller --
+ * not bio->bi_iter.bi_sector: bio_advance_iter_single() advances bi_sector
+ * by bytes >> SECTOR_SHIFT, so a bvec whose length is not a multiple of the
+ * sector size silently skews bi_sector against the bytes actually consumed
+ * and corrupts everything that follows.  Byte-granular bvec boundaries
+ * reach us from ITER_BVEC direct I/O submitters whose caller's bio_vec
+ * array is passed through as-is (bio_iov_bvec_set()).
+ *
+ * Returns the number of bytes processed, or 0 on error (the bio has then
+ * been completed).
  */
-static bool brd_rw_bvec(struct brd_device *brd, struct bio *bio)
+static unsigned int brd_rw_bvec(struct brd_device *brd, struct bio *bio,
+				loff_t pos)
 {
 	struct bio_vec bv = bio_iter_iovec(bio, bio->bi_iter);
-	sector_t sector = bio->bi_iter.bi_sector;
-	u32 offset = (sector & (PAGE_SECTORS - 1)) << SECTOR_SHIFT;
+	sector_t sector = pos >> SECTOR_SHIFT;
+	u32 offset = pos & (PAGE_SIZE - 1);
 	blk_opf_t opf = bio->bi_opf;
 	struct page *page;
 	void *kaddr;
@@ -167,14 +179,14 @@ static bool brd_rw_bvec(struct brd_device *brd, struct bio *bio)
 	bio_advance_iter_single(bio, &bio->bi_iter, bv.bv_len);
 	if (page)
 		put_page(page);
-	return true;
+	return bv.bv_len;
 
 out_error:
 	if (PTR_ERR(page) == -ENOMEM && (opf & REQ_NOWAIT))
 		bio_wouldblock_error(bio);
 	else
 		bio_io_error(bio);
-	return false;
+	return 0;
 }
 
 static void brd_do_discard(struct brd_device *brd, sector_t sector, u32 size)
@@ -202,6 +214,7 @@ static void brd_do_discard(struct brd_device *brd, sector_t sector, u32 size)
 static void brd_submit_bio(struct bio *bio)
 {
 	struct brd_device *brd = bio->bi_bdev->bd_disk->private_data;
+	loff_t pos;
 
 	if (unlikely(op_is_discard(bio->bi_opf))) {
 		brd_do_discard(brd, bio->bi_iter.bi_sector,
@@ -210,9 +223,13 @@ static void brd_submit_bio(struct bio *bio)
 		return;
 	}
 
+	pos = (loff_t)bio->bi_iter.bi_sector << SECTOR_SHIFT;
 	do {
-		if (!brd_rw_bvec(brd, bio))
+		unsigned int len = brd_rw_bvec(brd, bio, pos);
+
+		if (!len)
 			return;
+		pos += len;
 	} while (bio->bi_iter.bi_size);
 
 	bio_endio(bio);
-- 
2.52.0

[PATCH 2/4] zram: handle sub-page bvec segments without corrupting data

[Mike Snitzer]

zram's submit loops have two independent defects for a bvec whose
length is not PAGE_SIZE, and on a 4K-page kernel they compound into
silent full-page corruption for any ITER_BVEC direct I/O submitter
whose bio_vec array carries sub-page segments (bio_iov_bvec_set()
passes the caller's array through as-is).

First, on PAGE_SIZE == 4096 is_partial_io() is hardwired to false, on
the reasoning that logical_block_size == PAGE_SIZE guarantees
whole-page bvecs. It guarantees no such thing: queue limits constrain
a bio's starting sector and total size, not individual bvec lengths.
Every sub-page segment then takes the full-page fast path --
zram_write_page(zram, bvec->bv_page, index) consumes the entire page
and ignores bv_offset/bv_len completely -- so consecutive segments
that map to the same page index each rewrite the whole slot, and a
1 MiB write arriving as bv0=(684,3412) + 255x(0,4096) + (0,684) reads
back with every byte wrong while the write reports success. The read
side is equally exposed: zram_read_page() lands a full page in
bvec->bv_page, clobbering reader memory outside the bvec. Make
is_partial_io() honest on every page size; the partial-IO helpers it
routes to already exist and honor bv_offset/bv_len.

Second, the loops re-derive each segment's target page index and
in-page offset from iter.bi_sector, which bio_advance_iter_single()
advances by bytes >> SECTOR_SHIFT -- a sub-sector residue in any
segment length skews every subsequent segment's position while the
data cursor consumes the full length (the same defect just fixed in
brd). With partial detection made honest this would still misplace
data through the read-modify-write path. Track the device position as
a byte offset owned by the submit loop and advanced by the bytes each
segment actually processed.

Verified with a synthetic-bio reproducer: mid-page-offset geometries
(684, 160, 512) and the page-aligned control now all read back
byte-identical on a 4K-page kernel; before the fix the unaligned
geometries corrupted all 1 MiB silently.

With partial IO possible on every page size, the ZRAM_PARTIAL_IO
guard in read_from_bdev() is dead code -- drop it along with the
define.

Fixes: 1f7319c74275 ("zram: partial IO refactoring")
Fixes: 82ca875d2549 ("zram: refactor highlevel read and write handling")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 drivers/block/zram/zram_drv.c | 36 +++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 789c76dc8613..6c5ba814ba5b 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -217,18 +217,19 @@ static bool zram_can_store_page(struct zram *zram)
 	return !zram->limit_pages || alloced_pages <= zram->limit_pages;
 }
 
-#if PAGE_SIZE != 4096
+/*
+ * A whole-page bvec is required for the full-page fast paths, which
+ * consume bv_page outright and ignore bv_offset/bv_len.  The queue's
+ * logical_block_size == PAGE_SIZE only constrains a bio's starting
+ * sector and total size -- individual bvec lengths are not constrained
+ * by any queue limit, and ITER_BVEC direct I/O submitters pass the
+ * caller's bio_vec array through as-is (bio_iov_bvec_set()), so
+ * sub-page segments reach us on every PAGE_SIZE.
+ */
 static inline bool is_partial_io(struct bio_vec *bvec)
 {
 	return bvec->bv_len != PAGE_SIZE;
 }
-#define ZRAM_PARTIAL_IO		1
-#else
-static inline bool is_partial_io(struct bio_vec *bvec)
-{
-	return false;
-}
-#endif
 
 #if defined CONFIG_ZRAM_WRITEBACK || defined CONFIG_ZRAM_MULTI_COMP
 struct zram_pp_slot {
@@ -1510,11 +1511,8 @@ static int read_from_bdev(struct zram *zram, struct page *page,
 			  struct bio *parent)
 {
 	atomic64_inc(&zram->stats.bd_reads);
-	if (!parent) {
-		if (WARN_ON_ONCE(!IS_ENABLED(ZRAM_PARTIAL_IO)))
-			return -EIO;
+	if (!parent)
 		return read_from_bdev_sync(zram, page, index, blk_idx);
-	}
 	return read_from_bdev_async(zram, page, index, blk_idx, parent);
 }
 #else
@@ -2718,11 +2716,11 @@ static void zram_bio_read(struct zram *zram, struct bio *bio)
 {
 	unsigned long start_time = bio_start_io_acct(bio);
 	struct bvec_iter iter = bio->bi_iter;
+	loff_t pos = (loff_t)iter.bi_sector << SECTOR_SHIFT;
 
 	do {
-		unsigned long index = iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
-		u32 offset = (iter.bi_sector & (SECTORS_PER_PAGE - 1)) <<
-				SECTOR_SHIFT;
+		unsigned long index = pos >> PAGE_SHIFT;
+		u32 offset = pos & (PAGE_SIZE - 1);
 		struct bio_vec bv = bio_iter_iovec(bio, iter);
 
 		bv.bv_len = min_t(u32, bv.bv_len, PAGE_SIZE - offset);
@@ -2738,6 +2736,7 @@ static void zram_bio_read(struct zram *zram, struct bio *bio)
 		mark_slot_accessed(zram, index);
 		slot_unlock(zram, index);
 
+		pos += bv.bv_len;
 		bio_advance_iter_single(bio, &iter, bv.bv_len);
 	} while (iter.bi_size);
 
@@ -2749,11 +2748,11 @@ static void zram_bio_write(struct zram *zram, struct bio *bio)
 {
 	unsigned long start_time = bio_start_io_acct(bio);
 	struct bvec_iter iter = bio->bi_iter;
+	loff_t pos = (loff_t)iter.bi_sector << SECTOR_SHIFT;
 
 	do {
-		unsigned long index = iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
-		u32 offset = (iter.bi_sector & (SECTORS_PER_PAGE - 1)) <<
-				SECTOR_SHIFT;
+		unsigned long index = pos >> PAGE_SHIFT;
+		u32 offset = pos & (PAGE_SIZE - 1);
 		struct bio_vec bv = bio_iter_iovec(bio, iter);
 
 		bv.bv_len = min_t(u32, bv.bv_len, PAGE_SIZE - offset);
@@ -2768,6 +2767,7 @@ static void zram_bio_write(struct zram *zram, struct bio *bio)
 		mark_slot_accessed(zram, index);
 		slot_unlock(zram, index);
 
+		pos += bv.bv_len;
 		bio_advance_iter_single(bio, &iter, bv.bv_len);
 	} while (iter.bi_size);
 
-- 
2.52.0

[PATCH 3/4] nfsd: fetch direct I/O alignment for files handed to the filecache

[Mike Snitzer]

From: David Flynn <david.flynn@hammerspace.com>

nfsd_file_do_acquire() fetches a file's direct I/O alignment
attributes (nf_dio_mem_align, nf_dio_offset_align and
nf_dio_read_offset_align) only on the branch where NFSD opens the
file itself. The other branch, taken when the caller supplies an
already-open struct file, stores that file in the nfsd_file and
leaves the three alignment fields zero.

NFSv4 OPEN with CREATE takes the supplied-file branch:
nfsd4_create_file() creates the file with dentry_create() and passes
the resulting struct file as open->op_filp, which nfs4_get_vfs_file()
hands to nfsd_file_acquire_opened(). The cached nfsd_file therefore
keeps zero alignment for its whole life, and
nfsd_write_dio_iters_init() refuses direct I/O for every WRITE to a
file the client has just created.

Observed on an export whose backing device reports a dio_mem_align of
4: all 16 of 16 1 MiB NFSv4.2 writes that followed an OPEN(CREATE)
took the buffered path, while 16 of 16 writes to the same file
reopened without CREATE went direct.

Hoist the nfsd_file_get_dio_attrs() call out of the open branch so
that it runs once, for both branches, whenever the acquire has
otherwise succeeded. A getattr failure now fails the acquire on the
supplied-file branch exactly as it already does on the open branch.

Fixes: d11f6cd1bb4a ("NFSD: filecache: add STATX_DIOALIGN and STATX_DIO_READ_ALIGN support")
Assisted-by: Claude:claude-fable-5.1
Signed-off-by: David Flynn <david.flynn@hammerspace.com>
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfsd/filecache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index b9548eb17c77..e80f88787ef7 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -1285,9 +1285,9 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
 			}
 			status = nfserrno(ret);
 			trace_nfsd_file_open(nf, status);
-			if (status == nfs_ok)
-				status = nfsd_file_get_dio_attrs(fhp, nf);
 		}
+		if (status == nfs_ok)
+			status = nfsd_file_get_dio_attrs(fhp, nf);
 	} else
 		status = nfserr_jukebox;
 	/*
-- 
2.52.0

[PATCH 4/4] nfsd: fall back to buffered I/O when a direct write gets -EINVAL

[Mike Snitzer]

nfsd_dio_iter_is_aligned() approves a write iterator against the
file's STATX_DIOALIGN attributes (a whole-iterator iov_iter_alignment()
test against dio_mem_align), but the block stack applies stricter
geometry tests at bio split time: bio_split_io_at() checks each bvec's
offset and length against the queue's dma_alignment and may find no
valid block-size-aligned split at all. An ITER_BVEC WRITE payload can
pass the former and fail the latter: bio_iov_bvec_set() hands nfsd's
bvec array to the queue as-is, and the payload's first fragment starts
mid-page (the RPC header precedes it in the receive buffer), so the
iterator's interior page boundaries need not be logical-block aligned
and a bio the queue must split may have no valid split point. When
that happens, nfsd_direct_write() returned the -EINVAL to the client
as a failed WRITE (NFS4ERR_INVAL) -- for a perfectly valid request.

Observed against a brd-backed nvme-loop XFS export (dio_mem_align=4)
with 1 MiB WRITEs, e.g. arriving as 65 bvecs with bv0=(408,15976):
the gate admits the iterator, the block layer rejects it, and every
large write on the affected connection errors out (dd: Invalid
argument).

Treat -EINVAL from the direct attempt as "not direct-able": restore the
segment's iterator and retry it as (uncached when FOP_DONTCACHE)
buffered I/O, the same fallback nfsd_write_dio_iters_init() picks for
geometries it rejects itself.

The failed attempt must be assumed to have left the iterator advanced:
 ->write_iter() advances it while building and submitting bios before
the split-time rejection can fire, and vfs_iocb_iter_write() does not
revert on error. That is why the restore is a struct copy taken before
the attempt -- it snapshots the complete cursor by value (iov_offset,
count, bvec, nr_segs; the underlying bio_vec array is never mutated by
iteration), where iov_iter_revert() would need a byte count that an
error return does not provide. ki_pos is only advanced on success
(iomap_dio_complete() bumps it under ret > 0), so the retry lands at
the original offset, and any sectors a partially-split attempt already
reached are rewritten with the same data. The retry emits
nfsd_write_vector after the original nfsd_write_direct, so a fallback
is visible in tracing as the pair.

With this fix the same rig survives 30 fresh connections x 16 MiB of
page-aligned O_DIRECT client writes with zero client-visible errors
(fallback observed on 20 of 30 connections).

Fixes: 06c5c97293e3 ("NFSD: Implement NFSD_IO_DIRECT for NFS WRITE")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfsd/vfs.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 1af4f77f82fc..f43bbd0ae731 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1456,6 +1456,8 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
 
 	*cnt = 0;
 	for (i = 0; i < nsegs; i++) {
+		struct iov_iter saved_iter = segments[i].iter;
+
 		kiocb->ki_flags = segments[i].flags;
 		if (kiocb->ki_flags & IOCB_DIRECT)
 			trace_nfsd_write_direct(rqstp, fhp, kiocb->ki_pos,
@@ -1467,6 +1469,33 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		expected = iov_iter_count(&segments[i].iter);
 
 		host_err = vfs_iocb_iter_write(file, kiocb, &segments[i].iter);
+		if (unlikely(host_err == -EINVAL &&
+			     (kiocb->ki_flags & IOCB_DIRECT))) {
+			/*
+			 * nfsd_dio_iter_is_aligned() approves the iterator
+			 * against the file's STATX_DIOALIGN attributes, but
+			 * the block stack applies stricter geometry tests at
+			 * split time (bio_split_io_at() checks each bvec's
+			 * offset and length against the queue's dma_alignment
+			 * and may find no valid block-size-aligned split).
+			 * A receive-buffer iterator can pass the former and
+			 * still fail the latter at split time, so treat
+			 * -EINVAL from the direct attempt as "not direct-able"
+			 * and retry the segment as (uncached) buffered I/O
+			 * rather than failing the WRITE.  ki_pos is not
+			 * advanced on error, and any sectors the failed
+			 * attempt already reached are rewritten with the
+			 * same data.
+			 */
+			segments[i].iter = saved_iter;
+			kiocb->ki_flags &= ~IOCB_DIRECT;
+			if (file->f_op->fop_flags & FOP_DONTCACHE)
+				kiocb->ki_flags |= IOCB_DONTCACHE;
+			trace_nfsd_write_vector(rqstp, fhp, kiocb->ki_pos,
+						segments[i].iter.count);
+			host_err = vfs_iocb_iter_write(file, kiocb,
+						       &segments[i].iter);
+		}
 		if (host_err < 0)
 			return host_err;
 		*cnt += host_err;
-- 
2.52.0

[PATCH 2/4] zram: handle sub-page bvec segments without corrupting data

[Mike Snitzer]

zram's submit loops have two independent defects for a bvec whose
length is not PAGE_SIZE, and on a 4K-page kernel they compound into
silent full-page corruption for any ITER_BVEC direct I/O submitter
whose bio_vec array carries sub-page segments (bio_iov_bvec_set()
passes the caller's array through as-is).

First, on PAGE_SIZE == 4096 is_partial_io() is hardwired to false, on
the reasoning that logical_block_size == PAGE_SIZE guarantees
whole-page bvecs. It guarantees no such thing: queue limits constrain
a bio's starting sector and total size, not individual bvec lengths.
Every sub-page segment then takes the full-page fast path --
zram_write_page(zram, bvec->bv_page, index) consumes the entire page
and ignores bv_offset/bv_len completely -- so consecutive segments
that map to the same page index each rewrite the whole slot, and a
1 MiB write arriving as bv0=(684,3412) + 255x(0,4096) + (0,684) reads
back with every byte wrong while the write reports success. The read
side is equally exposed: zram_read_page() lands a full page in
bvec->bv_page, clobbering reader memory outside the bvec. Make
is_partial_io() honest on every page size; the partial-IO helpers it
routes to already exist and honor bv_offset/bv_len.

Second, the loops re-derive each segment's target page index and
in-page offset from iter.bi_sector, which bio_advance_iter_single()
advances by bytes >> SECTOR_SHIFT -- a sub-sector residue in any
segment length skews every subsequent segment's position while the
data cursor consumes the full length (the same defect just fixed in
brd). With partial detection made honest this would still misplace
data through the read-modify-write path. Track the device position as
a byte offset owned by the submit loop and advanced by the bytes each
segment actually processed.

Verified with a synthetic-bio reproducer: mid-page-offset geometries
(684, 160, 512) and the page-aligned control now all read back
byte-identical on a 4K-page kernel; before the fix the unaligned
geometries corrupted all 1 MiB silently.

With partial IO possible on every page size, the ZRAM_PARTIAL_IO
guard in read_from_bdev() is dead code -- drop it along with the
define.

Fixes: 1f7319c74275 ("zram: partial IO refactoring")
Fixes: 82ca875d2549 ("zram: refactor highlevel read and write handling")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 drivers/block/zram/zram_drv.c | 36 +++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 789c76dc8613..6c5ba814ba5b 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -217,18 +217,19 @@ static bool zram_can_store_page(struct zram *zram)
 	return !zram->limit_pages || alloced_pages <= zram->limit_pages;
 }
 
-#if PAGE_SIZE != 4096
+/*
+ * A whole-page bvec is required for the full-page fast paths, which
+ * consume bv_page outright and ignore bv_offset/bv_len.  The queue's
+ * logical_block_size == PAGE_SIZE only constrains a bio's starting
+ * sector and total size -- individual bvec lengths are not constrained
+ * by any queue limit, and ITER_BVEC direct I/O submitters pass the
+ * caller's bio_vec array through as-is (bio_iov_bvec_set()), so
+ * sub-page segments reach us on every PAGE_SIZE.
+ */
 static inline bool is_partial_io(struct bio_vec *bvec)
 {
 	return bvec->bv_len != PAGE_SIZE;
 }
-#define ZRAM_PARTIAL_IO		1
-#else
-static inline bool is_partial_io(struct bio_vec *bvec)
-{
-	return false;
-}
-#endif
 
 #if defined CONFIG_ZRAM_WRITEBACK || defined CONFIG_ZRAM_MULTI_COMP
 struct zram_pp_slot {
@@ -1510,11 +1511,8 @@ static int read_from_bdev(struct zram *zram, struct page *page,
 			  struct bio *parent)
 {
 	atomic64_inc(&zram->stats.bd_reads);
-	if (!parent) {
-		if (WARN_ON_ONCE(!IS_ENABLED(ZRAM_PARTIAL_IO)))
-			return -EIO;
+	if (!parent)
 		return read_from_bdev_sync(zram, page, index, blk_idx);
-	}
 	return read_from_bdev_async(zram, page, index, blk_idx, parent);
 }
 #else
@@ -2718,11 +2716,11 @@ static void zram_bio_read(struct zram *zram, struct bio *bio)
 {
 	unsigned long start_time = bio_start_io_acct(bio);
 	struct bvec_iter iter = bio->bi_iter;
+	loff_t pos = (loff_t)iter.bi_sector << SECTOR_SHIFT;
 
 	do {
-		unsigned long index = iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
-		u32 offset = (iter.bi_sector & (SECTORS_PER_PAGE - 1)) <<
-				SECTOR_SHIFT;
+		unsigned long index = pos >> PAGE_SHIFT;
+		u32 offset = pos & (PAGE_SIZE - 1);
 		struct bio_vec bv = bio_iter_iovec(bio, iter);
 
 		bv.bv_len = min_t(u32, bv.bv_len, PAGE_SIZE - offset);
@@ -2738,6 +2736,7 @@ static void zram_bio_read(struct zram *zram, struct bio *bio)
 		mark_slot_accessed(zram, index);
 		slot_unlock(zram, index);
 
+		pos += bv.bv_len;
 		bio_advance_iter_single(bio, &iter, bv.bv_len);
 	} while (iter.bi_size);
 
@@ -2749,11 +2748,11 @@ static void zram_bio_write(struct zram *zram, struct bio *bio)
 {
 	unsigned long start_time = bio_start_io_acct(bio);
 	struct bvec_iter iter = bio->bi_iter;
+	loff_t pos = (loff_t)iter.bi_sector << SECTOR_SHIFT;
 
 	do {
-		unsigned long index = iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
-		u32 offset = (iter.bi_sector & (SECTORS_PER_PAGE - 1)) <<
-				SECTOR_SHIFT;
+		unsigned long index = pos >> PAGE_SHIFT;
+		u32 offset = pos & (PAGE_SIZE - 1);
 		struct bio_vec bv = bio_iter_iovec(bio, iter);
 
 		bv.bv_len = min_t(u32, bv.bv_len, PAGE_SIZE - offset);
@@ -2768,6 +2767,7 @@ static void zram_bio_write(struct zram *zram, struct bio *bio)
 		mark_slot_accessed(zram, index);
 		slot_unlock(zram, index);
 
+		pos += bv.bv_len;
 		bio_advance_iter_single(bio, &iter, bv.bv_len);
 	} while (iter.bi_size);
 
-- 
2.52.0

[PATCH 3/4] nfsd: fetch direct I/O alignment for files handed to the filecache

[Mike Snitzer]

From: David Flynn <david.flynn@hammerspace.com>

nfsd_file_do_acquire() fetches a file's direct I/O alignment
attributes (nf_dio_mem_align, nf_dio_offset_align and
nf_dio_read_offset_align) only on the branch where NFSD opens the
file itself. The other branch, taken when the caller supplies an
already-open struct file, stores that file in the nfsd_file and
leaves the three alignment fields zero.

NFSv4 OPEN with CREATE takes the supplied-file branch:
nfsd4_create_file() creates the file with dentry_create() and passes
the resulting struct file as open->op_filp, which nfs4_get_vfs_file()
hands to nfsd_file_acquire_opened(). The cached nfsd_file therefore
keeps zero alignment for its whole life, and
nfsd_write_dio_iters_init() refuses direct I/O for every WRITE to a
file the client has just created.

Observed on an export whose backing device reports a dio_mem_align of
4: all 16 of 16 1 MiB NFSv4.2 writes that followed an OPEN(CREATE)
took the buffered path, while 16 of 16 writes to the same file
reopened without CREATE went direct.

Hoist the nfsd_file_get_dio_attrs() call out of the open branch so
that it runs once, for both branches, whenever the acquire has
otherwise succeeded. A getattr failure now fails the acquire on the
supplied-file branch exactly as it already does on the open branch.

Fixes: d11f6cd1bb4a ("NFSD: filecache: add STATX_DIOALIGN and STATX_DIO_READ_ALIGN support")
Assisted-by: Claude:claude-fable-5.1
Signed-off-by: David Flynn <david.flynn@hammerspace.com>
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfsd/filecache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index b9548eb17c77..e80f88787ef7 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -1285,9 +1285,9 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct net *net,
 			}
 			status = nfserrno(ret);
 			trace_nfsd_file_open(nf, status);
-			if (status == nfs_ok)
-				status = nfsd_file_get_dio_attrs(fhp, nf);
 		}
+		if (status == nfs_ok)
+			status = nfsd_file_get_dio_attrs(fhp, nf);
 	} else
 		status = nfserr_jukebox;
 	/*
-- 
2.52.0

[PATCH 4/4] nfsd: fall back to buffered I/O when a direct write gets -EINVAL

[Mike Snitzer]

nfsd_dio_iter_is_aligned() approves a write iterator against the
file's STATX_DIOALIGN attributes (a whole-iterator iov_iter_alignment()
test against dio_mem_align), but the block stack applies stricter
geometry tests at bio split time: bio_split_io_at() checks each bvec's
offset and length against the queue's dma_alignment and may find no
valid block-size-aligned split at all. An ITER_BVEC WRITE payload can
pass the former and fail the latter: bio_iov_bvec_set() hands nfsd's
bvec array to the queue as-is, and the payload's first fragment starts
mid-page (the RPC header precedes it in the receive buffer), so the
iterator's interior page boundaries need not be logical-block aligned
and a bio the queue must split may have no valid split point. When
that happens, nfsd_direct_write() returned the -EINVAL to the client
as a failed WRITE (NFS4ERR_INVAL) -- for a perfectly valid request.

Observed against a brd-backed nvme-loop XFS export (dio_mem_align=4)
with 1 MiB WRITEs, e.g. arriving as 65 bvecs with bv0=(408,15976):
the gate admits the iterator, the block layer rejects it, and every
large write on the affected connection errors out (dd: Invalid
argument).

Treat -EINVAL from the direct attempt as "not direct-able": restore the
segment's iterator and retry it as (uncached when FOP_DONTCACHE)
buffered I/O, the same fallback nfsd_write_dio_iters_init() picks for
geometries it rejects itself.

The failed attempt must be assumed to have left the iterator advanced:
 ->write_iter() advances it while building and submitting bios before
the split-time rejection can fire, and vfs_iocb_iter_write() does not
revert on error. That is why the restore is a struct copy taken before
the attempt -- it snapshots the complete cursor by value (iov_offset,
count, bvec, nr_segs; the underlying bio_vec array is never mutated by
iteration), where iov_iter_revert() would need a byte count that an
error return does not provide. ki_pos is only advanced on success
(iomap_dio_complete() bumps it under ret > 0), so the retry lands at
the original offset, and any sectors a partially-split attempt already
reached are rewritten with the same data. The retry emits
nfsd_write_vector after the original nfsd_write_direct, so a fallback
is visible in tracing as the pair.

With this fix the same rig survives 30 fresh connections x 16 MiB of
page-aligned O_DIRECT client writes with zero client-visible errors
(fallback observed on 20 of 30 connections).

Fixes: 06c5c97293e3 ("NFSD: Implement NFSD_IO_DIRECT for NFS WRITE")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfsd/vfs.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 1af4f77f82fc..f43bbd0ae731 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1456,6 +1456,8 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
 
 	*cnt = 0;
 	for (i = 0; i < nsegs; i++) {
+		struct iov_iter saved_iter = segments[i].iter;
+
 		kiocb->ki_flags = segments[i].flags;
 		if (kiocb->ki_flags & IOCB_DIRECT)
 			trace_nfsd_write_direct(rqstp, fhp, kiocb->ki_pos,
@@ -1467,6 +1469,33 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		expected = iov_iter_count(&segments[i].iter);
 
 		host_err = vfs_iocb_iter_write(file, kiocb, &segments[i].iter);
+		if (unlikely(host_err == -EINVAL &&
+			     (kiocb->ki_flags & IOCB_DIRECT))) {
+			/*
+			 * nfsd_dio_iter_is_aligned() approves the iterator
+			 * against the file's STATX_DIOALIGN attributes, but
+			 * the block stack applies stricter geometry tests at
+			 * split time (bio_split_io_at() checks each bvec's
+			 * offset and length against the queue's dma_alignment
+			 * and may find no valid block-size-aligned split).
+			 * A receive-buffer iterator can pass the former and
+			 * still fail the latter at split time, so treat
+			 * -EINVAL from the direct attempt as "not direct-able"
+			 * and retry the segment as (uncached) buffered I/O
+			 * rather than failing the WRITE.  ki_pos is not
+			 * advanced on error, and any sectors the failed
+			 * attempt already reached are rewritten with the
+			 * same data.
+			 */
+			segments[i].iter = saved_iter;
+			kiocb->ki_flags &= ~IOCB_DIRECT;
+			if (file->f_op->fop_flags & FOP_DONTCACHE)
+				kiocb->ki_flags |= IOCB_DONTCACHE;
+			trace_nfsd_write_vector(rqstp, fhp, kiocb->ki_pos,
+						segments[i].iter.count);
+			host_err = vfs_iocb_iter_write(file, kiocb,
+						       &segments[i].iter);
+		}
 		if (host_err < 0)
 			return host_err;
 		*cnt += host_err;
-- 
2.52.0

Re: [PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O

[Mike Snitzer]

Apologies for the git-send-email misfire.. still not sure what
happened, but will sort it out for future.

On Tue, Sep 08, 2026 at 12:34:35PM -0400, Mike Snitzer wrote:
> While qualifying NFSD's NFSD_IO_DIRECT write path with byte-level data
> verification, it was found that the ITER_BVEC payloads nfsd submits 
> expose silent data corruption in two bio-based block drivers and
> two defects in nfsd itself. Example problematic payloads is the first
> fragment starts mid-page because the RPC header precedes it in the
> receive buffer, and fragment lengths need not be sector multiples.
> bio_iov_bvec_set() passes such an array to the queue as-is; nothing
> below it validates per-bvec sector alignment.
> 
> Patches 1-2 fix silent corruption (write completes successfully, data
> lands wrong) and are stable candidates:
> 
> - brd re-derives each segment's device position from
>   bio->bi_iter.bi_sector, which bio_advance_iter_single() advances by
>   whole sectors only, so a sub-sector segment length skews everything
>   that follows.
> 
> - zram hardwires is_partial_io() to false on 4K-page kernels, sending
>   sub-page bvecs down a whole-page path that ignores bv_offset/bv_len
>   entirely, and has the same sector-cursor skew.
> 
> Both are verified with a synthetic-bio reproducer (stamped pattern,
> write, read back, compare) across mid-page and page-aligned
> geometries.
> 
> Patches 3-4 fix nfsd: the filecache never fetches DIO alignment
> attributes on the supplied-file acquire branch, so every WRITE to a
> file created via NFSv4 OPEN(CREATE) is refused direct I/O for the
> file's cached lifetime; and nfsd's statx-based DIO gate is weaker than
> bio_split_io_at()'s split-time checks, so an admitted iterator can
> still be rejected by the block layer -- retry the segment buffered
> instead of failing a valid WRITE with NFS4ERR_INVAL.
> 
> One open question for the iomap/block maintainers: should ITER_BVEC
> direct I/O with sub-sector bvec boundaries be validated or bounced
> centrally rather than trusted to every driver's iteration?  An audit
> of in-tree bio-based drivers found the same bi_sector-derived position
> pattern in dm-io, dm-log-writes, dm-writecache (pmem path) and
> dm-integrity -- unreachable through nfsd today only because dm queues
> advertise dma_alignment >= 511, which nfsd's alignment gate refuses.
> 
> Tested with the reproducer matrix on brd, zram and nvme-loop at 4K and
> 16K page size (aarch64) and 4K (x86_64), plus 30-connection NFS write
> rigs comparing source against export byte-for-byte: clean with the
> fixes, corrupting or erroring without them.
> 
> Mike Snitzer (3):
>   brd: iterate the bio by byte position, not bi_sector
>   zram: handle sub-page bvec segments without corrupting data
>   nfsd: fall back to buffered I/O when a direct write gets -EINVAL
> 
> David Flynn (1):
>   nfsd: fetch direct I/O alignment for files handed to the filecache
> 
>  drivers/block/brd.c           | 29 +++++++++++++++++++++++------
>  drivers/block/zram/zram_drv.c | 36 +++++++++++++++++------------------
>  fs/nfsd/filecache.c           |  4 ++--
>  fs/nfsd/vfs.c                 | 29 +++++++++++++++++++++++++++++
>  4 files changed, 77 insertions(+), 31 deletions(-)
> 
> --
> 2.52.0

Re: [PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O

[Chuck Lever]

On Tue, Sep 8, 2026, at 12:36 PM, Mike Snitzer wrote:
> Apologies for the git-send-email misfire.. still not sure what
> happened, but will sort it out for future.

3/4 looks sensible, and can go in now. 4/4 might need some
adjustment. Assuming, of course, there is no functional or
mechanical dependency on having 1/2 and 2/2 in the tree
first.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

Re: [PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O

[Mike Snitzer]

On Tue, Sep 08, 2026 at 01:48:14PM -0400, Chuck Lever wrote:
> 
> 
> On Tue, Sep 8, 2026, at 12:36 PM, Mike Snitzer wrote:
> > Apologies for the git-send-email misfire.. still not sure what
> > happened, but will sort it out for future.
> 
> 3/4 looks sensible, and can go in now. 4/4 might need some
> adjustment. Assuming, of course, there is no functional or
> mechanical dependency on having 1/2 and 2/2 in the tree
> first.

Right, 3/4 and 4/4 don't have dependency on 1/2 or 2/2.

Thanks,
Mike

Re: [PATCH 4/4] nfsd: fall back to buffered I/O when a direct write gets -EINVAL

[Chuck Lever]

On 9/8/26 12:34 PM, Mike Snitzer wrote:
> nfsd_dio_iter_is_aligned() approves a write iterator against the
> file's STATX_DIOALIGN attributes (a whole-iterator iov_iter_alignment()
> test against dio_mem_align), but the block stack applies stricter
> geometry tests at bio split time: bio_split_io_at() checks each bvec's
> offset and length against the queue's dma_alignment and may find no
> valid block-size-aligned split at all. An ITER_BVEC WRITE payload can
> pass the former and fail the latter: bio_iov_bvec_set() hands nfsd's
> bvec array to the queue as-is, and the payload's first fragment starts
> mid-page (the RPC header precedes it in the receive buffer), so the
> iterator's interior page boundaries need not be logical-block aligned
> and a bio the queue must split may have no valid split point. When
> that happens, nfsd_direct_write() returned the -EINVAL to the client
> as a failed WRITE (NFS4ERR_INVAL) -- for a perfectly valid request.
>
> Observed against a brd-backed nvme-loop XFS export (dio_mem_align=4)
> with 1 MiB WRITEs, e.g. arriving as 65 bvecs with bv0=(408,15976):
> the gate admits the iterator, the block layer rejects it, and every
> large write on the affected connection errors out (dd: Invalid
> argument).

Thanks for chasing this down. The bv0 numbers make the gate defect
clear: 15976 is not a multiple of the logical block size, so the
direct segment's first interior bvec boundary lands mid-sector. The
boundaries after that are page boundaries, which are fine.

A small correction for the commit message: nfsd_dio_iter_is_aligned()
doesn't exist. The gate is the first-bvec offset test in
nfsd_write_dio_iters_init(), and it checks only that one offset
against nf_dio_mem_align. Likewise bio_iov_bvec_set() is now
bio_iov_iter_set().

> Treat -EINVAL from the direct attempt as "not direct-able": restore the
> segment's iterator and retry it as (uncached when FOP_DONTCACHE)
> buffered I/O, the same fallback nfsd_write_dio_iters_init() picks for
> geometries it rejects itself.

[ ... ]

> @@ -1467,6 +1469,33 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		expected = iov_iter_count(&segments[i].iter);
>
>  		host_err = vfs_iocb_iter_write(file, kiocb, &segments[i].iter);
> +		if (unlikely(host_err == -EINVAL &&
> +			     (kiocb->ki_flags & IOCB_DIRECT))) {

[ ... ]

> +			segments[i].iter = saved_iter;
> +			kiocb->ki_flags &= ~IOCB_DIRECT;
> +			if (file->f_op->fop_flags & FOP_DONTCACHE)
> +				kiocb->ki_flags |= IOCB_DONTCACHE;
> +			trace_nfsd_write_vector(rqstp, fhp, kiocb->ki_pos,
> +						segments[i].iter.count);
> +			host_err = vfs_iocb_iter_write(file, kiocb,
> +						       &segments[i].iter);
> +		}

Per our discussion last October:

  https://lore.kernel.org/linux-nfs/aPXihwGTiA7bqTsN@infradead.org/

The conclusion then was that -EINVAL from ->write_iter can come from
a number of conditions in the filesystem, so NFSD can't treat it as
meaning only that the I/O was misaligned. That still holds, so I'd
rather not use -EINVAL to signal a retry. An -EINVAL that really is
the filesystem rejecting the request would now cost a second full
write attempt before surfacing anyway.

nfsd_write_dio_iters_init() already has the segment start and
nf_dio_offset_align, and after the first bvec every boundary is
page-aligned. If it also requires the first bvec's remaining length
(from the segment start) to be a multiple of offset_align and takes
the no_dio path otherwise, that rejects bv0=(408,15976) up front
using only data NFSD already has.

What would help me understand the failure even better:

- Which -EINVAL in bio_split_io_at() fired: the per-bvec dma_alignment
  test, or the zero-length result after ALIGN_DOWN()?

- On the reproducer, how does stx_dio_offset_align compare with the
  queue's logical_block_size?

If there turn out to be cases the gate can't predict from the statx
data, that seems like a question for the block and fs folks about
what error the filesystem should surface, rather than something to
work around in NFSD.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

Re: [PATCH 4/4] nfsd: fall back to buffered I/O when a direct write gets -EINVAL

[Chuck Lever]

On Tue, Sep 8, 2026, at 11:57 PM, David Flynn wrote:
> (Sorry in advance if I’m posting this wrong in some way - little new to 
> posting here…)
>
> Totally agree that NFSD must not interpret generic -EINVAL as an 
> alignment rejection... But, a boundary check would also excludes 
> geometries that capable storage stacks could write directly. We want 
> the actual filesystem/block decision to distinguish unsupported 
> geometry from a real error, while retaining every working zero-copy 
> case.

Fair enough, but that's a filesystem-community question, outside
of NFSD's domain. If they can agree to an API contract that NFSD
can use, then I don't have a quibble.

To make your argument, of course, you will need to provide a
real-world use case with an existing in-tree filesystem or device
that can demonstrate what you need.

In the meantime, I'd like to see the narrow issue that Mike
reported addressed in the current code. Fixing the current gate
is not fraught with these deeper architectural issues, and the
fix should be backported to LTS kernels.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

Re: [PATCH 3/4] nfsd: fetch direct I/O alignment for files handed to the filecache

[Chuck Lever]

On Tue, 08 Sep 2026 12:34:38 -0400, Mike Snitzer wrote:
> nfsd_file_do_acquire() fetches a file's direct I/O alignment
> attributes (nf_dio_mem_align, nf_dio_offset_align and
> nf_dio_read_offset_align) only on the branch where NFSD opens the
> file itself. The other branch, taken when the caller supplies an
> already-open struct file, stores that file in the nfsd_file and
> leaves the three alignment fields zero.
> 
> [...]

Applied to nfsd-testing, thanks!

[3/4] nfsd: fetch direct I/O alignment for files handed to the filecache
      commit: abf2077ee32058e60d3f49ecab265d3c4bd953d9

--
Chuck Lever

Re: [PATCH 4/4] nfsd: fall back to buffered I/O when a direct write gets -EINVAL

[Mike Snitzer]

On Tue, Sep 08, 2026 at 02:25:32PM -0400, Chuck Lever wrote:
> On 9/8/26 12:34 PM, Mike Snitzer wrote:
> > nfsd_dio_iter_is_aligned() approves a write iterator against the
> > file's STATX_DIOALIGN attributes (a whole-iterator iov_iter_alignment()
> > test against dio_mem_align), but the block stack applies stricter
> > geometry tests at bio split time: bio_split_io_at() checks each bvec's
> > offset and length against the queue's dma_alignment and may find no
> > valid block-size-aligned split at all. An ITER_BVEC WRITE payload can
> > pass the former and fail the latter: bio_iov_bvec_set() hands nfsd's
> > bvec array to the queue as-is, and the payload's first fragment starts
> > mid-page (the RPC header precedes it in the receive buffer), so the
> > iterator's interior page boundaries need not be logical-block aligned
> > and a bio the queue must split may have no valid split point. When
> > that happens, nfsd_direct_write() returned the -EINVAL to the client
> > as a failed WRITE (NFS4ERR_INVAL) -- for a perfectly valid request.
> >
> > Observed against a brd-backed nvme-loop XFS export (dio_mem_align=4)
> > with 1 MiB WRITEs, e.g. arriving as 65 bvecs with bv0=(408,15976):
> > the gate admits the iterator, the block layer rejects it, and every
> > large write on the affected connection errors out (dd: Invalid
> > argument).
> 
> Thanks for chasing this down. The bv0 numbers make the gate defect
> clear: 15976 is not a multiple of the logical block size, so the
> direct segment's first interior bvec boundary lands mid-sector. The
> boundaries after that are page boundaries, which are fine.
> 
> A small correction for the commit message: nfsd_dio_iter_is_aligned()
> doesn't exist. The gate is the first-bvec offset test in
> nfsd_write_dio_iters_init(), and it checks only that one offset
> against nf_dio_mem_align. Likewise bio_iov_bvec_set() is now
> bio_iov_iter_set().

As I'm sure you inferred, this patch was pulled out to the front as an
"upstream fixes" section -- of a broader series of changes David and I
have been developing to enhance SUNRPC to allow for wider use of
NFSD_DIRECT with TCP.  So that explains the inconsistency in functions
referenced.

> > Treat -EINVAL from the direct attempt as "not direct-able": restore the
> > segment's iterator and retry it as (uncached when FOP_DONTCACHE)
> > buffered I/O, the same fallback nfsd_write_dio_iters_init() picks for
> > geometries it rejects itself.
> 
> [ ... ]
> 
> > @@ -1467,6 +1469,33 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  		expected = iov_iter_count(&segments[i].iter);
> >
> >  		host_err = vfs_iocb_iter_write(file, kiocb, &segments[i].iter);
> > +		if (unlikely(host_err == -EINVAL &&
> > +			     (kiocb->ki_flags & IOCB_DIRECT))) {
> 
> [ ... ]
> 
> > +			segments[i].iter = saved_iter;
> > +			kiocb->ki_flags &= ~IOCB_DIRECT;
> > +			if (file->f_op->fop_flags & FOP_DONTCACHE)
> > +				kiocb->ki_flags |= IOCB_DONTCACHE;
> > +			trace_nfsd_write_vector(rqstp, fhp, kiocb->ki_pos,
> > +						segments[i].iter.count);
> > +			host_err = vfs_iocb_iter_write(file, kiocb,
> > +						       &segments[i].iter);
> > +		}
> 
> Per our discussion last October:
> 
>   https://lore.kernel.org/linux-nfs/aPXihwGTiA7bqTsN@infradead.org/
> 
> The conclusion then was that -EINVAL from ->write_iter can come from
> a number of conditions in the filesystem, so NFSD can't treat it as
> meaning only that the I/O was misaligned. That still holds, so I'd
> rather not use -EINVAL to signal a retry. An -EINVAL that really is
> the filesystem rejecting the request would now cost a second full
> write attempt before surfacing anyway.

Yes, I do recall that exchange now thanks for the reminder!

> nfsd_write_dio_iters_init() already has the segment start and
> nf_dio_offset_align, and after the first bvec every boundary is
> page-aligned. If it also requires the first bvec's remaining length
> (from the segment start) to be a multiple of offset_align and takes
> the no_dio path otherwise, that rejects bv0=(408,15976) up front
> using only data NFSD already has.

Turns out the gate in nfsd_write_dio_iters_init() is perfectly fine
for existing upstream SUNRPC TCP, copied rq_pages payloads are always
one contiguous page-tiled run, and contiguous runs always split
validly.

It is only with the broader set of SUNRPC TCP changes, that are
actively in development, where the gate needs to be tightened up.
 
> What would help me understand the failure even better:
> 
> - Which -EINVAL in bio_split_io_at() fired: the per-bvec dma_alignment
>   test, or the zero-length result after ALIGN_DOWN()?

It's the ALIGN_DOWN-to-zero branch - the per-bvec dma_alignment test
provably can't fire on gate-admitted iterators. The trigger is an
interior bvec discontinuity that was made possible by our
in-development changes.

> - On the reproducer, how does stx_dio_offset_align compare with the
>   queue's logical_block_size?

They are equal, dio_offset_align=512 == logical_block_size=512
(dio_mem_align=4 from dma_alignment=3).

> If there turn out to be cases the gate can't predict from the statx
> data, that seems like a question for the block and fs folks about
> what error the filesystem should surface, rather than something to
> work around in NFSD.

There isn't anything that needs a more sweeping review/decision from
the community. Basically Claude got it wrong that this was an existing
upstream problem that needed fixing. Please drop this patch 4/4.

I'll tighten up the gate in the broader patchset that is in
development. I'll keep iterating on it and hopefully be able to share
it soon.

Thanks,
Mike

Re: [PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O

[Christoph Hellwig]

On Tue, Sep 08, 2026 at 12:36:58PM -0400, Mike Snitzer wrote:
> Apologies for the git-send-email misfire.. still not sure what
> happened, but will sort it out for future.

When you do that please also don't mix unrelated bits.  The two
block driver fixes should go to linux-block, Jens and the relevant
maintainers and re pretty much unrelated to the nfsd bits.

Re: [PATCH 4/4] nfsd: fall back to buffered I/O when a direct write gets -EINVAL

[Christoph Hellwig]

On Tue, Sep 08, 2026 at 02:25:32PM -0400, Chuck Lever wrote:
> Per our discussion last October:
> 
>   https://lore.kernel.org/linux-nfs/aPXihwGTiA7bqTsN@infradead.org/
> 
> The conclusion then was that -EINVAL from ->write_iter can come from
> a number of conditions in the filesystem, so NFSD can't treat it as
> meaning only that the I/O was misaligned. That still holds, so I'd
> rather not use -EINVAL to signal a retry. An -EINVAL that really is
> the filesystem rejecting the request would now cost a second full
> write attempt before surfacing anyway.

Exactly.

> What would help me understand the failure even better:
> 
> - Which -EINVAL in bio_split_io_at() fired: the per-bvec dma_alignment
>   test, or the zero-length result after ALIGN_DOWN()?
> 
> - On the reproducer, how does stx_dio_offset_align compare with the
>   queue's logical_block_size?
> 
> If there turn out to be cases the gate can't predict from the statx
> data, that seems like a question for the block and fs folks about
> what error the filesystem should surface, rather than something to
> work around in NFSD.

We shouldn't have such a case, but if we have we need to have a clear
API to discover it.  Note that Keith has been looking into a proper
uapi to discover all the alignment pitfalls, which I hope we can
land rather sooner or later.  nfsd could consume it the same way
as userspace, and we could write userspace reproducer for any
corner cases we need to clarify.