| Message ID | 20230516223323.1383342-4-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mq-deadline: Improve support for zoned block devices | expand |
On 5/17/23 07:33, Bart Van Assche wrote: > Introduce a helper function for checking whether write serialization is > required if the operation will be sent to a zoned device. A second caller > for op_is_zoned_write() will be introduced in the next patch in this > series. > > Suggested-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com> > Cc: Ming Lei <ming.lei@redhat.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > include/linux/blkdev.h | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index db24cf98ccfb..a4f85781060c 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -1281,13 +1281,16 @@ static inline unsigned int bdev_zone_no(struct block_device *bdev, sector_t sec) > return disk_zone_no(bdev->bd_disk, sec); > } > > +/* Whether write serialization is required for @op on zoned devices. */ > +static inline bool op_is_zoned_write(enum req_op op) I do not like the name of this function as it has nothing to do with zoned devices. What about something like op_is_data_write() to clarify what is being tested ? > +{ > + return op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES; > +} > + > static inline bool bdev_op_is_zoned_write(struct block_device *bdev, > enum req_op op) > { > - if (!bdev_is_zoned(bdev)) > - return false; > - > - return op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES; > + return bdev_is_zoned(bdev) && op_is_zoned_write(op); > } Or if you really want to rewrite this, may be something like: static inline bool bdev_op_is_zoned_write(struct block_device *bdev, enum req_op op) { return bdev_is_zoned(bdev) && (op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES); } which is very easy to understand.
On 5/16/23 16:30, Damien Le Moal wrote: > Or if you really want to rewrite this, may be something like: > > static inline bool bdev_op_is_zoned_write(struct block_device *bdev, > enum req_op op) > { > return bdev_is_zoned(bdev) && > (op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES); > } > > which is very easy to understand. The op_is_zoned_write() function was introduced to use it in patch 4/11 of this series. Anyway, I will look into open-coding it. Bart.
On Tue, May 16, 2023 at 05:00:29PM -0700, Bart Van Assche wrote: > On 5/16/23 16:30, Damien Le Moal wrote: >> Or if you really want to rewrite this, may be something like: >> >> static inline bool bdev_op_is_zoned_write(struct block_device *bdev, >> enum req_op op) >> { >> return bdev_is_zoned(bdev) && >> (op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES); >> } >> >> which is very easy to understand. > > The op_is_zoned_write() function was introduced to use it in patch 4/11 of > this series. Anyway, I will look into open-coding it. I think the idea here is that we're testing for an operation that needs zone locking. Maybe that needs to be reflected in the name? op_needs_zone_write_locking() ?
On 2023/05/17 15:45, Christoph Hellwig wrote: > On Tue, May 16, 2023 at 05:00:29PM -0700, Bart Van Assche wrote: >> On 5/16/23 16:30, Damien Le Moal wrote: >>> Or if you really want to rewrite this, may be something like: >>> >>> static inline bool bdev_op_is_zoned_write(struct block_device *bdev, >>> enum req_op op) >>> { >>> return bdev_is_zoned(bdev) && >>> (op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES); >>> } >>> >>> which is very easy to understand. >> >> The op_is_zoned_write() function was introduced to use it in patch 4/11 of >> this series. Anyway, I will look into open-coding it. > > I think the idea here is that we're testing for an operation that needs > zone locking. Maybe that needs to be reflected in the name? > op_needs_zone_write_locking() ? Sounds better !
On 5/17/23 00:33, Bart Van Assche wrote: > Introduce a helper function for checking whether write serialization is > required if the operation will be sent to a zoned device. A second caller > for op_is_zoned_write() will be introduced in the next patch in this > series. > > Suggested-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com> > Cc: Ming Lei <ming.lei@redhat.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > include/linux/blkdev.h | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index db24cf98ccfb..a4f85781060c 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -1281,13 +1281,16 @@ static inline unsigned int bdev_zone_no(struct block_device *bdev, sector_t sec) > return disk_zone_no(bdev->bd_disk, sec); > } > > +/* Whether write serialization is required for @op on zoned devices. */ > +static inline bool op_is_zoned_write(enum req_op op) > +{ > + return op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES; > +} > + > static inline bool bdev_op_is_zoned_write(struct block_device *bdev, > enum req_op op) > { > - if (!bdev_is_zoned(bdev)) > - return false; > - > - return op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES; > + return bdev_is_zoned(bdev) && op_is_zoned_write(op); > } > > static inline sector_t bdev_zone_sectors(struct block_device *bdev) Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index db24cf98ccfb..a4f85781060c 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1281,13 +1281,16 @@ static inline unsigned int bdev_zone_no(struct block_device *bdev, sector_t sec) return disk_zone_no(bdev->bd_disk, sec); } +/* Whether write serialization is required for @op on zoned devices. */ +static inline bool op_is_zoned_write(enum req_op op) +{ + return op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES; +} + static inline bool bdev_op_is_zoned_write(struct block_device *bdev, enum req_op op) { - if (!bdev_is_zoned(bdev)) - return false; - - return op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES; + return bdev_is_zoned(bdev) && op_is_zoned_write(op); } static inline sector_t bdev_zone_sectors(struct block_device *bdev)