Re: [PATCH] block: Fix a deadlock related freezing zoned storage devices

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

On 5/23/25 10:20, Christoph Hellwig wrote:
> On Fri, May 23, 2025 at 10:10:30AM +0200, Damien Le Moal wrote:
>>> This is pretty ugly, and I honestly absolutely hate how there's quite a
>>> bit of zoned_whatever sprinkling throughout the core code. What's the
>>> reason for not unplugging here, unaligned writes? Because you should
>>> presumable have the exact same issues on non-zoned devices if they have
>>> IO stuck in a plug (and doesn't get unplugged) while someone is waiting
>>> on a freeze.
>>>
>>> A somewhat similar case was solved for IOPOLL and queue entering. That
>>> would be another thing to look at. Maybe a live enter could work if the
>>> plug itself pins it?
>>
>> What about this patch, completely untested...
> 
> This still looks extremely backwards as it messed up common core
> code for something that it shouldn't.  I'd still love to see an
> actual reproducer ahead of me, but I think the problem is that
> blk_zone_wplug_bio_work calls into the still fairly high-level
> submit_bio_noacct_nocheck for bios that already went through
> the submit_bio machinery.
> 
> Something like this completely untested patch:
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 8f15d1aa6eb8..6841af8a989c 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -1306,16 +1306,18 @@ static void blk_zone_wplug_bio_work(struct work_struct *work)
>  	spin_unlock_irqrestore(&zwplug->lock, flags);
>  
>  	bdev = bio->bi_bdev;
> -	submit_bio_noacct_nocheck(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))
> +	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);
> +	}

Indeed, this looks better and simpler. Will give it a spin to check.


-- 
Damien Le Moal
Western Digital Research