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

[Damien Le Moal <dlemoal@kernel.org>]

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