2021-05-26 08:46:26

by Wang Jianchao

Subject: [PATCH V2 5/7] ext4: get buddy cache after insert successfully

The getting of bd_bitmap_page and bd_buddy_page should be done
after insert the ext4_free_data successfully. Otherwise, nobody
can put them.

Signed-off-by: Wang Jianchao <[email protected]>
---
fs/ext4/mballoc.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index b23010c..3e8ad43 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5116,21 +5116,12 @@ static int ext4_insert_free_data( struct ext4_sb_info *sbi,
struct ext4_group_info *db = e4b->bd_info;
struct super_block *sb = e4b->bd_sb;
struct ext4_sb_info *sbi = EXT4_SB(sb);
+ bool first = RB_EMPTY_ROOT(&db->bb_free_root);

BUG_ON(!ext4_handle_valid(handle));
BUG_ON(e4b->bd_bitmap_page == NULL);
BUG_ON(e4b->bd_buddy_page == NULL);

- if (!db->bb_free_root.rb_node) {
- /* first free block exent. We need to
- protect buddy cache from being freed,
- * otherwise we'll refresh it from
- * on-disk bitmap and lose not-yet-available
- * blocks */
- get_page(e4b->bd_buddy_page);
- get_page(e4b->bd_bitmap_page);
- }
-
if (ext4_insert_free_data(sbi, &db->bb_free_root, nfd)) {
ext4_grp_locked_error(sb, e4b->bd_group, 0,
ext4_group_first_block_no(sb, e4b->bd_group) +
@@ -5140,6 +5131,15 @@ static int ext4_insert_free_data( struct ext4_sb_info *sbi,
return 0;
}

+ if (first) {
+ /* first free block exent. We need to protect buddy
+ * cache from being freed, otherwise we'll refresh it
+ * from on-disk bitmap and lose not-yet-available blocks
+ */
+ get_page(e4b->bd_buddy_page);
+ get_page(e4b->bd_bitmap_page);
+ }
+
spin_lock(&sbi->s_md_lock);
list_add_tail(&nfd->efd_list, &sbi->s_freed_data_list);
sbi->s_mb_free_pending += nfd->efd_count;
--
1.8.3.1

2021-06-23 03:06:35

by Theodore Ts'o

Subject: Re: [PATCH V2 5/7] ext4: get buddy cache after insert successfully

On Wed, May 26, 2021 at 04:43:43PM +0800, Wang Jianchao wrote:
> The getting of bd_bitmap_page and bd_buddy_page should be done
> after insert the ext4_free_data successfully. Otherwise, nobody
> can put them.
>
> Signed-off-by: Wang Jianchao <[email protected]>

This looks like it's a bug fix for patch 4/7 --- is that correct? If
so, maybe we should fold this into the previous patch?

Thanks,

- Ted