* [PATCH v3 1/7] block: use iomap_dirty_folio for block devices
2026-09-09 21:59 [PATCH v3 0/7] block device fixes for large block sizes, IOCB_NOWAIT, and direct I/O Tal Zussman
@ 2026-09-09 21:59 ` Tal Zussman
2026-09-09 21:59 ` [PATCH v3 2/7] block: take i_rwsem for the direct I/O write fallback Tal Zussman
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Tal Zussman @ 2026-09-09 21:59 UTC (permalink / raw)
To: Jens Axboe, Christoph Hellwig, Johannes Thumshirn,
Luis Chamberlain, Hannes Reinecke, Matthew Wilcox (Oracle),
John Garry, Christian Brauner, Darrick J. Wong, Keith Busch,
Martin K. Petersen
Cc: linux-block, linux-kernel, Sashiko, Tal Zussman
With CONFIG_BUFFER_HEAD=n, block devices are written back through iomap,
but def_blk_aops uses filemap_dirty_folio, which only sets PG_dirty. It
does not set the per-block dirty bits in the folio's iomap_folio_state,
so iomap_writeback_folio() finds no dirty range, submits no I/O and
clears PG_dirty, resulting in data loss.
Other iomap users set .dirty_folio to iomap_dirty_folio, which marks the
folio's blocks dirty before calling filemap_dirty_folio().
This is only observable with block size < folio size. With a single
block there is no iomap_folio_state to get out of sync and
iomap_writeback_folio() marks the whole folio dirty itself. For a
page-aligned device, this may require using the BLKBSZSET ioctl to set
the block size, which requires CAP_SYS_ADMIN. A device whose size is not
page aligned already gets a sub-page block size from
set_init_blocksize(), so no ioctl and no privilege is needed.
To reproduce, on a device with a sub-page block size, write a known
pattern with O_DIRECT, mmap the same range, store to it, msync() and
fsync(), then read it back with O_DIRECT. A reproducer is available at
[1].
[1] https://gist.github.com/tzussman/18ab05cba4b3fdc79cce0a69d1fd05b4
Fixes: 925c86a19bac ("fs: add CONFIG_BUFFER_HEAD")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260730-blk-dontcache-v7-0-3e8e6850068d%40columbia.edu?part=5
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
block/fops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/fops.c b/block/fops.c
index 2ce7c6c4714e..c57784773fe1 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -560,7 +560,7 @@ static int blkdev_writepages(struct address_space *mapping,
}
const struct address_space_operations def_blk_aops = {
- .dirty_folio = filemap_dirty_folio,
+ .dirty_folio = iomap_dirty_folio,
.release_folio = iomap_release_folio,
.invalidate_folio = iomap_invalidate_folio,
.read_folio = blkdev_read_folio,
--
2.39.5
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 2/7] block: take i_rwsem for the direct I/O write fallback
2026-09-09 21:59 [PATCH v3 0/7] block device fixes for large block sizes, IOCB_NOWAIT, and direct I/O Tal Zussman
2026-09-09 21:59 ` [PATCH v3 1/7] block: use iomap_dirty_folio for block devices Tal Zussman
@ 2026-09-09 21:59 ` Tal Zussman
2026-09-09 21:59 ` [PATCH v3 3/7] block: take i_rwsem for the splice read path Tal Zussman
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Tal Zussman @ 2026-09-09 21:59 UTC (permalink / raw)
To: Jens Axboe, Christoph Hellwig, Johannes Thumshirn,
Luis Chamberlain, Hannes Reinecke, Matthew Wilcox (Oracle),
John Garry, Christian Brauner, Darrick J. Wong, Keith Busch,
Martin K. Petersen
Cc: linux-block, linux-kernel, Sashiko, Tal Zussman
Commit c0e473a0d226 ("block: fix race between set_blocksize and read
paths") closed a race between set_blocksize() and block device I/O: with
large sector size support, set_blocksize() can change i_blkbits and the
mapping's minimum folio order while a concurrent reader still holds a
folio of the old, smaller order, leading to crashes. In particular, it
made blkdev_write_iter() wrap buffered writes in inode_lock_shared().
However, the direct I/O fallback path was missed in that conversion.
blkdev_write_iter() passes blkdev_buffered_write() as an argument to
direct_write_fallback() with no lock held. A direct write that completes
only partially then finishes as a buffered write with no protection.
This can cause a BUG by racing partial direct writes against
ioctl(BLKBSZSET). Writer threads issue O_DIRECT pwritev() with a
two-segment iovec whose second segment is an unreadable PROT_NONE
mapping. The direct path then writes the first segment, fails to pin the
second, and returns short, entering the fallback. A second thread keeps
toggling the second segment's protection so that some fallbacks get past
fault_in_iov_iter_readable() and reach the page cache, a third thread
populates the page cache with folios of the current block size via
pread() and readahead(), and a fourth thread toggles the block size
between 512 bytes and 64K with BLKBSZSET. The minimum folio order only
moves with block sizes above PAGE_SIZE, i.e. with
CONFIG_TRANSPARENT_HUGEPAGE raising BLK_MAX_BLOCK_SIZE to 64K.
On a CONFIG_DEBUG_VM kernel this yields the following BUG:
page dumped because: VM_BUG_ON_FOLIO(folio_order(folio) < mapping_min_folio_order(mapping))
kernel BUG at mm/filemap.c:858!
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
RIP: 0010:__filemap_add_folio+0x860/0x8d0
Call Trace:
filemap_add_folio+0xc9/0x1f0
__filemap_get_folio_mpol+0x240/0x660
iomap_write_begin+0xa87/0xd70
iomap_file_buffered_write+0x304/0x6a0
blkdev_write_iter+0x255/0x510
do_iter_readv_writev+0x23d/0x3c0
vfs_writev+0x211/0x7d0
do_pwritev+0x121/0x190
do_syscall_64+0x121/0x630
entry_SYSCALL_64_after_hwframe+0x77/0x7f
The same workload also trips WARN_ON_ONCE(pos >= folio_pos(folio) +
fsize) in iomap_trim_folio_range().
Fix this by calling blkdev_buffered_write() in the fallback path under
inode_lock_shared(), matching the plain buffered-write branch. With the
fix the same workload runs clean.
A short IOCB_NOWAIT direct write reaches the same fallback. Taking
i_rwsem there can now block behind set_blocksize(), and the fallback
already blocks on writeback of the data it copied in
direct_write_fallback(). blkdev_write_iter() already rejects a purely
buffered IOCB_NOWAIT write with -EOPNOTSUPP, so do not enter the
fallback for IOCB_NOWAIT at all: return the bytes the direct path
already wrote, or -EAGAIN if none, and let the caller retry.
The reproducer used was written by an LLM, and is available at [1].
[1] https://gist.github.com/tzussman/69d06bc57d42a42989eb038b1b5aeb74
Fixes: 3c20917120ce ("block/bdev: enable large folio support for large logical block sizes")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260730-blk-dontcache-v7-0-3e8e6850068d%40columbia.edu?part=5
Assisted-by: Claude:claude-fable-5
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
block/fops.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/block/fops.c b/block/fops.c
index c57784773fe1..d5f569333f46 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -765,9 +765,26 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
if (iocb->ki_flags & IOCB_DIRECT) {
ret = blkdev_direct_write(iocb, from);
- if (ret >= 0 && iov_iter_count(from))
- ret = direct_write_fallback(iocb, from, ret,
- blkdev_buffered_write(iocb, from));
+ if (ret >= 0 && iov_iter_count(from)) {
+ if (iocb->ki_flags & IOCB_NOWAIT) {
+ /*
+ * The buffered fallback blocks on i_rwsem and
+ * on writeback of the data it copied: return
+ * the short direct write instead and let the
+ * caller retry.
+ */
+ if (!ret)
+ ret = -EAGAIN;
+ } else {
+ ssize_t ret2;
+
+ inode_lock_shared(bd_inode);
+ ret2 = blkdev_buffered_write(iocb, from);
+ inode_unlock_shared(bd_inode);
+ ret = direct_write_fallback(iocb, from, ret,
+ ret2);
+ }
+ }
} else {
/*
* Take i_rwsem and invalidate_lock to avoid racing with
--
2.39.5
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 3/7] block: take i_rwsem for the splice read path
2026-09-09 21:59 [PATCH v3 0/7] block device fixes for large block sizes, IOCB_NOWAIT, and direct I/O Tal Zussman
2026-09-09 21:59 ` [PATCH v3 1/7] block: use iomap_dirty_folio for block devices Tal Zussman
2026-09-09 21:59 ` [PATCH v3 2/7] block: take i_rwsem for the direct I/O write fallback Tal Zussman
@ 2026-09-09 21:59 ` Tal Zussman
2026-09-09 21:59 ` [PATCH v3 4/7] block: honor IOCB_NOWAIT in the block device buffered " Tal Zussman
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Tal Zussman @ 2026-09-09 21:59 UTC (permalink / raw)
To: Jens Axboe, Christoph Hellwig, Johannes Thumshirn,
Luis Chamberlain, Hannes Reinecke, Matthew Wilcox (Oracle),
John Garry, Christian Brauner, Darrick J. Wong, Keith Busch,
Martin K. Petersen
Cc: linux-block, linux-kernel, Tal Zussman
def_blk_fops wires ->splice_read directly to filemap_splice_read(),
which allocates folios based on mapping_min_folio_order() without any
lock against set_blocksize(). A splice from a block device can race
set_blocksize() raising the minimum folio order and insert a folio that
is too small for the mapping. blkdev_read_iter() wraps filemap_read()
in inode_lock_shared() for this reason, but the splice path was missed.
Splicing from a block device while toggling the block size between 512
bytes and 64K with BLKBSZSET hits this within seconds on a
CONFIG_DEBUG_VM kernel:
page dumped because: VM_BUG_ON_FOLIO(folio_order(folio) < mapping_min_folio_order(mapping))
kernel BUG at mm/filemap.c:858!
Oops: invalid opcode: 0000 [#1] SMP NOPTI
RIP: 0010:__filemap_add_folio+0x51c/0x570
Call Trace:
filemap_add_folio+0x64/0x140
page_cache_ra_order+0x1dd/0x3d0
filemap_get_pages+0x153/0x760
filemap_splice_read+0x13f/0x300
splice_file_to_pipe+0xc0/0xd0
do_splice+0x6a8/0x890
__do_splice+0xb0/0x210
__x64_sys_splice+0x80/0x100
do_syscall_64+0x10e/0x520
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Take inode_lock_shared() around filemap_splice_read(), like the read
path does.
Fixes: 3c20917120ce ("block/bdev: enable large folio support for large logical block sizes")
Assisted-by: Claude:claude-fable-5
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
block/fops.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/block/fops.c b/block/fops.c
index d5f569333f46..a51814821100 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -855,6 +855,22 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
return ret;
}
+/*
+ * Take i_rwsem to avoid racing with set_blocksize changing i_blkbits/folio
+ * order and punching out the pagecache.
+ */
+static ssize_t blkdev_splice_read(struct file *in, loff_t *ppos,
+ struct pipe_inode_info *pipe, size_t len, unsigned int flags)
+{
+ struct inode *bd_inode = bdev_file_inode(in);
+ ssize_t ret;
+
+ inode_lock_shared(bd_inode);
+ ret = filemap_splice_read(in, ppos, pipe, len, flags);
+ inode_unlock_shared(bd_inode);
+ return ret;
+}
+
#define BLKDEV_FALLOC_FL_SUPPORTED \
(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | \
FALLOC_FL_ZERO_RANGE | FALLOC_FL_WRITE_ZEROES)
@@ -956,7 +972,7 @@ const struct file_operations def_blk_fops = {
#ifdef CONFIG_COMPAT
.compat_ioctl = compat_blkdev_ioctl,
#endif
- .splice_read = filemap_splice_read,
+ .splice_read = blkdev_splice_read,
.splice_write = iter_file_splice_write,
.fallocate = blkdev_fallocate,
.uring_cmd = blkdev_uring_cmd,
--
2.39.5
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 4/7] block: honor IOCB_NOWAIT in the block device buffered read path
2026-09-09 21:59 [PATCH v3 0/7] block device fixes for large block sizes, IOCB_NOWAIT, and direct I/O Tal Zussman
` (2 preceding siblings ...)
2026-09-09 21:59 ` [PATCH v3 3/7] block: take i_rwsem for the splice read path Tal Zussman
@ 2026-09-09 21:59 ` Tal Zussman
2026-09-09 22:05 ` [PATCH v3 7/7] block: remove dead metadata handling from the async direct I/O path Tal Zussman
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Tal Zussman @ 2026-09-09 21:59 UTC (permalink / raw)
To: Jens Axboe, Christoph Hellwig, Johannes Thumshirn,
Luis Chamberlain, Hannes Reinecke, Matthew Wilcox (Oracle),
John Garry, Christian Brauner, Darrick J. Wong, Keith Busch,
Martin K. Petersen
Cc: linux-block, linux-kernel, Sashiko, Tal Zussman
blkdev_read_iter() takes inode_lock_shared() unconditionally around
filemap_read(). Unlike blkdev_write_iter(), it does not reject
IOCB_NOWAIT for buffered I/O, so a non-blocking read, or the buffered
tail of a short IOCB_NOWAIT direct read, blocks behind set_blocksize()
holding i_rwsem across sync_blockdev().
A preadv2(RWF_NOWAIT) issued while another thread changes the block
size with a dirty page cache blocks for as long as sync_blockdev()
takes, 4 to 6 seconds on a scsi_debug device with delay=5.
Use inode_trylock_shared() for IOCB_NOWAIT and return the bytes the
direct path already read, or -EAGAIN if none, when the lock is
contended, preserving NOWAIT semantics.
Fixes: c0e473a0d226 ("block: fix race between set_blocksize and read paths")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260802-blkdev-fixes-v1-0-a82fc549fd74%40columbia.edu?part=2
Assisted-by: Claude:claude-fable-5
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
block/fops.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/block/fops.c b/block/fops.c
index a51814821100..a3a709697b40 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -845,7 +845,15 @@ static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
* Take i_rwsem and invalidate_lock to avoid racing with set_blocksize
* changing i_blkbits/folio order and punching out the pagecache.
*/
- inode_lock_shared(bd_inode);
+ if (iocb->ki_flags & IOCB_NOWAIT) {
+ if (!inode_trylock_shared(bd_inode)) {
+ if (!ret)
+ ret = -EAGAIN;
+ goto reexpand;
+ }
+ } else {
+ inode_lock_shared(bd_inode);
+ }
ret = filemap_read(iocb, to, ret);
inode_unlock_shared(bd_inode);
--
2.39.5
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 7/7] block: remove dead metadata handling from the async direct I/O path
2026-09-09 21:59 [PATCH v3 0/7] block device fixes for large block sizes, IOCB_NOWAIT, and direct I/O Tal Zussman
` (3 preceding siblings ...)
2026-09-09 21:59 ` [PATCH v3 4/7] block: honor IOCB_NOWAIT in the block device buffered " Tal Zussman
@ 2026-09-09 22:05 ` Tal Zussman
2026-09-10 10:40 ` Hannes Reinecke
2026-09-09 22:05 ` [PATCH v3 5/7] block: fail atomic writes instead of falling back to buffered I/O Tal Zussman
2026-09-09 22:05 ` [PATCH v3 6/7] block: unpin all pages of a bvec in bio_iov_iter_align_down() Tal Zussman
6 siblings, 1 reply; 10+ messages in thread
From: Tal Zussman @ 2026-09-09 22:05 UTC (permalink / raw)
To: Jens Axboe, Christoph Hellwig, Johannes Thumshirn,
Luis Chamberlain, Hannes Reinecke, Matthew Wilcox (Oracle),
John Garry, Christian Brauner, Darrick J. Wong, Keith Busch,
Martin K. Petersen
Cc: linux-block, linux-kernel, Sashiko, Tal Zussman
Since commit 2729a60bbfb9 ("block: don't silently ignore metadata for
sync read/write"), blkdev_direct_IO() sends every IOCB_HAS_METADATA
request to __blkdev_direct_IO(). Remove the now unreachable metadata
mapping in __blkdev_direct_IO_async() and the unmap in its completion
handler.
No functional change.
Assisted-by: Claude:claude-fable-5
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
block/fops.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/block/fops.c b/block/fops.c
index 0b614d76d128..6a86bffafa7a 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -306,9 +306,6 @@ static void blkdev_bio_end_io_async(struct bio *bio)
ret = blk_status_to_errno(bio->bi_status);
}
- if (bio_integrity(bio))
- bio_integrity_unmap_user(bio);
-
iocb->ki_complete(iocb, ret);
if (dio->flags & DIO_SHOULD_DIRTY) {
@@ -366,13 +363,6 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
task_io_account_write(bio->bi_iter.bi_size);
}
- if (iocb->ki_flags & IOCB_HAS_METADATA) {
- ret = bio_integrity_map_iter(bio, iocb->private);
- WRITE_ONCE(iocb->private, NULL);
- if (unlikely(ret))
- goto out_bio_put;
- }
-
if (iocb->ki_flags & IOCB_NOWAIT)
bio->bi_opf |= REQ_NOWAIT;
--
2.39.5
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 7/7] block: remove dead metadata handling from the async direct I/O path
2026-09-09 22:05 ` [PATCH v3 7/7] block: remove dead metadata handling from the async direct I/O path Tal Zussman
@ 2026-09-10 10:40 ` Hannes Reinecke
0 siblings, 0 replies; 10+ messages in thread
From: Hannes Reinecke @ 2026-09-10 10:40 UTC (permalink / raw)
To: Tal Zussman, Jens Axboe, Christoph Hellwig, Johannes Thumshirn,
Luis Chamberlain, Hannes Reinecke, Matthew Wilcox (Oracle),
John Garry, Christian Brauner, Darrick J. Wong, Keith Busch,
Martin K. Petersen
Cc: linux-block, linux-kernel, Sashiko
On 9/10/26 12:05 AM, Tal Zussman wrote:
> Since commit 2729a60bbfb9 ("block: don't silently ignore metadata for
> sync read/write"), blkdev_direct_IO() sends every IOCB_HAS_METADATA
> request to __blkdev_direct_IO(). Remove the now unreachable metadata
> mapping in __blkdev_direct_IO_async() and the unmap in its completion
> handler.
>
> No functional change.
>
> Assisted-by: Claude:claude-fable-5
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
> block/fops.c | 10 ----------
> 1 file changed, 10 deletions(-)
>
Maybe a 'Fixes' tag, since you already referenced the commit?
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 5/7] block: fail atomic writes instead of falling back to buffered I/O
2026-09-09 21:59 [PATCH v3 0/7] block device fixes for large block sizes, IOCB_NOWAIT, and direct I/O Tal Zussman
` (4 preceding siblings ...)
2026-09-09 22:05 ` [PATCH v3 7/7] block: remove dead metadata handling from the async direct I/O path Tal Zussman
@ 2026-09-09 22:05 ` Tal Zussman
2026-09-10 6:40 ` John Garry
2026-09-09 22:05 ` [PATCH v3 6/7] block: unpin all pages of a bvec in bio_iov_iter_align_down() Tal Zussman
6 siblings, 1 reply; 10+ messages in thread
From: Tal Zussman @ 2026-09-09 22:05 UTC (permalink / raw)
To: Jens Axboe, Christoph Hellwig, Johannes Thumshirn,
Luis Chamberlain, Hannes Reinecke, Matthew Wilcox (Oracle),
John Garry, Christian Brauner, Darrick J. Wong, Keith Busch,
Martin K. Petersen
Cc: linux-block, linux-kernel, Sashiko, Tal Zussman
An IOCB_ATOMIC direct write to a block device can silently lose its
torn-write guarantee in two ways:
1. blkdev_direct_write() turns an -EBUSY from page cache invalidation
into a 0 return, so the whole write is retried through
blkdev_buffered_write(), with no atomicity guarantee.
2. On a partial page pin, __blkdev_direct_IO_simple() and
__blkdev_direct_IO_async() submit what was pinned with REQ_ATOMIC
set and leave the rest to the buffered fallback.
The second case can be triggered deterministically. A 16K
pwritev2(RWF_ATOMIC) whose last page is PROT_NONE, on a scsi_debug
device with atomic_wr=1, completes short with only three of the four
pages written, violating RWF_ATOMIC semantics.
Fail the I/O instead. Make bio_iov_iter_get_pages() release the pins
and return -EINVAL when a REQ_ATOMIC bio doesn't cover the whole
iterator, since an atomic write is submitted as a single bio and a
short one would be torn. That covers iomap as well, where a partially
unmapped buffer could trip the WARN_ON_ONCE() in
iomap_dio_bio_iter_one(). The async block device path currently sets
REQ_ATOMIC after pinning, so set it before.
Skip the buffered fallback in blkdev_write_iter() for IOCB_ATOMIC, as
it already does for IOCB_NOWAIT, so the -EBUSY case returns -EAGAIN and
the caller retries, matching __iomap_dio_rw().
ext4 has the same fallback and only warns in it. For block devices both
ways in can be detected before any I/O is submitted, so fail early instead.
Fixes: caf336f81b3a ("block: Add fops atomic write support")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260802-blkdev-fixes-v1-0-a82fc549fd74%40columbia.edu?part=2
Assisted-by: Claude:claude-fable-5
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
block/bio.c | 29 ++++++++++++++++++++++-------
block/fops.c | 10 +++++-----
2 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 898b2f5ef8c8..63e266d861f1 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1284,6 +1284,7 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter,
unsigned mem_align_mask, unsigned len_align_mask)
{
iov_iter_extraction_t flags = 0;
+ int ret;
if (WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED)))
return -EIO;
@@ -1303,34 +1304,48 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter,
flags |= ITER_ALLOW_P2PDMA;
do {
- ssize_t ret;
+ ssize_t len;
- ret = iov_iter_extract_bvecs(iter, bio->bi_io_vec,
+ len = iov_iter_extract_bvecs(iter, bio->bi_io_vec,
BIO_MAX_SIZE - bio->bi_iter.bi_size,
&bio->bi_vcnt, bio->bi_max_vecs,
mem_align_mask, flags);
- if (ret <= 0) {
+ if (len <= 0) {
/*
* A misaligned vector fails the whole I/O. Release any
* pages pinned by earlier iterations before returning
* since this bio won't be submitted to release them.
*/
- if (ret == -EINVAL) {
+ if (len == -EINVAL) {
bio_release_pages(bio, false);
bio_clear_flag(bio, BIO_PAGE_PINNED);
bio->bi_vcnt = 0;
}
if (!bio->bi_vcnt)
- return ret;
+ return len;
break;
}
- bio->bi_iter.bi_size += ret;
+ bio->bi_iter.bi_size += len;
} while (iov_iter_count(iter) && !bio_full(bio, 0));
if (is_pci_p2pdma_page(bio->bi_io_vec->bv_page))
bio->bi_opf |= REQ_NOMERGE;
- return bio_iov_iter_align_down(bio, iter,
+ ret = bio_iov_iter_align_down(bio, iter,
&bio->bi_io_vec[bio->bi_vcnt - 1], len_align_mask);
+ if (ret)
+ return ret;
+
+ /*
+ * An atomic write is submitted as a single bio, so it has to cover
+ * the whole iterator or it would be torn.
+ */
+ if ((bio->bi_opf & REQ_ATOMIC) && iov_iter_count(iter)) {
+ bio_release_pages(bio, false);
+ bio_clear_flag(bio, BIO_PAGE_PINNED);
+ bio->bi_vcnt = 0;
+ return -EINVAL;
+ }
+ return 0;
}
static struct folio *folio_alloc_greedy(gfp_t gfp, size_t *size,
diff --git a/block/fops.c b/block/fops.c
index a3a709697b40..0b614d76d128 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -341,6 +341,8 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
bio->bi_write_stream = iocb->ki_write_stream;
bio->bi_end_io = blkdev_bio_end_io_async;
bio->bi_ioprio = iocb->ki_ioprio;
+ if (iocb->ki_flags & IOCB_ATOMIC)
+ bio->bi_opf |= REQ_ATOMIC;
/*
* Users don't rely on the iterator being in any particular
@@ -371,9 +373,6 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
goto out_bio_put;
}
- if (iocb->ki_flags & IOCB_ATOMIC)
- bio->bi_opf |= REQ_ATOMIC;
-
if (iocb->ki_flags & IOCB_NOWAIT)
bio->bi_opf |= REQ_NOWAIT;
@@ -766,10 +765,11 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
if (iocb->ki_flags & IOCB_DIRECT) {
ret = blkdev_direct_write(iocb, from);
if (ret >= 0 && iov_iter_count(from)) {
- if (iocb->ki_flags & IOCB_NOWAIT) {
+ if (iocb->ki_flags & (IOCB_NOWAIT | IOCB_ATOMIC)) {
/*
* The buffered fallback blocks on i_rwsem and
- * on writeback of the data it copied: return
+ * on writeback of the data it copied, and
+ * can't provide torn-write protection: return
* the short direct write instead and let the
* caller retry.
*/
--
2.39.5
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 5/7] block: fail atomic writes instead of falling back to buffered I/O
2026-09-09 22:05 ` [PATCH v3 5/7] block: fail atomic writes instead of falling back to buffered I/O Tal Zussman
@ 2026-09-10 6:40 ` John Garry
0 siblings, 0 replies; 10+ messages in thread
From: John Garry @ 2026-09-10 6:40 UTC (permalink / raw)
To: Tal Zussman, Jens Axboe, Christoph Hellwig, Johannes Thumshirn,
Luis Chamberlain, Hannes Reinecke, Matthew Wilcox (Oracle),
John Garry, Christian Brauner, Darrick J. Wong, Keith Busch,
Martin K. Petersen
Cc: linux-block, linux-kernel, Sashiko
On 9/9/26 23:05, Tal Zussman wrote:
> An IOCB_ATOMIC direct write to a block device can silently lose its
> torn-write guarantee in two ways:
>
> 1. blkdev_direct_write() turns an -EBUSY from page cache invalidation
> into a 0 return, so the whole write is retried through
> blkdev_buffered_write(), with no atomicity guarantee.
>
> 2. On a partial page pin, __blkdev_direct_IO_simple() and
> __blkdev_direct_IO_async() submit what was pinned with REQ_ATOMIC
> set and leave the rest to the buffered fallback.
This really should be 2x separate changes - 1x for fops.c and 1x for bio.c
>
> The second case can be triggered deterministically. A 16K
> pwritev2(RWF_ATOMIC) whose last page is PROT_NONE, on a scsi_debug
> device with atomic_wr=1, completes short with only three of the four
> pages written, violating RWF_ATOMIC semantics.
>
> Fail the I/O instead. Make bio_iov_iter_get_pages() release the pins
> and return -EINVAL when a REQ_ATOMIC bio doesn't cover the whole
> iterator, since an atomic write is submitted as a single bio and a
> short one would be torn. That covers iomap as well, where a partially
> unmapped buffer could trip the WARN_ON_ONCE() in
> iomap_dio_bio_iter_one(). The async block device path currently sets
> REQ_ATOMIC after pinning, so set it before.
>
> Skip the buffered fallback in blkdev_write_iter() for IOCB_ATOMIC, as
> it already does for IOCB_NOWAIT, so the -EBUSY case returns -EAGAIN and
> the caller retries, matching __iomap_dio_rw().
>
> ext4 has the same fallback and only warns in it. For block devices both
> ways in can be detected before any I/O is submitted, so fail early instead.
>
> Fixes: caf336f81b3a ("block: Add fops atomic write support")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260802-blkdev-fixes-v1-0-a82fc549fd74%40columbia.edu?part=2
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
> block/bio.c | 29 ++++++++++++++++++++++-------
> block/fops.c | 10 +++++-----
> 2 files changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index 898b2f5ef8c8..63e266d861f1 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1284,6 +1284,7 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter,
> unsigned mem_align_mask, unsigned len_align_mask)
> {
> iov_iter_extraction_t flags = 0;
> + int ret;
>
> if (WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED)))
> return -EIO;
> @@ -1303,34 +1304,48 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter,
> flags |= ITER_ALLOW_P2PDMA;
>
> do {
> - ssize_t ret;
> + ssize_t len;
iov_iter_extract_bvecs() local variable is called "size", so maybe use
the same here
>
> - ret = iov_iter_extract_bvecs(iter, bio->bi_io_vec,
> + len = iov_iter_extract_bvecs(iter, bio->bi_io_vec,
> BIO_MAX_SIZE - bio->bi_iter.bi_size,
> &bio->bi_vcnt, bio->bi_max_vecs,
> mem_align_mask, flags);
> - if (ret <= 0) {
> + if (len <= 0) {
> /*
> * A misaligned vector fails the whole I/O. Release any
> * pages pinned by earlier iterations before returning
> * since this bio won't be submitted to release them.
> */
> - if (ret == -EINVAL) {
> + if (len == -EINVAL) {
> bio_release_pages(bio, false);
> bio_clear_flag(bio, BIO_PAGE_PINNED);
> bio->bi_vcnt = 0;
> }
> if (!bio->bi_vcnt)
> - return ret;
> + return len;
> break;
> }
> - bio->bi_iter.bi_size += ret;
> + bio->bi_iter.bi_size += len;
> } while (iov_iter_count(iter) && !bio_full(bio, 0));
>
> if (is_pci_p2pdma_page(bio->bi_io_vec->bv_page))
> bio->bi_opf |= REQ_NOMERGE;
> - return bio_iov_iter_align_down(bio, iter,
> + ret = bio_iov_iter_align_down(bio, iter,
> &bio->bi_io_vec[bio->bi_vcnt - 1], len_align_mask);
> + if (ret)
> + return ret;
> +> + /*
> + * An atomic write is submitted as a single bio, so it has to cover
> + * the whole iterator or it would be torn.
> + */
> + if ((bio->bi_opf & REQ_ATOMIC) && iov_iter_count(iter)) {
> + bio_release_pages(bio, false);
> + bio_clear_flag(bio, BIO_PAGE_PINNED);
> + bio->bi_vcnt = 0;
> + return -EINVAL;
> + }
This all looks ok, but I'll check again ...
> + return 0;
> }
iomap_dio_bio_iter_one() can be updated at some stage to remove its own
check for improper length returned from bio_iov_iter_get_pages() for
IOCB_ATOMIC
>
> static struct folio *folio_alloc_greedy(gfp_t gfp, size_t *size,
> diff --git a/block/fops.c b/block/fops.c
> index a3a709697b40..0b614d76d128 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -341,6 +341,8 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
> bio->bi_write_stream = iocb->ki_write_stream;
> bio->bi_end_io = blkdev_bio_end_io_async;
> bio->bi_ioprio = iocb->ki_ioprio;
> + if (iocb->ki_flags & IOCB_ATOMIC)
> + bio->bi_opf |= REQ_ATOMIC;
>
> /*
> * Users don't rely on the iterator being in any particular
> @@ -371,9 +373,6 @@ static ssize_t __blkdev_direct_IO_async(struct kiocb *iocb,
> goto out_bio_put;
> }
>
> - if (iocb->ki_flags & IOCB_ATOMIC)
> - bio->bi_opf |= REQ_ATOMIC;
> -
> if (iocb->ki_flags & IOCB_NOWAIT)
> bio->bi_opf |= REQ_NOWAIT;
I think that you relocate this as well to have similar functionality
co-located
>
> @@ -766,10 +765,11 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
> if (iocb->ki_flags & IOCB_DIRECT) {
> ret = blkdev_direct_write(iocb, from);
> if (ret >= 0 && iov_iter_count(from)) {
> - if (iocb->ki_flags & IOCB_NOWAIT) {
> + if (iocb->ki_flags & (IOCB_NOWAIT | IOCB_ATOMIC)) {
An alternative could be to have iomap_file_buffered_write() reject
IOCB_ATOMIC.
> /*
> * The buffered fallback blocks on i_rwsem and
> - * on writeback of the data it copied: return
> + * on writeback of the data it copied, and
> + * can't provide torn-write protection: return
> * the short direct write instead and let the
> * caller retry.
> */
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 6/7] block: unpin all pages of a bvec in bio_iov_iter_align_down()
2026-09-09 21:59 [PATCH v3 0/7] block device fixes for large block sizes, IOCB_NOWAIT, and direct I/O Tal Zussman
` (5 preceding siblings ...)
2026-09-09 22:05 ` [PATCH v3 5/7] block: fail atomic writes instead of falling back to buffered I/O Tal Zussman
@ 2026-09-09 22:05 ` Tal Zussman
6 siblings, 0 replies; 10+ messages in thread
From: Tal Zussman @ 2026-09-09 22:05 UTC (permalink / raw)
To: Jens Axboe, Christoph Hellwig, Johannes Thumshirn,
Luis Chamberlain, Hannes Reinecke, Matthew Wilcox (Oracle),
John Garry, Christian Brauner, Darrick J. Wong, Keith Busch,
Martin K. Petersen
Cc: linux-block, linux-kernel, Sashiko, Tal Zussman
bio_iov_iter_align_down() drops trailing bvecs with unpin_user_page(),
but a bvec built by iov_iter_extract_bvecs() can span several pages of
one folio, each with its own pin. All but the first pin leak.
The partially trimmed bvec has the same problem. Shrinking bv_len does
not release the pins for the pages cut off by the trim, and
__bio_release_pages() only unpins the pages bv_len still covers at
completion.
Both issues occur only with a logical block size above PAGE_SIZE and a
large folio backing the user buffer. On a device with a 64K logical
block size, an O_DIRECT pwritev() from a hugetlb mapping that ends 16K
past a block boundary leaks one huge page per call, whether the
remainder is its own bvec or the tail of a larger one.
Unpin all pages of a dropped bvec with bvec_unpin(), and unpin the
pages trimmed off the last bvec as well. Move bvec_unpin() up and split
its page count into a helper so both sites share it.
Fixes: 20a0e6276edb ("block: align the bio after building it")
Assisted-by: Claude:claude-fable-5
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
block/bio.c | 38 ++++++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 63e266d861f1..521c362ae9bf 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1196,6 +1196,21 @@ bool bio_iov_iter_set(struct bio *bio, const struct iov_iter *iter)
return true;
}
+static unsigned int bvec_nr_pages(const struct bio_vec *bv)
+{
+ return (bv->bv_offset + bv->bv_len - 1) / PAGE_SIZE -
+ bv->bv_offset / PAGE_SIZE + 1;
+}
+
+static void bvec_unpin(struct bio_vec *bv, bool mark_dirty)
+{
+ struct folio *folio = bvec_folio(bv);
+
+ if (mark_dirty)
+ folio_mark_dirty_lock(folio);
+ unpin_user_folio(folio, bvec_nr_pages(bv));
+}
+
/*
* Aligns the bio size to the len_align_mask, releasing excessive bio vecs that
* __bio_iov_iter_get_pages may have inserted, and reverts the trimmed length
@@ -1205,6 +1220,7 @@ static int bio_iov_iter_align_down(struct bio *bio, struct iov_iter *iter,
struct bio_vec *bv, unsigned len_align_mask)
{
size_t nbytes = bio->bi_iter.bi_size & len_align_mask;
+ unsigned int npages;
if (!nbytes)
return 0;
@@ -1213,14 +1229,23 @@ static int bio_iov_iter_align_down(struct bio *bio, struct iov_iter *iter,
bio->bi_iter.bi_size -= nbytes;
while (nbytes >= bv->bv_len) {
if (bio_flagged(bio, BIO_PAGE_PINNED))
- unpin_user_page(bv->bv_page);
+ bvec_unpin(bv, false);
if (!--bio->bi_vcnt)
return -EFAULT;
nbytes -= bv->bv_len;
bv--;
}
+
+ /*
+ * __bio_release_pages() only unpins the pages still covered by
+ * bv_len, so drop the pins for the pages trimmed off here.
+ */
+ npages = bvec_nr_pages(bv);
bv->bv_len -= nbytes;
+ npages -= bvec_nr_pages(bv);
+ if (npages && bio_flagged(bio, BIO_PAGE_PINNED))
+ unpin_user_folio(bvec_folio(bv), npages);
return 0;
}
@@ -1503,17 +1528,6 @@ int bio_iov_iter_bounce(struct bio *bio, struct iov_iter *iter, size_t maxlen,
return bio_iov_iter_bounce_read(bio, iter, maxlen, minsize);
}
-static void bvec_unpin(struct bio_vec *bv, bool mark_dirty)
-{
- struct folio *folio = bvec_folio(bv);
- size_t nr_pages = (bv->bv_offset + bv->bv_len - 1) / PAGE_SIZE -
- bv->bv_offset / PAGE_SIZE + 1;
-
- if (mark_dirty)
- folio_mark_dirty_lock(folio);
- unpin_user_folio(folio, nr_pages);
-}
-
static void bio_iov_iter_unbounce_read(struct bio *bio, bool is_error,
bool mark_dirty)
{
--
2.39.5
^ permalink raw reply [flat|nested] 10+ messages in thread