Re: [PATCH v21 06/12] blk-zoned: Introduce a loop in blk_zone_wplug_bio_work()

Damien Le Moal
2025-07-18T07:17:12Z

On 7/18/25 05:58, Bart Van Assche wrote:
> Prepare for submitting multiple bios from inside a single
> blk_zone_wplug_bio_work() call. No functionality has been changed.
>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> block/blk-zoned.c | 72 +++++++++++++++++++++++------------------------
> 1 file changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 8fe6e545f300..6ef53f78fa3b 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -1283,47 +1283,47 @@ static void blk_zone_wplug_bio_work(struct work_struct *work)
> struct blk_zone_wplug *zwplug =
> container_of(work, struct blk_zone_wplug, bio_work);
> struct block_device *bdev;
> - unsigned long flags;
> struct bio *bio;
>
> - /*
> - * Submit the next plugged BIO. If we do not have any, clear
> - * the plugged flag.
> - */
> - spin_lock_irqsave(&zwplug->lock, flags);
> -
> + do {
> again:
> - bio = bio_list_pop(&zwplug->bio_list);
> - if (!bio) {
> - zwplug->flags &= ~BLK_ZONE_WPLUG_PLUGGED;
> - spin_unlock_irqrestore(&zwplug->lock, flags);
> - goto put_zwplug;
> - }
> -
> - trace_blk_zone_wplug_bio(zwplug->disk->queue, zwplug->zone_no,
> - bio->bi_iter.bi_sector, bio_sectors(bio));
> -
> - if (!blk_zone_wplug_prepare_bio(zwplug, bio)) {
> - blk_zone_wplug_bio_io_error(zwplug, bio);
> - goto again;
> - }
> -
> - spin_unlock_irqrestore(&zwplug->lock, flags);
> + /*
> + * Submit the next plugged BIO. If we do not have any, clear
> + * the plugged flag.
> + */
> + scoped_guard(spinlock_irqsave, &zwplug->lock) {

I am really not a fan of this. It adds one level of indentation without making
the code easier to read.

> + bio = bio_list_pop(&zwplug->bio_list);
> + if (!bio) {
> + zwplug->flags &= ~BLK_ZONE_WPLUG_PLUGGED;
> + goto put_zwplug;
> + }
> +
> + trace_blk_zone_wplug_bio(zwplug->disk->queue,
> + zwplug->zone_no,
> + bio->bi_iter.bi_sector,
> + bio_sectors(bio));
> +
> + if (!blk_zone_wplug_prepare_bio(zwplug, bio)) {
> + blk_zone_wplug_bio_io_error(zwplug, bio);
> + goto again;
> + }
> + }
>
> - bdev = bio->bi_bdev;
> + bdev = bio->bi_bdev;
>
> - /*
> - * blk-mq devices will reuse the extra reference on the request queue
> - * usage counter we took when the BIO was plugged, but the submission
> - * path for BIO-based devices will not do that. So drop this extra
> - * reference here.
> - */
> - if (bdev_test_flag(bdev, BD_HAS_SUBMIT_BIO)) {
> - bdev->bd_disk->fops->submit_bio(bio);
> - blk_queue_exit(bdev->bd_disk->queue);
> - } else {
> - blk_mq_submit_bio(bio);
> - }
> + /*
> + * blk-mq devices will reuse the extra reference on the request
> + * queue usage counter we took when the BIO was plugged, but the
> + * submission path for BIO-based devices will not do that. So
> + * drop this extra reference here.
> + */
> + if (bdev_test_flag(bdev, BD_HAS_SUBMIT_BIO)) {
> + bdev->bd_disk->fops->submit_bio(bio);
> + blk_queue_exit(bdev->bd_disk->queue);
> + } else {
> + blk_mq_submit_bio(bio);
> + }
> + } while (0);
>
> put_zwplug:
> /* Drop the reference we took in disk_zone_wplug_schedule_bio_work(). */

--
Damien Le Moal
Western Digital Research