[PATCH] f2fs: fix node_cnt race between extent node destroy and writeback

[PATCH] f2fs: fix node_cnt race between extent node destroy and writeback

[Yongpeng Yang]

From: Yongpeng Yang <yangyongpeng@xiaomi.com>

f2fs_destroy_extent_node() does not set FI_NO_EXTENT before clearing
extent nodes. When called from f2fs_drop_inode() with I_SYNC set,
concurrent kworker writeback can insert new extent nodes into the same
extent tree, racing with the destroy and triggering f2fs_bug_on() in
__destroy_extent_node(). The scenario is as follows:

drop inode                            writeback
 - iput
  - f2fs_drop_inode  // I_SYNC set
   - f2fs_destroy_extent_node
    - __destroy_extent_node
     - while (node_cnt) {
        write_lock(&et->lock)
        __free_extent_tree
        write_unlock(&et->lock)
                                       - __writeback_single_inode
                                        - f2fs_outplace_write_data
                                         - f2fs_update_read_extent_cache
                                          - __update_extent_tree_range
                                           // FI_NO_EXTENT not set,
                                           // insert new extent node
       } // node_cnt == 0, exit while
     - f2fs_bug_on(node_cnt)  // node_cnt > 0

Additionally, __update_extent_tree_range() only checks FI_NO_EXTENT for
EX_READ type, leaving EX_BLOCK_AGE updates completely unprotected.

This patch set FI_NO_EXTENT under et->lock in __destroy_extent_node(),
consistent with other callers (__update_extent_tree_range and
__drop_extent_tree) and check FI_NO_EXTENT for both EX_READ and
EX_BLOCK_AGE tree.

Fixes: 3fc5d5a182f6 ("f2fs: fix to shrink read extent node in batches")
Cc: stable@vger.kernel.org
Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
---
 fs/f2fs/extent_cache.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 0ed84cc065a7..87169fd29d89 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -119,9 +119,10 @@ 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;
@@ -644,6 +645,8 @@ 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);
 	}
@@ -688,12 +691,12 @@ static void __update_extent_tree_range(struct inode *inode,
 
 	write_lock(&et->lock);
 
-	if (type == EX_READ) {
-		if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
-			write_unlock(&et->lock);
-			return;
-		}
+	if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
+		write_unlock(&et->lock);
+		return;
+	}
 
+	if (type == EX_READ) {
 		prev = et->largest;
 		dei.len = 0;
 
-- 
2.43.0

Re: [PATCH] f2fs: fix node_cnt race between extent node destroy and writeback

[Chao Yu]

On 4/3/2026 10:40 PM, Yongpeng Yang wrote:
> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
> 
> f2fs_destroy_extent_node() does not set FI_NO_EXTENT before clearing
> extent nodes. When called from f2fs_drop_inode() with I_SYNC set,
> concurrent kworker writeback can insert new extent nodes into the same
> extent tree, racing with the destroy and triggering f2fs_bug_on() in
> __destroy_extent_node(). The scenario is as follows:
> 
> drop inode                            writeback
>   - iput
>    - f2fs_drop_inode  // I_SYNC set
>     - f2fs_destroy_extent_node
>      - __destroy_extent_node
>       - while (node_cnt) {
>          write_lock(&et->lock)
>          __free_extent_tree
>          write_unlock(&et->lock)
>                                         - __writeback_single_inode
>                                          - f2fs_outplace_write_data
>                                           - f2fs_update_read_extent_cache
>                                            - __update_extent_tree_range
>                                             // FI_NO_EXTENT not set,
>                                             // insert new extent node
>         } // node_cnt == 0, exit while
>       - f2fs_bug_on(node_cnt)  // node_cnt > 0
> 
> Additionally, __update_extent_tree_range() only checks FI_NO_EXTENT for
> EX_READ type, leaving EX_BLOCK_AGE updates completely unprotected.
> 
> This patch set FI_NO_EXTENT under et->lock in __destroy_extent_node(),
> consistent with other callers (__update_extent_tree_range and
> __drop_extent_tree) and check FI_NO_EXTENT for both EX_READ and
> EX_BLOCK_AGE tree.
> 
> Fixes: 3fc5d5a182f6 ("f2fs: fix to shrink read extent node in batches")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [f2fs-dev] [PATCH] f2fs: fix node_cnt race between extent node destroy and writeback

[patchwork-bot+f2fs]

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:

On Fri,  3 Apr 2026 22:40:17 +0800 you wrote:
> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
> 
> f2fs_destroy_extent_node() does not set FI_NO_EXTENT before clearing
> extent nodes. When called from f2fs_drop_inode() with I_SYNC set,
> concurrent kworker writeback can insert new extent nodes into the same
> extent tree, racing with the destroy and triggering f2fs_bug_on() in
> __destroy_extent_node(). The scenario is as follows:
> 
> [...]

Here is the summary with links:
  - [f2fs-dev] f2fs: fix node_cnt race between extent node destroy and writeback
    https://git.kernel.org/jaegeuk/f2fs/c/ed78aeebef05

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] f2fs: fix node_cnt race between extent node destroy and writeback

[Chao Yu]

On 4/3/26 22:40, Yongpeng Yang wrote:
> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
> 
> f2fs_destroy_extent_node() does not set FI_NO_EXTENT before clearing
> extent nodes. When called from f2fs_drop_inode() with I_SYNC set,
> concurrent kworker writeback can insert new extent nodes into the same
> extent tree, racing with the destroy and triggering f2fs_bug_on() in
> __destroy_extent_node(). The scenario is as follows:
> 
> drop inode                            writeback
>   - iput
>    - f2fs_drop_inode  // I_SYNC set
>     - f2fs_destroy_extent_node
>      - __destroy_extent_node
>       - while (node_cnt) {
>          write_lock(&et->lock)
>          __free_extent_tree
>          write_unlock(&et->lock)
>                                         - __writeback_single_inode
>                                          - f2fs_outplace_write_data
>                                           - f2fs_update_read_extent_cache
>                                            - __update_extent_tree_range
>                                             // FI_NO_EXTENT not set,
>                                             // insert new extent node
>         } // node_cnt == 0, exit while
>       - f2fs_bug_on(node_cnt)  // node_cnt > 0
> 
> Additionally, __update_extent_tree_range() only checks FI_NO_EXTENT for
> EX_READ type, leaving EX_BLOCK_AGE updates completely unprotected.
> 
> This patch set FI_NO_EXTENT under et->lock in __destroy_extent_node(),
> consistent with other callers (__update_extent_tree_range and
> __drop_extent_tree) and check FI_NO_EXTENT for both EX_READ and
> EX_BLOCK_AGE tree.

