All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: JeffleXu <jefflexu@linux.alibaba.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	joseph.qi@linux.alibaba.com, dm-devel@redhat.com,
	linux-block@vger.kernel.org, io-uring@vger.kernel.org
Subject: Re: [dm-devel] [PATCH v2 0/6] dm: support IO polling for bio-based dm device
Date: Sun, 31 Jan 2021 11:26:58 -0500	[thread overview]
Message-ID: <20210131162657.GA3164@redhat.com> (raw)
In-Reply-To: <2ed9966f-b390-085a-1a51-5bf65038d533@linux.alibaba.com>

On Wed, Jan 27 2021 at 10:06pm -0500,
JeffleXu <jefflexu@linux.alibaba.com> wrote:

> 
> 
> On 1/28/21 1:19 AM, Mike Snitzer wrote:
> > On Mon, Jan 25 2021 at  7:13am -0500,
> > Jeffle Xu <jefflexu@linux.alibaba.com> wrote:
> > 
> >> Since currently we have no simple but efficient way to implement the
> >> bio-based IO polling in the split-bio tracking style, this patch set
> >> turns to the original implementation mechanism that iterates and
> >> polls all underlying hw queues in polling mode. One optimization is
> >> introduced to mitigate the race of one hw queue among multiple polling
> >> instances.
> >>
> >> I'm still open to the split bio tracking mechanism, if there's
> >> reasonable way to implement it.
> >>
> >>
> >> [Performance Test]
> >> The performance is tested by fio (engine=io_uring) 4k randread on
> >> dm-linear device. The dm-linear device is built upon nvme devices,
> >> and every nvme device has one polling hw queue (nvme.poll_queues=1).
> >>
> >> Test Case		    | IOPS in IRQ mode | IOPS in polling mode | Diff
> >> 			    | (hipri=0)	       | (hipri=1)	      |
> >> --------------------------- | ---------------- | -------------------- | ----
> >> 3 target nvme, num_jobs = 1 | 198k 	       | 276k		      | ~40%
> >> 3 target nvme, num_jobs = 3 | 608k 	       | 705k		      | ~16%
> >> 6 target nvme, num_jobs = 6 | 1197k 	       | 1347k		      | ~13%
> >> 3 target nvme, num_jobs = 6 | 1285k 	       | 1293k		      | ~0%
> >>
> >> As the number of polling instances (num_jobs) increases, the
> >> performance improvement decreases, though it's still positive
> >> compared to the IRQ mode.
> > 
> > I think there is serious room for improvement for DM's implementation;
> > but the block changes for this are all we'd need for DM in the longrun
> > anyway (famous last words).
> 
> Agreed.
> 
> 
> > So on a block interface level I'm OK with
> > block patches 1-3.
> > 
> > I don't see why patch 5 is needed (said the same in reply to it; but I
> > just saw your reason below..).
> > 
> > Anyway, I can pick up DM patches 4 and 6 via linux-dm.git if Jens picks
> > up patches 1-3. Jens, what do you think?
> 
> cc Jens.
> 
> Also I will send a new version later, maybe some refactor on patch5 and
> some typo modifications.

Thinking further, there is no benefit to Jens picking up the block core
changes until the DM changes are ready.  While I think the refactoring
to expose the blk_poll (in patch 3) that supports blk-mq and bio-based
is reasonable -- Christoph correctly points out there is extra branching
that blk-mq must tolerate as implemented.  So even that needs followup
work as suggested here:
https://www.redhat.com/archives/dm-devel/2021-January/msg00397.html

Also, your followup about oversights in the the latest bio-based DM io
polling implementation speaks to all of this needing more time:
https://www.redhat.com/archives/dm-devel/2021-January/msg00436.html

You advocating going back to what is effectively the first RFC patchset
you proposed (with its underwhelming bio-based polling performance)
isn't a strong indication these changes are ready, or that we even have
a patch forward for how to make bio-based IO polling be worthwhile.

So: I retract my question to Jens about whether he'd pick up the block
core changes (while I think those are close, the corresponding DM
changes aren't).

Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel


      reply	other threads:[~2021-01-31 16:27 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-25 12:13 [dm-devel] [PATCH v2 0/6] dm: support IO polling for bio-based dm device Jeffle Xu
2021-01-25 12:13 ` Jeffle Xu
2021-01-25 12:13 ` [dm-devel] [PATCH v2 1/6] block: move definition of blk_qc_t to types.h Jeffle Xu
2021-01-25 12:13   ` Jeffle Xu
2021-01-25 12:13 ` [dm-devel] [PATCH v2 2/6] block: add queue_to_disk() to get gendisk from request_queue Jeffle Xu
2021-01-25 12:13   ` Jeffle Xu
2021-01-27 17:20   ` [dm-devel] " Mike Snitzer
2021-01-27 17:20     ` Mike Snitzer
2021-01-25 12:13 ` [dm-devel] [PATCH v2 3/6] block: add iopoll method to support bio-based IO polling Jeffle Xu
2021-01-25 12:13   ` Jeffle Xu
2021-01-27 17:14   ` [dm-devel] " Mike Snitzer
2021-01-27 17:14     ` Mike Snitzer
2021-01-28  8:40   ` [dm-devel] " Christoph Hellwig
2021-01-28  8:40     ` Christoph Hellwig
2021-01-28 11:52     ` [dm-devel] " JeffleXu
2021-01-28 11:52       ` JeffleXu
2021-01-28 14:36       ` [dm-devel] " Christoph Hellwig
2021-01-28 14:36         ` Christoph Hellwig
2021-01-25 12:13 ` [dm-devel] [PATCH v2 4/6] dm: always return BLK_QC_T_NONE for bio-based device Jeffle Xu
2021-01-25 12:13   ` Jeffle Xu
2021-01-25 12:13 ` [dm-devel] [PATCH v2 5/6] block: add QUEUE_FLAG_POLL_CAP flag Jeffle Xu
2021-01-25 12:13   ` Jeffle Xu
2021-01-27 17:13   ` [dm-devel] " Mike Snitzer
2021-01-27 17:13     ` Mike Snitzer
2021-01-28  2:07     ` [dm-devel] " JeffleXu
2021-01-28  2:07       ` JeffleXu
2021-01-25 12:13 ` [dm-devel] [PATCH v2 6/6] dm: support IO polling for bio-based dm device Jeffle Xu
2021-01-25 12:13   ` Jeffle Xu
2021-01-29  7:37   ` [dm-devel] " JeffleXu
2021-01-29  7:37     ` JeffleXu
2021-01-27 17:19 ` [dm-devel] [PATCH v2 0/6] " Mike Snitzer
2021-01-27 17:19   ` Mike Snitzer
2021-01-28  3:06   ` [dm-devel] " JeffleXu
2021-01-28  3:06     ` JeffleXu
2021-01-31 16:26     ` Mike Snitzer [this message]

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=20210131162657.GA3164@redhat.com \
    --to=snitzer@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@redhat.com \
    --cc=io-uring@vger.kernel.org \
    --cc=jefflexu@linux.alibaba.com \
    --cc=joseph.qi@linux.alibaba.com \
    --cc=linux-block@vger.kernel.org \
    /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.