Re: [PATCH v3 09/13] btrfs: introduce tree-log sub-space_info

[Naohiro Aota <Naohiro.Aota@wdc.com>]

On Thu Apr 17, 2025 at 9:44 PM JST, Josef Bacik wrote:
> On Wed, Apr 16, 2025 at 11:28:14PM +0900, Naohiro Aota wrote:
>> This commit introduces the tree-log sub-space_info, which is sub-space of
>> metadata space_info and dedicated for tree-log node allocation.
>> 
>> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
>> ---
>>  fs/btrfs/space-info.c |  4 ++++
>>  fs/btrfs/space-info.h |  1 +
>>  fs/btrfs/sysfs.c      | 10 +++++++++-
>>  3 files changed, 14 insertions(+), 1 deletion(-)
>> 
>> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
>> index 37e55298c082..4b2343a3a009 100644
>> --- a/fs/btrfs/space-info.c
>> +++ b/fs/btrfs/space-info.c
>> @@ -292,6 +292,10 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
>>  		if (flags & BTRFS_BLOCK_GROUP_DATA)
>>  			ret = create_space_info_sub_group(space_info, flags,
>>  							  SUB_GROUP_DATA_RELOC);
>> +		else if (flags & BTRFS_BLOCK_GROUP_METADATA)
>> +			ret = create_space_info_sub_group(space_info, flags,
>> +							  SUB_GROUP_METADATA_TREELOG);
>> +
>>  		if (ret == -ENOMEM)
>>  			return ret;
>>  		ASSERT(!ret);
>> diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
>> index 64641885babd..1aadf88e5789 100644
>> --- a/fs/btrfs/space-info.h
>> +++ b/fs/btrfs/space-info.h
>> @@ -100,6 +100,7 @@ enum btrfs_flush_state {
>>  
>>  enum btrfs_space_info_sub_group {
>>  	SUB_GROUP_DATA_RELOC = 0,
>> +	SUB_GROUP_METADATA_TREELOG = 0,
>
> This will mess up since they have the same value now.  Thanks,

They intensionally have the same value. Since SUB_GROUP_DATA_RELOC and
SUB_GROUP_METADATA_TREELOG are used to index space_info->sub_group array
and they are in the different root space_info (DATA vs METADATA), they
can have the same value.

But, yeah, making them as an ID would be more clean. So, I'm going to
make the values distinct and introduce a small inline function like
this.

static inline struct btrfs_space_info *btrfs_space_info_sub_group(struct btrfs_space_info *space_info,
                  enum btrfs_space_info_sub_group subgroup_id)
{
    if (subgroup_id == BTRFS_SUB_GROUP_PRIMARY)
        return space_info;
        
    if (space_info->flags & BTRFS_BLOCK_GROUP_DATA) {
        ASSERT(subgroup_id == BTRFS_SUB_GROUP_DATA_RELOC);
        return space_info->sub_group[0];
    } else if (space_info->flags & BTRFS_BLOCK_GROUP_METADATA) {
        ASSERT(subgroup_id == BTRFS_SUB_GROUP_METADATA_TREELOG);
        return space_info->sub_group[0];
    }
    
    /* Invalid combination */
    ASSERT(0);
    return NULL;
}

>
> Josef