| Message ID | 20240529071325.940910-7-naohiro.aota@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: zoned: proper "mkfs.btrfs -b" support | expand |
On Wed, May 29, 2024 at 04:13:21PM +0900, Naohiro Aota wrote: > Even with "mkfs.btrfs -b", mkfs.btrfs resets all the zones on the device. > Limit the reset target within the specified length. > > Also, we need to check that there is no active zone outside of the FS > range. Having an active zone outside FS reduces the number of zones btrfs > can write simultaneously. Technically, we can still scan all the device > zones and keep active zones outside FS intact and try to live with the > limited active zones. But, that will make btrfs operations harder. > > It is generally bad idea to use "-b" on a non-test usage on a device with > active zone limit in the first place. You really need to take care that FS > and outside the FS goes over the limit. That means you'll never be able to > use zones outside the FS anyway. > > So, until there is a strong request for that, I don't think it's worthwhile > to do so. > > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> > Reviewed-by: Qu Wenruo <wqu@suse.com> > --- > common/device-utils.c | 17 ++++++++++++----- > kernel-shared/zoned.c | 23 ++++++++++++++++++++++- > kernel-shared/zoned.h | 7 ++++--- > 3 files changed, 38 insertions(+), 9 deletions(-) > > diff --git a/common/device-utils.c b/common/device-utils.c > index 86942e0c7041..7df7d9ce39d8 100644 > --- a/common/device-utils.c > +++ b/common/device-utils.c > @@ -254,16 +254,23 @@ int btrfs_prepare_device(int fd, const char *file, u64 *byte_count_ret, > > if (!zinfo->emulated) { > if (opflags & PREP_DEVICE_VERBOSE) > - printf("Resetting device zones %s (%u zones) ...\n", > - file, zinfo->nr_zones); > + printf("Resetting device zones %s (%llu zones) ...\n", > + file, byte_count / zinfo->zone_size); > /* > * We cannot ignore zone reset errors for a zoned block > * device as this could result in the inability to write > * to non-empty sequential zones of the device. > */ > - if (btrfs_reset_all_zones(fd, zinfo)) { > - error("zoned: failed to reset device '%s' zones: %m", > - file); > + ret = btrfs_reset_zones(fd, zinfo, byte_count); > + if (ret) { > + if (ret == EBUSY) { > + error("zoned: device '%s' contains an active zone outside of the FS range", > + file); > + error("zoned: btrfs needs full control of active zones"); > + } else { > + error("zoned: failed to reset device '%s' zones: %m", > + file); > + } > goto err; > } > } > diff --git a/kernel-shared/zoned.c b/kernel-shared/zoned.c > index fb1e1388804e..b4244966ca36 100644 > --- a/kernel-shared/zoned.c > +++ b/kernel-shared/zoned.c > @@ -395,16 +395,24 @@ static int report_zones(int fd, const char *file, > * Discard blocks in the zones of a zoned block device. Process this with zone > * size granularity so that blocks in conventional zones are discarded using > * discard_range and blocks in sequential zones are reset though a zone reset. > + * > + * We need to ensure that zones outside of the FS is not active, so that > + * the FS can use all the active zones. Return EBUSY if there is an active > + * zone. > */ > -int btrfs_reset_all_zones(int fd, struct btrfs_zoned_device_info *zinfo) > +int btrfs_reset_zones(int fd, struct btrfs_zoned_device_info *zinfo, u64 byte_count) > { > unsigned int i; > int ret = 0; > > ASSERT(zinfo); > + ASSERT(IS_ALIGNED(byte_count, zinfo->zone_size)); > > /* Zone size granularity */ > for (i = 0; i < zinfo->nr_zones; i++) { > + if (byte_count == 0) > + break; > + > if (zinfo->zones[i].type == BLK_ZONE_TYPE_CONVENTIONAL) { > ret = device_discard_blocks(fd, > zinfo->zones[i].start << SECTOR_SHIFT, > @@ -419,7 +427,20 @@ int btrfs_reset_all_zones(int fd, struct btrfs_zoned_device_info *zinfo) > > if (ret) > return ret; > + > + byte_count -= zinfo->zone_size; > } > + for (; i < zinfo->nr_zones; i++) { > + const enum blk_zone_cond cond = zinfo->zones[i].cond; > + > + if (zinfo->zones[i].type == BLK_ZONE_TYPE_CONVENTIONAL) > + continue; > + if (cond == BLK_ZONE_COND_IMP_OPEN || > + cond == BLK_ZONE_COND_EXP_OPEN || > + cond == BLK_ZONE_COND_CLOSED) > + return EBUSY; Should this return -EBUSY? It should not matter for this case but by convention it would be better to use only negative errnos. I found another one that's in the same call chain that still returns plain errno, discard_range(). This should be fixed, possibly separately, so I'll keep your patch as is.
diff --git a/common/device-utils.c b/common/device-utils.c index 86942e0c7041..7df7d9ce39d8 100644 --- a/common/device-utils.c +++ b/common/device-utils.c @@ -254,16 +254,23 @@ int btrfs_prepare_device(int fd, const char *file, u64 *byte_count_ret, if (!zinfo->emulated) { if (opflags & PREP_DEVICE_VERBOSE) - printf("Resetting device zones %s (%u zones) ...\n", - file, zinfo->nr_zones); + printf("Resetting device zones %s (%llu zones) ...\n", + file, byte_count / zinfo->zone_size); /* * We cannot ignore zone reset errors for a zoned block * device as this could result in the inability to write * to non-empty sequential zones of the device. */ - if (btrfs_reset_all_zones(fd, zinfo)) { - error("zoned: failed to reset device '%s' zones: %m", - file); + ret = btrfs_reset_zones(fd, zinfo, byte_count); + if (ret) { + if (ret == EBUSY) { + error("zoned: device '%s' contains an active zone outside of the FS range", + file); + error("zoned: btrfs needs full control of active zones"); + } else { + error("zoned: failed to reset device '%s' zones: %m", + file); + } goto err; } } diff --git a/kernel-shared/zoned.c b/kernel-shared/zoned.c index fb1e1388804e..b4244966ca36 100644 --- a/kernel-shared/zoned.c +++ b/kernel-shared/zoned.c @@ -395,16 +395,24 @@ static int report_zones(int fd, const char *file, * Discard blocks in the zones of a zoned block device. Process this with zone * size granularity so that blocks in conventional zones are discarded using * discard_range and blocks in sequential zones are reset though a zone reset. + * + * We need to ensure that zones outside of the FS is not active, so that + * the FS can use all the active zones. Return EBUSY if there is an active + * zone. */ -int btrfs_reset_all_zones(int fd, struct btrfs_zoned_device_info *zinfo) +int btrfs_reset_zones(int fd, struct btrfs_zoned_device_info *zinfo, u64 byte_count) { unsigned int i; int ret = 0; ASSERT(zinfo); + ASSERT(IS_ALIGNED(byte_count, zinfo->zone_size)); /* Zone size granularity */ for (i = 0; i < zinfo->nr_zones; i++) { + if (byte_count == 0) + break; + if (zinfo->zones[i].type == BLK_ZONE_TYPE_CONVENTIONAL) { ret = device_discard_blocks(fd, zinfo->zones[i].start << SECTOR_SHIFT, @@ -419,7 +427,20 @@ int btrfs_reset_all_zones(int fd, struct btrfs_zoned_device_info *zinfo) if (ret) return ret; + + byte_count -= zinfo->zone_size; } + for (; i < zinfo->nr_zones; i++) { + const enum blk_zone_cond cond = zinfo->zones[i].cond; + + if (zinfo->zones[i].type == BLK_ZONE_TYPE_CONVENTIONAL) + continue; + if (cond == BLK_ZONE_COND_IMP_OPEN || + cond == BLK_ZONE_COND_EXP_OPEN || + cond == BLK_ZONE_COND_CLOSED) + return EBUSY; + } + return fsync(fd); } diff --git a/kernel-shared/zoned.h b/kernel-shared/zoned.h index 6eba86d266bf..2bf24cbba62a 100644 --- a/kernel-shared/zoned.h +++ b/kernel-shared/zoned.h @@ -149,7 +149,7 @@ bool btrfs_redirty_extent_buffer_for_zoned(struct btrfs_fs_info *fs_info, u64 start, u64 end); int btrfs_reset_chunk_zones(struct btrfs_fs_info *fs_info, u64 devid, u64 offset, u64 length); -int btrfs_reset_all_zones(int fd, struct btrfs_zoned_device_info *zinfo); +int btrfs_reset_zones(int fd, struct btrfs_zoned_device_info *zinfo, u64 byte_count); int zero_zone_blocks(int fd, struct btrfs_zoned_device_info *zinfo, off_t start, size_t len); int btrfs_wipe_temporary_sb(struct btrfs_fs_devices *fs_devices); @@ -203,8 +203,9 @@ static inline int btrfs_reset_chunk_zones(struct btrfs_fs_info *fs_info, return 0; } -static inline int btrfs_reset_all_zones(int fd, - struct btrfs_zoned_device_info *zinfo) +static inline int btrfs_reset_zones(int fd, + struct btrfs_zoned_device_info *zinfo, + u64 byte_count) { return -EOPNOTSUPP; }