diff mbox series

[v3,08/13] btrfs: introduce btrfs_space_info sub-group

Message ID ad0d95fe1fec479076594e78dd8ff489ac0a1e83.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

Commit Message

Naohiro Aota April 16, 2025, 2:28 p.m. UTC 
Current code assumes we have only one space_info for each block group type
(DATA, METADATA, and SYSTEM). We sometime needs multiple space_info to
manage special block groups.

One example is handling the data relocation block group for the zoned mode.
That block group is dedicated for writing relocated data and we cannot
allocate any regular extent from that block group, which is implemented in
the zoned extent allocator. That block group still belongs to the normal
data space_info. So, when all the normal data block groups are full and
there are some free space in the dedicated block group, the space_info
looks to have some free space, while it cannot allocate normal extent
anymore. That results in a strange ENOSPC error. We need to have a
space_info for the relocation data block group to represent the situation
properly.

This commit adds a basic infrastructure for having a "sub-group" of a
space_info: creation and removing. A sub-group space_info belongs to one of
the primary space_infos and has the same flags as its parent.

This commit first introduces the relocation data sub-space_info, and the
next commit will introduce tree-log sub-space_info. In the future, it could
be useful to implement tiered storage for btrfs e.g, by implementing a
sub-group space_info for block groups resides on a fast storage.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 fs/btrfs/block-group.c | 11 +++++++++++
 fs/btrfs/space-info.c  | 38 +++++++++++++++++++++++++++++++++++---
 fs/btrfs/space-info.h  |  8 ++++++++
 fs/btrfs/sysfs.c       | 16 +++++++++++++---
 4 files changed, 67 insertions(+), 6 deletions(-)

Comments

