[PATCH] ext4: don't report delalloc data as a hole on indirect-mapped inodes

[PATCH] ext4: don't report delalloc data as a hole on indirect-mapped inodes

[Daejun Park]

When a plain lookup finds no block, ext4_ind_map_blocks() sizes the hole
it reports by counting the empty subtrees under 'partial' in the on-disk
indirect tree.  That count knows nothing about delayed allocation, so a
range holding delalloc data that has not been written back yet is
reported as a plain hole.  The extent-mapped path does not have this
problem: ext4_ext_determine_insert_hole() trims the hole it found at the
first delayed extent before returning it.

ext4_map_blocks() consults the extent status tree before it calls into
the mapping layer, so a query that starts exactly on the delayed block
still finds it.  A query that starts earlier does not, because the hole
reported for the earlier block already spans the delayed one.  On a
4k-block filesystem, lseek(SEEK_DATA) from offset 0 on a file whose only
data block is at logical block N and is still dirty returns:

  N = 0..11      direct blocks                 N << 12
  N = 12         first single-indirect block   N << 12
  N = 13..1035   inside that indirect block    -1 ENXIO
  N = 1036       first double-indirect block   N << 12

Blocks 12 and 1036 survive because the query lands on the delayed block
itself.  Blocks 13..1035 are swallowed by the 1024-block hole reported
for block 12.  fiemap loses the same data for the same reason.

An ext3 filesystem mounted as ext4 hits this during ordinary use: ext3
inodes stay indirect mapped, while mount -t ext4 turns on delayed
allocation even though mounting ext3 with -o delalloc is explicitly
rejected.  install(1) from coreutils uses SEEK_DATA to locate data in
its source file, so installing a sparse file that has not been written
back silently produces a destination of the right size holding none of
the data:

  truncate -s 1G img && mkfs.ext3 -F img && mount -t ext4 img mnt
  echo | dd of=mnt/src bs=1 count=1 seek=64K
  install mnt/src mnt/dst
  cmp mnt/src mnt/dst        # differ: char 65537

Trim the hole at the first delayed extent, the way the extent-mapped
path does.  ext4_es_find_extent_range() does not clip the extent it
returns to the queried range, so skip one that begins at or before
m_lblk and clamp the result against the hole already found.

With this the sweep above returns N << 12 for every N, while the
nodelalloc and extent-mapped controls are unchanged.  generic/225,
generic/285, generic/286, generic/436, generic/448 and generic/490 pass
on ext4 made both with and without the extent feature.

Fixes: facab4d9711e ("ext4: return hole from ext4_map_blocks()")
Reported-by: Alexander Monakov <amonakov@ispras.ru>
Closes: https://lore.kernel.org/linux-ext4/594c17d9-c00f-e485-96fb-cedf27ce3aa3@ispras.ru/
Cc: stable@vger.kernel.org
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
 fs/ext4/indirect.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 5aec759eed70..4a1ad86a42bd 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -574,6 +574,7 @@ int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
 	/* Next simple case - plain lookup failed */
 	if ((flags & EXT4_GET_BLOCKS_CREATE) == 0) {
 		unsigned epb = inode->i_sb->s_blocksize / sizeof(u32);
+		struct extent_status es;
 		int i;
 
 		/*
@@ -589,6 +590,21 @@ int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
 		/* Fill in size of a hole we found */
 		map->m_pblk = 0;
 		map->m_len = umin(map->m_len, count);
+
+		/*
+		 * The hole was measured on the on-disk indirect tree, which
+		 * knows nothing about delayed allocation.  Trim it at the
+		 * first delayed extent, the way the extent-mapped path does
+		 * in ext4_ext_determine_insert_hole(), so that callers do not
+		 * mistake data that is not written back yet for a hole.  The
+		 * extent found is not clipped to the queried range, so it may
+		 * begin before m_lblk or past the end of the hole.
+		 */
+		ext4_es_find_extent_range(inode, &ext4_es_is_delayed,
+					  map->m_lblk,
+					  map->m_lblk + map->m_len - 1, &es);
+		if (es.es_len && es.es_lblk > map->m_lblk)
+			map->m_len = umin(map->m_len, es.es_lblk - map->m_lblk);
 		goto cleanup;
 	}
 

base-commit: 9091c97be34083587a75db174aab51551d8e8543
-- 
2.43.0

Re: [PATCH] ext4: don't report delalloc data as a hole on indirect-mapped inodes

[sashiko-bot]

