On Fri, Apr 15, 2022 at 07:18:44AM +0200, Christoph Hellwig wrote: > On Fri, Apr 15, 2022 at 11:47:03AM +0800, Ming Lei wrote: > > + /* make sure the bio is issued before polling */ > > + if (bio.bi_opf & REQ_POLLED) > > + blk_flush_plug(current->plug, false); > > I still think the core code should handle this. Without that we'd need > to export the blk_flush_plug for anything that would want to poll bios > from modules, in addition to it generally being a mess. See a proposed So far there isn't such usage yet. dm calls bio_poll() in ->iopoll(), and its caller(io_uring) will finish the plug. > patch for that below. I'd also split the flush aspect from the poll > aspect into two patches. > > > + if (bio.bi_opf & REQ_POLLED) > > + bio_poll(&bio, NULL, 0); > > + else > > blk_io_schedule(); > > Instead of this duplicate logic everywhere I'd just make bio_boll > call blk_io_schedule for the !REQ_POLLED case and simplify all the > callers. bio_poll() may be called with rcu read lock held, so I'd suggest to not mix the two together. > > > + if (dio->submit.poll_bio && > > + (dio->submit.poll_bio->bi_opf & > > + REQ_POLLED)) > > This indentation looks awfull,normal would be: > > if (dio->submit.poll_bio && > (dio->submit.poll_bio->bi_opf & REQ_POLLED)) That follows the indentation style of fs/iomap/direct-io.c for break in 'if'. > > --- > From 08ff61b0142eb708fc384cf867c72175561d974a Mon Sep 17 00:00:00 2001 > From: Christoph Hellwig <hch@xxxxxx> > Date: Fri, 15 Apr 2022 07:15:42 +0200 > Subject: blk-mq: don't plug for synchronously polled requests > > For synchronous polling to work, the bio must be issued to the driver from > the submit_bio call, otherwise ->bi_cookie won't be set. > > Based on a patch from Ming Lei. > > Reported-by: Changhui Zhong <czhong@xxxxxxxxxx> > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > --- > block/blk-mq.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index ed3ed86f7dd24..bcc7e3d11296c 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -2851,7 +2851,13 @@ void blk_mq_submit_bio(struct bio *bio) > return; > } > > - if (plug) > + /* > + * We can't plug for synchronously polled submissions, otherwise > + * bio->bi_cookie won't be set directly after submission, which is the > + * indicator used by the submitter to check if a bio needs polling. > + */ > + if (plug && > + (rq->bio->bi_opf & (REQ_POLLED | REQ_NOWAIT)) != REQ_POLLED) > blk_add_rq_to_plug(plug, rq); > else if ((rq->rq_flags & RQF_ELV) || > (rq->mq_hctx->dispatch_busy && It is nothing to do with REQ_NOWAIT. sync polled dio can be marked as REQ_NOWAIT by userspace too. If '--nowait=1' is added in the fio reproducer, io timeout is triggered too. Thanks, Ming
