On Thu 24-08-23 23:56:31, Wang Jianjian wrote:
> Commit 7a2fcbf7f85('ext4: don't use blocks freed but
> not yet committed in buddy cache init) walk the rbtree of
> freed data and mark them free in buddy to avoid reuse them
> before journal committing them, However, it is unnecessary to
> do that, because we have extra page references to buddy and bitmap
> pages, they will be released iff journal has committed and after
> process freed data.
So do you mean that buddy bitmap cannot be freed until the transaction that
frees the blocks in it commits? Indeed ext4_mb_free_metadata() grabs buddy
page references and ext4_free_data_in_buddy() drops them.
Perhaps I'd rephrase the changelog as:
Commit 7a2fcbf7f85 ("ext4: don't use blocks freed but not yet committed in
buddy cache init") added a code to mark as used blocks in the list of not yet
committed freed blocks during initialization of a buddy page. However
ext4_mb_free_metadata() makes sure buddy page is already loaded and takes a
reference to it so it cannot happen that ext4_mb_init_cache() is called
when efd list is non-empty. Just remove the
ext4_mb_generate_from_freelist() call.
> @@ -1274,7 +1272,6 @@ static int ext4_mb_init_cache(struct page *page, char *incore, gfp_t gfp)
>
> /* mark all preallocated blks used in in-core bitmap */
> ext4_mb_generate_from_pa(sb, data, group);
> - ext4_mb_generate_from_freelist(sb, data, group);
And just to be sure I'd add here:
WARN_ON_ONCE(!RB_EMPTY_ROOT(&grp->bb_free_root));
Honza
> ext4_unlock_group(sb, group);
>
> /* set incore so that the buddy information can be
> @@ -4440,30 +4437,6 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
> return false;
> }
>
> -/*
> - * the function goes through all block freed in the group
> - * but not yet committed and marks them used in in-core bitmap.
> - * buddy must be generated from this bitmap
> - * Need to be called with the ext4 group lock held
> - */
> -static void ext4_mb_generate_from_freelist(struct super_block *sb, void *bitmap,
> - ext4_group_t group)
> -{
> - struct rb_node *n;
> - struct ext4_group_info *grp;
> - struct ext4_free_data *entry;
> -
> - grp = ext4_get_group_info(sb, group);
> - n = rb_first(&(grp->bb_free_root));
> -
> - while (n) {
> - entry = rb_entry(n, struct ext4_free_data, efd_node);
> - mb_set_bits(bitmap, entry->efd_start_cluster, entry->efd_count);
> - n = rb_next(n);
> - }
> - return;
> -}
> -
> /*
> * the function goes through all preallocation in this group and marks them
> * used in in-core bitmap. buddy must be generated from this bitmap
> --
> 2.34.3
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
