Re: [PATCH 04/26] block: Introduce blk_zone_complete_request_bio()

Hannes Reinecke
2024-02-04T12:00:01Z

On 2/2/24 15:30, Damien Le Moal wrote:
> On completion of a zone append request, the request sector indicates the
> location of the written data. This value must be returned to the user
> through the BIO iter sector. This is done in 2 places: in
> blk_complete_request() and in req_bio_endio(). Introduce the inline
> helper function blk_zone_complete_request_bio() to avoid duplicating
> this BIO update for zone append requests, and to compile out this
> helper call when CONFIG_BLK_DEV_ZONED is not enabled.
>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
> block/blk-mq.c | 11 +++++------
> block/blk.h | 19 ++++++++++++++++++-
> 2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index bfebb8fcd248..f02e486a02ae 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -822,11 +822,11 @@ static void blk_complete_request(struct request *req)
> /* Completion has already been traced */
> bio_clear_flag(bio, BIO_TRACE_COMPLETION);
>
> - if (req_op(req) == REQ_OP_ZONE_APPEND)
> - bio->bi_iter.bi_sector = req->__sector;
> -
> - if (!is_flush)
> + if (!is_flush) {
> + blk_zone_complete_request_bio(req, bio);
> bio_endio(bio);
> + }
> +
> bio = next;
> } while (bio);
>
> @@ -928,8 +928,7 @@ bool blk_update_request(struct request *req, blk_status_t error,
>
> /* Don't actually finish bio if it's part of flush sequence */
> if (!bio->bi_iter.bi_size && !is_flush) {
> - if (req_op(req) == REQ_OP_ZONE_APPEND)
> - bio->bi_iter.bi_sector = req->__sector;
> + blk_zone_complete_request_bio(req, bio);
> bio_endio(bio);
> }
>
> diff --git a/block/blk.h b/block/blk.h
> index 913c93838a01..23f76b452e70 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -396,12 +396,29 @@ static inline struct bio *blk_queue_bounce(struct bio *bio,
>
> #ifdef CONFIG_BLK_DEV_ZONED
> void disk_free_zone_bitmaps(struct gendisk *disk);
> +static inline void blk_zone_complete_request_bio(struct request *rq,
> + struct bio *bio)
> +{
> + /*
> + * For zone append requests, the request sector indicates the location
> + * at which the BIO data was written. Return this value to the BIO
> + * issuer through the BIO iter sector.
> + */
> + if (req_op(rq) == REQ_OP_ZONE_APPEND)
> + bio->bi_iter.bi_sector = rq->__sector;
> +}
> int blkdev_report_zones_ioctl(struct block_device *bdev, unsigned int cmd,
> unsigned long arg);
> int blkdev_zone_mgmt_ioctl(struct block_device *bdev, blk_mode_t mode,
> unsigned int cmd, unsigned long arg);
> #else /* CONFIG_BLK_DEV_ZONED */
> -static inline void disk_free_zone_bitmaps(struct gendisk *disk) {}
> +static inline void disk_free_zone_bitmaps(struct gendisk *disk)
> +{
> +}
> +static inline void blk_zone_complete_request_bio(struct request *rq,
> + struct bio *bio)
> +{
> +}
> static inline int blkdev_report_zones_ioctl(struct block_device *bdev,
> unsigned int cmd, unsigned long arg)
> {

Well, it doesn't _actually_ complete the request nor the bio.
What about blk_zone_update_request_bio()?

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Ivo Totev, Andrew McDonald,
Werner Knoblich