On Thu, May 22, 2025 at 11:32:58AM -0700, Bart Van Assche wrote:
> On 5/22/25 10:38 AM, Jens Axboe wrote:
> > On 5/22/25 11:14 AM, Bart Van Assche wrote:
> > > static void __submit_bio(struct bio *bio)
> > > {
> > > /* If plug is not used, add new plug here to cache nsecs time. */
> > > @@ -633,8 +640,12 @@ static void __submit_bio(struct bio *bio)
> > > if (!bdev_test_flag(bio->bi_bdev, BD_HAS_SUBMIT_BIO)) {
> > > blk_mq_submit_bio(bio);
> > > - } else if (likely(bio_queue_enter(bio) == 0)) {
> > > + } else {
> > > struct gendisk *disk = bio->bi_bdev->bd_disk;
> > > + bool zwp = bio_zone_write_plugging(bio);
> > > +
> > > + if (unlikely(!zwp && bio_queue_enter(bio) != 0))
> > > + goto finish_plug;
> > >
> > > if ((bio->bi_opf & REQ_POLLED) &&
> > > !(disk->queue->limits.features & BLK_FEAT_POLL)) {
> > > @@ -643,9 +654,12 @@ static void __submit_bio(struct bio *bio)
> > > } else {
> > > disk->fops->submit_bio(bio);
> > > }
> > > - blk_queue_exit(disk->queue);
> > > +
> > > + if (!zwp)
> > > + blk_queue_exit(disk->queue);
> > > }
> >
> > 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?
>
> Hi Jens,
>
> q->q_usage_counter is not increased for bios on current->plug_list.
> q->q_usage_counter is increased before a bio is added to the zoned pluglist.
> So these two cases are different.
>
> I think it is important to hold a q->q_usage_counter reference for bios
> on the zoned plug list because bios are added to that list after bio
> splitting happened. Hence, request queue limits must not change while
> any bio is on the zoned plug list.
Hi Bart,
Can you share why request queue limit can't be changed after bio is added
to zoned plug list?
If it is really true, we may have to drain zoned plug list when freezing
queue.
Thanks,
Ming
