| Message ID | 20220427160255.300418-2-p.raghav@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/16] block: make blkdev_nr_zones and blk_queue_zone_no generic for npo2 zsze | expand |
On Wed, Apr 27, 2022 at 06:02:40PM +0200, Pankaj Raghav wrote: > 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> > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> > --- > block/blk-zoned.c | 8 +++++++- > include/linux/blkdev.h | 8 +++++++- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/block/blk-zoned.c b/block/blk-zoned.c > index 38cd840d8838..1dff4a8bd51d 100644 > --- a/block/blk-zoned.c > +++ b/block/blk-zoned.c > @@ -117,10 +117,16 @@ EXPORT_SYMBOL_GPL(__blk_req_zone_write_unlock); > 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 > Looks good. Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
On 4/27/22 09:02, Pankaj Raghav wrote: > 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> > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> > --- > block/blk-zoned.c | 8 +++++++- > include/linux/blkdev.h | 8 +++++++- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/block/blk-zoned.c b/block/blk-zoned.c > index 38cd840d8838..1dff4a8bd51d 100644 > --- a/block/blk-zoned.c > +++ b/block/blk-zoned.c > @@ -117,10 +117,16 @@ EXPORT_SYMBOL_GPL(__blk_req_zone_write_unlock); > 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); Does anyone need support for more than 4 billion sectors per zone? If not, do_div() should be sufficient. > 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); > } Same comment here. Thanks, Bart.
On 2022/05/04 1:37, Bart Van Assche wrote: > On 4/27/22 09:02, Pankaj Raghav wrote: >> 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> >> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> >> --- >> block/blk-zoned.c | 8 +++++++- >> include/linux/blkdev.h | 8 +++++++- >> 2 files changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/block/blk-zoned.c b/block/blk-zoned.c >> index 38cd840d8838..1dff4a8bd51d 100644 >> --- a/block/blk-zoned.c >> +++ b/block/blk-zoned.c >> @@ -117,10 +117,16 @@ EXPORT_SYMBOL_GPL(__blk_req_zone_write_unlock); >> 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); > > Does anyone need support for more than 4 billion sectors per zone? If > not, do_div() should be sufficient. > >> 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); >> } > > Same comment here. sector_t is 64-bits even on 32-bits arch, no ? so div64_u64 is needed here I think, which will be a simple regular division for 64-bit arch. > > Thanks, > > Bart.
On 2022-05-03 18:37, Bart Van Assche wrote: >> 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); > > Does anyone need support for more than 4 billion sectors per zone? If > not, do_div() should be sufficient. > You are absolutely right but blk_queue_zone_sectors explicitly changed the return type to uint32_t to sector_t in this commit: '113ab72 block: Fix potential overflow in blk_report_zones()'. I initially did have a do_div but later changed to div64_u64 to avoid any implicit down casting even though zone_sectors will be always limited to an unsigned int. >> 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); >> } > > Same comment here. > > Thanks, > > Bart.
On 4/27/22 09:02, Pankaj Raghav wrote: > 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> > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> > --- > block/blk-zoned.c | 8 +++++++- > include/linux/blkdev.h | 8 +++++++- > 2 files changed, 14 insertions(+), 2 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/block/blk-zoned.c b/block/blk-zoned.c index 38cd840d8838..1dff4a8bd51d 100644 --- a/block/blk-zoned.c +++ b/block/blk-zoned.c @@ -117,10 +117,16 @@ EXPORT_SYMBOL_GPL(__blk_req_zone_write_unlock); 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,