Re: [PATCH v6 05/31] ext4: recheck extent status tree before block allocation

[Zhang Yi]

On 9/24/2026 9:22 PM, Ojaswin Mujoo wrote:
> On Thu, Sep 03, 2026 at 08:35:17PM +0800, Zhang Yi wrote:
> > From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> >
> > After acquiring i_data_sem in write mode, recheck that the mapping
> > found via the extent status tree or disk query has not changed. A
> > racing truncate may have trimmed the extent between the earlier lookup
> > and the write lock acquisition, since writeback does not hold i_rwsem
> > or the folio locks covering the full extent. This could cause
> > ext4_map_create_blocks() to allocate blocks beyond the truncated range,
> > potentially leading to quota leaks in the upcomming iomap buffered
> > writeback path since the iomap writeback infrastructure caches extents
> > beyond the folio range.
> >
> > Therefore, if we find a valid extent and the sequence number has
> > changed, retry the entire lookup to obtain the correct trimmed mapping.
> >
> > Suggested-by: Jan Kara <jack@xxxxxxx>
> > Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> > ---
> >   fs/ext4/inode.c | 16 ++++++++++++++++
> >   1 file changed, 16 insertions(+)
> >
> > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > index 84991fe99071..b71b1d2588ae 100644
> > --- a/fs/ext4/inode.c
> > +++ b/fs/ext4/inode.c
> > @@ -734,6 +734,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
> >   	else
> >   		ext4_check_map_extents_env(inode);
> >   
> > +create_retry:
> >   	/* Lookup extent status tree firstly */
> >   	if (ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es, &map->m_seq)) {
> >   		if (ext4_es_is_written(&es) || ext4_es_is_unwritten(&es)) {
> > @@ -784,6 +785,8 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
> >   	down_read(&EXT4_I(inode)->i_data_sem);
> >   	retval = ext4_map_query_blocks(handle, inode, map, flags);
> >   	up_read((&EXT4_I(inode)->i_data_sem));
> > +	if (retval < 0)
> > +		return retval;
>
> Hey Zhang, I think there's a slight change in behavior that we return if
> the query fails now. Earlier we would have still tried the create_blocks
> call and, in case of transient errors like ENOMEM, might have suceeded.
> I don't think it's that big of a deal though.
>
> Rest looks fine and I agree with your reply to Sashiko, I think we
> can tweak those things if we ever encounter a livelock.
>
> Feel free to add:
>
> Reviewed-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>
>
> Also, if you don't mind can you add
>
> Link: https://lore.kernel.org/linux-ext4/b0781809-4759-4e12-be17-71555b764f48@xxxxxxxxx/
>
> (or the call trace) to the next version as it explains the issue pretty well.
> Looking at these things later takes some time to recall everything and
> such traces help a lot.

Hi, Ojaswin!

Thank you for the review and suggestion, this makes sense to me. I will
add this link in the next iteration.

Thanks,
Yi.

> Regards,
> ojaswin
>
> >   
> >   found:
> >   	if (retval > 0 && map->m_flags & EXT4_MAP_MAPPED) {
> > @@ -820,6 +823,19 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
> >   	 * with create == 1 flag.
> >   	 */
> >   	down_write(&EXT4_I(inode)->i_data_sem);
> > +
> > +	/*
> > +	 * Check the validity of the mapping found via the extent status
> > +	 * tree or the disk query. A racing truncate may have changed the
> > +	 * extent, since writeback does not hold i_rwsem or the folio locks
> > +	 * covering the full extent.
> > +	 */
> > +	if (map->m_seq != READ_ONCE(EXT4_I(inode)->i_es_seq)) {
> > +		up_write(&EXT4_I(inode)->i_data_sem);
> > +		map->m_flags = 0;
> > +		map->m_len = orig_mlen;
> > +		goto create_retry;
> > +	}
> >   	retval = ext4_map_create_blocks(handle, inode, map, flags);
> >   	up_write((&EXT4_I(inode)->i_data_sem));
> >   
> > --
> > 2.52.0