From: keith.busch@intel.com (Keith Busch)
Subject: [PATCH for-4.4] block: split bios to max possible length
Date: Wed, 6 Jan 2016 05:51:34 +0000 [thread overview]
Message-ID: <20160106055133.GA4868@localhost.localdomain> (raw)
In-Reply-To: <CACVXFVNQQNd8_3ti4AAhZB4KN0uJEN8sdmB+xsmpykJaCLfebA@mail.gmail.com>
On Wed, Jan 06, 2016@10:17:51AM +0800, Ming Lei wrote:
> Firstly we didn't split one single bio vector before bio splitting.
>
> Secondly, current bio split still doesn't support to split one single
> bvec into two, and it just makes the two bios shared the original
> bvec table, please see bio_split(), which calls bio_clone_fast()
> to do that, and the bvec table has been immutable at that time.
You are saying we can't split a bio in the middle of a vector?
bvec_iter_advance() says we can split anywhere.
> I understand your motivation in the two patches, actually before bio splitting,
> we don't do sg merge for nvme because of the flag of NO_SG_MERGE,
> which is ignored after bio splitting is introduced. So could you check if
> the nvme performance can be good by putting NO_SG_MERGE back
> in blk_bio_segment_split()? And the change should be simple, like the
> attached patch.
The nvme driver is okay to take physically merged pages. It splits them
into PRPs accordingly, and it's faster to DMA map physically contiguous
just because there are fewer segments to iterate, so NVMe would prefer
to let them coalesce.
> IMO, splitting is quite cheap,
Splitting is absolutely not cheap if your media is fast enough. I measure
every % of increased CPU instructions as the same % decrease in IOPs
with 3DXpoint.
But this patch was to split on stripe boundaries, which is an even worse
penalty if we get the split wrong.
> + bool no_sg_merge = !!test_bit(QUEUE_FLAG_NO_SG_MERGE, &q->queue_flags);
>
> bio_for_each_segment(bv, bio, iter) {
> - if (sectors + (bv.bv_len >> 9) > blk_max_size_offset(q, bio->bi_iter.bi_sector))
> + if (no_sg_merge)
> + goto new_segment;
Bad idea for NVMe. We need to split on SG gaps, which you've skipped,
or you will BUG_ON in the NVMe driver.
next prev parent reply other threads:[~2016-01-06 5:51 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-04 18:24 [PATCH for-4.4] block: split bios to max possible length Keith Busch
2016-01-05 4:54 ` Ming Lei
2016-01-05 15:09 ` Keith Busch
2016-01-06 2:17 ` Ming Lei
2016-01-06 5:51 ` Keith Busch [this message]
2016-01-06 6:29 ` Ming Lei
2016-01-06 6:53 ` Ming Lei
2016-01-06 15:18 ` Keith Busch
2016-01-06 15:43 ` Ming Lei
2016-01-06 16:21 ` Keith Busch
2016-01-07 0:14 ` Ming Lei
2016-01-07 10:46 ` Kent Overstreet
2016-01-09 11:10 ` Ming Lei
2016-01-06 15:29 ` Keith Busch
2016-01-06 9:46 ` Ming Lei
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=20160106055133.GA4868@localhost.localdomain \
--to=keith.busch@intel.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.