diff mbox series

[4/8] btrfs: zoned: reserve zones for an active metadata/system block group

Message ID df9fa11d7d8299b04b99d7276a764bfa318f5734.1690171333.git.naohiro.aota@wdc.com (mailing list archive)
State New, archived
Headers show
Series btrfs: zoned: write-time activation of metadata block group | expand

Commit Message

Naohiro Aota July 24, 2023, 4:18 a.m. UTC 
Ensure a metadata and system block group can be activated on write time, by
leaving a certain number of active zones when trying to activate a data
block group.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 fs/btrfs/zoned.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

Comments

Christoph Hellwig July 24, 2023, 3:06 p.m. UTC | #1 
> +static int reserved_zones(struct btrfs_fs_info *fs_info)
> +{
> +	const u64 flags[] = {BTRFS_BLOCK_GROUP_METADATA, BTRFS_BLOCK_GROUP_SYSTEM};
> +	int reserved = 0;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(flags); i++) {
> +		u64 profile = btrfs_get_alloc_profile(fs_info, flags[i]);
> +
> +		switch (profile) {
> +		case 0: /* single */
> +			reserved += 1;
> +			break;
> +		case BTRFS_BLOCK_GROUP_DUP:
> +			reserved += 2;
> +			break;
> +		default:
> +			ASSERT(0);
> +			break;
> +		}
> +	}
> +
> +	return reserved;
> +}

Shouldn't we just store the number of reserved zones in fs_info instead
of recalculating this over and over?
Naohiro Aota July 25, 2023, 9:07 a.m. UTC | #2 
On Mon, Jul 24, 2023 at 08:06:50AM -0700, Christoph Hellwig wrote:
> > +static int reserved_zones(struct btrfs_fs_info *fs_info)
> > +{
> > +	const u64 flags[] = {BTRFS_BLOCK_GROUP_METADATA, BTRFS_BLOCK_GROUP_SYSTEM};
> > +	int reserved = 0;
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(flags); i++) {
> > +		u64 profile = btrfs_get_alloc_profile(fs_info, flags[i]);
> > +
> > +		switch (profile) {
> > +		case 0: /* single */
> > +			reserved += 1;
> > +			break;
> > +		case BTRFS_BLOCK_GROUP_DUP:
> > +			reserved += 2;
> > +			break;
> > +		default:
> > +			ASSERT(0);
> > +			break;
> > +		}
> > +	}
> > +
> > +	return reserved;
> > +}
> 
> Shouldn't we just store the number of reserved zones in fs_info instead
> of recalculating this over and over?
> 

We can convert the profile from SINGLE to DUP while the FS is running. So,
simply storing it won't work.

But, we can hook the converting process and increase the reservation count
if it is converting to DUP? Then, we can calculate it on mount and record
it in fs_info.
Christoph Hellwig July 26, 2023, 1:07 p.m. UTC | #3 
On Tue, Jul 25, 2023 at 09:07:19AM +0000, Naohiro Aota wrote:
> We can convert the profile from SINGLE to DUP while the FS is running. So,
> simply storing it won't work.
> 
> But, we can hook the converting process and increase the reservation count
> if it is converting to DUP? Then, we can calculate it on mount and record
> it in fs_info.

That sounds like the right thing to do to me.
diff mbox series

Patch

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index dd86f1858c88..dbfa49c70c1a 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1867,6 +1867,35 @@  int btrfs_sync_zone_write_pointer(struct btrfs_device *tgt_dev, u64 logical,
 	return btrfs_zoned_issue_zeroout(tgt_dev, physical_pos, length);
 }
 
+/*
+ * Number of active zones reserved for one metadata block group and one
+ * system block group.
+ */
+static int reserved_zones(struct btrfs_fs_info *fs_info)
+{
+	const u64 flags[] = {BTRFS_BLOCK_GROUP_METADATA, BTRFS_BLOCK_GROUP_SYSTEM};
+	int reserved = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(flags); i++) {
+		u64 profile = btrfs_get_alloc_profile(fs_info, flags[i]);
+
+		switch (profile) {
+		case 0: /* single */
+			reserved += 1;
+			break;
+		case BTRFS_BLOCK_GROUP_DUP:
+			reserved += 2;
+			break;
+		default:
+			ASSERT(0);
+			break;
+		}
+	}
+
+	return reserved;
+}
+
 /*
  * Activate block group and underlying device zones
  *
@@ -1880,6 +1909,8 @@  bool btrfs_zone_activate(struct btrfs_block_group *block_group)
 	struct btrfs_space_info *space_info = block_group->space_info;
 	struct map_lookup *map;
 	struct btrfs_device *device;
+	const int reserved = (block_group->flags & BTRFS_BLOCK_GROUP_DATA) ?
+		reserved_zones(fs_info) : 0;
 	u64 physical;
 	bool ret;
 	int i;
@@ -1909,6 +1940,15 @@  bool btrfs_zone_activate(struct btrfs_block_group *block_group)
 		if (device->zone_info->max_active_zones == 0)
 			continue;
 
+		/*
+		 * For the data block group, leave active zones for one
+		 * metadata block group and one system block group.
+		 */
+		if (atomic_read(&device->zone_info->active_zones_left) <= reserved) {
+			ret = false;
+			goto out_unlock;
+		}
+
 		if (!btrfs_dev_set_active_zone(device, physical)) {
 			/* Cannot activate the zone */
 			ret = false;
@@ -2103,6 +2143,8 @@  bool btrfs_can_activate_zone(struct btrfs_fs_devices *fs_devices, u64 flags)
 {
 	struct btrfs_fs_info *fs_info = fs_devices->fs_info;
 	struct btrfs_device *device;
+	const int reserved = (flags & BTRFS_BLOCK_GROUP_DATA) ?
+		reserved_zones(fs_info) : 0;
 	bool ret = false;
 
 	if (!btrfs_is_zoned(fs_info))
@@ -2123,10 +2165,10 @@  bool btrfs_can_activate_zone(struct btrfs_fs_devices *fs_devices, u64 flags)
 
 		switch (flags & BTRFS_BLOCK_GROUP_PROFILE_MASK) {
 		case 0: /* single */
-			ret = (atomic_read(&zinfo->active_zones_left) >= 1);
+			ret = (atomic_read(&zinfo->active_zones_left) >= (1 + reserved));
 			break;
 		case BTRFS_BLOCK_GROUP_DUP:
-			ret = (atomic_read(&zinfo->active_zones_left) >= 2);
+			ret = (atomic_read(&zinfo->active_zones_left) >= (2 + reserved));
 			break;
 		}
 		if (ret)