I suffered below test failure, then I bisect to this change.

     generic/475  84s ... [failed, exit status 1]- output mismatch (see /share/git/fstests/results//generic/475.out.bad)
     --- tests/generic/475.out   2025-01-12 21:57:40.279440664 +0800
     +++ /share/git/fstests/results//generic/475.out.bad 2026-04-17 12:08:28.000000000 +0800
     @@ -1,2 +1,6 @@
      QA output created by 475
      Silence is golden.
     +mount: /mnt/scratch_f2fs: mount system call failed: Structure needs cleaning.
     +       dmesg(1) may have more information after failed mount system call.
     +mount failed
     +(see /share/git/fstests/results//generic/475.full for details)
     ...
     (Run 'diff -u /share/git/fstests/tests/generic/475.out /share/git/fstests/results//generic/475.out.bad'  to see the entire diff)


     generic/388  73s ... [failed, exit status 1]- output mismatch (see /share/git/fstests/results//generic/388.out.bad)
     --- tests/generic/388.out   2025-01-12 21:57:40.275440602 +0800
     +++ /share/git/fstests/results//generic/388.out.bad 2026-04-17 11:58:05.000000000 +0800
     @@ -1,2 +1,6 @@
      QA output created by 388
      Silence is golden.
     +mount: /mnt/scratch_f2fs: mount system call failed: Structure needs cleaning.
     +       dmesg(1) may have more information after failed mount system call.
     +cycle mount failed
     +(see /share/git/fstests/results//generic/388.full for details)
     ...
     (Run 'diff -u /share/git/fstests/tests/generic/388.out /share/git/fstests/results//generic/388.out.bad'  to see the entire diff)


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

I suspect we may miss any extent updates after we set FI_NO_EXTENT in
__destroy_extent_node(), result in failing in sanity_check_extent_cache().

Can we just relocate f2fs_bug_on(node_cnt) rather than complicated change?
Thoughts?

Thanks,

> 
> Fixes: 3fc5d5a182f6 ("f2fs: fix to shrink read extent node in batches")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
> ---
>   fs/f2fs/extent_cache.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
> index 0ed84cc065a7..87169fd29d89 100644
> --- a/fs/f2fs/extent_cache.c
> +++ b/fs/f2fs/extent_cache.c
> @@ -119,9 +119,10 @@ 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;
> @@ -644,6 +645,8 @@ 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);
>   	}
> @@ -688,12 +691,12 @@ static void __update_extent_tree_range(struct inode *inode,
>   
>   	write_lock(&et->lock);
>   
> -	if (type == EX_READ) {
> -		if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
> -			write_unlock(&et->lock);
> -			return;
> -		}
> +	if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
> +		write_unlock(&et->lock);
> +		return;
> +	}
>   
> +	if (type == EX_READ) {
>   		prev = et->largest;
>   		dei.len = 0;
>   

Re: [f2fs-dev] [PATCH] f2fs: fix node_cnt race between extent node destroy and writeback

[Yongpeng Yang]

On 4/17/26 17:00, Chao Yu via Linux-f2fs-devel wrote:
> On 4/3/26 22:40, Yongpeng Yang wrote:
>> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>
>> f2fs_destroy_extent_node() does not set FI_NO_EXTENT before clearing
>> extent nodes. When called from f2fs_drop_inode() with I_SYNC set,
>> concurrent kworker writeback can insert new extent nodes into the same
>> extent tree, racing with the destroy and triggering f2fs_bug_on() in
>> __destroy_extent_node(). The scenario is as follows:
>>
>> drop inode                            writeback
>>   - iput
>>    - f2fs_drop_inode  // I_SYNC set
>>     - f2fs_destroy_extent_node
>>      - __destroy_extent_node
>>       - while (node_cnt) {
>>          write_lock(&et->lock)
>>          __free_extent_tree
>>          write_unlock(&et->lock)
>>                                         - __writeback_single_inode
>>                                          - f2fs_outplace_write_data
>>                                           - f2fs_update_read_extent_cache
>>                                            - __update_extent_tree_range
>>                                             // FI_NO_EXTENT not set,
>>                                             // insert new extent node
>>         } // node_cnt == 0, exit while
>>       - f2fs_bug_on(node_cnt)  // node_cnt > 0
>>
>> Additionally, __update_extent_tree_range() only checks FI_NO_EXTENT for
>> EX_READ type, leaving EX_BLOCK_AGE updates completely unprotected.
>>
>> This patch set FI_NO_EXTENT under et->lock in __destroy_extent_node(),
>> consistent with other callers (__update_extent_tree_range and
>> __drop_extent_tree) and check FI_NO_EXTENT for both EX_READ and
>> EX_BLOCK_AGE tree.
> 
> I suffered below test failure, then I bisect to this change.
> 
>     generic/475  84s ... [failed, exit status 1]- output mismatch (see /
> share/git/fstests/results//generic/475.out.bad)
>     --- tests/generic/475.out   2025-01-12 21:57:40.279440664 +0800
>     +++ /share/git/fstests/results//generic/475.out.bad 2026-04-17
> 12:08:28.000000000 +0800
>     @@ -1,2 +1,6 @@
>      QA output created by 475
>      Silence is golden.
>     +mount: /mnt/scratch_f2fs: mount system call failed: Structure needs
> cleaning.
>     +       dmesg(1) may have more information after failed mount system
> call.
>     +mount failed
>     +(see /share/git/fstests/results//generic/475.full for details)
>     ...
>     (Run 'diff -u /share/git/fstests/tests/generic/475.out /share/git/
> fstests/results//generic/475.out.bad'  to see the entire diff)
> 
> 
>     generic/388  73s ... [failed, exit status 1]- output mismatch (see /
> share/git/fstests/results//generic/388.out.bad)
>     --- tests/generic/388.out   2025-01-12 21:57:40.275440602 +0800
>     +++ /share/git/fstests/results//generic/388.out.bad 2026-04-17
> 11:58:05.000000000 +0800
>     @@ -1,2 +1,6 @@
>      QA output created by 388
>      Silence is golden.
>     +mount: /mnt/scratch_f2fs: mount system call failed: Structure needs
> cleaning.
>     +       dmesg(1) may have more information after failed mount system
> call.
>     +cycle mount failed
>     +(see /share/git/fstests/results//generic/388.full for details)
>     ...
>     (Run 'diff -u /share/git/fstests/tests/generic/388.out /share/git/
> fstests/results//generic/388.out.bad'  to see the entire diff)
> 
> 
>     F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761) extent
> info [220057, 57, 6] is incorrect, run fsck to fix
> 
> I suspect we may miss any extent updates after we set FI_NO_EXTENT in
> __destroy_extent_node(), result in failing in sanity_check_extent_cache().
> 
> Can we just relocate f2fs_bug_on(node_cnt) rather than complicated change?
> Thoughts?

Oh, I overlooked largest extent. How about relocate
f2fs_bug_on(node_cnt) to __destroy_extent_tree?

static void __destroy_extent_tree(struct inode *inode, enum extent_type
type)

        /* free all extent info belong to this extent tree */
        node_cnt = __destroy_extent_node(inode, type);
+       f2fs_bug_on(sbi, atomic_read(&et->node_cnt));

Thanks
Yongpeng,

> 
> Thanks,
> 
>>
>> Fixes: 3fc5d5a182f6 ("f2fs: fix to shrink read extent node in batches")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
>> ---
>>   fs/f2fs/extent_cache.c | 17 ++++++++++-------
>>   1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>> index 0ed84cc065a7..87169fd29d89 100644
>> --- a/fs/f2fs/extent_cache.c
>> +++ b/fs/f2fs/extent_cache.c
>> @@ -119,9 +119,10 @@ 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;
>> @@ -644,6 +645,8 @@ 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);
>>       }
>> @@ -688,12 +691,12 @@ static void __update_extent_tree_range(struct
>> inode *inode,
>>         write_lock(&et->lock);
>>   -    if (type == EX_READ) {
>> -        if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>> -            write_unlock(&et->lock);
>> -            return;
>> -        }
>> +    if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>> +        write_unlock(&et->lock);
>> +        return;
>> +    }
>>   +    if (type == EX_READ) {
>>           prev = et->largest;
>>           dei.len = 0;
>>   
> 
> 
> 
> _______________________________________________
> 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 node_cnt race between extent node destroy and writeback

[Chao Yu]

On 4/17/26 21:26, Yongpeng Yang wrote:
> 
> On 4/17/26 17:00, Chao Yu via Linux-f2fs-devel wrote:
>> On 4/3/26 22:40, Yongpeng Yang wrote:
>>> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>>
>>> f2fs_destroy_extent_node() does not set FI_NO_EXTENT before clearing
>>> extent nodes. When called from f2fs_drop_inode() with I_SYNC set,
>>> concurrent kworker writeback can insert new extent nodes into the same
>>> extent tree, racing with the destroy and triggering f2fs_bug_on() in
>>> __destroy_extent_node(). The scenario is as follows:
>>>
>>> drop inode                            writeback
>>>    - iput
>>>     - f2fs_drop_inode  // I_SYNC set
>>>      - f2fs_destroy_extent_node
>>>       - __destroy_extent_node
>>>        - while (node_cnt) {
>>>           write_lock(&et->lock)
>>>           __free_extent_tree
>>>           write_unlock(&et->lock)
>>>                                          - __writeback_single_inode
>>>                                           - f2fs_outplace_write_data
>>>                                            - f2fs_update_read_extent_cache
>>>                                             - __update_extent_tree_range
>>>                                              // FI_NO_EXTENT not set,
>>>                                              // insert new extent node
>>>          } // node_cnt == 0, exit while
>>>        - f2fs_bug_on(node_cnt)  // node_cnt > 0
>>>
>>> Additionally, __update_extent_tree_range() only checks FI_NO_EXTENT for
>>> EX_READ type, leaving EX_BLOCK_AGE updates completely unprotected.
>>>
>>> This patch set FI_NO_EXTENT under et->lock in __destroy_extent_node(),
>>> consistent with other callers (__update_extent_tree_range and
>>> __drop_extent_tree) and check FI_NO_EXTENT for both EX_READ and
>>> EX_BLOCK_AGE tree.
>>
>> I suffered below test failure, then I bisect to this change.
>>
>>      generic/475  84s ... [failed, exit status 1]- output mismatch (see /
>> share/git/fstests/results//generic/475.out.bad)
>>      --- tests/generic/475.out   2025-01-12 21:57:40.279440664 +0800
>>      +++ /share/git/fstests/results//generic/475.out.bad 2026-04-17
>> 12:08:28.000000000 +0800
>>      @@ -1,2 +1,6 @@
>>       QA output created by 475
>>       Silence is golden.
>>      +mount: /mnt/scratch_f2fs: mount system call failed: Structure needs
>> cleaning.
>>      +       dmesg(1) may have more information after failed mount system
>> call.
>>      +mount failed
>>      +(see /share/git/fstests/results//generic/475.full for details)
>>      ...
>>      (Run 'diff -u /share/git/fstests/tests/generic/475.out /share/git/
>> fstests/results//generic/475.out.bad'  to see the entire diff)
>>
>>
>>      generic/388  73s ... [failed, exit status 1]- output mismatch (see /
>> share/git/fstests/results//generic/388.out.bad)
>>      --- tests/generic/388.out   2025-01-12 21:57:40.275440602 +0800
>>      +++ /share/git/fstests/results//generic/388.out.bad 2026-04-17
>> 11:58:05.000000000 +0800
>>      @@ -1,2 +1,6 @@
>>       QA output created by 388
>>       Silence is golden.
>>      +mount: /mnt/scratch_f2fs: mount system call failed: Structure needs
>> cleaning.
>>      +       dmesg(1) may have more information after failed mount system
>> call.
>>      +cycle mount failed
>>      +(see /share/git/fstests/results//generic/388.full for details)
>>      ...
>>      (Run 'diff -u /share/git/fstests/tests/generic/388.out /share/git/
>> fstests/results//generic/388.out.bad'  to see the entire diff)
>>
>>
>>      F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761) extent
>> info [220057, 57, 6] is incorrect, run fsck to fix
>>
>> I suspect we may miss any extent updates after we set FI_NO_EXTENT in
>> __destroy_extent_node(), result in failing in sanity_check_extent_cache().
>>
>> Can we just relocate f2fs_bug_on(node_cnt) rather than complicated change?
>> Thoughts?
> 
> Oh, I overlooked largest extent. How about relocate
> f2fs_bug_on(node_cnt) to __destroy_extent_tree?
> 
> static void __destroy_extent_tree(struct inode *inode, enum extent_type
> type)
> 
>          /* free all extent info belong to this extent tree */
>          node_cnt = __destroy_extent_node(inode, type);
> +       f2fs_bug_on(sbi, atomic_read(&et->node_cnt));

	/* free all extent info belong to this extent tree */
	node_cnt = __destroy_extent_node(inode, type);

	/* delete extent tree entry in radix tree */
	mutex_lock(&eti->extent_tree_lock);
	f2fs_bug_on(sbi, atomic_read(&et->node_cnt));  <---

Oh, it has already checked node_cnt, so, maybe we can just remove the check in
__destroy_extent_node()?

Thanks,


> 
> Thanks
> Yongpeng,
> 
>>
>> Thanks,
>>
>>>
>>> Fixes: 3fc5d5a182f6 ("f2fs: fix to shrink read extent node in batches")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>> ---
>>>    fs/f2fs/extent_cache.c | 17 ++++++++++-------
>>>    1 file changed, 10 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>> index 0ed84cc065a7..87169fd29d89 100644
>>> --- a/fs/f2fs/extent_cache.c
>>> +++ b/fs/f2fs/extent_cache.c
>>> @@ -119,9 +119,10 @@ 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;
>>> @@ -644,6 +645,8 @@ 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);
>>>        }
>>> @@ -688,12 +691,12 @@ static void __update_extent_tree_range(struct
>>> inode *inode,
>>>          write_lock(&et->lock);
>>>    -    if (type == EX_READ) {
>>> -        if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>> -            write_unlock(&et->lock);
>>> -            return;
>>> -        }
>>> +    if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>> +        write_unlock(&et->lock);
>>> +        return;
>>> +    }
>>>    +    if (type == EX_READ) {
>>>            prev = et->largest;
>>>            dei.len = 0;
>>>    
>>
>>
>>
>> _______________________________________________
>> 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 node_cnt race between extent node destroy and writeback

[Yongpeng Yang]

On 4/18/26 8:51 AM, Chao Yu via Linux-f2fs-devel wrote:
> On 4/17/26 21:26, Yongpeng Yang wrote:
>>
>> On 4/17/26 17:00, Chao Yu via Linux-f2fs-devel wrote:
>>> On 4/3/26 22:40, Yongpeng Yang wrote:
>>>> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>>>
>>>> f2fs_destroy_extent_node() does not set FI_NO_EXTENT before clearing
>>>> extent nodes. When called from f2fs_drop_inode() with I_SYNC set,
>>>> concurrent kworker writeback can insert new extent nodes into the same
>>>> extent tree, racing with the destroy and triggering f2fs_bug_on() in
>>>> __destroy_extent_node(). The scenario is as follows:
>>>>
>>>> drop inode                            writeback
>>>>    - iput
>>>>     - f2fs_drop_inode  // I_SYNC set
>>>>      - f2fs_destroy_extent_node
>>>>       - __destroy_extent_node
>>>>        - while (node_cnt) {
>>>>           write_lock(&et->lock)
>>>>           __free_extent_tree
>>>>           write_unlock(&et->lock)
>>>>                                          - __writeback_single_inode
>>>>                                           - f2fs_outplace_write_data
>>>>                                            - 
>>>> f2fs_update_read_extent_cache
>>>>                                             - 
>>>> __update_extent_tree_range
>>>>                                              // FI_NO_EXTENT not set,
>>>>                                              // insert new extent node
>>>>          } // node_cnt == 0, exit while
>>>>        - f2fs_bug_on(node_cnt)  // node_cnt > 0
>>>>
>>>> Additionally, __update_extent_tree_range() only checks FI_NO_EXTENT for
>>>> EX_READ type, leaving EX_BLOCK_AGE updates completely unprotected.
>>>>
>>>> This patch set FI_NO_EXTENT under et->lock in __destroy_extent_node(),
>>>> consistent with other callers (__update_extent_tree_range and
>>>> __drop_extent_tree) and check FI_NO_EXTENT for both EX_READ and
>>>> EX_BLOCK_AGE tree.
>>>
>>> I suffered below test failure, then I bisect to this change.
>>>
>>>      generic/475  84s ... [failed, exit status 1]- output mismatch 
>>> (see /
>>> share/git/fstests/results//generic/475.out.bad)
>>>      --- tests/generic/475.out   2025-01-12 21:57:40.279440664 +0800
>>>      +++ /share/git/fstests/results//generic/475.out.bad 2026-04-17
>>> 12:08:28.000000000 +0800
>>>      @@ -1,2 +1,6 @@
>>>       QA output created by 475
>>>       Silence is golden.
>>>      +mount: /mnt/scratch_f2fs: mount system call failed: Structure 
>>> needs
>>> cleaning.
>>>      +       dmesg(1) may have more information after failed mount 
>>> system
>>> call.
>>>      +mount failed
>>>      +(see /share/git/fstests/results//generic/475.full for details)
>>>      ...
>>>      (Run 'diff -u /share/git/fstests/tests/generic/475.out /share/git/
>>> fstests/results//generic/475.out.bad'  to see the entire diff)
>>>
>>>
>>>      generic/388  73s ... [failed, exit status 1]- output mismatch 
>>> (see /
>>> share/git/fstests/results//generic/388.out.bad)
>>>      --- tests/generic/388.out   2025-01-12 21:57:40.275440602 +0800
>>>      +++ /share/git/fstests/results//generic/388.out.bad 2026-04-17
>>> 11:58:05.000000000 +0800
>>>      @@ -1,2 +1,6 @@
>>>       QA output created by 388
>>>       Silence is golden.
>>>      +mount: /mnt/scratch_f2fs: mount system call failed: Structure 
>>> needs
>>> cleaning.
>>>      +       dmesg(1) may have more information after failed mount 
>>> system
>>> call.
>>>      +cycle mount failed
>>>      +(see /share/git/fstests/results//generic/388.full for details)
>>>      ...
>>>      (Run 'diff -u /share/git/fstests/tests/generic/388.out /share/git/
>>> fstests/results//generic/388.out.bad'  to see the entire diff)
>>>
>>>
>>>      F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761) extent
>>> info [220057, 57, 6] is incorrect, run fsck to fix
>>>
>>> I suspect we may miss any extent updates after we set FI_NO_EXTENT in
>>> __destroy_extent_node(), result in failing in 
>>> sanity_check_extent_cache().
>>>
>>> Can we just relocate f2fs_bug_on(node_cnt) rather than complicated 
>>> change?
>>> Thoughts?
>>
>> Oh, I overlooked largest extent. How about relocate
>> f2fs_bug_on(node_cnt) to __destroy_extent_tree?
>>
>> static void __destroy_extent_tree(struct inode *inode, enum extent_type
>> type)
>>
>>          /* free all extent info belong to this extent tree */
>>          node_cnt = __destroy_extent_node(inode, type);
>> +       f2fs_bug_on(sbi, atomic_read(&et->node_cnt));
> 
>      /* free all extent info belong to this extent tree */
>      node_cnt = __destroy_extent_node(inode, type);
> 
>      /* delete extent tree entry in radix tree */
>      mutex_lock(&eti->extent_tree_lock);
>      f2fs_bug_on(sbi, atomic_read(&et->node_cnt));  <---
> 
> Oh, it has already checked node_cnt, so, maybe we can just remove the 
> check in
> __destroy_extent_node()?

Yes. BTW, is it correct to remove the call to f2fs_destroy_extent_node()
in f2fs_drop_inode()? It seems this call is unnecessary, since
f2fs_evict_inode() will eventually delete all extent nodes properly.

Thanks
Yongpeng,

> 
> Thanks,
> 
> 
>>
>> Thanks
>> Yongpeng,
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Fixes: 3fc5d5a182f6 ("f2fs: fix to shrink read extent node in batches")
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>>> ---
>>>>    fs/f2fs/extent_cache.c | 17 ++++++++++-------
>>>>    1 file changed, 10 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>> index 0ed84cc065a7..87169fd29d89 100644
>>>> --- a/fs/f2fs/extent_cache.c
>>>> +++ b/fs/f2fs/extent_cache.c
>>>> @@ -119,9 +119,10 @@ 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;
>>>> @@ -644,6 +645,8 @@ 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);
>>>>        }
>>>> @@ -688,12 +691,12 @@ static void __update_extent_tree_range(struct
>>>> inode *inode,
>>>>          write_lock(&et->lock);
>>>>    -    if (type == EX_READ) {
>>>> -        if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>>> -            write_unlock(&et->lock);
>>>> -            return;
>>>> -        }
>>>> +    if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>>> +        write_unlock(&et->lock);
>>>> +        return;
>>>> +    }
>>>>    +    if (type == EX_READ) {
>>>>            prev = et->largest;
>>>>            dei.len = 0;
>>>
>>>
>>>
>>> _______________________________________________
>>> 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

Re: [f2fs-dev] [PATCH] f2fs: fix node_cnt race between extent node destroy and writeback

[Chao Yu]

On 4/19/2026 12:29 AM, Yongpeng Yang wrote:
> 
> On 4/18/26 8:51 AM, Chao Yu via Linux-f2fs-devel wrote:
>> On 4/17/26 21:26, Yongpeng Yang wrote:
>>>
>>> On 4/17/26 17:00, Chao Yu via Linux-f2fs-devel wrote:
>>>> On 4/3/26 22:40, Yongpeng Yang wrote:
>>>>> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>>>>
>>>>> f2fs_destroy_extent_node() does not set FI_NO_EXTENT before clearing
>>>>> extent nodes. When called from f2fs_drop_inode() with I_SYNC set,
>>>>> concurrent kworker writeback can insert new extent nodes into the same
>>>>> extent tree, racing with the destroy and triggering f2fs_bug_on() in
>>>>> __destroy_extent_node(). The scenario is as follows:
>>>>>
>>>>> drop inode                            writeback
>>>>>     - iput
>>>>>      - f2fs_drop_inode  // I_SYNC set
>>>>>       - f2fs_destroy_extent_node
>>>>>        - __destroy_extent_node
>>>>>         - while (node_cnt) {
>>>>>            write_lock(&et->lock)
>>>>>            __free_extent_tree
>>>>>            write_unlock(&et->lock)
>>>>>                                           - __writeback_single_inode
>>>>>                                            - f2fs_outplace_write_data
>>>>>                                             -
>>>>> f2fs_update_read_extent_cache
>>>>>                                              -
>>>>> __update_extent_tree_range
>>>>>                                               // FI_NO_EXTENT not set,
>>>>>                                               // insert new extent node
>>>>>           } // node_cnt == 0, exit while
>>>>>         - f2fs_bug_on(node_cnt)  // node_cnt > 0
>>>>>
>>>>> Additionally, __update_extent_tree_range() only checks FI_NO_EXTENT for
>>>>> EX_READ type, leaving EX_BLOCK_AGE updates completely unprotected.
>>>>>
>>>>> This patch set FI_NO_EXTENT under et->lock in __destroy_extent_node(),
>>>>> consistent with other callers (__update_extent_tree_range and
>>>>> __drop_extent_tree) and check FI_NO_EXTENT for both EX_READ and
>>>>> EX_BLOCK_AGE tree.
>>>>
>>>> I suffered below test failure, then I bisect to this change.
>>>>
>>>>       generic/475  84s ... [failed, exit status 1]- output mismatch
>>>> (see /
>>>> share/git/fstests/results//generic/475.out.bad)
>>>>       --- tests/generic/475.out   2025-01-12 21:57:40.279440664 +0800
>>>>       +++ /share/git/fstests/results//generic/475.out.bad 2026-04-17
>>>> 12:08:28.000000000 +0800
>>>>       @@ -1,2 +1,6 @@
>>>>        QA output created by 475
>>>>        Silence is golden.
>>>>       +mount: /mnt/scratch_f2fs: mount system call failed: Structure
>>>> needs
>>>> cleaning.
>>>>       +       dmesg(1) may have more information after failed mount
>>>> system
>>>> call.
>>>>       +mount failed
>>>>       +(see /share/git/fstests/results//generic/475.full for details)
>>>>       ...
>>>>       (Run 'diff -u /share/git/fstests/tests/generic/475.out /share/git/
>>>> fstests/results//generic/475.out.bad'  to see the entire diff)
>>>>
>>>>
>>>>       generic/388  73s ... [failed, exit status 1]- output mismatch
>>>> (see /
>>>> share/git/fstests/results//generic/388.out.bad)
>>>>       --- tests/generic/388.out   2025-01-12 21:57:40.275440602 +0800
>>>>       +++ /share/git/fstests/results//generic/388.out.bad 2026-04-17
>>>> 11:58:05.000000000 +0800
>>>>       @@ -1,2 +1,6 @@
>>>>        QA output created by 388
>>>>        Silence is golden.
>>>>       +mount: /mnt/scratch_f2fs: mount system call failed: Structure
>>>> needs
>>>> cleaning.
>>>>       +       dmesg(1) may have more information after failed mount
>>>> system
>>>> call.
>>>>       +cycle mount failed
>>>>       +(see /share/git/fstests/results//generic/388.full for details)
>>>>       ...
>>>>       (Run 'diff -u /share/git/fstests/tests/generic/388.out /share/git/
>>>> fstests/results//generic/388.out.bad'  to see the entire diff)
>>>>
>>>>
>>>>       F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761) extent
>>>> info [220057, 57, 6] is incorrect, run fsck to fix
>>>>
>>>> I suspect we may miss any extent updates after we set FI_NO_EXTENT in
>>>> __destroy_extent_node(), result in failing in
>>>> sanity_check_extent_cache().
>>>>
>>>> Can we just relocate f2fs_bug_on(node_cnt) rather than complicated
>>>> change?
>>>> Thoughts?
>>>
>>> Oh, I overlooked largest extent. How about relocate
>>> f2fs_bug_on(node_cnt) to __destroy_extent_tree?
>>>
>>> static void __destroy_extent_tree(struct inode *inode, enum extent_type
>>> type)
>>>
>>>           /* free all extent info belong to this extent tree */
>>>           node_cnt = __destroy_extent_node(inode, type);
>>> +       f2fs_bug_on(sbi, atomic_read(&et->node_cnt));
>>
>>       /* free all extent info belong to this extent tree */
>>       node_cnt = __destroy_extent_node(inode, type);
>>
>>       /* delete extent tree entry in radix tree */
>>       mutex_lock(&eti->extent_tree_lock);
>>       f2fs_bug_on(sbi, atomic_read(&et->node_cnt));  <---
>>
>> Oh, it has already checked node_cnt, so, maybe we can just remove the
>> check in
>> __destroy_extent_node()?
> 
> Yes. BTW, is it correct to remove the call to f2fs_destroy_extent_node()
> in f2fs_drop_inode()? It seems this call is unnecessary, since
> f2fs_evict_inode() will eventually delete all extent nodes properly.

I think it's fine to keep it according to original intention "destroy
extent_tree for the truncation case" introduced from 3e72f721390d
("f2fs: use extent_cache by default"). It helps the performance w/
in batch extent node release.

Thanks,

> 
> Thanks
> Yongpeng,
> 
>>
>> Thanks,
>>
>>
>>>
>>> Thanks
>>> Yongpeng,
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> Fixes: 3fc5d5a182f6 ("f2fs: fix to shrink read extent node in batches")
>>>>> Cc: stable@vger.kernel.org
>>>>> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>>>> ---
>>>>>     fs/f2fs/extent_cache.c | 17 ++++++++++-------
>>>>>     1 file changed, 10 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>>> index 0ed84cc065a7..87169fd29d89 100644
>>>>> --- a/fs/f2fs/extent_cache.c
>>>>> +++ b/fs/f2fs/extent_cache.c
>>>>> @@ -119,9 +119,10 @@ 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;
>>>>> @@ -644,6 +645,8 @@ 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);
>>>>>         }
>>>>> @@ -688,12 +691,12 @@ static void __update_extent_tree_range(struct
>>>>> inode *inode,
>>>>>           write_lock(&et->lock);
>>>>>     -    if (type == EX_READ) {
>>>>> -        if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>>>> -            write_unlock(&et->lock);
>>>>> -            return;
>>>>> -        }
>>>>> +    if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>>>> +        write_unlock(&et->lock);
>>>>> +        return;
>>>>> +    }
>>>>>     +    if (type == EX_READ) {
>>>>>             prev = et->largest;
>>>>>             dei.len = 0;
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> 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
> 

Re: [f2fs-dev] [PATCH] f2fs: fix node_cnt race between extent node destroy and writeback

[Yongpeng Yang]

On 4/20/26 15:28, Chao Yu via Linux-f2fs-devel wrote:
> On 4/19/2026 12:29 AM, Yongpeng Yang wrote:
>>
>> On 4/18/26 8:51 AM, Chao Yu via Linux-f2fs-devel wrote:
>>> On 4/17/26 21:26, Yongpeng Yang wrote:
>>>>
>>>> On 4/17/26 17:00, Chao Yu via Linux-f2fs-devel wrote:
>>>>> On 4/3/26 22:40, Yongpeng Yang wrote:
>>>>>> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>>>>>
>>>>>> f2fs_destroy_extent_node() does not set FI_NO_EXTENT before clearing
>>>>>> extent nodes. When called from f2fs_drop_inode() with I_SYNC set,
>>>>>> concurrent kworker writeback can insert new extent nodes into the
>>>>>> same
>>>>>> extent tree, racing with the destroy and triggering f2fs_bug_on() in
>>>>>> __destroy_extent_node(). The scenario is as follows:
>>>>>>
>>>>>> drop inode                            writeback
>>>>>>     - iput
>>>>>>      - f2fs_drop_inode  // I_SYNC set
>>>>>>       - f2fs_destroy_extent_node
>>>>>>        - __destroy_extent_node
>>>>>>         - while (node_cnt) {
>>>>>>            write_lock(&et->lock)
>>>>>>            __free_extent_tree
>>>>>>            write_unlock(&et->lock)
>>>>>>                                           - __writeback_single_inode
>>>>>>                                            - f2fs_outplace_write_data
>>>>>>                                             -
>>>>>> f2fs_update_read_extent_cache
>>>>>>                                              -
>>>>>> __update_extent_tree_range
>>>>>>                                               // FI_NO_EXTENT not
>>>>>> set,
>>>>>>                                               // insert new extent
>>>>>> node
>>>>>>           } // node_cnt == 0, exit while
>>>>>>         - f2fs_bug_on(node_cnt)  // node_cnt > 0
>>>>>>
>>>>>> Additionally, __update_extent_tree_range() only checks
>>>>>> FI_NO_EXTENT for
>>>>>> EX_READ type, leaving EX_BLOCK_AGE updates completely unprotected.
>>>>>>
>>>>>> This patch set FI_NO_EXTENT under et->lock in
>>>>>> __destroy_extent_node(),
>>>>>> consistent with other callers (__update_extent_tree_range and
>>>>>> __drop_extent_tree) and check FI_NO_EXTENT for both EX_READ and
>>>>>> EX_BLOCK_AGE tree.
>>>>>
>>>>> I suffered below test failure, then I bisect to this change.
>>>>>
>>>>>       generic/475  84s ... [failed, exit status 1]- output mismatch
>>>>> (see /
>>>>> share/git/fstests/results//generic/475.out.bad)
>>>>>       --- tests/generic/475.out   2025-01-12 21:57:40.279440664 +0800
>>>>>       +++ /share/git/fstests/results//generic/475.out.bad 2026-04-17
>>>>> 12:08:28.000000000 +0800
>>>>>       @@ -1,2 +1,6 @@
>>>>>        QA output created by 475
>>>>>        Silence is golden.
>>>>>       +mount: /mnt/scratch_f2fs: mount system call failed: Structure
>>>>> needs
>>>>> cleaning.
>>>>>       +       dmesg(1) may have more information after failed mount
>>>>> system
>>>>> call.
>>>>>       +mount failed
>>>>>       +(see /share/git/fstests/results//generic/475.full for details)
>>>>>       ...
>>>>>       (Run 'diff -u /share/git/fstests/tests/generic/475.out /
>>>>> share/git/
>>>>> fstests/results//generic/475.out.bad'  to see the entire diff)
>>>>>
>>>>>
>>>>>       generic/388  73s ... [failed, exit status 1]- output mismatch
>>>>> (see /
>>>>> share/git/fstests/results//generic/388.out.bad)
>>>>>       --- tests/generic/388.out   2025-01-12 21:57:40.275440602 +0800
>>>>>       +++ /share/git/fstests/results//generic/388.out.bad 2026-04-17
>>>>> 11:58:05.000000000 +0800
>>>>>       @@ -1,2 +1,6 @@
>>>>>        QA output created by 388
>>>>>        Silence is golden.
>>>>>       +mount: /mnt/scratch_f2fs: mount system call failed: Structure
>>>>> needs
>>>>> cleaning.
>>>>>       +       dmesg(1) may have more information after failed mount
>>>>> system
>>>>> call.
>>>>>       +cycle mount failed
>>>>>       +(see /share/git/fstests/results//generic/388.full for details)
>>>>>       ...
>>>>>       (Run 'diff -u /share/git/fstests/tests/generic/388.out /
>>>>> share/git/
>>>>> fstests/results//generic/388.out.bad'  to see the entire diff)
>>>>>
>>>>>
>>>>>       F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761)
>>>>> extent
>>>>> info [220057, 57, 6] is incorrect, run fsck to fix
>>>>>
>>>>> I suspect we may miss any extent updates after we set FI_NO_EXTENT in
>>>>> __destroy_extent_node(), result in failing in
>>>>> sanity_check_extent_cache().
>>>>>
>>>>> Can we just relocate f2fs_bug_on(node_cnt) rather than complicated
>>>>> change?
>>>>> Thoughts?
>>>>
>>>> Oh, I overlooked largest extent. How about relocate
>>>> f2fs_bug_on(node_cnt) to __destroy_extent_tree?
>>>>
>>>> static void __destroy_extent_tree(struct inode *inode, enum extent_type
>>>> type)
>>>>
>>>>           /* free all extent info belong to this extent tree */
>>>>           node_cnt = __destroy_extent_node(inode, type);
>>>> +       f2fs_bug_on(sbi, atomic_read(&et->node_cnt));
>>>
>>>       /* free all extent info belong to this extent tree */
>>>       node_cnt = __destroy_extent_node(inode, type);
>>>
>>>       /* delete extent tree entry in radix tree */
>>>       mutex_lock(&eti->extent_tree_lock);
>>>       f2fs_bug_on(sbi, atomic_read(&et->node_cnt));  <---
>>>
>>> Oh, it has already checked node_cnt, so, maybe we can just remove the
>>> check in
>>> __destroy_extent_node()?
>>
>> Yes. BTW, is it correct to remove the call to f2fs_destroy_extent_node()
>> in f2fs_drop_inode()? It seems this call is unnecessary, since
>> f2fs_evict_inode() will eventually delete all extent nodes properly.
> 
> I think it's fine to keep it according to original intention "destroy
> extent_tree for the truncation case" introduced from 3e72f721390d
> ("f2fs: use extent_cache by default"). It helps the performance w/
> in batch extent node release.

Oh, I see. This patch has already been merged into the dev branch. Which
of the following approaches would be more appropriate?
1. Drop the current patch from the dev branch, then submit a patch to
remove the f2fs_bug_on() in __destroy_extent_node.
2. Send two patches: the first reverts the change, and the second
removes the f2fs_bug_on() in __destroy_extent_node().

Thanks
Yongpeng,

> 
> Thanks,
> 
>>
>> Thanks
>> Yongpeng,
>>
>>>
>>> Thanks,
>>>
>>>
>>>>
>>>> Thanks
>>>> Yongpeng,
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>>>
>>>>>> Fixes: 3fc5d5a182f6 ("f2fs: fix to shrink read extent node in
>>>>>> batches")
>>>>>> Cc: stable@vger.kernel.org
>>>>>> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>>>>> ---
>>>>>>     fs/f2fs/extent_cache.c | 17 ++++++++++-------
>>>>>>     1 file changed, 10 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>>>> index 0ed84cc065a7..87169fd29d89 100644
>>>>>> --- a/fs/f2fs/extent_cache.c
>>>>>> +++ b/fs/f2fs/extent_cache.c
>>>>>> @@ -119,9 +119,10 @@ 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;
>>>>>> @@ -644,6 +645,8 @@ 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);
>>>>>>         }
>>>>>> @@ -688,12 +691,12 @@ static void __update_extent_tree_range(struct
>>>>>> inode *inode,
>>>>>>           write_lock(&et->lock);
>>>>>>     -    if (type == EX_READ) {
>>>>>> -        if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>>>>> -            write_unlock(&et->lock);
>>>>>> -            return;
>>>>>> -        }
>>>>>> +    if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>>>>> +        write_unlock(&et->lock);
>>>>>> +        return;
>>>>>> +    }
>>>>>>     +    if (type == EX_READ) {
>>>>>>             prev = et->largest;
>>>>>>             dei.len = 0;
>>>>>
>>>>>
>>>>>

Re: [f2fs-dev] [PATCH] f2fs: fix node_cnt race between extent node destroy and writeback

[Chao Yu]

On 4/21/2026 4:44 PM, Yongpeng Yang wrote:
> 
> On 4/20/26 15:28, Chao Yu via Linux-f2fs-devel wrote:
>> On 4/19/2026 12:29 AM, Yongpeng Yang wrote:
>>>
>>> On 4/18/26 8:51 AM, Chao Yu via Linux-f2fs-devel wrote:
>>>> On 4/17/26 21:26, Yongpeng Yang wrote:
>>>>>
>>>>> On 4/17/26 17:00, Chao Yu via Linux-f2fs-devel wrote:
>>>>>> On 4/3/26 22:40, Yongpeng Yang wrote:
>>>>>>> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>>>>>>
>>>>>>> f2fs_destroy_extent_node() does not set FI_NO_EXTENT before clearing
>>>>>>> extent nodes. When called from f2fs_drop_inode() with I_SYNC set,
>>>>>>> concurrent kworker writeback can insert new extent nodes into the
>>>>>>> same
>>>>>>> extent tree, racing with the destroy and triggering f2fs_bug_on() in
>>>>>>> __destroy_extent_node(). The scenario is as follows:
>>>>>>>
>>>>>>> drop inode                            writeback
>>>>>>>      - iput
>>>>>>>       - f2fs_drop_inode  // I_SYNC set
>>>>>>>        - f2fs_destroy_extent_node
>>>>>>>         - __destroy_extent_node
>>>>>>>          - while (node_cnt) {
>>>>>>>             write_lock(&et->lock)
>>>>>>>             __free_extent_tree
>>>>>>>             write_unlock(&et->lock)
>>>>>>>                                            - __writeback_single_inode
>>>>>>>                                             - f2fs_outplace_write_data
>>>>>>>                                              -
>>>>>>> f2fs_update_read_extent_cache
>>>>>>>                                               -
>>>>>>> __update_extent_tree_range
>>>>>>>                                                // FI_NO_EXTENT not
>>>>>>> set,
>>>>>>>                                                // insert new extent
>>>>>>> node
>>>>>>>            } // node_cnt == 0, exit while
>>>>>>>          - f2fs_bug_on(node_cnt)  // node_cnt > 0
>>>>>>>
>>>>>>> Additionally, __update_extent_tree_range() only checks
>>>>>>> FI_NO_EXTENT for
>>>>>>> EX_READ type, leaving EX_BLOCK_AGE updates completely unprotected.
>>>>>>>
>>>>>>> This patch set FI_NO_EXTENT under et->lock in
>>>>>>> __destroy_extent_node(),
>>>>>>> consistent with other callers (__update_extent_tree_range and
>>>>>>> __drop_extent_tree) and check FI_NO_EXTENT for both EX_READ and
>>>>>>> EX_BLOCK_AGE tree.
>>>>>>
>>>>>> I suffered below test failure, then I bisect to this change.
>>>>>>
>>>>>>        generic/475  84s ... [failed, exit status 1]- output mismatch
>>>>>> (see /
>>>>>> share/git/fstests/results//generic/475.out.bad)
>>>>>>        --- tests/generic/475.out   2025-01-12 21:57:40.279440664 +0800
>>>>>>        +++ /share/git/fstests/results//generic/475.out.bad 2026-04-17
>>>>>> 12:08:28.000000000 +0800
>>>>>>        @@ -1,2 +1,6 @@
>>>>>>         QA output created by 475
>>>>>>         Silence is golden.
>>>>>>        +mount: /mnt/scratch_f2fs: mount system call failed: Structure
>>>>>> needs
>>>>>> cleaning.
>>>>>>        +       dmesg(1) may have more information after failed mount
>>>>>> system
>>>>>> call.
>>>>>>        +mount failed
>>>>>>        +(see /share/git/fstests/results//generic/475.full for details)
>>>>>>        ...
>>>>>>        (Run 'diff -u /share/git/fstests/tests/generic/475.out /
>>>>>> share/git/
>>>>>> fstests/results//generic/475.out.bad'  to see the entire diff)
>>>>>>
>>>>>>
>>>>>>        generic/388  73s ... [failed, exit status 1]- output mismatch
>>>>>> (see /
>>>>>> share/git/fstests/results//generic/388.out.bad)
>>>>>>        --- tests/generic/388.out   2025-01-12 21:57:40.275440602 +0800
>>>>>>        +++ /share/git/fstests/results//generic/388.out.bad 2026-04-17
>>>>>> 11:58:05.000000000 +0800
>>>>>>        @@ -1,2 +1,6 @@
>>>>>>         QA output created by 388
>>>>>>         Silence is golden.
>>>>>>        +mount: /mnt/scratch_f2fs: mount system call failed: Structure
>>>>>> needs
>>>>>> cleaning.
>>>>>>        +       dmesg(1) may have more information after failed mount
>>>>>> system
>>>>>> call.
>>>>>>        +cycle mount failed
>>>>>>        +(see /share/git/fstests/results//generic/388.full for details)
>>>>>>        ...
>>>>>>        (Run 'diff -u /share/git/fstests/tests/generic/388.out /
>>>>>> share/git/
>>>>>> fstests/results//generic/388.out.bad'  to see the entire diff)
>>>>>>
>>>>>>
>>>>>>        F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761)
>>>>>> extent
>>>>>> info [220057, 57, 6] is incorrect, run fsck to fix
>>>>>>
>>>>>> I suspect we may miss any extent updates after we set FI_NO_EXTENT in
>>>>>> __destroy_extent_node(), result in failing in
>>>>>> sanity_check_extent_cache().
>>>>>>
>>>>>> Can we just relocate f2fs_bug_on(node_cnt) rather than complicated
>>>>>> change?
>>>>>> Thoughts?
>>>>>
>>>>> Oh, I overlooked largest extent. How about relocate
>>>>> f2fs_bug_on(node_cnt) to __destroy_extent_tree?
>>>>>
>>>>> static void __destroy_extent_tree(struct inode *inode, enum extent_type
>>>>> type)
>>>>>
>>>>>            /* free all extent info belong to this extent tree */
>>>>>            node_cnt = __destroy_extent_node(inode, type);
>>>>> +       f2fs_bug_on(sbi, atomic_read(&et->node_cnt));
>>>>
>>>>        /* free all extent info belong to this extent tree */
>>>>        node_cnt = __destroy_extent_node(inode, type);
>>>>
>>>>        /* delete extent tree entry in radix tree */
>>>>        mutex_lock(&eti->extent_tree_lock);
>>>>        f2fs_bug_on(sbi, atomic_read(&et->node_cnt));  <---
>>>>
>>>> Oh, it has already checked node_cnt, so, maybe we can just remove the
>>>> check in
>>>> __destroy_extent_node()?
>>>
>>> Yes. BTW, is it correct to remove the call to f2fs_destroy_extent_node()
>>> in f2fs_drop_inode()? It seems this call is unnecessary, since
>>> f2fs_evict_inode() will eventually delete all extent nodes properly.
>>
>> I think it's fine to keep it according to original intention "destroy
>> extent_tree for the truncation case" introduced from 3e72f721390d
>> ("f2fs: use extent_cache by default"). It helps the performance w/
>> in batch extent node release.
> 
> Oh, I see. This patch has already been merged into the dev branch. Which
> of the following approaches would be more appropriate?
> 1. Drop the current patch from the dev branch, then submit a patch to
> remove the f2fs_bug_on() in __destroy_extent_node.
> 2. Send two patches: the first reverts the change, and the second
> removes the f2fs_bug_on() in __destroy_extent_node().

It's near the end of merge window, I think we need to keep dev as
it is, and create another patch to revert previous change and drop
the f2fs_bug_on() as well, what do you think?

To Jaegeuk, thoughts?

Thanks,

> 
> Thanks
> Yongpeng,
> 
>>
>> Thanks,
>>
>>>
>>> Thanks
>>> Yongpeng,
>>>
>>>>
>>>> Thanks,
>>>>
>>>>
>>>>>
>>>>> Thanks
>>>>> Yongpeng,
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>>
>>>>>>> Fixes: 3fc5d5a182f6 ("f2fs: fix to shrink read extent node in
>>>>>>> batches")
>>>>>>> Cc: stable@vger.kernel.org
>>>>>>> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>>>>>> ---
>>>>>>>      fs/f2fs/extent_cache.c | 17 ++++++++++-------
>>>>>>>      1 file changed, 10 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>>>>> index 0ed84cc065a7..87169fd29d89 100644
>>>>>>> --- a/fs/f2fs/extent_cache.c
>>>>>>> +++ b/fs/f2fs/extent_cache.c
>>>>>>> @@ -119,9 +119,10 @@ 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;
>>>>>>> @@ -644,6 +645,8 @@ 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);
>>>>>>>          }
>>>>>>> @@ -688,12 +691,12 @@ static void __update_extent_tree_range(struct
>>>>>>> inode *inode,
>>>>>>>            write_lock(&et->lock);
>>>>>>>      -    if (type == EX_READ) {
>>>>>>> -        if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>>>>>> -            write_unlock(&et->lock);
>>>>>>> -            return;
>>>>>>> -        }
>>>>>>> +    if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>>>>>> +        write_unlock(&et->lock);
>>>>>>> +        return;
>>>>>>> +    }
>>>>>>>      +    if (type == EX_READ) {
>>>>>>>              prev = et->largest;
>>>>>>>              dei.len = 0;
>>>>>>
>>>>>>
>>>>>>
>