Johannes Thumshirn April 16, 2025, 2:56 p.m. UTC | #1 
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Josef Bacik April 17, 2025, 12:43 p.m. UTC | #2 
On Wed, Apr 16, 2025 at 11:28:13PM +0900, Naohiro Aota wrote:
> Current code assumes we have only one space_info for each block group type
> (DATA, METADATA, and SYSTEM). We sometime needs multiple space_info to
> manage special block groups.
> 
> One example is handling the data relocation block group for the zoned mode.
> That block group is dedicated for writing relocated data and we cannot
> allocate any regular extent from that block group, which is implemented in
> the zoned extent allocator. That block group still belongs to the normal
> data space_info. So, when all the normal data block groups are full and
> there are some free space in the dedicated block group, the space_info
> looks to have some free space, while it cannot allocate normal extent
> anymore. That results in a strange ENOSPC error. We need to have a
> space_info for the relocation data block group to represent the situation
> properly.
> 
> This commit adds a basic infrastructure for having a "sub-group" of a
> space_info: creation and removing. A sub-group space_info belongs to one of
> the primary space_infos and has the same flags as its parent.
> 
> This commit first introduces the relocation data sub-space_info, and the
> next commit will introduce tree-log sub-space_info. In the future, it could
> be useful to implement tiered storage for btrfs e.g, by implementing a
> sub-group space_info for block groups resides on a fast storage.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  fs/btrfs/block-group.c | 11 +++++++++++
>  fs/btrfs/space-info.c  | 38 +++++++++++++++++++++++++++++++++++---
>  fs/btrfs/space-info.h  |  8 ++++++++
>  fs/btrfs/sysfs.c       | 16 +++++++++++++---
>  4 files changed, 67 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 846c9737ff5a..475353b0b32c 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -4411,6 +4411,17 @@ static void check_removing_space_info(struct btrfs_space_info *space_info)
>  {
>  	struct btrfs_fs_info *info = space_info->fs_info;
>  
> +	if (space_info->subgroup_id == SUB_GROUP_PRIMARY) {
> +		/* This is a top space_info, proceeds its children first. */
> +		for (int i = 0; i < BTRFS_SPACE_INFO_SUB_GROUP_MAX; i++) {
> +			if (space_info->sub_group[i]) {
> +				check_removing_space_info(space_info->sub_group[i]);
> +				kfree(space_info->sub_group[i]);
> +				space_info->sub_group[i] = NULL;
> +			}
> +		}
> +	}
> +
>  	/*
>  	 * Do not hide this behind enospc_debug, this is actually
>  	 * important and indicates a real bug if this happens.
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 2489c2a16123..37e55298c082 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -249,16 +249,38 @@ static void init_space_info(struct btrfs_fs_info *info,
>  	INIT_LIST_HEAD(&space_info->priority_tickets);
>  	space_info->clamp = 1;
>  	btrfs_update_space_info_chunk_size(space_info, calc_chunk_size(info, flags));
> +	space_info->subgroup_id = SUB_GROUP_PRIMARY;
>  
>  	if (btrfs_is_zoned(info))
>  		space_info->bg_reclaim_threshold = BTRFS_DEFAULT_ZONED_RECLAIM_THRESH;
>  }
>  
> +static int create_space_info_sub_group(struct btrfs_space_info *parent, u64 flags,
> +				       enum btrfs_space_info_sub_group id)
> +{
> +	struct btrfs_fs_info *fs_info = parent->fs_info;
> +	struct btrfs_space_info *sub_space;
> +
> +	ASSERT(parent->subgroup_id == SUB_GROUP_PRIMARY);
> +	ASSERT(id != SUB_GROUP_PRIMARY);
> +
> +	sub_space = kzalloc(sizeof(*sub_space), GFP_NOFS);
> +	if (!sub_space)
> +		return -ENOMEM;
> +
> +	init_space_info(fs_info, sub_space, flags);
> +	parent->sub_group[id] = sub_space;
> +	sub_space->parent = parent;
> +	sub_space->subgroup_id = id;
> +
> +	return btrfs_sysfs_add_space_info_type(fs_info, sub_space);
> +}
> +
>  static int create_space_info(struct btrfs_fs_info *info, u64 flags)
>  {
>  
>  	struct btrfs_space_info *space_info;
> -	int ret;
> +	int ret = 0;
>  
>  	space_info = kzalloc(sizeof(*space_info), GFP_NOFS);
>  	if (!space_info)
> @@ -266,6 +288,15 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
>  
>  	init_space_info(info, space_info, flags);
>  
> +	if (btrfs_is_zoned(info)) {
> +		if (flags & BTRFS_BLOCK_GROUP_DATA)
> +			ret = create_space_info_sub_group(space_info, flags,
> +							  SUB_GROUP_DATA_RELOC);
> +		if (ret == -ENOMEM)
> +			return ret;
> +		ASSERT(!ret);
> +	}
> +
>  	ret = btrfs_sysfs_add_space_info_type(info, space_info);
>  	if (ret)
>  		return ret;
> @@ -561,8 +592,9 @@ static void __btrfs_dump_space_info(const struct btrfs_fs_info *fs_info,
>  	lockdep_assert_held(&info->lock);
>  
>  	/* The free space could be negative in case of overcommit */
> -	btrfs_info(fs_info, "space_info %s has %lld free, is %sfull",
> -		   flag_str,
> +	btrfs_info(fs_info,
> +		   "space_info %s (sub-group id %d) has %lld free, is %sfull",
> +		   flag_str, info->subgroup_id,
>  		   (s64)(info->total_bytes - btrfs_space_info_used(info, true)),
>  		   info->full ? "" : "not ");
>  	btrfs_info(fs_info,
> diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
> index 7459b4eb99cd..64641885babd 100644
> --- a/fs/btrfs/space-info.h
> +++ b/fs/btrfs/space-info.h
> @@ -98,8 +98,16 @@ enum btrfs_flush_state {
>  	RESET_ZONES		= 12,
>  };
>  
> +enum btrfs_space_info_sub_group {
> +	SUB_GROUP_DATA_RELOC = 0,
> +	SUB_GROUP_PRIMARY = -1,
> +};
> +#define BTRFS_SPACE_INFO_SUB_GROUP_MAX 1

We want to avoid namespace pollution, so rename these to have a btrfs prefix,
and do something like

enum btrfs_space_info_sub_group {
	BTRFS_SUB_GROUP_DATA_RELOC = 0,
	BTRFS_SUB_GROUP_MAX,
	BTRFS_SUB_GROUP_PRIMARY = -1,
};

And then you can remove the define.  Thanks,

Josef
diff mbox series

Patch

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 846c9737ff5a..475353b0b32c 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -4411,6 +4411,17 @@  static void check_removing_space_info(struct btrfs_space_info *space_info)
 {
 	struct btrfs_fs_info *info = space_info->fs_info;
 
+	if (space_info->subgroup_id == SUB_GROUP_PRIMARY) {
+		/* This is a top space_info, proceeds its children first. */
+		for (int i = 0; i < BTRFS_SPACE_INFO_SUB_GROUP_MAX; i++) {
+			if (space_info->sub_group[i]) {
+				check_removing_space_info(space_info->sub_group[i]);
+				kfree(space_info->sub_group[i]);
+				space_info->sub_group[i] = NULL;
+			}
+		}
+	}
+
 	/*
 	 * Do not hide this behind enospc_debug, this is actually
 	 * important and indicates a real bug if this happens.
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 2489c2a16123..37e55298c082 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -249,16 +249,38 @@  static void init_space_info(struct btrfs_fs_info *info,
 	INIT_LIST_HEAD(&space_info->priority_tickets);
 	space_info->clamp = 1;
 	btrfs_update_space_info_chunk_size(space_info, calc_chunk_size(info, flags));
+	space_info->subgroup_id = SUB_GROUP_PRIMARY;
 
 	if (btrfs_is_zoned(info))
 		space_info->bg_reclaim_threshold = BTRFS_DEFAULT_ZONED_RECLAIM_THRESH;
 }
 
+static int create_space_info_sub_group(struct btrfs_space_info *parent, u64 flags,
+				       enum btrfs_space_info_sub_group id)
+{
+	struct btrfs_fs_info *fs_info = parent->fs_info;
+	struct btrfs_space_info *sub_space;
+
+	ASSERT(parent->subgroup_id == SUB_GROUP_PRIMARY);
+	ASSERT(id != SUB_GROUP_PRIMARY);
+
+	sub_space = kzalloc(sizeof(*sub_space), GFP_NOFS);
+	if (!sub_space)
+		return -ENOMEM;
+
+	init_space_info(fs_info, sub_space, flags);
+	parent->sub_group[id] = sub_space;
+	sub_space->parent = parent;
+	sub_space->subgroup_id = id;
+
+	return btrfs_sysfs_add_space_info_type(fs_info, sub_space);
+}
+
 static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 {
 
 	struct btrfs_space_info *space_info;
-	int ret;
+	int ret = 0;
 
 	space_info = kzalloc(sizeof(*space_info), GFP_NOFS);
 	if (!space_info)
@@ -266,6 +288,15 @@  static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 
 	init_space_info(info, space_info, flags);
 
+	if (btrfs_is_zoned(info)) {
+		if (flags & BTRFS_BLOCK_GROUP_DATA)
+			ret = create_space_info_sub_group(space_info, flags,
+							  SUB_GROUP_DATA_RELOC);
+		if (ret == -ENOMEM)
+			return ret;
+		ASSERT(!ret);
+	}
+
 	ret = btrfs_sysfs_add_space_info_type(info, space_info);
 	if (ret)
 		return ret;
@@ -561,8 +592,9 @@  static void __btrfs_dump_space_info(const struct btrfs_fs_info *fs_info,
 	lockdep_assert_held(&info->lock);
 
 	/* The free space could be negative in case of overcommit */
-	btrfs_info(fs_info, "space_info %s has %lld free, is %sfull",
-		   flag_str,
+	btrfs_info(fs_info,
+		   "space_info %s (sub-group id %d) has %lld free, is %sfull",
+		   flag_str, info->subgroup_id,
 		   (s64)(info->total_bytes - btrfs_space_info_used(info, true)),
 		   info->full ? "" : "not ");
 	btrfs_info(fs_info,
diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
index 7459b4eb99cd..64641885babd 100644
--- a/fs/btrfs/space-info.h
+++ b/fs/btrfs/space-info.h
@@ -98,8 +98,16 @@  enum btrfs_flush_state {
 	RESET_ZONES		= 12,
 };
 
+enum btrfs_space_info_sub_group {
+	SUB_GROUP_DATA_RELOC = 0,
+	SUB_GROUP_PRIMARY = -1,
+};
+#define BTRFS_SPACE_INFO_SUB_GROUP_MAX 1
 struct btrfs_space_info {
 	struct btrfs_fs_info *fs_info;
+	struct btrfs_space_info *parent;
+	struct btrfs_space_info *sub_group[BTRFS_SPACE_INFO_SUB_GROUP_MAX];
+	int subgroup_id;
 	spinlock_t lock;
 
 	u64 total_bytes;	/* total bytes in the space,
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index b9af74498b0c..92caa5d09e2f 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1930,15 +1930,25 @@  void btrfs_sysfs_remove_space_info(struct btrfs_space_info *space_info)
 	kobject_put(&space_info->kobj);
 }
 
-static const char *alloc_name(u64 flags)
+static const char *alloc_name(struct btrfs_space_info *space_info)
 {
+	u64 flags = space_info->flags;
+
 	switch (flags) {
 	case BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_DATA:
 		return "mixed";
 	case BTRFS_BLOCK_GROUP_METADATA:
 		return "metadata";
 	case BTRFS_BLOCK_GROUP_DATA:
-		return "data";
+		switch (space_info->subgroup_id) {
+		case SUB_GROUP_PRIMARY:
+			return "data";
+		case SUB_GROUP_DATA_RELOC:
+			return "data-reloc";
+		default:
+			WARN_ON_ONCE(1);
+			return "data (unknown sub-group)";
+		}
 	case BTRFS_BLOCK_GROUP_SYSTEM:
 		return "system";
 	default:
@@ -1958,7 +1968,7 @@  int btrfs_sysfs_add_space_info_type(struct btrfs_fs_info *fs_info,
 
 	ret = kobject_init_and_add(&space_info->kobj, &space_info_ktype,
 				   fs_info->space_info_kobj, "%s",
-				   alloc_name(space_info->flags));
+				   alloc_name(space_info));
 	if (ret) {
 		kobject_put(&space_info->kobj);
 		return ret;