| Message ID | 20230418224002.1195163-4-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mq-deadline: Improve support for zoned block devices | expand |
On Tue, Apr 18, 2023 at 03:39:54PM -0700, Bart Van Assche wrote: > +/** > + * blk_rq_is_seq_zoned_write() - Whether @rq is a zoned write for which the order matters. Maybe: * blk_rq_is_seq_zoned_write() - check if @rq needs zoned write serialization > +bool blk_rq_is_seq_zoned_write(struct request *rq) > +{ > + switch (req_op(rq)) { > + case REQ_OP_WRITE: > + case REQ_OP_WRITE_ZEROES: > + return blk_rq_zone_is_seq(rq); > + case REQ_OP_ZONE_APPEND: > + default: The REQ_OP_ZONE_APPEND case here is superflous. Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
On 4/18/23 21:50, Christoph Hellwig wrote: > On Tue, Apr 18, 2023 at 03:39:54PM -0700, Bart Van Assche wrote: >> +/** >> + * blk_rq_is_seq_zoned_write() - Whether @rq is a zoned write for which the order matters. > > Maybe: > > * blk_rq_is_seq_zoned_write() - check if @rq needs zoned write serialization That looks better to me :-) >> +bool blk_rq_is_seq_zoned_write(struct request *rq) >> +{ >> + switch (req_op(rq)) { >> + case REQ_OP_WRITE: >> + case REQ_OP_WRITE_ZEROES: >> + return blk_rq_zone_is_seq(rq); >> + case REQ_OP_ZONE_APPEND: >> + default: > > The REQ_OP_ZONE_APPEND case here is superflous. Agreed, but I'd like to keep it since last time I posted this patch I was asked whether I had perhaps overlooked the REQ_OP_ZONE_APPEND case. I added "case REQ_OP_ZONE_APPEND:" to prevent such questions. Are you OK with keeping "case REQ_OP_ZONE_APPEND:" or do you perhaps prefer that I remove it? Bart.
On 4/20/23 06:12, Bart Van Assche wrote: > On 4/18/23 21:50, Christoph Hellwig wrote: >> On Tue, Apr 18, 2023 at 03:39:54PM -0700, Bart Van Assche wrote: >>> +/** >>> + * blk_rq_is_seq_zoned_write() - Whether @rq is a zoned write for which the order matters. >> >> Maybe: >> >> * blk_rq_is_seq_zoned_write() - check if @rq needs zoned write serialization > > That looks better to me :-) > >>> +bool blk_rq_is_seq_zoned_write(struct request *rq) >>> +{ >>> + switch (req_op(rq)) { >>> + case REQ_OP_WRITE: >>> + case REQ_OP_WRITE_ZEROES: >>> + return blk_rq_zone_is_seq(rq); >>> + case REQ_OP_ZONE_APPEND: >>> + default: >> >> The REQ_OP_ZONE_APPEND case here is superflous. > > Agreed, but I'd like to keep it since last time I posted this patch I > was asked whether I had perhaps overlooked the REQ_OP_ZONE_APPEND case. > I added "case REQ_OP_ZONE_APPEND:" to prevent such questions. Are you OK > with keeping "case REQ_OP_ZONE_APPEND:" or do you perhaps prefer that I > remove it? > Could also have a comment on top of the switch explicitly saying that only WRITE and WRITE ZEROES need to be checked, and that all other commands, including zone append writes, do not have strong reordering requirements. This way, no need to superfluous cases. > Bart. >
On Thu, Apr 20, 2023 at 10:03:16AM +0900, Damien Le Moal wrote: > Could also have a comment on top of the switch explicitly saying that only WRITE > and WRITE ZEROES need to be checked, and that all other commands, including zone > append writes, do not have strong reordering requirements. This way, no need to > superfluous cases. Yes, I think a good comment would be more useful here.
diff --git a/block/blk-zoned.c b/block/blk-zoned.c index c93a26ce4670..9b9cd6adfd1b 100644 --- a/block/blk-zoned.c +++ b/block/blk-zoned.c @@ -52,6 +52,26 @@ const char *blk_zone_cond_str(enum blk_zone_cond zone_cond) } EXPORT_SYMBOL_GPL(blk_zone_cond_str); +/** + * blk_rq_is_seq_zoned_write() - Whether @rq is a zoned write for which the order matters. + * @rq: Request to examine. + * + * In this context sequential zone means either a sequential write required or + * to a sequential write preferred zone. + */ +bool blk_rq_is_seq_zoned_write(struct request *rq) +{ + switch (req_op(rq)) { + case REQ_OP_WRITE: + case REQ_OP_WRITE_ZEROES: + return blk_rq_zone_is_seq(rq); + case REQ_OP_ZONE_APPEND: + default: + return false; + } +} +EXPORT_SYMBOL_GPL(blk_rq_is_seq_zoned_write); + /* * Return true if a request is a write requests that needs zone write locking. */ @@ -60,10 +80,7 @@ bool blk_req_needs_zone_write_lock(struct request *rq) if (!rq->q->disk->seq_zones_wlock) return false; - if (queue_op_is_zoned_write(rq->q, req_op(rq))) - return blk_rq_zone_is_seq(rq); - - return false; + return blk_rq_is_seq_zoned_write(rq); } EXPORT_SYMBOL_GPL(blk_req_needs_zone_write_lock); diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index 06caacd77ed6..e498b85bc470 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -1164,6 +1164,7 @@ static inline unsigned int blk_rq_zone_is_seq(struct request *rq) return disk_zone_is_seq(rq->q->disk, blk_rq_pos(rq)); } +bool blk_rq_is_seq_zoned_write(struct request *rq); bool blk_req_needs_zone_write_lock(struct request *rq); bool blk_req_zone_write_trylock(struct request *rq); void __blk_req_zone_write_lock(struct request *rq); @@ -1194,6 +1195,11 @@ static inline bool blk_req_can_dispatch_to_zone(struct request *rq) return !blk_req_zone_is_write_locked(rq); } #else /* CONFIG_BLK_DEV_ZONED */ +static inline bool blk_rq_is_seq_zoned_write(struct request *rq) +{ + return false; +} + static inline bool blk_req_needs_zone_write_lock(struct request *rq) { return false;
Introduce the function blk_rq_is_seq_zoned_write(). This function will be used in later patches to preserve the order of zoned writes for which the order needs to be preserved. Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Ming Lei <ming.lei@redhat.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- block/blk-zoned.c | 25 +++++++++++++++++++++---- include/linux/blk-mq.h | 6 ++++++ 2 files changed, 27 insertions(+), 4 deletions(-)