| Message ID | 20230106083317.93938-3-p.raghav@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] block: remove superfluous check for request queue in bdev_is_zoned | expand |
On 1/6/23 17:33, Pankaj Raghav wrote: > Instead of open coding to check for zone start, add a helper to improve > readability and store the logic in one place. > > bdev_offset_from_zone_start() will be used later in the series. > > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> > --- > block/blk-core.c | 2 +- > block/blk-zoned.c | 4 ++-- > include/linux/blkdev.h | 18 ++++++++++++++++++ > 3 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index 9321767470dc..0405b3144e7a 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -573,7 +573,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 (bio->bi_iter.bi_sector & (bdev_zone_sectors(bio->bi_bdev) - 1) || > + if (!bdev_is_zone_start(bio->bi_bdev, bio->bi_iter.bi_sector) || > !bio_zone_is_seq(bio)) > return BLK_STS_IOERR; > > diff --git a/block/blk-zoned.c b/block/blk-zoned.c > index db829401d8d0..614b575be899 100644 > --- a/block/blk-zoned.c > +++ b/block/blk-zoned.c > @@ -277,10 +277,10 @@ int blkdev_zone_mgmt(struct block_device *bdev, enum req_op op, > return -EINVAL; > > /* Check alignment (handle eventual smaller last zone) */ > - if (sector & (zone_sectors - 1)) > + if (!bdev_is_zone_start(bdev, sector)) > return -EINVAL; > > - if ((nr_sectors & (zone_sectors - 1)) && end_sector != capacity) > + if (!bdev_is_zone_start(bdev, nr_sectors) && end_sector != capacity) > return -EINVAL; > > /* > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index 0e40b014c40b..04b7cbfd7a2a 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -715,6 +715,7 @@ static inline unsigned int disk_zone_no(struct gendisk *disk, sector_t sector) > { > return 0; > } > + whiteline change > static inline unsigned int bdev_max_open_zones(struct block_device *bdev) > { > return 0; > @@ -1304,6 +1305,23 @@ static inline sector_t bdev_zone_sectors(struct block_device *bdev) > return q->limits.chunk_sectors; > } > > +static inline sector_t bdev_offset_from_zone_start(struct block_device *bdev, > + sector_t sec) > +{ > + if (!bdev_is_zoned(bdev)) this helper should never be called outside of code supporting zones. So why this check ? > + return 0; > + > + return sec & (bdev_zone_sectors(bdev) - 1); > +} > + > +static inline bool bdev_is_zone_start(struct block_device *bdev, sector_t sec) > +{ > + if (!bdev_is_zoned(bdev)) > + return false; Same here. > + > + return bdev_offset_from_zone_start(bdev, sec) == 0; > +} > + > static inline int queue_dma_alignment(const struct request_queue *q) > { > return q ? q->limits.dma_alignment : 511;
On 1/6/23 00:33, Pankaj Raghav wrote: > +static inline sector_t bdev_offset_from_zone_start(struct block_device *bdev, > + sector_t sec) > +{ > + if (!bdev_is_zoned(bdev)) > + return 0; > + > + return sec & (bdev_zone_sectors(bdev) - 1); > +} > + > +static inline bool bdev_is_zone_start(struct block_device *bdev, sector_t sec) > +{ > + if (!bdev_is_zoned(bdev)) > + return false; > + > + return bdev_offset_from_zone_start(bdev, sec) == 0; > +} A nit: 'sector_t sector' is much more common in the block layer than 'sector_t sec'. Please consider changing 'sec' into 'sector'. Thanks, Bart. -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
On 1/6/23 00:33, Pankaj Raghav wrote: > Instead of open coding to check for zone start, add a helper to improve > readability and store the logic in one place. > > bdev_offset_from_zone_start() will be used later in the series. > > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> Looks good. Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> -ck -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
diff --git a/block/blk-core.c b/block/blk-core.c index 9321767470dc..0405b3144e7a 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -573,7 +573,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 (bio->bi_iter.bi_sector & (bdev_zone_sectors(bio->bi_bdev) - 1) || + if (!bdev_is_zone_start(bio->bi_bdev, bio->bi_iter.bi_sector) || !bio_zone_is_seq(bio)) return BLK_STS_IOERR; diff --git a/block/blk-zoned.c b/block/blk-zoned.c index db829401d8d0..614b575be899 100644 --- a/block/blk-zoned.c +++ b/block/blk-zoned.c @@ -277,10 +277,10 @@ int blkdev_zone_mgmt(struct block_device *bdev, enum req_op op, return -EINVAL; /* Check alignment (handle eventual smaller last zone) */ - if (sector & (zone_sectors - 1)) + if (!bdev_is_zone_start(bdev, sector)) return -EINVAL; - if ((nr_sectors & (zone_sectors - 1)) && end_sector != capacity) + if (!bdev_is_zone_start(bdev, nr_sectors) && end_sector != capacity) return -EINVAL; /* diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 0e40b014c40b..04b7cbfd7a2a 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -715,6 +715,7 @@ static inline unsigned int disk_zone_no(struct gendisk *disk, sector_t sector) { return 0; } + static inline unsigned int bdev_max_open_zones(struct block_device *bdev) { return 0; @@ -1304,6 +1305,23 @@ static inline sector_t bdev_zone_sectors(struct block_device *bdev) return q->limits.chunk_sectors; } +static inline sector_t bdev_offset_from_zone_start(struct block_device *bdev, + sector_t sec) +{ + if (!bdev_is_zoned(bdev)) + return 0; + + return sec & (bdev_zone_sectors(bdev) - 1); +} + +static inline bool bdev_is_zone_start(struct block_device *bdev, sector_t sec) +{ + if (!bdev_is_zoned(bdev)) + return false; + + return bdev_offset_from_zone_start(bdev, sec) == 0; +} + static inline int queue_dma_alignment(const struct request_queue *q) { return q ? q->limits.dma_alignment : 511;
Instead of open coding to check for zone start, add a helper to improve readability and store the logic in one place. bdev_offset_from_zone_start() will be used later in the series. Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> --- block/blk-core.c | 2 +- block/blk-zoned.c | 4 ++-- include/linux/blkdev.h | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-)