* Re: [PATCH v3] ext4: don't report delalloc data as a hole on indirect-mapped inodes
2026-08-27 8:48 ` [PATCH v3] ext4: don't report delalloc data as a hole on indirect-mapped inodes Daejun Park
@ 2026-08-27 8:58 ` sashiko-bot
2026-08-27 11:48 ` Jan Kara
2026-08-27 12:39 ` Zhang Yi
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-27 8:58 UTC (permalink / raw)
To: Daejun Park; +Cc: linux-ext4, tytso
> 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: it 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:
>
> [ ... ]
> Reported-by: Alexander Monakov <amonakov@ispras.ru>
> Closes: https://lore.kernel.org/linux-ext4/594c17d9-c00f-e485-96fb-cedf27ce3aa3@ispras.ru/
> Suggested-by: Jan Kara <jack@suse.cz>
> 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/20260827084829epcms2p6ecc0c39a71d0c3eebf9cb1ade77314f6@epcms2p6?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] ext4: don't report delalloc data as a hole on indirect-mapped inodes
2026-08-27 8:48 ` [PATCH v3] ext4: don't report delalloc data as a hole on indirect-mapped inodes Daejun Park
2026-08-27 8:58 ` sashiko-bot
@ 2026-08-27 11:48 ` Jan Kara
2026-08-27 12:39 ` Zhang Yi
2 siblings, 0 replies; 4+ messages in thread
From: Jan Kara @ 2026-08-27 11:48 UTC (permalink / raw)
To: Daejun Park
Cc: tytso@mit.edu, adilger.kernel@dilger.ca, jack@suse.cz,
yi.zhang@huawei.com, ritesh.list@gmail.com,
libaokun@linux.alibaba.com, ojaswin@linux.ibm.com,
linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org,
amonakov@ispras.ru
On Thu 27-08-26 17:48:29, 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: it 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
>
> Reconciling a hole with the extent status tree does not depend on how
> the hole was found, so instead of open coding it a second time, split
> the delalloc handling out of ext4_ext_determine_insert_hole() into
> ext4_determine_insert_hole(), which takes the hole the caller located,
> and call that from ext4_ind_map_blocks() as well. All the special cases
> and the comments explaining them stay in one place.
>
> Two things change for indirect-mapped inodes. They now cache the holes
> they report, as the extent-mapped path already does: ext4_punch_hole()
> already puts EXTENT_STATUS_HOLE on such inodes, ext4_map_blocks() and
> ext4_da_map_blocks() already handle a cached hole, the latter by
> replacing it with the delayed extent, and consecutive hole entries
> merge, so the tree does not grow with the length of a scan. And when a
> delayed extent covers the queried block itself - a race, since
> ext4_map_blocks() found nothing in the tree just before - the hole is
> now reported only up to the end of that extent instead of in full.
>
> The indirect path passes hole_start == lblk, so the "delalloc extent in
> front of the queried range" case cannot be reached there:
> __es_tree_search() never returns an extent ending before the block it
> was searched for.
>
> 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/448 and generic/490 pass on ext4 made both with and
> without the extent feature. generic/286 and generic/436 pass on
> extent-mapped ext4 and are skipped without it, since they need
> fallocate.
>
> 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/
> Suggested-by: Jan Kara <jack@suse.cz>
> Cc: stable@vger.kernel.org
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Ok, good catch (from Sashiko I assume) about the error handling. The fix
still looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> v1: https://lore.kernel.org/linux-ext4/20260826052446epcms2p5a415be5fbfe23b8a32786f0ae6d05aea@epcms2p5/
> v2: https://lore.kernel.org/linux-ext4/20260827043928epcms2p54597035aadd1eca0556ae9b0585694f9@epcms2p5/
>
> v2: rather than open coding the delalloc trim in ext4_ind_map_blocks(),
> split it out of ext4_ext_determine_insert_hole() into
> ext4_determine_insert_hole() and call that from both paths, as Jan
> suggested.
>
> v3: only reconcile when the lookup really did find a hole.
> ext4_get_branch() returns a non-NULL partial when it failed as
> well, and ext4_ind_map_blocks() checks err only after this branch,
> so v2 cached a hole for a range it could not read. The error was
> still returned, but every later read of that range was answered
> from the cache as zeros instead of retrying the disk. Reproduced
> by pointing an inode's single-indirect block past the end of the
> filesystem: v2 caches a hole for the whole 1024-block subtree and
> reads succeed with zeros where they must return -EIO.
> Also corrected the xfstests sentence: generic/286 and generic/436
> need fallocate, so they are skipped rather than passing on a
> filesystem made without the extent feature.
>
> fs/ext4/ext4.h | 3 +++
> fs/ext4/extents.c | 26 +++++++++++++-------------
> fs/ext4/indirect.c | 17 +++++++++++++++++
> 3 files changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 7fd078de265d..f5eff74631a4 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3893,6 +3893,9 @@ extern void ext4_ext_tree_init(handle_t *handle, struct inode *inode);
> extern int ext4_ext_index_trans_blocks(struct inode *inode, int extents);
> extern int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
> struct ext4_map_blocks *map, int flags);
> +ext4_lblk_t ext4_determine_insert_hole(struct inode *inode, ext4_lblk_t lblk,
> + ext4_lblk_t hole_start,
> + ext4_lblk_t hole_len);
> extern int ext4_ext_truncate(handle_t *, struct inode *);
> extern int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
> ext4_lblk_t end);
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 836396ea7912..e2b7565e2cb4 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4190,21 +4190,19 @@ static int get_implied_cluster_alloc(struct super_block *sb,
> }
>
> /*
> - * Determine hole length around the given logical block, first try to
> - * locate and expand the hole from the given @path, and then adjust it
> - * if it's partially or completely converted to delayed extents, insert
> - * it into the extent cache tree if it's indeed a hole, finally return
> - * the length of the determined extent.
> + * Adjust the hole [@hole_start, @hole_start + @hole_len) the caller found
> + * around the queried block @lblk if it's partially or completely converted
> + * to delayed extents, insert it into the extent cache tree if it's indeed a
> + * hole, finally return the length of the determined extent starting at
> + * @lblk. @hole_start must not be behind @lblk.
> */
> -static ext4_lblk_t ext4_ext_determine_insert_hole(struct inode *inode,
> - struct ext4_ext_path *path,
> - ext4_lblk_t lblk)
> +ext4_lblk_t ext4_determine_insert_hole(struct inode *inode, ext4_lblk_t lblk,
> + ext4_lblk_t hole_start,
> + ext4_lblk_t hole_len)
> {
> - ext4_lblk_t hole_start, len;
> + ext4_lblk_t len = hole_len;
> struct extent_status es;
>
> - hole_start = lblk;
> - len = ext4_ext_find_hole(inode, path, &hole_start);
> again:
> ext4_es_find_extent_range(inode, &ext4_es_is_delayed, hole_start,
> hole_start + len - 1, &es);
> @@ -4371,9 +4369,11 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
> * we couldn't try to create block if flags doesn't contain EXT4_GET_BLOCKS_CREATE
> */
> if ((flags & EXT4_GET_BLOCKS_CREATE) == 0) {
> - ext4_lblk_t len;
> + ext4_lblk_t hole_start = map->m_lblk, len;
>
> - len = ext4_ext_determine_insert_hole(inode, path, map->m_lblk);
> + len = ext4_ext_find_hole(inode, path, &hole_start);
> + len = ext4_determine_insert_hole(inode, map->m_lblk,
> + hole_start, len);
>
> map->m_pblk = 0;
> map->m_len = min_t(unsigned int, map->m_len, len);
> diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
> index 5aec759eed70..a495080df710 100644
> --- a/fs/ext4/indirect.c
> +++ b/fs/ext4/indirect.c
> @@ -586,6 +586,23 @@ int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
> for (i = partial - chain + 1; i < depth; i++)
> count = count * epb + (epb - offsets[i] - 1);
> count++;
> +
> + /*
> + * The count knows nothing about delayed allocation, so let
> + * the common helper reconcile it with the extent status
> + * tree. Only when the lookup really did find a hole:
> + * ext4_get_branch() also returns a non-NULL partial when it
> + * failed, and caching a hole for a range it could not read
> + * would answer every later read from that cache instead of
> + * retrying. Clamp the count first, a subtree can be bigger
> + * than the logical block space when the block size is large.
> + */
> + if (!err) {
> + count = umin(count, EXT_MAX_BLOCKS - map->m_lblk);
> + count = ext4_determine_insert_hole(inode, map->m_lblk,
> + map->m_lblk, count);
> + }
> +
> /* Fill in size of a hole we found */
> map->m_pblk = 0;
> map->m_len = umin(map->m_len, count);
>
> base-commit: 9091c97be34083587a75db174aab51551d8e8543
> --
> 2.43.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3] ext4: don't report delalloc data as a hole on indirect-mapped inodes
2026-08-27 8:48 ` [PATCH v3] ext4: don't report delalloc data as a hole on indirect-mapped inodes Daejun Park
2026-08-27 8:58 ` sashiko-bot
2026-08-27 11:48 ` Jan Kara
@ 2026-08-27 12:39 ` Zhang Yi
2 siblings, 0 replies; 4+ messages in thread
From: Zhang Yi @ 2026-08-27 12:39 UTC (permalink / raw)
To: daejun7.park
Cc: tytso@mit.edu, adilger.kernel@dilger.ca, jack@suse.cz,
ritesh.list@gmail.com, libaokun@linux.alibaba.com,
ojaswin@linux.ibm.com, linux-ext4@vger.kernel.org,
linux-kernel@vger.kernel.org, amonakov@ispras.ru
On 8/27/2026 4:48 PM, 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: it 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
>
> Reconciling a hole with the extent status tree does not depend on how
> the hole was found, so instead of open coding it a second time, split
> the delalloc handling out of ext4_ext_determine_insert_hole() into
> ext4_determine_insert_hole(), which takes the hole the caller located,
> and call that from ext4_ind_map_blocks() as well. All the special cases
> and the comments explaining them stay in one place.
>
> Two things change for indirect-mapped inodes. They now cache the holes
> they report, as the extent-mapped path already does: ext4_punch_hole()
> already puts EXTENT_STATUS_HOLE on such inodes, ext4_map_blocks() and
> ext4_da_map_blocks() already handle a cached hole, the latter by
> replacing it with the delayed extent, and consecutive hole entries
> merge, so the tree does not grow with the length of a scan. And when a
> delayed extent covers the queried block itself - a race, since
> ext4_map_blocks() found nothing in the tree just before - the hole is
> now reported only up to the end of that extent instead of in full.
>
> The indirect path passes hole_start == lblk, so the "delalloc extent in
> front of the queried range" case cannot be reached there:
> __es_tree_search() never returns an extent ending before the block it
> was searched for.
>
> 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/448 and generic/490 pass on ext4 made both with and
> without the extent feature. generic/286 and generic/436 pass on
> extent-mapped ext4 and are skipped without it, since they need
> fallocate.
>
> 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/
> Suggested-by: Jan Kara <jack@suse.cz>
> Cc: stable@vger.kernel.org
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Looks good to me.
Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
> ---
> v1: https://lore.kernel.org/linux-ext4/20260826052446epcms2p5a415be5fbfe23b8a32786f0ae6d05aea@epcms2p5/
> v2: https://lore.kernel.org/linux-ext4/20260827043928epcms2p54597035aadd1eca0556ae9b0585694f9@epcms2p5/
>
> v2: rather than open coding the delalloc trim in ext4_ind_map_blocks(),
> split it out of ext4_ext_determine_insert_hole() into
> ext4_determine_insert_hole() and call that from both paths, as Jan
> suggested.
>
> v3: only reconcile when the lookup really did find a hole.
> ext4_get_branch() returns a non-NULL partial when it failed as
> well, and ext4_ind_map_blocks() checks err only after this branch,
> so v2 cached a hole for a range it could not read. The error was
> still returned, but every later read of that range was answered
> from the cache as zeros instead of retrying the disk. Reproduced
> by pointing an inode's single-indirect block past the end of the
> filesystem: v2 caches a hole for the whole 1024-block subtree and
> reads succeed with zeros where they must return -EIO.
> Also corrected the xfstests sentence: generic/286 and generic/436
> need fallocate, so they are skipped rather than passing on a
> filesystem made without the extent feature.
>
> fs/ext4/ext4.h | 3 +++
> fs/ext4/extents.c | 26 +++++++++++++-------------
> fs/ext4/indirect.c | 17 +++++++++++++++++
> 3 files changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 7fd078de265d..f5eff74631a4 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3893,6 +3893,9 @@ extern void ext4_ext_tree_init(handle_t *handle, struct inode *inode);
> extern int ext4_ext_index_trans_blocks(struct inode *inode, int extents);
> extern int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
> struct ext4_map_blocks *map, int flags);
> +ext4_lblk_t ext4_determine_insert_hole(struct inode *inode, ext4_lblk_t lblk,
> + ext4_lblk_t hole_start,
> + ext4_lblk_t hole_len);
> extern int ext4_ext_truncate(handle_t *, struct inode *);
> extern int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
> ext4_lblk_t end);
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 836396ea7912..e2b7565e2cb4 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4190,21 +4190,19 @@ static int get_implied_cluster_alloc(struct super_block *sb,
> }
>
> /*
> - * Determine hole length around the given logical block, first try to
> - * locate and expand the hole from the given @path, and then adjust it
> - * if it's partially or completely converted to delayed extents, insert
> - * it into the extent cache tree if it's indeed a hole, finally return
> - * the length of the determined extent.
> + * Adjust the hole [@hole_start, @hole_start + @hole_len) the caller found
> + * around the queried block @lblk if it's partially or completely converted
> + * to delayed extents, insert it into the extent cache tree if it's indeed a
> + * hole, finally return the length of the determined extent starting at
> + * @lblk. @hole_start must not be behind @lblk.
> */
> -static ext4_lblk_t ext4_ext_determine_insert_hole(struct inode *inode,
> - struct ext4_ext_path *path,
> - ext4_lblk_t lblk)
> +ext4_lblk_t ext4_determine_insert_hole(struct inode *inode, ext4_lblk_t lblk,
> + ext4_lblk_t hole_start,
> + ext4_lblk_t hole_len)
> {
> - ext4_lblk_t hole_start, len;
> + ext4_lblk_t len = hole_len;
> struct extent_status es;
>
> - hole_start = lblk;
> - len = ext4_ext_find_hole(inode, path, &hole_start);
> again:
> ext4_es_find_extent_range(inode, &ext4_es_is_delayed, hole_start,
> hole_start + len - 1, &es);
> @@ -4371,9 +4369,11 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
> * we couldn't try to create block if flags doesn't contain EXT4_GET_BLOCKS_CREATE
> */
> if ((flags & EXT4_GET_BLOCKS_CREATE) == 0) {
> - ext4_lblk_t len;
> + ext4_lblk_t hole_start = map->m_lblk, len;
>
> - len = ext4_ext_determine_insert_hole(inode, path, map->m_lblk);
> + len = ext4_ext_find_hole(inode, path, &hole_start);
> + len = ext4_determine_insert_hole(inode, map->m_lblk,
> + hole_start, len);
>
> map->m_pblk = 0;
> map->m_len = min_t(unsigned int, map->m_len, len);
> diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
> index 5aec759eed70..a495080df710 100644
> --- a/fs/ext4/indirect.c
> +++ b/fs/ext4/indirect.c
> @@ -586,6 +586,23 @@ int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
> for (i = partial - chain + 1; i < depth; i++)
> count = count * epb + (epb - offsets[i] - 1);
> count++;
> +
> + /*
> + * The count knows nothing about delayed allocation, so let
> + * the common helper reconcile it with the extent status
> + * tree. Only when the lookup really did find a hole:
> + * ext4_get_branch() also returns a non-NULL partial when it
> + * failed, and caching a hole for a range it could not read
> + * would answer every later read from that cache instead of
> + * retrying. Clamp the count first, a subtree can be bigger
> + * than the logical block space when the block size is large.
> + */
> + if (!err) {
> + count = umin(count, EXT_MAX_BLOCKS - map->m_lblk);
> + count = ext4_determine_insert_hole(inode, map->m_lblk,
> + map->m_lblk, count);
> + }
> +
> /* Fill in size of a hole we found */
> map->m_pblk = 0;
> map->m_len = umin(map->m_len, count);
>
> base-commit: 9091c97be34083587a75db174aab51551d8e8543
^ permalink raw reply [flat|nested] 4+ messages in thread