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

[Qu Wenruo <wqu@suse.com>]

Gentle ping?

This patch has multiple reviewed-by tags, but I still didn't see it in 
the upstream rc1 nor vfs.all branch.

Thanks,
Qu

在 2026/8/5 07:29, 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>
> ---
> v4:
> - Always align down to fs block size
> 
> 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 | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index b3368d64e81b..01f26fb3b3b2 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -594,12 +594,19 @@ 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 = i_blocksize(iter->inode);
> +	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)