| Message ID | bf7314dc720dbeb1de64b95512cc796fdaba7ef3.1744813603.git.naohiro.aota@wdc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btrfs: zoned: split out space_info for dedicated block groups | expand |
On Wed, Apr 16, 2025 at 11:28:11PM +0900, Naohiro Aota wrote: > Take an optional btrfs_space_info argument in btrfs_chunk_alloc(). If > specified, btrfs_chunk_alloc() works on the space_info. If not, the default > space_info is used as the same as before. > > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> For consistency sake I'd prefer if you'd just update the callers to lookup the space_info and pass that in as appropriate. In fact a lot of these callers already have the block group or space_info available, so we could avoid the extra overhead of doing a lookup > --- > fs/btrfs/block-group.c | 19 ++++++++++++------- > fs/btrfs/block-group.h | 3 ++- > fs/btrfs/extent-tree.c | 2 +- > fs/btrfs/space-info.c | 2 +- > fs/btrfs/transaction.c | 2 +- > 5 files changed, 17 insertions(+), 11 deletions(-) > > diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c > index b700d80089d3..12cc9069d4bb 100644 > --- a/fs/btrfs/block-group.c > +++ b/fs/btrfs/block-group.c > @@ -3018,7 +3018,7 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache, > */ > alloc_flags = btrfs_get_alloc_profile(fs_info, cache->flags); > if (alloc_flags != cache->flags) { > - ret = btrfs_chunk_alloc(trans, alloc_flags, > + ret = btrfs_chunk_alloc(trans, NULL, alloc_flags, > CHUNK_ALLOC_FORCE); Here we just do cache->space_info; > /* > * ENOSPC is allowed here, we may have enough space > @@ -3047,7 +3047,7 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache, > goto unlock_out; > > alloc_flags = btrfs_get_alloc_profile(fs_info, cache->space_info->flags); > - ret = btrfs_chunk_alloc(trans, alloc_flags, CHUNK_ALLOC_FORCE); > + ret = btrfs_chunk_alloc(trans, NULL, alloc_flags, CHUNK_ALLOC_FORCE); Same here. > if (ret < 0) > goto out; > /* > @@ -3899,7 +3899,7 @@ int btrfs_force_chunk_alloc(struct btrfs_trans_handle *trans, u64 type) > { > u64 alloc_flags = btrfs_get_alloc_profile(trans->fs_info, type); > > - return btrfs_chunk_alloc(trans, alloc_flags, CHUNK_ALLOC_FORCE); > + return btrfs_chunk_alloc(trans, NULL, alloc_flags, CHUNK_ALLOC_FORCE); Here we'd have to lookup. > } > > static struct btrfs_block_group *do_chunk_alloc(struct btrfs_trans_handle *trans, u64 flags) > @@ -4102,12 +4102,15 @@ static struct btrfs_block_group *do_chunk_alloc(struct btrfs_trans_handle *trans > * - return 0 if it doesn't need to allocate a new chunk, > * - return 1 if it successfully allocates a chunk, > * - return errors including -ENOSPC otherwise. > + * > + * @space_info can optionally be specified to make a new chunk belong to it. If > + * it is NULL, it is set automatically. > */ > -int btrfs_chunk_alloc(struct btrfs_trans_handle *trans, u64 flags, > +int btrfs_chunk_alloc(struct btrfs_trans_handle *trans, > + struct btrfs_space_info *space_info, u64 flags, > enum btrfs_chunk_alloc_enum force) > { > struct btrfs_fs_info *fs_info = trans->fs_info; > - struct btrfs_space_info *space_info; > struct btrfs_block_group *ret_bg; > bool wait_for_alloc = false; > bool should_alloc = false; > @@ -4146,8 +4149,10 @@ int btrfs_chunk_alloc(struct btrfs_trans_handle *trans, u64 flags, > if (flags & BTRFS_BLOCK_GROUP_SYSTEM) > return -ENOSPC; > > - space_info = btrfs_find_space_info(fs_info, flags); > - ASSERT(space_info); > + if (!space_info) { > + space_info = btrfs_find_space_info(fs_info, flags); > + ASSERT(space_info); > + } > > do { > spin_lock(&space_info->lock); > diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h > index 36937eeab9b8..c01f3af726a1 100644 > --- a/fs/btrfs/block-group.h > +++ b/fs/btrfs/block-group.h > @@ -342,7 +342,8 @@ int btrfs_add_reserved_bytes(struct btrfs_block_group *cache, > bool force_wrong_size_class); > void btrfs_free_reserved_bytes(struct btrfs_block_group *cache, > u64 num_bytes, int delalloc); > -int btrfs_chunk_alloc(struct btrfs_trans_handle *trans, u64 flags, > +int btrfs_chunk_alloc(struct btrfs_trans_handle *trans, > + struct btrfs_space_info *space_info, u64 flags, > enum btrfs_chunk_alloc_enum force); > int btrfs_force_chunk_alloc(struct btrfs_trans_handle *trans, u64 type); > void check_system_chunk(struct btrfs_trans_handle *trans, const u64 type); > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index a68a8a07caff..1dad1a42c9c1 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -4159,7 +4159,7 @@ static int find_free_extent_update_loop(struct btrfs_fs_info *fs_info, > return ret; > } > > - ret = btrfs_chunk_alloc(trans, ffe_ctl->flags, > + ret = btrfs_chunk_alloc(trans, NULL, ffe_ctl->flags, > CHUNK_ALLOC_FORCE_FOR_EXTENT); We'd have to look up here. > > /* Do not bail out on ENOSPC since we can do more. */ > diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c > index d6d33ab754ba..2489c2a16123 100644 > --- a/fs/btrfs/space-info.c > +++ b/fs/btrfs/space-info.c > @@ -817,7 +817,7 @@ static void flush_space(struct btrfs_fs_info *fs_info, > ret = PTR_ERR(trans); > break; > } > - ret = btrfs_chunk_alloc(trans, > + ret = btrfs_chunk_alloc(trans, space_info, > btrfs_get_alloc_profile(fs_info, space_info->flags), > (state == ALLOC_CHUNK) ? CHUNK_ALLOC_NO_FORCE : > CHUNK_ALLOC_FORCE); > diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c > index 39e48bf610a1..670e0527996c 100644 > --- a/fs/btrfs/transaction.c > +++ b/fs/btrfs/transaction.c > @@ -763,7 +763,7 @@ start_transaction(struct btrfs_root *root, unsigned int num_items, > if (do_chunk_alloc && num_bytes) { > u64 flags = h->block_rsv->space_info->flags; > > - btrfs_chunk_alloc(h, btrfs_get_alloc_profile(fs_info, flags), > + btrfs_chunk_alloc(h, NULL, btrfs_get_alloc_profile(fs_info, flags), > CHUNK_ALLOC_NO_FORCE); Here we just do h->block_rsv->space_info. Thanks, Josef
On Thu Apr 17, 2025 at 9:38 PM JST, Josef Bacik wrote: > On Wed, Apr 16, 2025 at 11:28:11PM +0900, Naohiro Aota wrote: >> Take an optional btrfs_space_info argument in btrfs_chunk_alloc(). If >> specified, btrfs_chunk_alloc() works on the space_info. If not, the default >> space_info is used as the same as before. >> >> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> >> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > For consistency sake I'd prefer if you'd just update the callers to lookup the > space_info and pass that in as appropriate. In fact a lot of these callers > already have the block group or space_info available, so we could avoid the > extra overhead of doing a lookup Sure. I'll revise this patch and next one in that way.
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index b700d80089d3..12cc9069d4bb 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -3018,7 +3018,7 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache, */ alloc_flags = btrfs_get_alloc_profile(fs_info, cache->flags); if (alloc_flags != cache->flags) { - ret = btrfs_chunk_alloc(trans, alloc_flags, + ret = btrfs_chunk_alloc(trans, NULL, alloc_flags, CHUNK_ALLOC_FORCE); /* * ENOSPC is allowed here, we may have enough space @@ -3047,7 +3047,7 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache, goto unlock_out; alloc_flags = btrfs_get_alloc_profile(fs_info, cache->space_info->flags); - ret = btrfs_chunk_alloc(trans, alloc_flags, CHUNK_ALLOC_FORCE); + ret = btrfs_chunk_alloc(trans, NULL, alloc_flags, CHUNK_ALLOC_FORCE); if (ret < 0) goto out; /* @@ -3899,7 +3899,7 @@ int btrfs_force_chunk_alloc(struct btrfs_trans_handle *trans, u64 type) { u64 alloc_flags = btrfs_get_alloc_profile(trans->fs_info, type); - return btrfs_chunk_alloc(trans, alloc_flags, CHUNK_ALLOC_FORCE); + return btrfs_chunk_alloc(trans, NULL, alloc_flags, CHUNK_ALLOC_FORCE); } static struct btrfs_block_group *do_chunk_alloc(struct btrfs_trans_handle *trans, u64 flags) @@ -4102,12 +4102,15 @@ static struct btrfs_block_group *do_chunk_alloc(struct btrfs_trans_handle *trans * - return 0 if it doesn't need to allocate a new chunk, * - return 1 if it successfully allocates a chunk, * - return errors including -ENOSPC otherwise. + * + * @space_info can optionally be specified to make a new chunk belong to it. If + * it is NULL, it is set automatically. */ -int btrfs_chunk_alloc(struct btrfs_trans_handle *trans, u64 flags, +int btrfs_chunk_alloc(struct btrfs_trans_handle *trans, + struct btrfs_space_info *space_info, u64 flags, enum btrfs_chunk_alloc_enum force) { struct btrfs_fs_info *fs_info = trans->fs_info; - struct btrfs_space_info *space_info; struct btrfs_block_group *ret_bg; bool wait_for_alloc = false; bool should_alloc = false; @@ -4146,8 +4149,10 @@ int btrfs_chunk_alloc(struct btrfs_trans_handle *trans, u64 flags, if (flags & BTRFS_BLOCK_GROUP_SYSTEM) return -ENOSPC; - space_info = btrfs_find_space_info(fs_info, flags); - ASSERT(space_info); + if (!space_info) { + space_info = btrfs_find_space_info(fs_info, flags); + ASSERT(space_info); + } do { spin_lock(&space_info->lock); diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h index 36937eeab9b8..c01f3af726a1 100644 --- a/fs/btrfs/block-group.h +++ b/fs/btrfs/block-group.h @@ -342,7 +342,8 @@ int btrfs_add_reserved_bytes(struct btrfs_block_group *cache, bool force_wrong_size_class); void btrfs_free_reserved_bytes(struct btrfs_block_group *cache, u64 num_bytes, int delalloc); -int btrfs_chunk_alloc(struct btrfs_trans_handle *trans, u64 flags, +int btrfs_chunk_alloc(struct btrfs_trans_handle *trans, + struct btrfs_space_info *space_info, u64 flags, enum btrfs_chunk_alloc_enum force); int btrfs_force_chunk_alloc(struct btrfs_trans_handle *trans, u64 type); void check_system_chunk(struct btrfs_trans_handle *trans, const u64 type); diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index a68a8a07caff..1dad1a42c9c1 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -4159,7 +4159,7 @@ static int find_free_extent_update_loop(struct btrfs_fs_info *fs_info, return ret; } - ret = btrfs_chunk_alloc(trans, ffe_ctl->flags, + ret = btrfs_chunk_alloc(trans, NULL, ffe_ctl->flags, CHUNK_ALLOC_FORCE_FOR_EXTENT); /* Do not bail out on ENOSPC since we can do more. */ diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c index d6d33ab754ba..2489c2a16123 100644 --- a/fs/btrfs/space-info.c +++ b/fs/btrfs/space-info.c @@ -817,7 +817,7 @@ static void flush_space(struct btrfs_fs_info *fs_info, ret = PTR_ERR(trans); break; } - ret = btrfs_chunk_alloc(trans, + ret = btrfs_chunk_alloc(trans, space_info, btrfs_get_alloc_profile(fs_info, space_info->flags), (state == ALLOC_CHUNK) ? CHUNK_ALLOC_NO_FORCE : CHUNK_ALLOC_FORCE); diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index 39e48bf610a1..670e0527996c 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -763,7 +763,7 @@ start_transaction(struct btrfs_root *root, unsigned int num_items, if (do_chunk_alloc && num_bytes) { u64 flags = h->block_rsv->space_info->flags; - btrfs_chunk_alloc(h, btrfs_get_alloc_profile(fs_info, flags), + btrfs_chunk_alloc(h, NULL, btrfs_get_alloc_profile(fs_info, flags), CHUNK_ALLOC_NO_FORCE); }