[PATCH] f2fs: fix incorrect FI_NO_EXTENT handling in __destroy_extent_node()

[PATCH] f2fs: fix incorrect FI_NO_EXTENT handling in __destroy_extent_node()

[Yongpeng Yang]

From: yangyongpeng <yangyongpeng@xiaomi.com>

When __destroy_extent_node() sets the inode flag FI_NO_EXTENT, it does
not reset the length of the largest extent to 0 and update the inode
folio. Since modifications to the extent tree are disallowed afterward,
the cached largest extent may become stale. This can trigger the
following error in xfstests generic/388:

F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761) extent info [220057, 57, 6] is incorrect, run fsck to fix

In the f2fs_drop_inode path, __destroy_extent_node() does not need to
guarantee that et->node_cnt is 0, because concurrency with writeback
is expected in this path, and writeback may update the extent cache.

This patch updates __destroy_extent_node() to avoid setting the inode
flag FI_NO_EXTENT, and to remove the check zero of et->node_cnt.

Fixes: ed78aeebef05 ("f2fs: fix node_cnt race between extent node destroy and writeback")
Cc: stable@vger.kernel.org
Reported-by: Chao Yu <chao@kernel.org>
Suggested-by: Chao Yu <chao@kernel.org>
Signed-off-by: yangyongpeng <yangyongpeng@xiaomi.com>
---
 fs/f2fs/extent_cache.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 87169fd29d89..3adbead27953 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -645,14 +645,10 @@ static unsigned int __destroy_extent_node(struct inode *inode,
 
 	while (atomic_read(&et->node_cnt)) {
 		write_lock(&et->lock);
-		if (!is_inode_flag_set(inode, FI_NO_EXTENT))
-			set_inode_flag(inode, FI_NO_EXTENT);
 		node_cnt += __free_extent_tree(sbi, et, nr_shrink);
 		write_unlock(&et->lock);
 	}
 
-	f2fs_bug_on(sbi, atomic_read(&et->node_cnt));
-
 	return node_cnt;
 }
 
-- 
2.43.0

Re: [PATCH] f2fs: fix incorrect FI_NO_EXTENT handling in __destroy_extent_node()

[Chao Yu]

On 4/22/2026 3:35 PM, Yongpeng Yang wrote:
> From: yangyongpeng <yangyongpeng@xiaomi.com>
> 
> When __destroy_extent_node() sets the inode flag FI_NO_EXTENT, it does
> not reset the length of the largest extent to 0 and update the inode
> folio. Since modifications to the extent tree are disallowed afterward,
> the cached largest extent may become stale. This can trigger the
> following error in xfstests generic/388:
> 
> F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761) extent info [220057, 57, 6] is incorrect, run fsck to fix
> 
> In the f2fs_drop_inode path, __destroy_extent_node() does not need to
> guarantee that et->node_cnt is 0, because concurrency with writeback
> is expected in this path, and writeback may update the extent cache.
> 
> This patch updates __destroy_extent_node() to avoid setting the inode
> flag FI_NO_EXTENT, and to remove the check zero of et->node_cnt.
> 
> Fixes: ed78aeebef05 ("f2fs: fix node_cnt race between extent node destroy and writeback")
> Cc: stable@vger.kernel.org
> Reported-by: Chao Yu <chao@kernel.org>
> Suggested-by: Chao Yu <chao@kernel.org>
> Signed-off-by: yangyongpeng <yangyongpeng@xiaomi.com>
> ---
>   fs/f2fs/extent_cache.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index 87169fd29d89..3adbead27953 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -645,14 +645,10 @@ static unsigned int __destroy_extent_node(struct inode *inode,
>   
>   	while (atomic_read(&et->node_cnt)) {
>   		write_lock(&et->lock);
> -		if (!is_inode_flag_set(inode, FI_NO_EXTENT))
> -			set_inode_flag(inode, FI_NO_EXTENT);

We'd better revert all change lines in "f2fs: fix node_cnt race between
extent node destroy and writeback"?

Thanks,

>   		node_cnt += __free_extent_tree(sbi, et, nr_shrink);
>   		write_unlock(&et->lock);
>   	}
>   
> -	f2fs_bug_on(sbi, atomic_read(&et->node_cnt));
> -
>   	return node_cnt;
>   }
>   

