All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org, linux-mm@kvack.org,
	linux-xfs@vger.kernel.org, Changhui Zhong <czhong@redhat.com>
Subject: Re: [PATCH V2] block: avoid io timeout in case of sync polled dio
Date: Tue, 19 Apr 2022 07:39:24 +0200	[thread overview]
Message-ID: <20220419053924.GA31720@lst.de> (raw)
In-Reply-To: <Yl0e/YBPGydwVAE7@T590>

On Mon, Apr 18, 2022 at 04:19:09PM +0800, Ming Lei wrote:
> But there isn't any such users from module now. Maybe never, since sync
> polled dio becomes legacy after io_uring is invented.

I thought about that a bit, but if we decided synchronous polled I/O
is not in major user anymore (which I think) and we think it is enough
of a burden to support (which I'm not sure of, but this patch points to)
then it might be time to remove it.

> Do we have such potential use case in which explicit flush plug is
> needed except for polled io in __blkdev_direct_IO_simple() and swap_readpage()?
> 
> If there is, I am happy to add one flag for bypassing plug in blk core
> code.

I think the point is that we need to offer sensible APIs for I/O
methods we want to support.

> > 
> > > iomap is one good example to show this point, since it does flush the plug
> > > before call bio_poll(), see __iomap_dio_rw().
> > 
> > iomap does not do a manual plug flush anywhere.
> > 
> > iomap does finish the plug before polling, which makes sense.
> > 
> > Now of course __blkdev_direct_IO_simple doesn't even use a plug
> > to start with, so I'm wondering what plug this patch even tries
> > to flush?
>  
> At least blkdev_write_iter(), and __swap_writepage() might call
> into ->direct_IO with one plug too.
> 
> Not mention loop driver can call into ->direct_IO directly, and we
> should have applied plug for batching submission in loop_process_work().

The loop driver still calls through the read/write iter method, and
the swap code ->direct_IO path is not used for block devices (and
completley broken right now, see the patch series from Neil).

But the loop driver is a good point, even for the iomap case as the
blk_finish_plug would only flush the plug that is on-stack in
__iomap_dio_rw, it would not help you with any plug holder by caller
higher in the stack.

  reply	other threads:[~2022-04-19  5:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-15  3:47 [PATCH V2] block: avoid io timeout in case of sync polled dio Ming Lei
2022-04-15  5:18 ` Christoph Hellwig
2022-04-15 11:00   ` Ming Lei
2022-04-16  5:49     ` Christoph Hellwig
2022-04-16  9:03       ` Ming Lei
2022-04-18  5:12         ` Christoph Hellwig
2022-04-18  8:19           ` Ming Lei
2022-04-19  5:39             ` Christoph Hellwig [this message]
2022-04-19  7:47               ` Ming Lei
2022-04-19  8:15                 ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220419053924.GA31720@lst.de \
    --to=hch@lst.de \
    --cc=axboe@kernel.dk \
    --cc=czhong@redhat.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.