* [PATCH v5 1/7] block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2 zsze
2022-05-23 16:15 ` [PATCH v5 0/7] support non power of 2 zoned devices Pankaj Raghav
@ 2022-05-23 16:15 ` Pankaj Raghav
2022-05-23 16:15 ` [PATCH v5 2/7] block: allow blk-zoned devices to have non-power-of-2 zone size Pankaj Raghav
` (5 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Pankaj Raghav @ 2022-05-23 16:15 UTC (permalink / raw)
To: axboe, hch, snitzer, damien.lemoal, hare, Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Pankaj Raghav, Luis Chamberlain
Adapt blkdev_nr_zones and blk_queue_zone_no function so that it can
also work for non-power-of-2 zone sizes.
As the existing deployments of zoned devices had power-of-2
assumption, power-of-2 optimized calculation is kept for those devices.
There are no direct hot paths modified and the changes just
introduce one new branch per call.
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
block/blk-zoned.c | 12 +++++++++---
include/linux/blkdev.h | 8 +++++++-
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 38cd840d8838..e7eec513dd42 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -111,16 +111,22 @@ EXPORT_SYMBOL_GPL(__blk_req_zone_write_unlock);
* blkdev_nr_zones - Get number of zones
* @disk: Target gendisk
*
- * Return the total number of zones of a zoned block device. For a block
- * device without zone capabilities, the number of zones is always 0.
+ * Return the total number of zones of a zoned block device, including the
+ * eventual small last zone if present. For a block device without zone
+ * capabilities, the number of zones is always 0.
*/
unsigned int blkdev_nr_zones(struct gendisk *disk)
{
sector_t zone_sectors = blk_queue_zone_sectors(disk->queue);
+ sector_t capacity = get_capacity(disk);
if (!blk_queue_is_zoned(disk->queue))
return 0;
- return (get_capacity(disk) + zone_sectors - 1) >> ilog2(zone_sectors);
+
+ if (is_power_of_2(zone_sectors))
+ return (capacity + zone_sectors - 1) >> ilog2(zone_sectors);
+
+ return div64_u64(capacity + zone_sectors - 1, zone_sectors);
}
EXPORT_SYMBOL_GPL(blkdev_nr_zones);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 60d016138997..c4e4c7071b7b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -665,9 +665,15 @@ static inline unsigned int blk_queue_nr_zones(struct request_queue *q)
static inline unsigned int blk_queue_zone_no(struct request_queue *q,
sector_t sector)
{
+ sector_t zone_sectors = blk_queue_zone_sectors(q);
+
if (!blk_queue_is_zoned(q))
return 0;
- return sector >> ilog2(q->limits.chunk_sectors);
+
+ if (is_power_of_2(zone_sectors))
+ return sector >> ilog2(zone_sectors);
+
+ return div64_u64(sector, zone_sectors);
}
static inline bool blk_queue_zone_is_seq(struct request_queue *q,
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v5 2/7] block: allow blk-zoned devices to have non-power-of-2 zone size
2022-05-23 16:15 ` [PATCH v5 0/7] support non power of 2 zoned devices Pankaj Raghav
2022-05-23 16:15 ` [PATCH v5 1/7] block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2 zsze Pankaj Raghav
@ 2022-05-23 16:15 ` Pankaj Raghav
2022-05-24 5:19 ` Hannes Reinecke
2022-05-23 16:15 ` [PATCH v5 3/7] nvme: zns: Allow ZNS drives that have non-power_of_2 " Pankaj Raghav
` (4 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Pankaj Raghav @ 2022-05-23 16:15 UTC (permalink / raw)
To: axboe, hch, snitzer, damien.lemoal, hare, Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Pankaj Raghav, Luis Chamberlain
Checking if a given sector is aligned to a zone is a common
operation that is performed for zoned devices. Add
blk_queue_is_zone_start helper to check for this instead of opencoding it
everywhere.
Convert the calculations on zone size to be generic instead of relying on
power_of_2 based logic in the block layer using the helpers wherever
possible.
The only hot path affected by this change for power_of_2 zoned devices
is in blk_check_zone_append() but blk_queue_is_zone_start() helper is
used to optimize the calculation for po2 zone sizes. Note that the append
path cannot be accessed by direct raw access to the block device but only
through a filesystem abstraction.
Finally, allow non power of 2 zoned devices provided that their zone
capacity and zone size are equal. The main motivation to allow non
power_of_2 zoned device is to remove the unmapped LBA between zcap and
zsze for devices that cannot have a power_of_2 zcap.
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
block/blk-core.c | 3 +--
block/blk-zoned.c | 26 ++++++++++++++++++++------
include/linux/blkdev.h | 21 +++++++++++++++++++++
3 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index bc0506772152..bf1eae142118 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -630,8 +630,7 @@ static inline blk_status_t blk_check_zone_append(struct request_queue *q,
return BLK_STS_NOTSUPP;
/* The bio sector must point to the start of a sequential zone */
- if (pos & (blk_queue_zone_sectors(q) - 1) ||
- !blk_queue_zone_is_seq(q, pos))
+ if (!blk_queue_is_zone_start(q, pos) || !blk_queue_zone_is_seq(q, pos))
return BLK_STS_IOERR;
/*
diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index e7eec513dd42..665993b13668 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -288,10 +288,10 @@ int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op,
return -EINVAL;
/* Check alignment (handle eventual smaller last zone) */
- if (sector & (zone_sectors - 1))
+ if (!blk_queue_is_zone_start(q, sector))
return -EINVAL;
- if ((nr_sectors & (zone_sectors - 1)) && end_sector != capacity)
+ if (!blk_queue_is_zone_start(q, nr_sectors) && end_sector != capacity)
return -EINVAL;
/*
@@ -489,14 +489,28 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx,
* smaller last zone.
*/
if (zone->start == 0) {
- if (zone->len == 0 || !is_power_of_2(zone->len)) {
- pr_warn("%s: Invalid zoned device with non power of two zone size (%llu)\n",
- disk->disk_name, zone->len);
+ if (zone->len == 0) {
+ pr_warn("%s: Invalid zone size", disk->disk_name);
+ return -ENODEV;
+ }
+
+ /*
+ * Don't allow zoned device with non power_of_2 zone size with
+ * zone capacity less than zone size.
+ */
+ if (!is_power_of_2(zone->len) &&
+ zone->capacity < zone->len) {
+ pr_warn("%s: Invalid zone capacity for non power of 2 zone size",
+ disk->disk_name);
return -ENODEV;
}
args->zone_sectors = zone->len;
- args->nr_zones = (capacity + zone->len - 1) >> ilog2(zone->len);
+ /*
+ * Division is used to calculate nr_zones for both power_of_2
+ * and non power_of_2 zone sizes as it is not in the hot path.
+ */
+ args->nr_zones = div64_u64(capacity + zone->len - 1, zone->len);
} else if (zone->start + args->zone_sectors < capacity) {
if (zone->len != args->zone_sectors) {
pr_warn("%s: Invalid zoned device with non constant zone size\n",
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c4e4c7071b7b..f5c7a41032ba 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -676,6 +676,21 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
return div64_u64(sector, zone_sectors);
}
+static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
+{
+ sector_t zone_sectors = blk_queue_zone_sectors(q);
+ u64 remainder = 0;
+
+ if (!blk_queue_is_zoned(q))
+ return false;
+
+ if (is_power_of_2(zone_sectors))
+ return IS_ALIGNED(sec, zone_sectors);
+
+ div64_u64_rem(sec, zone_sectors, &remainder);
+ return remainder == 0;
+}
+
static inline bool blk_queue_zone_is_seq(struct request_queue *q,
sector_t sector)
{
@@ -722,6 +737,12 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
{
return 0;
}
+
+static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
+{
+ return false;
+}
+
static inline unsigned int queue_max_open_zones(const struct request_queue *q)
{
return 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v5 2/7] block: allow blk-zoned devices to have non-power-of-2 zone size
2022-05-23 16:15 ` [PATCH v5 2/7] block: allow blk-zoned devices to have non-power-of-2 zone size Pankaj Raghav
@ 2022-05-24 5:19 ` Hannes Reinecke
2022-05-24 7:10 ` Pankaj Raghav
0 siblings, 1 reply; 19+ messages in thread
From: Hannes Reinecke @ 2022-05-24 5:19 UTC (permalink / raw)
To: Pankaj Raghav, axboe, hch, snitzer, damien.lemoal,
Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Luis Chamberlain
On 5/23/22 18:15, Pankaj Raghav wrote:
> Checking if a given sector is aligned to a zone is a common
> operation that is performed for zoned devices. Add
> blk_queue_is_zone_start helper to check for this instead of opencoding it
> everywhere.
>
> Convert the calculations on zone size to be generic instead of relying on
> power_of_2 based logic in the block layer using the helpers wherever
> possible.
>
> The only hot path affected by this change for power_of_2 zoned devices
> is in blk_check_zone_append() but blk_queue_is_zone_start() helper is
> used to optimize the calculation for po2 zone sizes. Note that the append
> path cannot be accessed by direct raw access to the block device but only
> through a filesystem abstraction.
>
> Finally, allow non power of 2 zoned devices provided that their zone
> capacity and zone size are equal. The main motivation to allow non
> power_of_2 zoned device is to remove the unmapped LBA between zcap and
> zsze for devices that cannot have a power_of_2 zcap.
>
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
> block/blk-core.c | 3 +--
> block/blk-zoned.c | 26 ++++++++++++++++++++------
> include/linux/blkdev.h | 21 +++++++++++++++++++++
> 3 files changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index bc0506772152..bf1eae142118 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -630,8 +630,7 @@ static inline blk_status_t blk_check_zone_append(struct request_queue *q,
> return BLK_STS_NOTSUPP;
>
> /* The bio sector must point to the start of a sequential zone */
> - if (pos & (blk_queue_zone_sectors(q) - 1) ||
> - !blk_queue_zone_is_seq(q, pos))
> + if (!blk_queue_is_zone_start(q, pos) || !blk_queue_zone_is_seq(q, pos))
> return BLK_STS_IOERR;
>
> /*
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index e7eec513dd42..665993b13668 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -288,10 +288,10 @@ int blkdev_zone_mgmt(struct block_device *bdev, enum req_opf op,
> return -EINVAL;
>
> /* Check alignment (handle eventual smaller last zone) */
> - if (sector & (zone_sectors - 1))
> + if (!blk_queue_is_zone_start(q, sector))
> return -EINVAL;
>
> - if ((nr_sectors & (zone_sectors - 1)) && end_sector != capacity)
> + if (!blk_queue_is_zone_start(q, nr_sectors) && end_sector != capacity)
> return -EINVAL;
>
> /*
> @@ -489,14 +489,28 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx,
> * smaller last zone.
> */
> if (zone->start == 0) {
> - if (zone->len == 0 || !is_power_of_2(zone->len)) {
> - pr_warn("%s: Invalid zoned device with non power of two zone size (%llu)\n",
> - disk->disk_name, zone->len);
> + if (zone->len == 0) {
> + pr_warn("%s: Invalid zone size", disk->disk_name);
> + return -ENODEV;
> + }
> +
> + /*
> + * Don't allow zoned device with non power_of_2 zone size with
> + * zone capacity less than zone size.
> + */
> + if (!is_power_of_2(zone->len) &&
> + zone->capacity < zone->len) {
> + pr_warn("%s: Invalid zone capacity for non power of 2 zone size",
> + disk->disk_name);
> return -ENODEV;
> }
>
> args->zone_sectors = zone->len;
> - args->nr_zones = (capacity + zone->len - 1) >> ilog2(zone->len);
> + /*
> + * Division is used to calculate nr_zones for both power_of_2
> + * and non power_of_2 zone sizes as it is not in the hot path.
> + */
> + args->nr_zones = div64_u64(capacity + zone->len - 1, zone->len);
> } else if (zone->start + args->zone_sectors < capacity) {
> if (zone->len != args->zone_sectors) {
> pr_warn("%s: Invalid zoned device with non constant zone size\n",
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index c4e4c7071b7b..f5c7a41032ba 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -676,6 +676,21 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
> return div64_u64(sector, zone_sectors);
> }
>
> +static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
> +{
> + sector_t zone_sectors = blk_queue_zone_sectors(q);
> + u64 remainder = 0;
> +
> + if (!blk_queue_is_zoned(q))
> + return false;
> +
Not sure if we need this here; surely blk_queue_zone_sectors() will
already barf, and none of the callers did this check.
> + if (is_power_of_2(zone_sectors))
> + return IS_ALIGNED(sec, zone_sectors);
> +
> + div64_u64_rem(sec, zone_sectors, &remainder);
> + return remainder == 0;
> +}
> +
> static inline bool blk_queue_zone_is_seq(struct request_queue *q,
> sector_t sector)
> {
> @@ -722,6 +737,12 @@ static inline unsigned int blk_queue_zone_no(struct request_queue *q,
> {
> return 0;
> }
> +
> +static inline bool blk_queue_is_zone_start(struct request_queue *q, sector_t sec)
> +{
> + return false;
> +}
> +
> static inline unsigned int queue_max_open_zones(const struct request_queue *q)
> {
> return 0;
Otherwise looks good.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v5 2/7] block: allow blk-zoned devices to have non-power-of-2 zone size
2022-05-24 5:19 ` Hannes Reinecke
@ 2022-05-24 7:10 ` Pankaj Raghav
2022-05-24 7:14 ` Hannes Reinecke
0 siblings, 1 reply; 19+ messages in thread
From: Pankaj Raghav @ 2022-05-24 7:10 UTC (permalink / raw)
To: Hannes Reinecke, axboe, hch, snitzer, damien.lemoal,
Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Luis Chamberlain
On 5/24/22 07:19, Hannes Reinecke wrote:
>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>> index c4e4c7071b7b..f5c7a41032ba 100644
>> --- a/include/linux/blkdev.h
>> +++ b/include/linux/blkdev.h
>> @@ -676,6 +676,21 @@ static inline unsigned int
>> blk_queue_zone_no(struct request_queue *q,
>> return div64_u64(sector, zone_sectors);
>> }
>> +static inline bool blk_queue_is_zone_start(struct request_queue *q,
>> sector_t sec)
>> +{
>> + sector_t zone_sectors = blk_queue_zone_sectors(q);
>> + u64 remainder = 0;
>> +
>> + if (!blk_queue_is_zoned(q))
>> + return false;
>> +
>
> Not sure if we need this here; surely blk_queue_zone_sectors() will
> already barf, and none of the callers did this check.
>
I totally agree with you but all the other blk_queue_* functions had
this defensive check and I didn't want to break that pattern:
static inline unsigned int blk_queue_zone_no(struct request_queue *q,
sector_t sector)
{
....
if (!blk_queue_is_zoned(q))
return 0;
....
}
>> + if (is_power_of_2(zone_sectors))
>> + return IS_ALIGNED(sec, zone_sectors);
>> +
And, if the chunk sectors is 0, then we will do the npo2 calculation
resulting in a divide by zero even though chances of that happening is
very very low as you pointed out.
>> + div64_u64_rem(sec, zone_sectors, &remainder);
>> + return remainder == 0;
>> +}
>> +
>> static inline bool blk_queue_zone_is_seq(struct request_queue *q,
>> sector_t sector)
>> {
>> @@ -722,6 +737,12 @@ static inline unsigned int
>> blk_queue_zone_no(struct request_queue *q,
>> {
>> return 0;
>> }
>> +
>> +static inline bool blk_queue_is_zone_start(struct request_queue *q,
>> sector_t sec)
>> +{
>> + return false;
>> +}
>> +
>> static inline unsigned int queue_max_open_zones(const struct
>> request_queue *q)
>> {
>> return 0;
> Otherwise looks good.
>
Thanks!
> Cheers,
>
> Hannes
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v5 2/7] block: allow blk-zoned devices to have non-power-of-2 zone size
2022-05-24 7:10 ` Pankaj Raghav
@ 2022-05-24 7:14 ` Hannes Reinecke
0 siblings, 0 replies; 19+ messages in thread
From: Hannes Reinecke @ 2022-05-24 7:14 UTC (permalink / raw)
To: Pankaj Raghav, axboe, hch, snitzer, damien.lemoal,
Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Luis Chamberlain
On 5/24/22 09:10, Pankaj Raghav wrote:
> On 5/24/22 07:19, Hannes Reinecke wrote:
>
>>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>>> index c4e4c7071b7b..f5c7a41032ba 100644
>>> --- a/include/linux/blkdev.h
>>> +++ b/include/linux/blkdev.h
>>> @@ -676,6 +676,21 @@ static inline unsigned int
>>> blk_queue_zone_no(struct request_queue *q,
>>> return div64_u64(sector, zone_sectors);
>>> }
>>> +static inline bool blk_queue_is_zone_start(struct request_queue *q,
>>> sector_t sec)
>>> +{
>>> + sector_t zone_sectors = blk_queue_zone_sectors(q);
>>> + u64 remainder = 0;
>>> +
>>> + if (!blk_queue_is_zoned(q))
>>> + return false;
>>> +
>>
>> Not sure if we need this here; surely blk_queue_zone_sectors() will
>> already barf, and none of the callers did this check.
>>
> I totally agree with you but all the other blk_queue_* functions had
> this defensive check and I didn't want to break that pattern:
>
Ah, fair enough.
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5 3/7] nvme: zns: Allow ZNS drives that have non-power_of_2 zone size
2022-05-23 16:15 ` [PATCH v5 0/7] support non power of 2 zoned devices Pankaj Raghav
2022-05-23 16:15 ` [PATCH v5 1/7] block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2 zsze Pankaj Raghav
2022-05-23 16:15 ` [PATCH v5 2/7] block: allow blk-zoned devices to have non-power-of-2 zone size Pankaj Raghav
@ 2022-05-23 16:15 ` Pankaj Raghav
2022-05-24 5:21 ` Hannes Reinecke
2022-05-23 16:15 ` [PATCH v5 4/7] nvmet: Allow ZNS target to support non-power_of_2 zone sizes Pankaj Raghav
` (3 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Pankaj Raghav @ 2022-05-23 16:15 UTC (permalink / raw)
To: axboe, hch, snitzer, damien.lemoal, hare, Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Pankaj Raghav, Luis Chamberlain
Remove the condition which disallows non-power_of_2 zone size ZNS drive
to be updated and use generic method to calculate number of zones
instead of relying on log and shift based calculation on zone size.
The power_of_2 calculation has been replaced directly with generic
calculation without special handling. Both modified functions are not
used in hot paths, they are only used during initialization &
revalidation of the ZNS device.
As rounddown macro from math.h does not work for 32 bit architectures,
round down operation is open coded.
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
drivers/nvme/host/zns.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 9f81beb4df4e..d92f937d5cb9 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -101,13 +101,6 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
}
ns->zsze = nvme_lba_to_sect(ns, le64_to_cpu(id->lbafe[lbaf].zsze));
- if (!is_power_of_2(ns->zsze)) {
- dev_warn(ns->ctrl->device,
- "invalid zone size:%llu for namespace:%u\n",
- ns->zsze, ns->head->ns_id);
- status = -ENODEV;
- goto free_data;
- }
blk_queue_set_zoned(ns->disk, BLK_ZONED_HM);
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
@@ -128,8 +121,13 @@ static void *nvme_zns_alloc_report_buffer(struct nvme_ns *ns,
const size_t min_bufsize = sizeof(struct nvme_zone_report) +
sizeof(struct nvme_zone_descriptor);
+ /*
+ * Division is used to calculate nr_zones with no special handling
+ * for power of 2 zone sizes as this function is not invoked in a
+ * hot path
+ */
nr_zones = min_t(unsigned int, nr_zones,
- get_capacity(ns->disk) >> ilog2(ns->zsze));
+ div64_u64(get_capacity(ns->disk), ns->zsze));
bufsize = sizeof(struct nvme_zone_report) +
nr_zones * sizeof(struct nvme_zone_descriptor);
@@ -182,6 +180,7 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
int ret, zone_idx = 0;
unsigned int nz, i;
size_t buflen;
+ u64 remainder = 0;
if (ns->head->ids.csi != NVME_CSI_ZNS)
return -EINVAL;
@@ -197,7 +196,11 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
c.zmr.zrasf = NVME_ZRASF_ZONE_REPORT_ALL;
c.zmr.pr = NVME_REPORT_ZONE_PARTIAL;
- sector &= ~(ns->zsze - 1);
+ /*
+ * Round down the sector value to the nearest zone start
+ */
+ div64_u64_rem(sector, ns->zsze, &remainder);
+ sector -= remainder;
while (zone_idx < nr_zones && sector < get_capacity(ns->disk)) {
memset(report, 0, buflen);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v5 3/7] nvme: zns: Allow ZNS drives that have non-power_of_2 zone size
2022-05-23 16:15 ` [PATCH v5 3/7] nvme: zns: Allow ZNS drives that have non-power_of_2 " Pankaj Raghav
@ 2022-05-24 5:21 ` Hannes Reinecke
0 siblings, 0 replies; 19+ messages in thread
From: Hannes Reinecke @ 2022-05-24 5:21 UTC (permalink / raw)
To: Pankaj Raghav, axboe, hch, snitzer, damien.lemoal,
Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Luis Chamberlain
On 5/23/22 18:15, Pankaj Raghav wrote:
> Remove the condition which disallows non-power_of_2 zone size ZNS drive
> to be updated and use generic method to calculate number of zones
> instead of relying on log and shift based calculation on zone size.
>
> The power_of_2 calculation has been replaced directly with generic
> calculation without special handling. Both modified functions are not
> used in hot paths, they are only used during initialization &
> revalidation of the ZNS device.
>
> As rounddown macro from math.h does not work for 32 bit architectures,
> round down operation is open coded.
>
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
> drivers/nvme/host/zns.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
> index 9f81beb4df4e..d92f937d5cb9 100644
> --- a/drivers/nvme/host/zns.c
> +++ b/drivers/nvme/host/zns.c
> @@ -101,13 +101,6 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
> }
>
> ns->zsze = nvme_lba_to_sect(ns, le64_to_cpu(id->lbafe[lbaf].zsze));
> - if (!is_power_of_2(ns->zsze)) {
> - dev_warn(ns->ctrl->device,
> - "invalid zone size:%llu for namespace:%u\n",
> - ns->zsze, ns->head->ns_id);
> - status = -ENODEV;
> - goto free_data;
> - }
>
> blk_queue_set_zoned(ns->disk, BLK_ZONED_HM);
> blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
> @@ -128,8 +121,13 @@ static void *nvme_zns_alloc_report_buffer(struct nvme_ns *ns,
> const size_t min_bufsize = sizeof(struct nvme_zone_report) +
> sizeof(struct nvme_zone_descriptor);
>
> + /*
> + * Division is used to calculate nr_zones with no special handling
> + * for power of 2 zone sizes as this function is not invoked in a
> + * hot path
> + */
> nr_zones = min_t(unsigned int, nr_zones,
> - get_capacity(ns->disk) >> ilog2(ns->zsze));
> + div64_u64(get_capacity(ns->disk), ns->zsze));
>
> bufsize = sizeof(struct nvme_zone_report) +
> nr_zones * sizeof(struct nvme_zone_descriptor);
> @@ -182,6 +180,7 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
> int ret, zone_idx = 0;
> unsigned int nz, i;
> size_t buflen;
> + u64 remainder = 0;
>
> if (ns->head->ids.csi != NVME_CSI_ZNS)
> return -EINVAL;
> @@ -197,7 +196,11 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
> c.zmr.zrasf = NVME_ZRASF_ZONE_REPORT_ALL;
> c.zmr.pr = NVME_REPORT_ZONE_PARTIAL;
>
> - sector &= ~(ns->zsze - 1);
> + /*
> + * Round down the sector value to the nearest zone start
> + */
> + div64_u64_rem(sector, ns->zsze, &remainder);
> + sector -= remainder;
> while (zone_idx < nr_zones && sector < get_capacity(ns->disk)) {
> memset(report, 0, buflen);
>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5 4/7] nvmet: Allow ZNS target to support non-power_of_2 zone sizes
2022-05-23 16:15 ` [PATCH v5 0/7] support non power of 2 zoned devices Pankaj Raghav
` (2 preceding siblings ...)
2022-05-23 16:15 ` [PATCH v5 3/7] nvme: zns: Allow ZNS drives that have non-power_of_2 " Pankaj Raghav
@ 2022-05-23 16:15 ` Pankaj Raghav
2022-05-23 16:15 ` [PATCH v5 5/7] null_blk: allow non power of 2 zoned devices Pankaj Raghav
` (2 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Pankaj Raghav @ 2022-05-23 16:15 UTC (permalink / raw)
To: axboe, hch, snitzer, damien.lemoal, hare, Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Pankaj Raghav, Bart Van Assche,
Johannes Thumshirn, Luis Chamberlain
A generic bdev_zone_no helper is added to calculate zone number for a given
sector in a block device. This helper internally uses blk_queue_zone_no to
find the zone number.
Use the helper bdev_zone_no() to calculate nr of zones. This let's us
make modifications to the math if needed in one place and adds now
support for npo2 zone devices.
Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
drivers/nvme/target/zns.c | 2 +-
include/linux/blkdev.h | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index e34718b09550..e41b6a6ef048 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -243,7 +243,7 @@ static unsigned long nvmet_req_nr_zones_from_slba(struct nvmet_req *req)
unsigned int sect = nvmet_lba_to_sect(req->ns, req->cmd->zmr.slba);
return blkdev_nr_zones(req->ns->bdev->bd_disk) -
- (sect >> ilog2(bdev_zone_sectors(req->ns->bdev)));
+ bdev_zone_no(req->ns->bdev, sect);
}
static unsigned long get_nr_zones_from_buf(struct nvmet_req *req, u32 bufsize)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index f5c7a41032ba..ed8742a72dcb 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1382,6 +1382,13 @@ static inline sector_t bdev_zone_sectors(struct block_device *bdev)
return 0;
}
+static inline unsigned int bdev_zone_no(struct block_device *bdev, sector_t sec)
+{
+ struct request_queue *q = bdev_get_queue(bdev);
+
+ return blk_queue_zone_no(q, sec);
+}
+
static inline unsigned int bdev_max_open_zones(struct block_device *bdev)
{
struct request_queue *q = bdev_get_queue(bdev);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v5 5/7] null_blk: allow non power of 2 zoned devices
2022-05-23 16:15 ` [PATCH v5 0/7] support non power of 2 zoned devices Pankaj Raghav
` (3 preceding siblings ...)
2022-05-23 16:15 ` [PATCH v5 4/7] nvmet: Allow ZNS target to support non-power_of_2 zone sizes Pankaj Raghav
@ 2022-05-23 16:15 ` Pankaj Raghav
2022-05-24 2:40 ` kernel test robot
2022-05-24 19:07 ` kernel test robot
2022-05-23 16:16 ` [PATCH v5 6/7] null_blk: use zone_size_sects_shift for " Pankaj Raghav
2022-05-23 16:16 ` [PATCH v5 7/7] dm-zoned: ensure only power of 2 zone sizes are allowed Pankaj Raghav
6 siblings, 2 replies; 19+ messages in thread
From: Pankaj Raghav @ 2022-05-23 16:15 UTC (permalink / raw)
To: axboe, hch, snitzer, damien.lemoal, hare, Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Pankaj Raghav, Luis Chamberlain
Convert the power of 2 based calculation with zone size to be generic in
null_zone_no with optimization for power of 2 based zone sizes.
The nr_zones calculation in null_init_zoned_dev has been replaced with a
division without special handling for power of 2 based zone sizes as
this function is called only during the initialization and will not
invoked in the hot path.
Performance Measurement:
Device:
zone size = 128M, blocksize=4k
FIO cmd:
fio --name=zbc --filename=/dev/nullb0 --direct=1 --zonemode=zbd --size=23G
--io_size=<iosize> --ioengine=io_uring --iodepth=<iod> --rw=<mode> --bs=4k
--loops=4
The following results are an average of 4 runs on AMD Ryzen 5 5600X with
32GB of RAM:
Sequential Write:
x-----------------x---------------------------------x---------------------------------x
| IOdepth | 8 | 16 |
x-----------------x---------------------------------x---------------------------------x
| | KIOPS |BW(MiB/s) | Lat(usec) | KIOPS |BW(MiB/s) | Lat(usec) |
x-----------------x---------------------------------x---------------------------------x
| Without patch | 578 | 2257 | 12.80 | 576 | 2248 | 25.78 |
x-----------------x---------------------------------x---------------------------------x
| With patch | 581 | 2268 | 12.74 | 576 | 2248 | 25.85 |
x-----------------x---------------------------------x---------------------------------x
Sequential read:
x-----------------x---------------------------------x---------------------------------x
| IOdepth | 8 | 16 |
x-----------------x---------------------------------x---------------------------------x
| | KIOPS |BW(MiB/s) | Lat(usec) | KIOPS |BW(MiB/s) | Lat(usec) |
x-----------------x---------------------------------x---------------------------------x
| Without patch | 667 | 2605 | 11.79 | 675 | 2637 | 23.49 |
x-----------------x---------------------------------x---------------------------------x
| With patch | 667 | 2605 | 11.79 | 675 | 2638 | 23.48 |
x-----------------x---------------------------------x---------------------------------x
Random read:
x-----------------x---------------------------------x---------------------------------x
| IOdepth | 8 | 16 |
x-----------------x---------------------------------x---------------------------------x
| | KIOPS |BW(MiB/s) | Lat(usec) | KIOPS |BW(MiB/s) | Lat(usec) |
x-----------------x---------------------------------x---------------------------------x
| Without patch | 522 | 2038 | 15.05 | 514 | 2006 | 30.87 |
x-----------------x---------------------------------x---------------------------------x
| With patch | 522 | 2039 | 15.04 | 523 | 2042 | 30.33 |
x-----------------x---------------------------------x---------------------------------x
Minor variations are noticed in Sequential write with io depth 8 and
in random read with io depth 16. But overall no noticeable differences
were noticed
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
drivers/block/null_blk/main.c | 5 ++---
drivers/block/null_blk/zoned.c | 14 +++++++-------
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index c441a4972064..1dec51d69674 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1930,9 +1930,8 @@ static int null_validate_conf(struct nullb_device *dev)
if (dev->queue_mode == NULL_Q_BIO)
dev->mbps = 0;
- if (dev->zoned &&
- (!dev->zone_size || !is_power_of_2(dev->zone_size))) {
- pr_err("zone_size must be power-of-two\n");
+ if (dev->zoned && !dev->zone_size) {
+ pr_err("Invalid zero zone size\n");
return -EINVAL;
}
diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index dae54dd1aeac..00c34e65ef0a 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c
@@ -13,7 +13,10 @@ static inline sector_t mb_to_sects(unsigned long mb)
static inline unsigned int null_zone_no(struct nullb_device *dev, sector_t sect)
{
- return sect >> ilog2(dev->zone_size_sects);
+ if (is_power_of_2(dev->zone_size_sects))
+ return sect >> ilog2(dev->zone_size_sects);
+
+ return div64_u64(sect, dev->zone_size_sects);
}
static inline void null_lock_zone_res(struct nullb_device *dev)
@@ -62,10 +65,6 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)
sector_t sector = 0;
unsigned int i;
- if (!is_power_of_2(dev->zone_size)) {
- pr_err("zone_size must be power-of-two\n");
- return -EINVAL;
- }
if (dev->zone_size > dev->size) {
pr_err("Zone size larger than device capacity\n");
return -EINVAL;
@@ -83,8 +82,9 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)
zone_capacity_sects = mb_to_sects(dev->zone_capacity);
dev_capacity_sects = mb_to_sects(dev->size);
dev->zone_size_sects = mb_to_sects(dev->zone_size);
- dev->nr_zones = round_up(dev_capacity_sects, dev->zone_size_sects)
- >> ilog2(dev->zone_size_sects);
+ dev->nr_zones =
+ div64_u64(roundup(dev_capacity_sects, dev->zone_size_sects),
+ dev->zone_size_sects);
dev->zones = kvmalloc_array(dev->nr_zones, sizeof(struct nullb_zone),
GFP_KERNEL | __GFP_ZERO);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v5 5/7] null_blk: allow non power of 2 zoned devices
2022-05-23 16:15 ` [PATCH v5 5/7] null_blk: allow non power of 2 zoned devices Pankaj Raghav
@ 2022-05-24 2:40 ` kernel test robot
2022-05-24 7:30 ` Pankaj Raghav
2022-05-24 19:07 ` kernel test robot
1 sibling, 1 reply; 19+ messages in thread
From: kernel test robot @ 2022-05-24 2:40 UTC (permalink / raw)
To: Pankaj Raghav, axboe, hch, snitzer, damien.lemoal, hare,
Johannes.Thumshirn
Cc: llvm, kbuild-all, linux-nvme, dm-devel, dsterba, jiangbo.365,
linux-kernel, gost.dev, linux-block, jaegeuk, Pankaj Raghav,
Luis Chamberlain
Hi Pankaj,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on device-mapper-dm/for-next linus/master hch-configfs/for-next v5.18 next-20220523]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Pankaj-Raghav/block-make-blkdev_nr_zones-and-blk_queue_zone_no-generic-for-npo2-zsze/20220524-011616
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: hexagon-randconfig-r045-20220523 (https://download.01.org/0day-ci/archive/20220524/202205241034.izkLMTcH-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 10c9ecce9f6096e18222a331c5e7d085bd813f75)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/3d3c81da0adbd40eb0d2125327b7e227582b2a37
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Pankaj-Raghav/block-make-blkdev_nr_zones-and-blk_queue_zone_no-generic-for-npo2-zsze/20220524-011616
git checkout 3d3c81da0adbd40eb0d2125327b7e227582b2a37
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "__hexagon_umoddi3" [drivers/block/null_blk/null_blk.ko] undefined!
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v5 5/7] null_blk: allow non power of 2 zoned devices
2022-05-24 2:40 ` kernel test robot
@ 2022-05-24 7:30 ` Pankaj Raghav
2022-05-24 15:22 ` Nathan Chancellor
0 siblings, 1 reply; 19+ messages in thread
From: Pankaj Raghav @ 2022-05-24 7:30 UTC (permalink / raw)
To: kernel test robot, axboe, hch, snitzer, damien.lemoal, hare,
Johannes.Thumshirn
Cc: llvm, kbuild-all, linux-nvme, dm-devel, dsterba, jiangbo.365,
linux-kernel, gost.dev, linux-block, jaegeuk, Luis Chamberlain
On 5/24/22 04:40, kernel test robot wrote:
> Hi Pankaj,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on axboe-block/for-next]
> [also build test ERROR on device-mapper-dm/for-next linus/master hch-configfs/for-next v5.18 next-20220523]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://protect2.fireeye.com/v1/url?k=8acc50e6-d557681b-8acddba9-000babff317b-0ca211d60a57a8c6&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fgithub.com%2Fintel-lab-lkp%2Flinux%2Fcommits%2FPankaj-Raghav%2Fblock-make-blkdev_nr_zones-and-blk_queue_zone_no-generic-for-npo2-zsze%2F20220524-011616
> base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
> config: hexagon-randconfig-r045-20220523 (https://protect2.fireeye.com/v1/url?k=0dc32741-52581fbc-0dc2ac0e-000babff317b-fb6f258f0c80ebb9&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fdownload.01.org%2F0day-ci%2Farchive%2F20220524%2F202205241034.izkLMTcH-lkp%40intel.com%2Fconfig)
> compiler: clang version 15.0.0 (https://protect2.fireeye.com/v1/url?k=afbb4f4f-f02077b2-afbac400-000babff317b-a4413fa4cefe1877&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project 10c9ecce9f6096e18222a331c5e7d085bd813f75)
> reproduce (this is a W=1 build):
> wget https://protect2.fireeye.com/v1/url?k=718573dc-2e1e4b21-7184f893-000babff317b-b6c06a1c569b0d77&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fraw.githubusercontent.com%2Fintel%2Flkp-tests%2Fmaster%2Fsbin%2Fmake.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://protect2.fireeye.com/v1/url?k=247d070f-7be63ff2-247c8c40-000babff317b-6fd7f2f6a5a5bc60&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fgithub.com%2Fintel-lab-lkp%2Flinux%2Fcommit%2F3d3c81da0adbd40eb0d2125327b7e227582b2a37
> git remote add linux-review https://protect2.fireeye.com/v1/url?k=081be8db-5780d026-081a6394-000babff317b-d12fdca0fccd0493&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fgithub.com%2Fintel-lab-lkp%2Flinux
> git fetch --no-tags linux-review Pankaj-Raghav/block-make-blkdev_nr_zones-and-blk_queue_zone_no-generic-for-npo2-zsze/20220524-011616
> git checkout 3d3c81da0adbd40eb0d2125327b7e227582b2a37
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>, old ones prefixed by <<):
>
>>> ERROR: modpost: "__hexagon_umoddi3" [drivers/block/null_blk/null_blk.ko] undefined!
I am able to apply my patches cleanly against next-20220523, build it
without any errors with GCC and boot into it in my x86_64 machine. Not
sure why this error is popping up.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 5/7] null_blk: allow non power of 2 zoned devices
2022-05-24 7:30 ` Pankaj Raghav
@ 2022-05-24 15:22 ` Nathan Chancellor
2022-05-24 15:32 ` Pankaj Raghav
0 siblings, 1 reply; 19+ messages in thread
From: Nathan Chancellor @ 2022-05-24 15:22 UTC (permalink / raw)
To: Pankaj Raghav
Cc: kernel test robot, axboe, hch, snitzer, damien.lemoal, hare,
Johannes.Thumshirn, llvm, kbuild-all, linux-nvme, dm-devel,
dsterba, jiangbo.365, linux-kernel, gost.dev, linux-block,
jaegeuk, Luis Chamberlain
On Tue, May 24, 2022 at 09:30:41AM +0200, Pankaj Raghav wrote:
> On 5/24/22 04:40, kernel test robot wrote:
> > Hi Pankaj,
> >
> > Thank you for the patch! Yet something to improve:
> >
> > [auto build test ERROR on axboe-block/for-next]
> > [also build test ERROR on device-mapper-dm/for-next linus/master hch-configfs/for-next v5.18 next-20220523]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch]
> >
> > url: https://protect2.fireeye.com/v1/url?k=8acc50e6-d557681b-8acddba9-000babff317b-0ca211d60a57a8c6&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fgithub.com%2Fintel-lab-lkp%2Flinux%2Fcommits%2FPankaj-Raghav%2Fblock-make-blkdev_nr_zones-and-blk_queue_zone_no-generic-for-npo2-zsze%2F20220524-011616
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
> > config: hexagon-randconfig-r045-20220523 (https://protect2.fireeye.com/v1/url?k=0dc32741-52581fbc-0dc2ac0e-000babff317b-fb6f258f0c80ebb9&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fdownload.01.org%2F0day-ci%2Farchive%2F20220524%2F202205241034.izkLMTcH-lkp%40intel.com%2Fconfig)
> > compiler: clang version 15.0.0 (https://protect2.fireeye.com/v1/url?k=afbb4f4f-f02077b2-afbac400-000babff317b-a4413fa4cefe1877&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project 10c9ecce9f6096e18222a331c5e7d085bd813f75)
> > reproduce (this is a W=1 build):
> > wget https://protect2.fireeye.com/v1/url?k=718573dc-2e1e4b21-7184f893-000babff317b-b6c06a1c569b0d77&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fraw.githubusercontent.com%2Fintel%2Flkp-tests%2Fmaster%2Fsbin%2Fmake.cross -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # https://protect2.fireeye.com/v1/url?k=247d070f-7be63ff2-247c8c40-000babff317b-6fd7f2f6a5a5bc60&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fgithub.com%2Fintel-lab-lkp%2Flinux%2Fcommit%2F3d3c81da0adbd40eb0d2125327b7e227582b2a37
> > git remote add linux-review https://protect2.fireeye.com/v1/url?k=081be8db-5780d026-081a6394-000babff317b-d12fdca0fccd0493&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fgithub.com%2Fintel-lab-lkp%2Flinux
> > git fetch --no-tags linux-review Pankaj-Raghav/block-make-blkdev_nr_zones-and-blk_queue_zone_no-generic-for-npo2-zsze/20220524-011616
> > git checkout 3d3c81da0adbd40eb0d2125327b7e227582b2a37
> > # save the config file
> > mkdir build_dir && cp config build_dir/.config
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
> >
> > If you fix the issue, kindly add following tag where applicable
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All errors (new ones prefixed by >>, old ones prefixed by <<):
> >
> >>> ERROR: modpost: "__hexagon_umoddi3" [drivers/block/null_blk/null_blk.ko] undefined!
>
> I am able to apply my patches cleanly against next-20220523, build it
> without any errors with GCC and boot into it in my x86_64 machine. Not
> sure why this error is popping up.
Do a 32-bit build and you'll see it. With GCC 12.1.0, ARCH=i386
defconfig + CONFIG_BLK_DEV_NULL_BLK=y + CONFIG_BLK_DEV_ZONED=y
reproduces it for me:
$ make -skj"$(nproc)" ARCH=i386 defconfig menuconfig all
ld: drivers/block/null_blk/zoned.o: in function `null_init_zoned_dev':
zoned.c:(.text+0x112e): undefined reference to `__umoddi3'
...
roundup() does a plain division with a 64-bit dividend, which will cause
issues with 32-bit architectures. I suspect that you should really be
using DIV_ROUND_UP_SECTOR_T() for the nr_zones calculation or maybe one
of the other rounding macros in include/linux/math.h but I am not sure.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 5/7] null_blk: allow non power of 2 zoned devices
2022-05-24 15:22 ` Nathan Chancellor
@ 2022-05-24 15:32 ` Pankaj Raghav
0 siblings, 0 replies; 19+ messages in thread
From: Pankaj Raghav @ 2022-05-24 15:32 UTC (permalink / raw)
To: Nathan Chancellor
Cc: kernel test robot, axboe, hch, snitzer, damien.lemoal, hare,
Johannes.Thumshirn, llvm, kbuild-all, linux-nvme, dm-devel,
dsterba, jiangbo.365, linux-kernel, gost.dev, linux-block,
jaegeuk, Luis Chamberlain
On 5/24/22 17:22, Nathan Chancellor wrote:
>>> git remote add linux-review https://protect2.fireeye.com/v1/url?k=081be8db-5780d026-081a6394-000babff317b-d12fdca0fccd0493&q=1&e=5ef82219-ef70-445f-a7d0-0ae0a30be69f&u=https%3A%2F%2Fgithub.com%2Fintel-lab-lkp%2Flinux
>>> git fetch --no-tags linux-review Pankaj-Raghav/block-make-blkdev_nr_zones-and-blk_queue_zone_no-generic-for-npo2-zsze/20220524-011616
>>> git checkout 3d3c81da0adbd40eb0d2125327b7e227582b2a37
>>> # save the config file
>>> mkdir build_dir && cp config build_dir/.config
>>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
>>>
>>> If you fix the issue, kindly add following tag where applicable
>>> Reported-by: kernel test robot <lkp@intel.com>
>>>
>>> All errors (new ones prefixed by >>, old ones prefixed by <<):
>>>
>>>>> ERROR: modpost: "__hexagon_umoddi3" [drivers/block/null_blk/null_blk.ko] undefined!
>>
>> I am able to apply my patches cleanly against next-20220523, build it
>> without any errors with GCC and boot into it in my x86_64 machine. Not
>> sure why this error is popping up.
>
> Do a 32-bit build and you'll see it. With GCC 12.1.0, ARCH=i386
> defconfig + CONFIG_BLK_DEV_NULL_BLK=y + CONFIG_BLK_DEV_ZONED=y
> reproduces it for me:
>
> $ make -skj"$(nproc)" ARCH=i386 defconfig menuconfig all
> ld: drivers/block/null_blk/zoned.o: in function `null_init_zoned_dev':
> zoned.c:(.text+0x112e): undefined reference to `__umoddi3'
> ...
>
Ah.. I didn't see anything about 32 bit ARCH in the email so it didn't
strike to me that was the problem. It said ARCH=hexagon and I had no
idea what it was. Thanks for reproducing it.
> roundup() does a plain division with a 64-bit dividend, which will cause
> issues with 32-bit architectures. I suspect that you should really be
> using DIV_ROUND_UP_SECTOR_T() for the nr_zones calculation or maybe one
> of the other rounding macros in include/linux/math.h but I am not sure.
>
That must be it. I will fix it. Thanks Nathan.
> Cheers,
> Nathan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 5/7] null_blk: allow non power of 2 zoned devices
2022-05-23 16:15 ` [PATCH v5 5/7] null_blk: allow non power of 2 zoned devices Pankaj Raghav
2022-05-24 2:40 ` kernel test robot
@ 2022-05-24 19:07 ` kernel test robot
1 sibling, 0 replies; 19+ messages in thread
From: kernel test robot @ 2022-05-24 19:07 UTC (permalink / raw)
To: Pankaj Raghav, axboe, hch, snitzer, damien.lemoal, hare,
Johannes.Thumshirn
Cc: llvm, kbuild-all, linux-nvme, dm-devel, dsterba, jiangbo.365,
linux-kernel, gost.dev, linux-block, jaegeuk, Pankaj Raghav,
Luis Chamberlain
Hi Pankaj,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on device-mapper-dm/for-next linus/master hch-configfs/for-next v5.18 next-20220524]
[cannot apply to linux-nvme/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Pankaj-Raghav/block-make-blkdev_nr_zones-and-blk_queue_zone_no-generic-for-npo2-zsze/20220524-011616
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: i386-randconfig-a015 (https://download.01.org/0day-ci/archive/20220525/202205250258.tP8p4wdJ-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 10c9ecce9f6096e18222a331c5e7d085bd813f75)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/3d3c81da0adbd40eb0d2125327b7e227582b2a37
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Pankaj-Raghav/block-make-blkdev_nr_zones-and-blk_queue_zone_no-generic-for-npo2-zsze/20220524-011616
git checkout 3d3c81da0adbd40eb0d2125327b7e227582b2a37
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: __umoddi3
>>> referenced by zoned.c:89 (drivers/block/null_blk/zoned.c:89)
>>> block/null_blk/zoned.o:(null_init_zoned_dev) in archive drivers/built-in.a
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5 6/7] null_blk: use zone_size_sects_shift for power of 2 zoned devices
2022-05-23 16:15 ` [PATCH v5 0/7] support non power of 2 zoned devices Pankaj Raghav
` (4 preceding siblings ...)
2022-05-23 16:15 ` [PATCH v5 5/7] null_blk: allow non power of 2 zoned devices Pankaj Raghav
@ 2022-05-23 16:16 ` Pankaj Raghav
2022-05-23 18:11 ` Luis Chamberlain
2022-05-24 5:22 ` Hannes Reinecke
2022-05-23 16:16 ` [PATCH v5 7/7] dm-zoned: ensure only power of 2 zone sizes are allowed Pankaj Raghav
6 siblings, 2 replies; 19+ messages in thread
From: Pankaj Raghav @ 2022-05-23 16:16 UTC (permalink / raw)
To: axboe, hch, snitzer, damien.lemoal, hare, Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Pankaj Raghav, Damien Le Moal
Instead of doing is_power_of_2 and ilog2 operation for every IO, cache
the zone_size_sects_shift variable and use it for power of 2 zoned
devices.
This variable will be set to zero for non power of 2 zoned devices.
Suggested-by: Damien Le Moal <damien.lemoal@wdc.com>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
drivers/block/null_blk/null_blk.h | 6 ++++++
drivers/block/null_blk/zoned.c | 10 ++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
index 78eb56b0ca55..3d6e41a9491f 100644
--- a/drivers/block/null_blk/null_blk.h
+++ b/drivers/block/null_blk/null_blk.h
@@ -74,6 +74,12 @@ struct nullb_device {
unsigned int imp_close_zone_no;
struct nullb_zone *zones;
sector_t zone_size_sects;
+ /*
+ * zone_size_sects_shift is only useful when the zone size is
+ * power of 2. This variable is set to zero when zone size is non
+ * power of 2.
+ */
+ unsigned int zone_size_sects_shift;
bool need_zone_res_mgmt;
spinlock_t zone_res_lock;
diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
index 00c34e65ef0a..806bef98ac83 100644
--- a/drivers/block/null_blk/zoned.c
+++ b/drivers/block/null_blk/zoned.c
@@ -13,8 +13,8 @@ static inline sector_t mb_to_sects(unsigned long mb)
static inline unsigned int null_zone_no(struct nullb_device *dev, sector_t sect)
{
- if (is_power_of_2(dev->zone_size_sects))
- return sect >> ilog2(dev->zone_size_sects);
+ if (dev->zone_size_sects_shift)
+ return sect >> dev->zone_size_sects_shift;
return div64_u64(sect, dev->zone_size_sects);
}
@@ -82,6 +82,12 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)
zone_capacity_sects = mb_to_sects(dev->zone_capacity);
dev_capacity_sects = mb_to_sects(dev->size);
dev->zone_size_sects = mb_to_sects(dev->zone_size);
+
+ if (is_power_of_2(dev->zone_size_sects))
+ dev->zone_size_sects_shift = ilog2(dev->zone_size_sects);
+ else
+ dev->zone_size_sects_shift = 0;
+
dev->nr_zones =
div64_u64(roundup(dev_capacity_sects, dev->zone_size_sects),
dev->zone_size_sects);
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v5 6/7] null_blk: use zone_size_sects_shift for power of 2 zoned devices
2022-05-23 16:16 ` [PATCH v5 6/7] null_blk: use zone_size_sects_shift for " Pankaj Raghav
@ 2022-05-23 18:11 ` Luis Chamberlain
2022-05-24 5:22 ` Hannes Reinecke
1 sibling, 0 replies; 19+ messages in thread
From: Luis Chamberlain @ 2022-05-23 18:11 UTC (permalink / raw)
To: Pankaj Raghav
Cc: axboe, hch, snitzer, damien.lemoal, hare, Johannes.Thumshirn,
linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Damien Le Moal
On Mon, May 23, 2022 at 06:16:00PM +0200, Pankaj Raghav wrote:
> Instead of doing is_power_of_2 and ilog2 operation for every IO, cache
> the zone_size_sects_shift variable and use it for power of 2 zoned
> devices.
>
> This variable will be set to zero for non power of 2 zoned devices.
>
> Suggested-by: Damien Le Moal <damien.lemoal@wdc.com>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Luis
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 6/7] null_blk: use zone_size_sects_shift for power of 2 zoned devices
2022-05-23 16:16 ` [PATCH v5 6/7] null_blk: use zone_size_sects_shift for " Pankaj Raghav
2022-05-23 18:11 ` Luis Chamberlain
@ 2022-05-24 5:22 ` Hannes Reinecke
1 sibling, 0 replies; 19+ messages in thread
From: Hannes Reinecke @ 2022-05-24 5:22 UTC (permalink / raw)
To: Pankaj Raghav, axboe, hch, snitzer, damien.lemoal,
Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Damien Le Moal
On 5/23/22 18:16, Pankaj Raghav wrote:
> Instead of doing is_power_of_2 and ilog2 operation for every IO, cache
> the zone_size_sects_shift variable and use it for power of 2 zoned
> devices.
>
> This variable will be set to zero for non power of 2 zoned devices.
>
> Suggested-by: Damien Le Moal <damien.lemoal@wdc.com>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
> drivers/block/null_blk/null_blk.h | 6 ++++++
> drivers/block/null_blk/zoned.c | 10 ++++++++--
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
> index 78eb56b0ca55..3d6e41a9491f 100644
> --- a/drivers/block/null_blk/null_blk.h
> +++ b/drivers/block/null_blk/null_blk.h
> @@ -74,6 +74,12 @@ struct nullb_device {
> unsigned int imp_close_zone_no;
> struct nullb_zone *zones;
> sector_t zone_size_sects;
> + /*
> + * zone_size_sects_shift is only useful when the zone size is
> + * power of 2. This variable is set to zero when zone size is non
> + * power of 2.
> + */
> + unsigned int zone_size_sects_shift;
> bool need_zone_res_mgmt;
> spinlock_t zone_res_lock;
>
> diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
> index 00c34e65ef0a..806bef98ac83 100644
> --- a/drivers/block/null_blk/zoned.c
> +++ b/drivers/block/null_blk/zoned.c
> @@ -13,8 +13,8 @@ static inline sector_t mb_to_sects(unsigned long mb)
>
> static inline unsigned int null_zone_no(struct nullb_device *dev, sector_t sect)
> {
> - if (is_power_of_2(dev->zone_size_sects))
> - return sect >> ilog2(dev->zone_size_sects);
> + if (dev->zone_size_sects_shift)
> + return sect >> dev->zone_size_sects_shift;
>
> return div64_u64(sect, dev->zone_size_sects);
> }
> @@ -82,6 +82,12 @@ int null_init_zoned_dev(struct nullb_device *dev, struct request_queue *q)
> zone_capacity_sects = mb_to_sects(dev->zone_capacity);
> dev_capacity_sects = mb_to_sects(dev->size);
> dev->zone_size_sects = mb_to_sects(dev->zone_size);
> +
> + if (is_power_of_2(dev->zone_size_sects))
> + dev->zone_size_sects_shift = ilog2(dev->zone_size_sects);
> + else
> + dev->zone_size_sects_shift = 0;
> +
> dev->nr_zones =
> div64_u64(roundup(dev_capacity_sects, dev->zone_size_sects),
> dev->zone_size_sects);
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5 7/7] dm-zoned: ensure only power of 2 zone sizes are allowed
2022-05-23 16:15 ` [PATCH v5 0/7] support non power of 2 zoned devices Pankaj Raghav
` (5 preceding siblings ...)
2022-05-23 16:16 ` [PATCH v5 6/7] null_blk: use zone_size_sects_shift for " Pankaj Raghav
@ 2022-05-23 16:16 ` Pankaj Raghav
6 siblings, 0 replies; 19+ messages in thread
From: Pankaj Raghav @ 2022-05-23 16:16 UTC (permalink / raw)
To: axboe, hch, snitzer, damien.lemoal, hare, Johannes.Thumshirn
Cc: linux-nvme, dm-devel, dsterba, jiangbo.365, linux-kernel,
gost.dev, linux-block, jaegeuk, Luis Chamberlain, Pankaj Raghav
From: Luis Chamberlain <mcgrof@kernel.org>
Today dm-zoned relies on the assumption that you have a zone size
with a power of 2. Even though the block layer today enforces this
requirement, these devices do exist and so provide a stop-gap measure
to ensure these devices cannot be used by mistake
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
drivers/md/dm-zone.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/md/dm-zone.c b/drivers/md/dm-zone.c
index 57daa86c19cf..f87a1eda8252 100644
--- a/drivers/md/dm-zone.c
+++ b/drivers/md/dm-zone.c
@@ -231,6 +231,18 @@ static int dm_revalidate_zones(struct mapped_device *md, struct dm_table *t)
struct request_queue *q = md->queue;
unsigned int noio_flag;
int ret;
+ struct block_device *bdev = md->disk->part0;
+ sector_t zone_sectors;
+ char bname[BDEVNAME_SIZE];
+
+ zone_sectors = bdev_zone_sectors(bdev);
+
+ if (!is_power_of_2(zone_sectors)) {
+ DMWARN("%s: %s only power of two zone size supported",
+ dm_device_name(md),
+ bdevname(bdev, bname));
+ return -EINVAL;
+ }
/*
* Check if something changed. If yes, cleanup the current resources
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread