public inbox for [email protected]
 help / color / mirror / Atom feed
From: Jeffle Xu <[email protected]>
To: [email protected], [email protected], [email protected]
Cc: [email protected], [email protected],
	[email protected]
Subject: [PATCH v4 0/2] block, iomap: disable iopoll for split bio
Date: Tue, 17 Nov 2020 15:56:23 +0800	[thread overview]
Message-ID: <[email protected]> (raw)

This patchset is to fix the potential hang occurred in sync polling.

Please refer the following link for background info and the v1 patch:
https://patchwork.kernel.org/project/linux-block/patch/[email protected]/

The first patch disables iopoll for split bio in block layer, which is
suggested by Ming Lei.


The second patch disables iopoll when one dio need to be split into
multiple bios. As for this patch, Ming Lei had ever asked what's the
expected behaviour of upper layers when simply clear IOCB_HIPRI in
the direct routine of blkdev fs, iomap-based fs. Currently there are
two parts concerning IOCB_HIPRI (or io polling). One is the sync
polling logic embedded in the direct IO routine. In this case, sync
polling won't be executed any more since IOCB_HIPRI flag has been
cleared from iocb->ki_flags. Consider the following code snippet:

fs/block_dev.c: __blkdev_direct_IO
	for (;;) {
		...
		if (!(iocb->ki_flags & IOCB_HIPRI) ||
		    !blk_poll(bdev_get_queue(bdev), qc, true))
			blk_io_schedule();
	}

fs/iomap/direct-io.c: __iomap_dio_rw
	for (;;) {
		...
		if (!(iocb->ki_flags & IOCB_HIPRI) ||
		    !dio->submit.last_queue ||
		    !blk_poll(dio->submit.last_queue,
				 dio->submit.cookie, true))
			blk_io_schedule();
	}


The other part is io_uring. 

fs/io_uring.c: 
io_iopoll_getevents
  io_do_iopoll
    list_for_each_entry_safe(...) {
      ret = kiocb->ki_filp->f_op->iopoll(kiocb, spin);
    }

In this case, though the split bios have been enqueued into DEFAULT
hw queues, io_uring will still poll POLL hw queues. When polling on
the cookie returned by split bio, blk_poll() will return 0 immediately
since the hw queue type check added in patch 1. If there's no other
bio in the POLL hw queues, io_do_iopoll() will loop indefinitely
until the split bio is completed by interrupt of DEFAULT queue. Indeed
there may be a pulse of high CPU sys in this time window here, but it
is no worse than before. After all io_do_iopoll() will still get stuck
in this loop when there's only one bio (that we are polling on) in POLL
hw queue, before this patch applied.

The situation described above may be less impossible. As long as there
are other bios in POLL hw queue, work of io_do_iopoll() is still
meaningful as it *helps* reap these other bios in POLL hw queue, while
the split bios are still completed by interrupt of DEFAULT hw queue.


changes since v3:
- patch 1: add hw queue type check in blk_poll(), so that cookie returned
  by split bio won't get into the real polling routine

changes since v2:
- tune the line length of patch 1
- fix the condition checking whether split needed in patch 2

changes since v1:
- adopt the fix suggested by Ming Lei, to disable iopoll for split bio directly
- disable iopoll in direct IO routine of blkdev fs and iomap

Jeffle Xu (2):
  block: disable iopoll for split bio
  block,iomap: disable iopoll when split needed

 block/blk-merge.c    |  7 +++++++
 block/blk-mq.c       |  6 ++++--
 fs/block_dev.c       |  9 +++++++++
 fs/iomap/direct-io.c | 10 ++++++++++
 4 files changed, 30 insertions(+), 2 deletions(-)

-- 
2.27.0


             reply	other threads:[~2020-11-17  7:57 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-17  7:56 Jeffle Xu [this message]
2020-11-17  7:56 ` [PATCH v4 1/2] block: disable iopoll for split bio Jeffle Xu
2020-11-19  3:06   ` JeffleXu
2020-11-19 17:52   ` Christoph Hellwig
2020-11-20  9:22     ` JeffleXu
2020-11-17  7:56 ` [PATCH v4 2/2] block,iomap: disable iopoll when split needed Jeffle Xu
2020-11-17 17:37   ` Darrick J. Wong
2020-11-18  1:56     ` JeffleXu
2020-11-19 17:55   ` Christoph Hellwig
2020-11-20 10:06     ` JeffleXu
2020-11-24 11:25       ` Christoph Hellwig
2020-11-25  7:03         ` JeffleXu
2020-11-17 12:51 ` [PATCH v4 0/2] block, iomap: disable iopoll for split bio JeffleXu
2020-11-18  9:50   ` JeffleXu

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 \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox