Hi Jens,
It is still unclear whether the support for non-po2 zone size devices
will be added anytime soon [1]. I have extracted out the cleanup
patches that doesn't do any functional change but improves the
readability by adding helpers. This also helps a bit to
maintain off-tree support as there is an interest to have this feature
in some companies.
I have retained the reviewed by tags for the commits that did not change
from the main patches I sent before[1].
[1] https://lore.kernel.org/lkml/[email protected]/
Luis Chamberlain (1):
dm-zoned: ensure only power of 2 zone sizes are allowed
Pankaj Raghav (6):
block: remove superfluous check for request queue in bdev_is_zoned
block: add a new helper bdev_{is_zone_start, offset_from_zone_start}
nvmet: introduce bdev_zone_no helper
zonefs: use bdev_zone_no helper to calculate the zone index
dm-zone: use generic helpers to calculate offset from zone start
dm: call dm_zone_endio after the target endio callback for zoned
devices
block/blk-core.c | 2 +-
block/blk-zoned.c | 4 ++--
drivers/md/dm-zone.c | 8 +++-----
drivers/md/dm-zoned-target.c | 8 ++++++++
drivers/md/dm.c | 8 ++++----
drivers/nvme/target/zns.c | 3 +--
fs/zonefs/super.c | 8 +++-----
fs/zonefs/zonefs.h | 1 -
include/linux/blkdev.h | 28 +++++++++++++++++++++++-----
9 files changed, 45 insertions(+), 25 deletions(-)
--
2.25.1
Use the bdev_offset_from_zone_start() helper function to calculate
the offset from zone start instead of open coding.
Signed-off-by: Pankaj Raghav <[email protected]>
Reviewed-by: Luis Chamberlain <[email protected]>
Reviewed-by: Damien Le Moal <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>
Reviewed-by: Johannes Thumshirn <[email protected]>
Reviewed-by: Mike Snitzer <[email protected]>
---
drivers/md/dm-zone.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-zone.c b/drivers/md/dm-zone.c
index 3dafc0e8b7a9..ac6fc1293d41 100644
--- a/drivers/md/dm-zone.c
+++ b/drivers/md/dm-zone.c
@@ -390,7 +390,8 @@ static bool dm_zone_map_bio_begin(struct mapped_device *md,
case REQ_OP_WRITE_ZEROES:
case REQ_OP_WRITE:
/* Writes must be aligned to the zone write pointer */
- if ((clone->bi_iter.bi_sector & (zsectors - 1)) != zwp_offset)
+ if (bdev_offset_from_zone_start(md->disk->part0,
+ clone->bi_iter.bi_sector) != zwp_offset)
return false;
break;
case REQ_OP_ZONE_APPEND:
@@ -602,11 +603,8 @@ void dm_zone_endio(struct dm_io *io, struct bio *clone)
*/
if (clone->bi_status == BLK_STS_OK &&
bio_op(clone) == REQ_OP_ZONE_APPEND) {
- sector_t mask =
- (sector_t)bdev_zone_sectors(disk->part0) - 1;
-
orig_bio->bi_iter.bi_sector +=
- clone->bi_iter.bi_sector & mask;
+ bdev_offset_from_zone_start(disk->part0, clone->bi_iter.bi_sector);
}
return;
--
2.25.1
From: Luis Chamberlain <[email protected]>
dm-zoned relies on the assumption that the zone size is a
power-of-2(po2) and the zone capacity is same as the zone size.
Ensure only po2 devices can be used as dm-zoned target until a native
support for zoned devices with non-po2 zone size is added.
Reviewed-by: Hannes Reinecke <[email protected]>
Reviewed-by: Damien Le Moal <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>
Reviewed-by: Mike Snitzer <[email protected]>
Reviewed-by: Johannes Thumshirn <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
Signed-off-by: Pankaj Raghav <[email protected]>
---
drivers/md/dm-zoned-target.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index 95b132b52f33..9325bf5dee81 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -792,6 +792,10 @@ static int dmz_fixup_devices(struct dm_target *ti)
return -EINVAL;
}
zone_nr_sectors = bdev_zone_sectors(bdev);
+ if (!is_power_of_2(zone_nr_sectors)) {
+ ti->error = "Zone size is not a power-of-2 number of sectors";
+ return -EINVAL;
+ }
zoned_dev->zone_nr_sectors = zone_nr_sectors;
zoned_dev->nr_zones = bdev_nr_zones(bdev);
}
@@ -804,6 +808,10 @@ static int dmz_fixup_devices(struct dm_target *ti)
return -EINVAL;
}
zoned_dev->zone_nr_sectors = bdev_zone_sectors(bdev);
+ if (!is_power_of_2(zoned_dev->zone_nr_sectors)) {
+ ti->error = "Zone size is not a power-of-2 number of sectors";
+ return -EINVAL;
+ }
zoned_dev->nr_zones = bdev_nr_zones(bdev);
}
--
2.25.1
dm_zone_endio() updates the bi_sector of orig bio for zoned devices that
uses either native append or append emulation, and it is called before the
endio of the target. But target endio can still update the clone bio
after dm_zone_endio is called, thereby, the orig bio does not contain
the updated information anymore.
Currently, this is not a problem as the targets that support zoned devices
such as dm-zoned, dm-linear, and dm-crypt do not have an endio function,
and even if they do (such as dm-flakey), they don't modify the
bio->bi_iter.bi_sector of the cloned bio that is used to update the
orig_bio's bi_sector in dm_zone_endio function.
Call dm_zone_endio for zoned devices after calling the target's endio
function.
Reviewed-by: Mike Snitzer <[email protected]>
Reviewed-by: Bart Van Assche <[email protected]>
Signed-off-by: Pankaj Raghav <[email protected]>
---
drivers/md/dm.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index b424a6ee27ba..fdef74fe8bd1 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1109,10 +1109,6 @@ static void clone_endio(struct bio *bio)
disable_write_zeroes(md);
}
- if (static_branch_unlikely(&zoned_enabled) &&
- unlikely(bdev_is_zoned(bio->bi_bdev)))
- dm_zone_endio(io, bio);
-
if (endio) {
int r = endio(ti, bio, &error);
switch (r) {
@@ -1141,6 +1137,10 @@ static void clone_endio(struct bio *bio)
}
}
+ if (static_branch_unlikely(&zoned_enabled) &&
+ unlikely(bdev_is_zoned(bio->bi_bdev)))
+ dm_zone_endio(io, bio);
+
if (static_branch_unlikely(&swap_bios_enabled) &&
unlikely(swap_bios_limit(ti, bio)))
up(&md->swap_bios_semaphore);
--
2.25.1
On Fri, Jan 06, 2023 at 09:33:16AM +0100, Pankaj Raghav wrote:
> Use the bdev_offset_from_zone_start() helper function to calculate
> the offset from zone start instead of open coding.
>
> Signed-off-by: Pankaj Raghav <[email protected]>
> Reviewed-by: Luis Chamberlain <[email protected]>
> Reviewed-by: Damien Le Moal <[email protected]>
> Reviewed-by: Bart Van Assche <[email protected]>
> Reviewed-by: Johannes Thumshirn <[email protected]>
> Reviewed-by: Mike Snitzer <[email protected]>
> ---
> drivers/md/dm-zone.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/dm-zone.c b/drivers/md/dm-zone.c
> index 3dafc0e8b7a9..ac6fc1293d41 100644
> --- a/drivers/md/dm-zone.c
> +++ b/drivers/md/dm-zone.c
> @@ -390,7 +390,8 @@ static bool dm_zone_map_bio_begin(struct mapped_device *md,
> case REQ_OP_WRITE_ZEROES:
> case REQ_OP_WRITE:
> /* Writes must be aligned to the zone write pointer */
> - if ((clone->bi_iter.bi_sector & (zsectors - 1)) != zwp_offset)
> + if (bdev_offset_from_zone_start(md->disk->part0,
> + clone->bi_iter.bi_sector) != zwp_offset)
I can't see how this actually cleans antyhing up, while it does add an
overly long line.
> if (clone->bi_status == BLK_STS_OK &&
> bio_op(clone) == REQ_OP_ZONE_APPEND) {
> orig_bio->bi_iter.bi_sector +=
> - clone->bi_iter.bi_sector & mask;
> + bdev_offset_from_zone_start(disk->part0, clone->bi_iter.bi_sector);
Same here.
On Fri, Jan 06, 2023 at 09:33:17AM +0100, Pankaj Raghav wrote:
> dm_zone_endio() updates the bi_sector of orig bio for zoned devices that
> uses either native append or append emulation, and it is called before the
> endio of the target. But target endio can still update the clone bio
> after dm_zone_endio is called, thereby, the orig bio does not contain
> the updated information anymore.
>
> Currently, this is not a problem as the targets that support zoned devices
> such as dm-zoned, dm-linear, and dm-crypt do not have an endio function,
> and even if they do (such as dm-flakey), they don't modify the
> bio->bi_iter.bi_sector of the cloned bio that is used to update the
> orig_bio's bi_sector in dm_zone_endio function.
>
> Call dm_zone_endio for zoned devices after calling the target's endio
> function.
This looks sensible, but I fail to see why we need this or how it fits
into the earlier block layer part of the series.
Given that we don't even support the non pow of 2 zones in the block
layer I don't see why this is needed. But either way it doesn't really
seem to fit into this series.
On 2023-01-10 07:58, Christoph Hellwig wrote:
> On Fri, Jan 06, 2023 at 09:33:17AM +0100, Pankaj Raghav wrote:
>> dm_zone_endio() updates the bi_sector of orig bio for zoned devices that
>> uses either native append or append emulation, and it is called before the
>> endio of the target. But target endio can still update the clone bio
>> after dm_zone_endio is called, thereby, the orig bio does not contain
>> the updated information anymore.
>>
>> Currently, this is not a problem as the targets that support zoned devices
>> such as dm-zoned, dm-linear, and dm-crypt do not have an endio function,
>> and even if they do (such as dm-flakey), they don't modify the
>> bio->bi_iter.bi_sector of the cloned bio that is used to update the
>> orig_bio's bi_sector in dm_zone_endio function.
>>
>> Call dm_zone_endio for zoned devices after calling the target's endio
>> function.
>
> This looks sensible, but I fail to see why we need this or how it fits
> into the earlier block layer part of the series.
>
I just extracted the cleanup from my old series. Do you think it is better
to send it as a separate patch instead of adding it along in this series?
On 2023-01-10 07:56, Christoph Hellwig wrote:
> Given that we don't even support the non pow of 2 zones in the block
> layer I don't see why this is needed. But either way it doesn't really
> seem to fit into this series.
You are right. It is just an extra stop gap, but it is not needed as block
layer does not support pow of 2 anyway! I will remove this patch in the
next version.
On 2023-01-10 07:57, Christoph Hellwig wrote:
> On Fri, Jan 06, 2023 at 09:33:16AM +0100, Pankaj Raghav wrote:
>> Use the bdev_offset_from_zone_start() helper function to calculate
>> the offset from zone start instead of open coding.
>>
>> Signed-off-by: Pankaj Raghav <[email protected]>
>> Reviewed-by: Luis Chamberlain <[email protected]>
>> Reviewed-by: Damien Le Moal <[email protected]>
>> Reviewed-by: Bart Van Assche <[email protected]>
>> Reviewed-by: Johannes Thumshirn <[email protected]>
>> Reviewed-by: Mike Snitzer <[email protected]>
>> ---
>> drivers/md/dm-zone.c | 8 +++-----
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/md/dm-zone.c b/drivers/md/dm-zone.c
>> index 3dafc0e8b7a9..ac6fc1293d41 100644
>> --- a/drivers/md/dm-zone.c
>> +++ b/drivers/md/dm-zone.c
>> @@ -390,7 +390,8 @@ static bool dm_zone_map_bio_begin(struct mapped_device *md,
>> case REQ_OP_WRITE_ZEROES:
>> case REQ_OP_WRITE:
>> /* Writes must be aligned to the zone write pointer */
>> - if ((clone->bi_iter.bi_sector & (zsectors - 1)) != zwp_offset)
>> + if (bdev_offset_from_zone_start(md->disk->part0,
>> + clone->bi_iter.bi_sector) != zwp_offset)
>
> I can't see how this actually cleans antyhing up, while it does add an
> overly long line.
>
While I do agree with the overly long line comment, I feel it makes the
intent more clear, as it is easy to overlook this math operation.
>> if (clone->bi_status == BLK_STS_OK &&
>> bio_op(clone) == REQ_OP_ZONE_APPEND) {
>> orig_bio->bi_iter.bi_sector +=
>> - clone->bi_iter.bi_sector & mask;
>> + bdev_offset_from_zone_start(disk->part0, clone->bi_iter.bi_sector);
>
> Same here.
On Tue, Jan 10, 2023 at 10:07:39AM +0100, Pankaj Raghav wrote:
> I just extracted the cleanup from my old series. Do you think it is better
> to send it as a separate patch instead of adding it along in this series?
dm and block have different maintainers. So unless there is a
dependency it's better to split the patches out.