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);- } else {
blk_mq_submit_bio(bio);- }
Indeed, this looks better and simpler. Will give it a spin to check.