Re: [f2fs-dev] [PATCH] f2fs: fix incorrect FI_NO_EXTENT handling in __destroy_extent_node()

[Yongpeng Yang]

On 4/22/26 20:33, Chao Yu via Linux-f2fs-devel wrote:
> On 4/22/2026 3:35 PM, Yongpeng Yang wrote:
>> From: yangyongpeng <yangyongpeng@xiaomi.com>
>>
>> When __destroy_extent_node() sets the inode flag FI_NO_EXTENT, it does
>> not reset the length of the largest extent to 0 and update the inode
>> folio. Since modifications to the extent tree are disallowed afterward,
>> the cached largest extent may become stale. This can trigger the
>> following error in xfstests generic/388:
>>
>> F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761) extent
>> info [220057, 57, 6] is incorrect, run fsck to fix
>>
>> In the f2fs_drop_inode path, __destroy_extent_node() does not need to
>> guarantee that et->node_cnt is 0, because concurrency with writeback
>> is expected in this path, and writeback may update the extent cache.
>>
>> This patch updates __destroy_extent_node() to avoid setting the inode
>> flag FI_NO_EXTENT, and to remove the check zero of et->node_cnt.
>>
>> Fixes: ed78aeebef05 ("f2fs: fix node_cnt race between extent node
>> destroy and writeback")
>> Cc: stable@vger.kernel.org
>> Reported-by: Chao Yu <chao@kernel.org>
>> Suggested-by: Chao Yu <chao@kernel.org>
>> Signed-off-by: yangyongpeng <yangyongpeng@xiaomi.com>
>> ---
>>   fs/f2fs/extent_cache.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>> index 87169fd29d89..3adbead27953 100644
>> --- a/fs/f2fs/extent_cache.c
>> +++ b/fs/f2fs/extent_cache.c
>> @@ -645,14 +645,10 @@ static unsigned int __destroy_extent_node(struct
>> inode *inode,
>>         while (atomic_read(&et->node_cnt)) {
>>           write_lock(&et->lock);
>> -        if (!is_inode_flag_set(inode, FI_NO_EXTENT))
>> -            set_inode_flag(inode, FI_NO_EXTENT);
> 
> We'd better revert all change lines in "f2fs: fix node_cnt race between
> extent node destroy and writeback"?

The others all check whether FI_NO_EXTENT is set. When it is set,
inserting an age extent is disallowed, so nothing was removed.

Thanks
Yongpeng,

> 
> Thanks,
> 
>>           node_cnt += __free_extent_tree(sbi, et, nr_shrink);
>>           write_unlock(&et->lock);
>>       }
>>   -    f2fs_bug_on(sbi, atomic_read(&et->node_cnt));
>> -
>>       return node_cnt;
>>   }
>>   
> 
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: fix incorrect FI_NO_EXTENT handling in __destroy_extent_node()

[Chao Yu]

On 4/24/26 17:45, Yongpeng Yang wrote:
> 
> On 4/22/26 20:33, Chao Yu via Linux-f2fs-devel wrote:
>> On 4/22/2026 3:35 PM, Yongpeng Yang wrote:
>>> From: yangyongpeng <yangyongpeng@xiaomi.com>
>>>
>>> When __destroy_extent_node() sets the inode flag FI_NO_EXTENT, it does
>>> not reset the length of the largest extent to 0 and update the inode
>>> folio. Since modifications to the extent tree are disallowed afterward,
>>> the cached largest extent may become stale. This can trigger the
>>> following error in xfstests generic/388:
>>>
>>> F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761) extent
>>> info [220057, 57, 6] is incorrect, run fsck to fix
>>>
>>> In the f2fs_drop_inode path, __destroy_extent_node() does not need to
>>> guarantee that et->node_cnt is 0, because concurrency with writeback
>>> is expected in this path, and writeback may update the extent cache.
>>>
>>> This patch updates __destroy_extent_node() to avoid setting the inode
>>> flag FI_NO_EXTENT, and to remove the check zero of et->node_cnt.
>>>
>>> Fixes: ed78aeebef05 ("f2fs: fix node_cnt race between extent node
>>> destroy and writeback")
>>> Cc: stable@vger.kernel.org
>>> Reported-by: Chao Yu <chao@kernel.org>
>>> Suggested-by: Chao Yu <chao@kernel.org>
>>> Signed-off-by: yangyongpeng <yangyongpeng@xiaomi.com>
>>> ---
>>>    fs/f2fs/extent_cache.c | 4 ----
>>>    1 file changed, 4 deletions(-)
>>>
>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>> index 87169fd29d89..3adbead27953 100644
>>> --- a/fs/f2fs/extent_cache.c
>>> +++ b/fs/f2fs/extent_cache.c
>>> @@ -645,14 +645,10 @@ static unsigned int __destroy_extent_node(struct
>>> inode *inode,
>>>          while (atomic_read(&et->node_cnt)) {
>>>            write_lock(&et->lock);
>>> -        if (!is_inode_flag_set(inode, FI_NO_EXTENT))
>>> -            set_inode_flag(inode, FI_NO_EXTENT);
>>
>> We'd better revert all change lines in "f2fs: fix node_cnt race between
>> extent node destroy and writeback"?
> 
> The others all check whether FI_NO_EXTENT is set. When it is set,
> inserting an age extent is disallowed, so nothing was removed.

diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 87169fd29d89..0ed84cc065a7 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -119,10 +119,9 @@ static bool __may_extent_tree(struct inode *inode, enum extent_type type)
         if (!__init_may_extent_tree(inode, type))
                 return false;

-       if (is_inode_flag_set(inode, FI_NO_EXTENT))
-               return false;
-
         if (type == EX_READ) {
+               if (is_inode_flag_set(inode, FI_NO_EXTENT))
+                       return false;
                 if (is_inode_flag_set(inode, FI_COMPRESSED_FILE) &&
                                  !f2fs_sb_has_readonly(F2FS_I_SB(inode)))
                         return false;

...

@@ -691,12 +688,12 @@ static void __update_extent_tree_range(struct inode *inode,

         write_lock(&et->lock);

-       if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
-               write_unlock(&et->lock);
-               return;
-       }
-
         if (type == EX_READ) {
+               if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
+                       write_unlock(&et->lock);
+                       return;
+               }
+
                 prev = et->largest;
                 dei.len = 0;

Hmm, I'm not sure I understood you correctly, if you want to keep above codes, what
about changing in another patch w/ correct commit message?

Thanks,


> 
> Thanks
> Yongpeng,
> 
>>
>> Thanks,
>>
>>>            node_cnt += __free_extent_tree(sbi, et, nr_shrink);
>>>            write_unlock(&et->lock);
>>>        }
>>>    -    f2fs_bug_on(sbi, atomic_read(&et->node_cnt));
>>> -
>>>        return node_cnt;
>>>    }
>>>    
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 

Re: [f2fs-dev] [PATCH] f2fs: fix incorrect FI_NO_EXTENT handling in __destroy_extent_node()

[Yongpeng Yang]

On 4/27/26 15:38, Chao Yu via Linux-f2fs-devel wrote:
> On 4/24/26 17:45, Yongpeng Yang wrote:
>>
>> On 4/22/26 20:33, Chao Yu via Linux-f2fs-devel wrote:
>>> On 4/22/2026 3:35 PM, Yongpeng Yang wrote:
>>>> From: yangyongpeng <yangyongpeng@xiaomi.com>
>>>>
>>>> When __destroy_extent_node() sets the inode flag FI_NO_EXTENT, it does
>>>> not reset the length of the largest extent to 0 and update the inode
>>>> folio. Since modifications to the extent tree are disallowed afterward,
>>>> the cached largest extent may become stale. This can trigger the
>>>> following error in xfstests generic/388:
>>>>
>>>> F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761) extent
>>>> info [220057, 57, 6] is incorrect, run fsck to fix
>>>>
>>>> In the f2fs_drop_inode path, __destroy_extent_node() does not need to
>>>> guarantee that et->node_cnt is 0, because concurrency with writeback
>>>> is expected in this path, and writeback may update the extent cache.
>>>>
>>>> This patch updates __destroy_extent_node() to avoid setting the inode
>>>> flag FI_NO_EXTENT, and to remove the check zero of et->node_cnt.
>>>>
>>>> Fixes: ed78aeebef05 ("f2fs: fix node_cnt race between extent node
>>>> destroy and writeback")
>>>> Cc: stable@vger.kernel.org
>>>> Reported-by: Chao Yu <chao@kernel.org>
>>>> Suggested-by: Chao Yu <chao@kernel.org>
>>>> Signed-off-by: yangyongpeng <yangyongpeng@xiaomi.com>
>>>> ---
>>>>    fs/f2fs/extent_cache.c | 4 ----
>>>>    1 file changed, 4 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>> index 87169fd29d89..3adbead27953 100644
>>>> --- a/fs/f2fs/extent_cache.c
>>>> +++ b/fs/f2fs/extent_cache.c
>>>> @@ -645,14 +645,10 @@ static unsigned int __destroy_extent_node(struct
>>>> inode *inode,
>>>>          while (atomic_read(&et->node_cnt)) {
>>>>            write_lock(&et->lock);
>>>> -        if (!is_inode_flag_set(inode, FI_NO_EXTENT))
>>>> -            set_inode_flag(inode, FI_NO_EXTENT);
>>>
>>> We'd better revert all change lines in "f2fs: fix node_cnt race between
>>> extent node destroy and writeback"?
>>
>> The others all check whether FI_NO_EXTENT is set. When it is set,
>> inserting an age extent is disallowed, so nothing was removed.
> 
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index 87169fd29d89..0ed84cc065a7 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -119,10 +119,9 @@ static bool __may_extent_tree(struct inode *inode,
> enum extent_type type)
>         if (!__init_may_extent_tree(inode, type))
>                 return false;
> 
> -       if (is_inode_flag_set(inode, FI_NO_EXTENT))
> -               return false;
> -
>         if (type == EX_READ) {
> +               if (is_inode_flag_set(inode, FI_NO_EXTENT))
> +                       return false;

This should be revert. The EX_BLOCK_AGE extent tree type was added later
and has never been governed by FI_NO_EXTENT. After reverting the commit
ed78aeebef05, having FI_NO_EXTENT set no longer implies that the inode
needs to be evicted, so rejecting updates to EX_BLOCK_AGE extents based
on this flag no longer makes sense. Therefore, this part of the change
should be dropped.

>                 if (is_inode_flag_set(inode, FI_COMPRESSED_FILE) &&
>                                  !f2fs_sb_has_readonly(F2FS_I_SB(inode)))
>                         return false;
> 
> ...
> 
> @@ -691,12 +688,12 @@ static void __update_extent_tree_range(struct
> inode *inode,
> 
>         write_lock(&et->lock);
> 
> -       if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
> -               write_unlock(&et->lock);
> -               return;
> -       }

This also should be revert. All callers of this function already invoke
__may_extent_tree() to verify whether FI_NO_EXTENT is set. So, this
check are dead code.

> -
>         if (type == EX_READ) {
> +               if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
> +                       write_unlock(&et->lock);
> +                       return;
> +               }
> +
>                 prev = et->largest;
>                 dei.len = 0;
> 
> Hmm, I'm not sure I understood you correctly, if you want to keep above
> codes, what
> about changing in another patch w/ correct commit message?

Yes, I mean that, but I was mistaken. I'll fix above issue in v2 patch.

Thanks
Yongpeng,

> 
> Thanks,
> 
> 
>>
>> Thanks
>> Yongpeng,
>>
>>>
>>> Thanks,
>>>
>>>>            node_cnt += __free_extent_tree(sbi, et, nr_shrink);
>>>>            write_unlock(&et->lock);
>>>>        }
>>>>    -    f2fs_bug_on(sbi, atomic_read(&et->node_cnt));
>>>> -
>>>>        return node_cnt;
>>>>    }
>>>>    
>>>
>>>
>>>
>>> _______________________________________________
>>> Linux-f2fs-devel mailing list
>>> Linux-f2fs-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>
> 
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel