| Message ID | 20220623232603.3751912-3-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve zoned storage write performance | expand |
On 6/24/22 08:25, Bart Van Assche wrote: > Introduce a function that makes it easy to verify whether a write > request is for a sequential write required or sequential write preferred > zone. Simplify blk_req_needs_zone_write_lock() by using the new function. > > Cc: Damien Le Moal <damien.lemoal@wdc.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> Looks good. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > --- > block/blk-zoned.c | 14 +------------- > include/linux/blk-mq.h | 23 +++++++++++++++++++++++ > 2 files changed, 24 insertions(+), 13 deletions(-) > > diff --git a/block/blk-zoned.c b/block/blk-zoned.c > index 38cd840d8838..cafcbc508dfb 100644 > --- a/block/blk-zoned.c > +++ b/block/blk-zoned.c > @@ -57,19 +57,7 @@ EXPORT_SYMBOL_GPL(blk_zone_cond_str); > */ > bool blk_req_needs_zone_write_lock(struct request *rq) > { > - if (!rq->q->seq_zones_wlock) > - return false; > - > - if (blk_rq_is_passthrough(rq)) > - return false; > - > - switch (req_op(rq)) { > - case REQ_OP_WRITE_ZEROES: > - case REQ_OP_WRITE: > - return blk_rq_zone_is_seq(rq); > - default: > - return false; > - } > + return rq->q->seq_zones_wlock && blk_rq_is_zoned_seq_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 909d47e34b7c..d5930797b84d 100644 > --- a/include/linux/blk-mq.h > +++ b/include/linux/blk-mq.h > @@ -1136,6 +1136,24 @@ static inline unsigned int blk_rq_zone_is_seq(struct request *rq) > return blk_queue_zone_is_seq(rq->q, blk_rq_pos(rq)); > } > > +/** > + * blk_rq_is_zoned_seq_write() - Whether @rq is a write request for a sequential zone. > + * @rq: Request to examine. > + * > + * In this context sequential zone means either a sequential write required or > + * a sequential write preferred zone. > + */ > +static inline bool blk_rq_is_zoned_seq_write(struct request *rq) > +{ > + switch (req_op(rq)) { > + case REQ_OP_WRITE: > + case REQ_OP_WRITE_ZEROES: > + return blk_rq_zone_is_seq(rq); > + default: > + return false; > + } > +} > + > 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); > @@ -1166,6 +1184,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_zoned_seq_write(struct request *rq) > +{ > + return false; > +} > + > static inline bool blk_req_needs_zone_write_lock(struct request *rq) > { > return false;
On Thu, Jun 23, 2022 at 04:25:59PM -0700, Bart Van Assche wrote: > Introduce a function that makes it easy to verify whether a write > request is for a sequential write required or sequential write preferred > zone. Simplify blk_req_needs_zone_write_lock() by using the new function. > > Cc: Damien Le Moal <damien.lemoal@wdc.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > block/blk-zoned.c | 14 +------------- > include/linux/blk-mq.h | 23 +++++++++++++++++++++++ > 2 files changed, 24 insertions(+), 13 deletions(-) > > diff --git a/block/blk-zoned.c b/block/blk-zoned.c > index 38cd840d8838..cafcbc508dfb 100644 > --- a/block/blk-zoned.c > +++ b/block/blk-zoned.c > @@ -57,19 +57,7 @@ EXPORT_SYMBOL_GPL(blk_zone_cond_str); > */ > bool blk_req_needs_zone_write_lock(struct request *rq) > { > - if (!rq->q->seq_zones_wlock) > - return false; > - > - if (blk_rq_is_passthrough(rq)) > - return false; > - > - switch (req_op(rq)) { > - case REQ_OP_WRITE_ZEROES: > - case REQ_OP_WRITE: > - return blk_rq_zone_is_seq(rq); > - default: > - return false; > - } > + return rq->q->seq_zones_wlock && blk_rq_is_zoned_seq_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 909d47e34b7c..d5930797b84d 100644 > --- a/include/linux/blk-mq.h > +++ b/include/linux/blk-mq.h > @@ -1136,6 +1136,24 @@ static inline unsigned int blk_rq_zone_is_seq(struct request *rq) > return blk_queue_zone_is_seq(rq->q, blk_rq_pos(rq)); > } > > +/** > + * blk_rq_is_zoned_seq_write() - Whether @rq is a write request for a sequential zone. This doesn't parse and is overly long at the same time.
On 6/23/22 23:07, Christoph Hellwig wrote: > On Thu, Jun 23, 2022 at 04:25:59PM -0700, Bart Van Assche wrote: >> +/** >> + * blk_rq_is_zoned_seq_write() - Whether @rq is a write request for a sequential zone. > > This doesn't parse and is overly long at the same time. Hi Christoph, It is not clear to me why you wrote that "this doesn't parse"? Are you referring to the function name or to the function description? I think the text at the right side of the hyphen is grammatically correct. Although I'm in favor of respecting the 80 column limit, I haven't found any explanation in Documentation/doc-guide/kernel-doc.rst about whether or not splitting the brief description across multiple lines is supported. Thanks, Bart.
diff --git a/block/blk-zoned.c b/block/blk-zoned.c index 38cd840d8838..cafcbc508dfb 100644 --- a/block/blk-zoned.c +++ b/block/blk-zoned.c @@ -57,19 +57,7 @@ EXPORT_SYMBOL_GPL(blk_zone_cond_str); */ bool blk_req_needs_zone_write_lock(struct request *rq) { - if (!rq->q->seq_zones_wlock) - return false; - - if (blk_rq_is_passthrough(rq)) - return false; - - switch (req_op(rq)) { - case REQ_OP_WRITE_ZEROES: - case REQ_OP_WRITE: - return blk_rq_zone_is_seq(rq); - default: - return false; - } + return rq->q->seq_zones_wlock && blk_rq_is_zoned_seq_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 909d47e34b7c..d5930797b84d 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -1136,6 +1136,24 @@ static inline unsigned int blk_rq_zone_is_seq(struct request *rq) return blk_queue_zone_is_seq(rq->q, blk_rq_pos(rq)); } +/** + * blk_rq_is_zoned_seq_write() - Whether @rq is a write request for a sequential zone. + * @rq: Request to examine. + * + * In this context sequential zone means either a sequential write required or + * a sequential write preferred zone. + */ +static inline bool blk_rq_is_zoned_seq_write(struct request *rq) +{ + switch (req_op(rq)) { + case REQ_OP_WRITE: + case REQ_OP_WRITE_ZEROES: + return blk_rq_zone_is_seq(rq); + default: + return false; + } +} + 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); @@ -1166,6 +1184,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_zoned_seq_write(struct request *rq) +{ + return false; +} + static inline bool blk_req_needs_zone_write_lock(struct request *rq) { return false;
Introduce a function that makes it easy to verify whether a write request is for a sequential write required or sequential write preferred zone. Simplify blk_req_needs_zone_write_lock() by using the new function. Cc: Damien Le Moal <damien.lemoal@wdc.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- block/blk-zoned.c | 14 +------------- include/linux/blk-mq.h | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 13 deletions(-)