[PATCH v2] f2fs: wait for inode record work before clearing ino bitmaps

Wenjie Qi posted 1 patch 3 weeks, 3 days ago
There is a newer version of this series
fs/f2fs/checkpoint.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v2] f2fs: wait for inode record work before clearing ino bitmaps
Posted by Wenjie Qi 3 weeks, 3 days ago
APPEND/UPDATE inode state recording was moved to the inode eviction
workqueue. These entries were later converted to bitmap values stored in
XArrays, but the workqueue drain was left behind in the list cleanup loop
where it is now a no-op.

During unmount, inode eviction work can therefore remain queued when
f2fs_release_ino_entry() destroys the bitmap XArrays. A delayed worker can
repopulate them before the workqueue is finally destroyed, leaking newly
allocated XArray nodes when the F2FS superblock is freed.

Wait for APPEND/UPDATE inode record work before destroying each bitmap
XArray, restoring the required ordering.

Fixes: 9a9ee7408a1f ("f2fs: reduce memory footprint of ino management")
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
---
v2:
- Remove the no-op wait from the ORPHAN/FLUSH cleanup loop.
- Wait once before bitmap XArray cleanup since APPEND/UPDATE share evict_wq.

 fs/f2fs/checkpoint.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 4b59f30ef45d5..642a771e0f63c 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -887,8 +887,6 @@ void f2fs_release_ino_entry(struct f2fs_sb_info *sbi, bool all)
 	for (i = all ? ORPHAN_INO : FLUSH_INO; i <= FLUSH_INO; i++) {
 		struct inode_management *im = &sbi->im[i];
 
-		f2fs_wait_for_inode_record(sbi, i);
-
 		spin_lock(&im->ino_lock);
 		list_for_each_entry_safe(e, tmp, &im->ino_list, list) {
 			list_del(&e->list);
@@ -899,6 +897,8 @@ void f2fs_release_ino_entry(struct f2fs_sb_info *sbi, bool all)
 		spin_unlock(&im->ino_lock);
 	}
 
+	f2fs_wait_for_inode_record(sbi, APPEND_INO);
+
 	for (i = APPEND_INO; i < MAX_INO_ENTRY; i++) {
 		struct inode_management *im = &sbi->im[i];
 
-- 
2.43.0
Re: [PATCH v2] f2fs: wait for inode record work before clearing ino bitmaps
Posted by Chao Yu 3 weeks, 3 days ago
On 9/1/26 22:08, Wenjie Qi wrote:
> APPEND/UPDATE inode state recording was moved to the inode eviction
> workqueue. These entries were later converted to bitmap values stored in
> XArrays, but the workqueue drain was left behind in the list cleanup loop
> where it is now a no-op.
> 
> During unmount, inode eviction work can therefore remain queued when
> f2fs_release_ino_entry() destroys the bitmap XArrays. A delayed worker can
> repopulate them before the workqueue is finally destroyed, leaking newly
> allocated XArray nodes when the F2FS superblock is freed.
> 
> Wait for APPEND/UPDATE inode record work before destroying each bitmap
> XArray, restoring the required ordering.
> 
> Fixes: 9a9ee7408a1f ("f2fs: reduce memory footprint of ino management")
> Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> ---
> v2:
> - Remove the no-op wait from the ORPHAN/FLUSH cleanup loop.
> - Wait once before bitmap XArray cleanup since APPEND/UPDATE share evict_wq.
> 
>  fs/f2fs/checkpoint.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 4b59f30ef45d5..642a771e0f63c 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -887,8 +887,6 @@ void f2fs_release_ino_entry(struct f2fs_sb_info *sbi, bool all)
>  	for (i = all ? ORPHAN_INO : FLUSH_INO; i <= FLUSH_INO; i++) {
>  		struct inode_management *im = &sbi->im[i];
>  
> -		f2fs_wait_for_inode_record(sbi, i);
> -
>  		spin_lock(&im->ino_lock);
>  		list_for_each_entry_safe(e, tmp, &im->ino_list, list) {
>  			list_del(&e->list);
> @@ -899,6 +897,8 @@ void f2fs_release_ino_entry(struct f2fs_sb_info *sbi, bool all)
>  		spin_unlock(&im->ino_lock);
>  	}
>  
> +	f2fs_wait_for_inode_record(sbi, APPEND_INO);

It may missed to wait UPDATE_INO?

Thanks

> +
>  	for (i = APPEND_INO; i < MAX_INO_ENTRY; i++) {
>  		struct inode_management *im = &sbi->im[i];
>
Re: [f2fs-dev] [PATCH v2] f2fs: wait for inode record work before clearing ino bitmaps
Posted by Wenjie Qi 3 weeks, 3 days ago
Hi Chao,

It does not miss UPDATE_INO. The mode argument only selects whether to
wait: both APPEND_INO and UPDATE_INO call
flush_workqueue(sbi->evict_wq), and both updates are queued on the same
eviction workqueue. Therefore, one call with APPEND_INO also drains
pending UPDATE_INO work. A second call with UPDATE_INO would flush the
same queue again.

Thanks,
Wenjie
Re: [f2fs-dev] [PATCH v2] f2fs: wait for inode record work before clearing ino bitmaps
Posted by Chao Yu 3 weeks, 3 days ago
On 9/2/26 16:16, Wenjie Qi wrote:
> Hi Chao,
> 
> It does not miss UPDATE_INO. The mode argument only selects whether to
> wait: both APPEND_INO and UPDATE_INO call
> flush_workqueue(sbi->evict_wq), and both updates are queued on the same
> eviction workqueue. Therefore, one call with APPEND_INO also drains
> pending UPDATE_INO work. A second call with UPDATE_INO would flush the
> same queue again.

Alright, how about call flush_workqueue(sbi->evict_wq) directly and drop
f2fs_wait_for_inode_record()?

Thanks,

> 
> Thanks,
> Wenjie