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

> 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 ?

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);
         } 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,
>