[PATCH v3] iomap: follow the alignment requirement for iomap_dio_hole_iter()

[PATCH v3] iomap: follow the alignment requirement for iomap_dio_hole_iter()

[Qu Wenruo]

[BUG]
On the latest development branch, btrfs with 8K block size on 4K page
sized systems will fail all test cases that run fsstress.

One very short example would be:

 # $fsstress -n 4 -d $mnt -s 1785675805 -v
 0/0: dwrite - no filename
 0/1: creat f0 x:0 0 0
 0/1: creat add id=0,parent=-1
 0/2: write dontcache f0[259 1 0 0 0 0] [816411,3620] 0
 0/3: dread - xfsctl(XFS_IOC_DIOINFO) f0[259 1 0 0 32 820031] return 25, fallback to stat()
 0/3: dread f0[259 1 0 0 32 820031] [483328,81920] 0

Which triggered the following ASSERT():

 assertion failed: IS_ALIGNED(state->start, blocksize) && IS_ALIGNED(state->end + 1, blocksize), in fs/btrfs/extent-io-tree.c:346 (unaligned extent state, blocksize=8192 start=487424 end=565247 state=0x0)
 ------------[ cut here ]------------
 kernel BUG at fs/btrfs/extent-io-tree.c:346!
 Oops: invalid opcode: 0000 [#1] SMP
 CPU: 4 UID: 0 PID: 648 Comm: fsstress Tainted: G            E       7.2.0-rc5-custom+ #431 PREEMPT(full)  7c507bd40d65e4d871e698b22d80f1306bbec543
 Tainted: [E]=UNSIGNED_MODULE
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022
 RIP: 0010:validate_extent_state.part.0.isra.0.cold+0x24/0x26 [btrfs]
 Call Trace:
  <TASK>
  insert_state+0x34/0x1a0 [btrfs 6924944583e4bc91a7c5183a1e7908e745a9b0c8]
  set_extent_bit+0x440/0x8d0 [btrfs 6924944583e4bc91a7c5183a1e7908e745a9b0c8]
  btrfs_lock_extent_bits+0x58/0x380 [btrfs 6924944583e4bc91a7c5183a1e7908e745a9b0c8]
  btrfs_dio_iomap_begin+0x319/0xb70 [btrfs 6924944583e4bc91a7c5183a1e7908e745a9b0c8]
  iomap_iter+0x1a2/0x370
  __iomap_dio_rw+0x236/0x8d0
  iomap_dio_rw+0x12/0x30
  btrfs_direct_read+0x15f/0x290 [btrfs 6924944583e4bc91a7c5183a1e7908e745a9b0c8]
  btrfs_file_read_iter+0x42/0x90 [btrfs 6924944583e4bc91a7c5183a1e7908e745a9b0c8]
  vfs_read+0x25e/0x380
  ksys_read+0x73/0xe0
  do_syscall_64+0xe1/0x790
  entry_SYSCALL_64_after_hwframe+0x4b/0x53
 RIP: 0033:0x7fc3f129318e
  </TASK>

[CAUSE]
iomap_dio_hole_iter() is responsible for zeroing out the buffer for a
hole, which calls iov_iter_zero() to zero the range.

However btrfs disables page fault during its __iomap_dio_rw() call, so
iov_iter_zero() can fail at any page boundary.

When the fs block size is larger than page size, iov_iter_zero() may
only have zeroed one page, which is not aligned to the fs block size.

Such one page long range is passed back to btrfs, which triggers the
above ASSERT().

[FIX]
For iomap_dio_hole_iter() round down the copied length, and revert
any excessive range that is beyond the aligned copied length.
Also use @aligned_copied for the size increment and iter advancement.

This should only affect btrfs, which is the only fs utilizing iomap dio
with page fault disabled.

Fixes: 001397f5ef49 ("iomap: add IOMAP_DIO_FSBLOCK_ALIGNED flag")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
v3:
- Fix a NULL pointer dereference where @bdev can be NULL for holes

v2:
- Rebased to the latest vfs tree

- Slightly rewords the reproducer
  In fact no special reproducer needed, any test case running fsstress
  can easily trigger it.

  The example provided is just the shortest sequence I used to debug the
  crash.
---
 fs/iomap/direct-io.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index b3368d64e81b..10f0020de477 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -409,7 +409,10 @@ static inline unsigned int iomap_dio_alignment(struct inode *inode,
 {
 	if (dio_flags & IOMAP_DIO_FSBLOCK_ALIGNED)
 		return i_blocksize(inode);
-	return bdev_logical_block_size(bdev);
+	/* @bdev can be NULL for hole cases. */
+	if (bdev)
+		return bdev_logical_block_size(bdev);
+	return SECTOR_SIZE;
 }
 
 static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
@@ -594,12 +597,20 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 
 static int iomap_dio_hole_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 {
-	loff_t length = iov_iter_zero(iomap_length(iter), dio->submit.iter);
+	loff_t copied = iov_iter_zero(iomap_length(iter), dio->submit.iter);
+	unsigned int bs = iomap_dio_alignment(iter->inode, iter->iomap.bdev,
+					      dio->flags);
+	loff_t aligned_copied = round_down(copied, bs);
 
-	dio->size += length;
-	if (!length)
+	/*
+	 * If fs block size is larger than page size, page fault failure
+	 * can cause @copied to be page aligned but not fs block aligned.
+	 */
+	iov_iter_revert(dio->submit.iter, copied - aligned_copied);
+	dio->size += aligned_copied;
+	if (!aligned_copied)
 		return -EFAULT;
-	return iomap_iter_advance(iter, length);
+	return iomap_iter_advance(iter, aligned_copied);
 }
 
 static int iomap_dio_inline_iter(struct iomap_iter *iomi, struct iomap_dio *dio)
-- 
2.54.0

Re: [PATCH v3] iomap: follow the alignment requirement for iomap_dio_hole_iter()

[Christoph Hellwig]

On Sat, Aug 01, 2026 at 07:17:26AM +0930, Qu Wenruo wrote:
> +++ b/fs/iomap/direct-io.c
> @@ -409,7 +409,10 @@ static inline unsigned int iomap_dio_alignment(struct inode *inode,
>  {
>  	if (dio_flags & IOMAP_DIO_FSBLOCK_ALIGNED)
>  		return i_blocksize(inode);
> -	return bdev_logical_block_size(bdev);
> +	/* @bdev can be NULL for hole cases. */
> +	if (bdev)
> +		return bdev_logical_block_size(bdev);
> +	return SECTOR_SIZE;

No.  As said for previous versions you should not use
iomap_dio_alignment.  There is no such things as sub-fsblock alignment
for holes.  All mappings need to be aligned on fs blocks, you can do
sub-mapping granularity I/O in it, but not align on it.  So don't
call iomap_dio_alignment, but instead align down the shortened range
from the fault to a fs block boundary.

And really sort out the btrfs locking that we can't get rid of this
premature faulting as it keeps messing up core code way too much.

Re: [PATCH v3] iomap: follow the alignment requirement for iomap_dio_hole_iter()

[Qu Wenruo]

在 2026/8/4 22:31, Christoph Hellwig 写道:
> On Sat, Aug 01, 2026 at 07:17:26AM +0930, Qu Wenruo wrote:
>> +++ b/fs/iomap/direct-io.c
>> @@ -409,7 +409,10 @@ static inline unsigned int iomap_dio_alignment(struct inode *inode,
>>   {
>>   	if (dio_flags & IOMAP_DIO_FSBLOCK_ALIGNED)
>>   		return i_blocksize(inode);
>> -	return bdev_logical_block_size(bdev);
>> +	/* @bdev can be NULL for hole cases. */
>> +	if (bdev)
>> +		return bdev_logical_block_size(bdev);
>> +	return SECTOR_SIZE;
> 
> No.  As said for previous versions you should not use
> iomap_dio_alignment.  There is no such things as sub-fsblock alignment
> for holes.  All mappings need to be aligned on fs blocks, you can do
> sub-mapping granularity I/O in it, but not align on it.  So don't
> call iomap_dio_alignment, but instead align down the shortened range
> from the fault to a fs block boundary.

OK, got it now.

> 
> And really sort out the btrfs locking that we can't get rid of this
> premature faulting as it keeps messing up core code way too much.

That will be a long term work, meanwhile I'm only fixing a short-term 
bug, already affecting existing bs > ps support.

I hate the btrfs internal locking mechanism just as you, but it won't be 
an easy thing to do in the short term.

Thanks,
Qu