Re: [PATCH] ext4: cache full extents during mapping lookup

On 7/22/2026 11:00 AM, changfengnan wrote:
> 
>> From: "Zhang Yi"<[email protected]>
>> Date:  Tue, Jul 21, 2026, 15:43
>> Subject:  Re: [PATCH] ext4: cache full extents during mapping lookup
>> To: "Fengnan Chang"<[email protected]>
>> Cc: <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>, <[email protected]>
>> On 7/20/2026 3:51 PM, Fengnan Chang wrote:
>>> The extent status (ES) tree and extent tree leaf buffers are reclaimed
>>> independently.  An ES entry can be reclaimed while the leaf buffer remains
>>> cached with BH_Verified set.
>>>   
>>> When this happens, ext4_es_lookup_extent() misses.  Since the leaf buffer
>>> is already verified, __read_extent_tree_block() returns it without calling
>>> ext4_cache_extents().  ext4_ext_map_blocks() returns the part of the extent
>>> covered by the request, and ext4_map_query_blocks() caches only that range.
>>> Reads to other blocks in the same extent then keep missing the ES tree and
>>> walking the extent tree.
>>>   
>>> Cache the full extent in ext4_ext_map_blocks() before the returned mapping
>>> is limited to the requested range.  Check the ES tree under the read lock
>>> first.  If the full extent is not there, ext4_es_cache_extent() checks
>>> again under the write lock before inserting it.
>>>   
>>> Do not do this when ext4_ext_map_blocks() is called with
>>> EXT4_GET_BLOCKS_CREATE or EXT4_EX_NOCACHE.  NOCACHE paths can change extent
>>> mappings, so caching ranges outside the request can leave stale ES entries.
>>> Cache both written and unwritten extents, as ext4_cache_extents() does.
>>>   
>>> Tested with fio 4K random direct reads using libaio at iodepth 128 on a
>>> 128 GiB file with 208 on-disk extents.  The results are averages of three
>>> 10-second runs:
>>>   
>>>                   before reclaim    after reclaim
>>>    unpatched        419.6k IOPS       274.6k IOPS
>>>    patched          426.1k IOPS       423.7k IOPS
>>>   
>>> Average completion latency went from 303.02 to 462.78 us without the patch,
>>> and from 298.36 to 300.07 us with the patch.
>>>   
>>> No latency regression was seen in A/B test.
>>>   
>>> Suggested-by: Jan Kara <[email protected]>
>>> Link: https://lore.kernel.org/r/[email protected]
>>> Signed-off-by: Fengnan Chang <[email protected]>
>>   
>> Thank you for the patch! This overall looks good to me, just some minor
>> suggestions below.
>>   
>>> ---
>>>   fs/ext4/extents.c        |  9 +++++++++
>>>   fs/ext4/extents_status.c | 33 +++++++++++++++++++++++++++++++++
>>>   fs/ext4/extents_status.h |  3 +++
>>>   3 files changed, 45 insertions(+)
>>>   
>>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>>> index 91c97af64b317..f43dd9c3bc44e 100644
>>> --- a/fs/ext4/extents.c
>>> +++ b/fs/ext4/extents.c
>>> @@ -4310,6 +4310,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
>>>                   ext4_lblk_t ee_block = le32_to_cpu(ex->ee_block);
>>>                   ext4_fsblk_t ee_start = ext4_ext_pblock(ex);
>>>                   unsigned short ee_len;
>>> +                unsigned int status;
>>>   
>>>   
>>>                   /*
>>> @@ -4317,6 +4318,8 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
>>>                    * we split out initialized portions during a write.
>>>                    */
>>>                   ee_len = ext4_ext_get_actual_len(ex);
>>> +                status = ext4_ext_is_unwritten(ex) ?
>>> +                         EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
>>>   
>>>                   trace_ext4_ext_show_extent(inode, ee_block, ee_start, ee_len);
>>>   
>>> @@ -4328,6 +4331,12 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
>>>                           ext_debug(inode, "%u fit into %u:%d -> %llu\n",
>>>                                     map->m_lblk, ee_block, ee_len, newblock);
>>>   
>>> +                        if (!(flags & (EXT4_GET_BLOCKS_CREATE |
>>> +                                       EXT4_EX_NOCACHE)))
>>> +                                ext4_es_cache_extent_if_missing(inode, ee_block,
>>> +                                                                ee_len, ee_start,
>>> +                                                                status);
>>   
>> After this patch, I think the calls to ext4_es_cache_extent() in
>> ext4_map_query_blocks_next_in_leaf() and ext4_map_query_blocks() can
>> also be replaced with this helper.
>>   
>>> +
>>>                           /*
>>>                            * If the extent is initialized check whether the
>>>                            * caller wants to convert it to unwritten.
>>> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
>>> index 6e4a191e82191..83fe2068ab656 100644
>>> --- a/fs/ext4/extents_status.c
>>> +++ b/fs/ext4/extents_status.c
>>> @@ -1082,6 +1082,39 @@ void ext4_es_cache_extent(struct inode *inode, ext4_lblk_t lblk,
>>>                              ext4_es_pblock(&chkes), ext4_es_status(&chkes));
>>>   }
>>>   
>>> +/*
>>> + * Avoid taking i_es_lock for writing if the entire extent is already cached.
>>> + * ext4_es_cache_extent() rechecks under the write lock if the cache changes
>>> + * after this read-side check.
>>> + */
>>> +void ext4_es_cache_extent_if_missing(struct inode *inode, ext4_lblk_t lblk,
>>   
>> I'd prefer to move this logic into ext4_es_cache_extent() and add a
>> parameter(e.g., read_search) to control whether to do pre-search unbder
>> the read lock. If the extent is likely to already exist on the status
>> tree, we can pass 1. Sounds good?
> 
> ext4_es_cache_extent add a parameter  to control whether to do pre-search
> sounds good.
> I have some questions about when to perform a pre-search.
> Are you meaning that a pre-search should also be performed when
> ext4_map_query_blocks_next_in_leaf and ext4_map_query_blocks call
> ext4_es_cache_extent ?
> ext4_map_query_blocks_next_in_leaf is ok , but I think it only makes sense
> to perform a pre-search for ext4_es_cache_extent in ext4_map_query_blocks
> when in extent mode, because in extent mode, the entire leaf is cached, so
> it’s likely that the extent already exists—but this isn’t the case in indirect mode.
> Right ?

Yes.

> 
> I’d like to make the following change:
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 91c97af64b317..2ffb5027aa1ee 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -537,12 +537,12 @@ static void ext4_cache_extents(struct inode *inode,
>   
>                   if (prev && (prev != lblk))
>                           ext4_es_cache_extent(inode, prev, lblk - prev, ~0,
> -                                             EXTENT_STATUS_HOLE);
> +                                             EXTENT_STATUS_HOLE, false);
>   
>                   if (ext4_ext_is_unwritten(ex))
>                           status = EXTENT_STATUS_UNWRITTEN;
>                   ext4_es_cache_extent(inode, lblk, len,
> -                                     ext4_ext_pblock(ex), status);
> +                                     ext4_ext_pblock(ex), status, false);
>                   prev = lblk + len;
>           }
>   }
> @@ -4239,7 +4239,8 @@ static ext4_lblk_t ext4_ext_determine_insert_hole(struct inode *inode,
>   insert_hole:
>           /* Put just found gap into cache to speed up subsequent requests */
>           ext_debug(inode, " -> %u:%u\n", hole_start, len);
> -        ext4_es_cache_extent(inode, hole_start, len, ~0, EXTENT_STATUS_HOLE);
> +        ext4_es_cache_extent(inode, hole_start, len, ~0, EXTENT_STATUS_HOLE,
> +                             false);
>   
>           /* Update hole_len to reflect hole size after lblk */
>           if (hole_start != lblk)
> @@ -4310,6 +4311,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
>                   ext4_lblk_t ee_block = le32_to_cpu(ex->ee_block);
>                   ext4_fsblk_t ee_start = ext4_ext_pblock(ex);
>                   unsigned short ee_len;
> +                unsigned int status;
>   
>   
>                   /*
> @@ -4317,6 +4319,8 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
>                    * we split out initialized portions during a write.
>                    */
>                   ee_len = ext4_ext_get_actual_len(ex);
> +                status = ext4_ext_is_unwritten(ex) ?
> +                         EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
>   
>                   trace_ext4_ext_show_extent(inode, ee_block, ee_start, ee_len);
>   
> @@ -4328,6 +4332,12 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
>                           ext_debug(inode, "%u fit into %u:%d -> %llu\n",
>                                     map->m_lblk, ee_block, ee_len, newblock);
>   
> +                        if (!(flags & (EXT4_GET_BLOCKS_CREATE |
> +                                       EXT4_EX_NOCACHE)))
> +                                ext4_es_cache_extent(inode, ee_block, ee_len,
> +                                                     ee_start, status,
> +                                                     true);
> +
>                           /*
>                            * If the extent is initialized check whether the
>                            * caller wants to convert it to unwritten.
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 6e4a191e82191..5a03878b06160 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -1023,7 +1023,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>    */
>   void ext4_es_cache_extent(struct inode *inode, ext4_lblk_t lblk,
>                             ext4_lblk_t len, ext4_fsblk_t pblk,
> -                          unsigned int status)
> +                          unsigned int status, bool pre_search)
>   {
>           struct extent_status *es;
>           struct extent_status chkes, newes;
> @@ -1043,6 +1043,21 @@ void ext4_es_cache_extent(struct inode *inode, ext4_lblk_t lblk,
>   
>           BUG_ON(end < lblk);
>   
> +        /*
> +         * Avoid taking i_es_lock for writing if the entire extent is already
> +         * cached. The write-locked search below rechecks after a miss.
> +         */
> +        if (pre_search) {
> +                read_lock(&EXT4_I(inode)->i_es_lock);
> +                es = __es_tree_search(&EXT4_I(inode)->i_es_tree.root, lblk);
> +                if (es && es->es_lblk <= lblk && ext4_es_end(es) >= end &&
> +                    !__es_check_extent_status(es, status, NULL)) {
> +                        read_unlock(&EXT4_I(inode)->i_es_lock);
> +                        return;
> +                }
> +                read_unlock(&EXT4_I(inode)->i_es_lock);
> +        }
> +
>           write_lock(&EXT4_I(inode)->i_es_lock);
>           es = __es_tree_search(&EXT4_I(inode)->i_es_tree.root, lblk);
>           if (es && es->es_lblk <= end) {
> diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
> index f3396cf32b446..c2da72e3c82ba 100644
> --- a/fs/ext4/extents_status.h
> +++ b/fs/ext4/extents_status.h
> @@ -139,7 +139,7 @@ extern void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>                                     bool delalloc_reserve_used);
>   extern void ext4_es_cache_extent(struct inode *inode, ext4_lblk_t lblk,
>                                    ext4_lblk_t len, ext4_fsblk_t pblk,
> -                                 unsigned int status);
> +                                 unsigned int status, bool pre_search);
>   extern void ext4_es_remove_extent(struct inode *inode, ext4_lblk_t lblk,
>                                     ext4_lblk_t len);
>   extern void ext4_es_find_extent_range(struct inode *inode,
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index ce99807c5f5b2..341b7c649e133 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -523,7 +523,7 @@ static int ext4_map_query_blocks_next_in_leaf(handle_t *handle,
>   
>           if (retval <= 0) {
>                   ext4_es_cache_extent(inode, map->m_lblk, map->m_len,
> -                                     map->m_pblk, status);
> +                                     map->m_pblk, status, true);
>                   return map->m_len;
>           }
>   
> @@ -546,11 +546,11 @@ static int ext4_map_query_blocks_next_in_leaf(handle_t *handle,
>                           status == status2) {
>                   ext4_es_cache_extent(inode, map->m_lblk,
>                                        map->m_len + map2.m_len, map->m_pblk,
> -                                     status);
> +                                     status, true);
>                   map->m_len += map2.m_len;
>           } else {
>                   ext4_es_cache_extent(inode, map->m_lblk, map->m_len,
> -                                     map->m_pblk, status);
> +                                     map->m_pblk, status, true);
>           }
>   
>           return map->m_len;
> @@ -562,6 +562,8 @@ int ext4_map_query_blocks(handle_t *handle, struct inode *inode,
>           unsigned int status;
>           int retval;
>           unsigned int orig_mlen = map->m_len;
> +        bool pre_search = ext4_test_inode_flag(inode,
> +                                              EXT4_INODE_EXTENTS);
>   
>           flags &= EXT4_EX_QUERY_FILTER;
>           if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> @@ -593,7 +595,7 @@ int ext4_map_query_blocks(handle_t *handle, struct inode *inode,
>                   status = map->m_flags & EXT4_MAP_UNWRITTEN ?
>                                   EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
>                   ext4_es_cache_extent(inode, map->m_lblk, map->m_len,
> -                                     map->m_pblk, status);
> +                                     map->m_pblk, status, pre_search);

It would be helpful to add a comment here. Otherwise, this looks good to
me.

Thanks,
Yi.

>           } else {
>                   retval = ext4_map_query_blocks_next_in_leaf(handle, inode, map,
>                                                               orig_mlen);
> -- 
> 
>>   
>> Thanks,
>> Yi.
>>   
>>> +                                     ext4_lblk_t len, ext4_fsblk_t pblk,
>>> +                                     unsigned int status)
>>> +{
>>> +        struct extent_status *es;
>>> +        ext4_lblk_t end;
>>> +        bool cached;
>>> +
>>> +        if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY)
>>> +                return;
>>> +
>>> +        if (!len)
>>> +                return;
>>> +
>>> +        end = lblk + len - 1;
>>> +        if (WARN_ON_ONCE(end < lblk))
>>> +                return;
>>> +
>>> +        read_lock(&EXT4_I(inode)->i_es_lock);
>>> +        es = __es_tree_search(&EXT4_I(inode)->i_es_tree.root, lblk);
>>> +        cached = es && es->es_lblk <= lblk && ext4_es_end(es) >= end &&
>>> +                 !__es_check_extent_status(es, status, NULL);
>>> +        read_unlock(&EXT4_I(inode)->i_es_lock);
>>> +
>>> +        if (!cached)
>>> +                ext4_es_cache_extent(inode, lblk, len, pblk, status);
>>> +}
>>> +
>>>   /*
>>>    * ext4_es_lookup_extent() looks up an extent in extent status tree.
>>>    *
>>> diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
>>> index f3396cf32b446..d8fab84911352 100644
>>> --- a/fs/ext4/extents_status.h
>>> +++ b/fs/ext4/extents_status.h
>>> @@ -140,6 +140,9 @@ extern void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>>>   extern void ext4_es_cache_extent(struct inode *inode, ext4_lblk_t lblk,
>>>                                    ext4_lblk_t len, ext4_fsblk_t pblk,
>>>                                    unsigned int status);
>>> +void ext4_es_cache_extent_if_missing(struct inode *inode, ext4_lblk_t lblk,
>>> +                                     ext4_lblk_t len, ext4_fsblk_t pblk,
>>> +                                     unsigned int status);
>>>   extern void ext4_es_remove_extent(struct inode *inode, ext4_lblk_t lblk,
>>>                                     ext4_lblk_t len);
>>>   extern void ext4_es_find_extent_range(struct inode *inode,
>>   
>