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

[PATCH] 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 the following fsstress workload:

 # $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]
Btrfs has recently introduced more strict extent_state alignment checks,
which means functions like btrfs_lock_extent() will require the range
to be block size aligned.

But in the above workload, iomap_dio_hole_iter() only zeroed 4K, then
hit a page fault. In that case the iov_iter is still advanced 4K bytes,
then btrfs_dio_iomap_begin() is called for the remaining range, which is
no longer block size (8K) aligned, triggering the ASSERT().

This shows that, although iomap_dio_bio_iter() is properly respecting
the alignment requirement, iomap_dio_hole_iter() is not doing the same
thing.

[FIX]
Extract a helper, iomap_dio_alignment(), to calculate the alignment first.

Then 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.

Fixes: 001397f5ef49 ("iomap: add IOMAP_DIO_FSBLOCK_ALIGNED flag")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/iomap/direct-io.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index e2cd5f92babe..94747963cb1f 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -403,6 +403,18 @@ static ssize_t iomap_dio_bio_iter_one(struct iomap_iter *iter,
 	return ret;
 }
 
+/*
+ * File systems that write out of place and always allocate new blocks
+ * need each bio to be block aligned as that's the unit of allocation.
+ */
+static unsigned int iomap_dio_alignment(const struct iomap_iter *iter,
+					const struct iomap_dio *dio)
+{
+	if (dio->flags & IOMAP_DIO_FSBLOCK_ALIGNED)
+		return i_blocksize(iter->inode);
+	return bdev_logical_block_size(iter->iomap.bdev);
+}
+
 static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 {
 	const struct iomap *iomap = &iter->iomap;
@@ -414,18 +426,9 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 	bool need_zeroout = false;
 	u64 copied = 0;
 	size_t orig_count;
-	unsigned int alignment;
+	const unsigned int alignment = iomap_dio_alignment(iter, dio);
 	ssize_t ret = 0;
 
-	/*
-	 * File systems that write out of place and always allocate new blocks
-	 * need each bio to be block aligned as that's the unit of allocation.
-	 */
-	if (dio->flags & IOMAP_DIO_FSBLOCK_ALIGNED)
-		alignment = fs_block_size;
-	else
-		alignment = bdev_logical_block_size(iomap->bdev);
-
 	if ((pos | length) & (alignment - 1))
 		return -EINVAL;
 
@@ -588,12 +591,15 @@ 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);
+	const unsigned int alignment = iomap_dio_alignment(iter, dio);
+	const loff_t aligned_copied = round_down(copied, alignment);
 
-	dio->size += length;
-	if (!length)
+	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] iomap: follow the alignment requirement for iomap_dio_hole_iter()

[Christoph Hellwig]

On Thu, Jul 30, 2026 at 10:52:10AM +0930, Qu Wenruo wrote:
> [BUG]
> On the latest development branch, btrfs with 8K block size on 4K page
> sized systems will fail the following fsstress workload:
> 
>  # $fsstress -n 4 -d $mnt -s 1785675805 -v

Please add this to xfstests.

> +/*
> + * File systems that write out of place and always allocate new blocks
> + * need each bio to be block aligned as that's the unit of allocation.
> + */
> +static unsigned int iomap_dio_alignment(const struct iomap_iter *iter,
> +					const struct iomap_dio *dio)
> +{
> +	if (dio->flags & IOMAP_DIO_FSBLOCK_ALIGNED)
> +		return i_blocksize(iter->inode);
> +	return bdev_logical_block_size(iter->iomap.bdev);
> +}

This already exists in the VFS iomap tree (with a slightly different
prototype).

>  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);
> +	const unsigned int alignment = iomap_dio_alignment(iter, dio);
> +	const loff_t aligned_copied = round_down(copied, alignment);
>  
> -	dio->size += length;
> -	if (!length)
> +	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);
>  }

iomap generall expects extents to be block aligned, how do you end up
with non-aligned reporting here?

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

[Qu Wenruo]

在 2026/7/30 17:15, Christoph Hellwig 写道:
> On Thu, Jul 30, 2026 at 10:52:10AM +0930, Qu Wenruo wrote:
>> [BUG]
>> On the latest development branch, btrfs with 8K block size on 4K page
>> sized systems will fail the following fsstress workload:
>>
>>   # $fsstress -n 4 -d $mnt -s 1785675805 -v
> 
> Please add this to xfstests.

In that case I'll create a more dedicated reproducer.
> 
>> +/*
>> + * File systems that write out of place and always allocate new blocks
>> + * need each bio to be block aligned as that's the unit of allocation.
>> + */
>> +static unsigned int iomap_dio_alignment(const struct iomap_iter *iter,
>> +					const struct iomap_dio *dio)
>> +{
>> +	if (dio->flags & IOMAP_DIO_FSBLOCK_ALIGNED)
>> +		return i_blocksize(iter->inode);
>> +	return bdev_logical_block_size(iter->iomap.bdev);
>> +}
> 
> This already exists in the VFS iomap tree (with a slightly different
> prototype).
> 
>>   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);
>> +	const unsigned int alignment = iomap_dio_alignment(iter, dio);
>> +	const loff_t aligned_copied = round_down(copied, alignment);
>>   
>> -	dio->size += length;
>> -	if (!length)
>> +	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);
>>   }
> 
> iomap generall expects extents to be block aligned, how do you end up
> with non-aligned reporting here?

The dio read buffer is 2 pages (matching the 8K alignment), but only the 
first page is faulted in.

Furthermore btrfs has disabled page fault during dio read, so the 2nd 
page will not be faulted in.

Thus iov_iter_zero() only got to zero the first page.

Btrfs always returned a hole that is properly aligned, but as long as bs 
 > ps, the page fault can always break in the middle, causing unaligned 
range.

Thanks,
Qu

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

[Christoph Hellwig]

On Thu, Jul 30, 2026 at 06:06:17PM +0930, Qu Wenruo wrote:
> > iomap generall expects extents to be block aligned, how do you end up
> > with non-aligned reporting here?
> 
> The dio read buffer is 2 pages (matching the 8K alignment), but only the
> first page is faulted in.
> 
> Furthermore btrfs has disabled page fault during dio read, so the 2nd page
> will not be faulted in.
> 
> Thus iov_iter_zero() only got to zero the first page.
> 
> Btrfs always returned a hole that is properly aligned, but as long as bs >
> ps, the page fault can always break in the middle, causing unaligned range.

Urgg, we really need to get rid of this nofault behavior rather sooner
than later.  But I think the iomap should always be fsblock alignment.
While we can support smaller I/O, the mapping should always be block
aligned.