> When a plain lookup finds no block, ext4_ind_map_blocks() sizes the hole
> it reports by counting the empty subtrees under 'partial' in the on-disk
> indirect tree.  That count knows nothing about delayed allocation, so a
> range holding delalloc data that has not been written back yet is
> reported as a plain hole.  The extent-mapped path does not have this
> problem: ext4_ext_determine_insert_hole() trims the hole it found at the
> first delayed extent before returning it.
> 
> ext4_map_blocks() consults the extent status tree before it calls into
> the mapping layer, so a query that starts exactly on the delayed block
> still finds it.  A query that starts earlier does not, because the hole
> reported for the earlier block already spans the delayed one.  On a
> 4k-block filesystem, lseek(SEEK_DATA) from offset 0 on a file whose only
> data block is at logical block N and is still dirty returns:
> 
> [ ... ]
> Fixes: facab4d9711e ("ext4: return hole from ext4_map_blocks()")
> Reported-by: Alexander Monakov <amonakov@ispras.ru>
> Closes: https://lore.kernel.org/linux-ext4/594c17d9-c00f-e485-96fb-cedf27ce3aa3@ispras.ru/
> Cc: stable@vger.kernel.org
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826052446epcms2p5a415be5fbfe23b8a32786f0ae6d05aea@epcms2p5?part=1

Re: [PATCH] ext4: don't report delalloc data as a hole on indirect-mapped inodes

[Jan Kara]

On Wed 26-08-26 14:24:46, Daejun Park wrote:
> When a plain lookup finds no block, ext4_ind_map_blocks() sizes the hole
> it reports by counting the empty subtrees under 'partial' in the on-disk
> indirect tree.  That count knows nothing about delayed allocation, so a
> range holding delalloc data that has not been written back yet is
> reported as a plain hole.  The extent-mapped path does not have this
> problem: ext4_ext_determine_insert_hole() trims the hole it found at the
> first delayed extent before returning it.
> 
> ext4_map_blocks() consults the extent status tree before it calls into
> the mapping layer, so a query that starts exactly on the delayed block
> still finds it.  A query that starts earlier does not, because the hole
> reported for the earlier block already spans the delayed one.  On a
> 4k-block filesystem, lseek(SEEK_DATA) from offset 0 on a file whose only
> data block is at logical block N and is still dirty returns:
> 
>   N = 0..11      direct blocks                 N << 12
>   N = 12         first single-indirect block   N << 12
>   N = 13..1035   inside that indirect block    -1 ENXIO
>   N = 1036       first double-indirect block   N << 12
> 
> Blocks 12 and 1036 survive because the query lands on the delayed block
> itself.  Blocks 13..1035 are swallowed by the 1024-block hole reported
> for block 12.  fiemap loses the same data for the same reason.
> 
> An ext3 filesystem mounted as ext4 hits this during ordinary use: ext3
> inodes stay indirect mapped, while mount -t ext4 turns on delayed
> allocation even though mounting ext3 with -o delalloc is explicitly
> rejected.  install(1) from coreutils uses SEEK_DATA to locate data in
> its source file, so installing a sparse file that has not been written
> back silently produces a destination of the right size holding none of
> the data:
> 
>   truncate -s 1G img && mkfs.ext3 -F img && mount -t ext4 img mnt
>   echo | dd of=mnt/src bs=1 count=1 seek=64K
>   install mnt/src mnt/dst
>   cmp mnt/src mnt/dst        # differ: char 65537
> 
> Trim the hole at the first delayed extent, the way the extent-mapped
> path does.  ext4_es_find_extent_range() does not clip the extent it
> returns to the queried range, so skip one that begins at or before
> m_lblk and clamp the result against the hole already found.
> 
> With this the sweep above returns N << 12 for every N, while the
> nodelalloc and extent-mapped controls are unchanged.  generic/225,
> generic/285, generic/286, generic/436, generic/448 and generic/490 pass
> on ext4 made both with and without the extent feature.
> 
> Fixes: facab4d9711e ("ext4: return hole from ext4_map_blocks()")
> Reported-by: Alexander Monakov <amonakov@ispras.ru>
> Closes: https://lore.kernel.org/linux-ext4/594c17d9-c00f-e485-96fb-cedf27ce3aa3@ispras.ru/
> Cc: stable@vger.kernel.org
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>

Thanks for report and the fix! So I think a better fix here is to lift most
of the logic of ext4_ext_determine_insert_hole() into a helper function
like:

ext4_lblk_t ext4_determine_insert_hole(struct inode *inode,
				ext4_lblk_t hole_start, ext4_lblk_t hole_len)

and then use it from both ext4_ext_map_block() and ext4_ind_map_blocks().
That avoids the duplication and also have all the comments regarding
various special cases (like delalloc extent covering map->m_lblk) in one
place.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR