Re: [PATCH] ext4: cache full extents during mapping lookup
From: changfengnan
Date: Tue Jul 21 2026 - 23:01:20 EST
> From: "Zhang Yi"<yi.zhang@xxxxxxxxxxxxxxx>
> Date: Tue, Jul 21, 2026, 15:43
> Subject: Re: [PATCH] ext4: cache full extents during mapping lookup
> To: "Fengnan Chang"<changfengnan@xxxxxxxxxxxxx>
> Cc: <linux-ext4@xxxxxxxxxxxxxxx>, <linux-kernel@xxxxxxxxxxxxxxx>, <tytso@xxxxxxx>, <adilger.kernel@xxxxxxxxx>, <libaokun@xxxxxxxxxxxxxxxxx>, <jack@xxxxxxx>, <ojaswin@xxxxxxxxxxxxx>, <ritesh.list@xxxxxxxxx>
> 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 <jack@xxxxxxx>
> > Link: https://lore.kernel.org/r/20260714114358.93335-1-changfengnan@xxxxxxxxxxxxx
> > Signed-off-by: Fengnan Chang <changfengnan@xxxxxxxxxxxxx>
>
> 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,
>