Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1182523 > unrolled thread

Re: [PATCH v5 00/11] simplify block layer based on immutable biovecs

Started byMing Lin <mlin@kernel.org>
First post2015-07-13 07:20 +0200
Last post2015-07-14 23:00 +0200
Articles 2 — 1 participant

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v5 00/11] simplify block layer based on immutable biovecs Ming Lin <mlin@kernel.org> - 2015-07-13 07:20 +0200
    Re: [PATCH v5 00/11] simplify block layer based on immutable biovecs Ming Lin <mlin@kernel.org> - 2015-07-14 23:00 +0200

#1182523 — Re: [PATCH v5 00/11] simplify block layer based on immutable biovecs

FromMing Lin <mlin@kernel.org>
Date2015-07-13 07:20 +0200
SubjectRe: [PATCH v5 00/11] simplify block layer based on immutable biovecs
Message-ID<pLEjM-Zo-5@gated-at.bofh.it>
On Mon, 2015-07-06 at 00:11 -0700, mlin@kernel.org wrote:
> Hi Mike,
> 
> On Wed, 2015-06-10 at 17:46 -0400, Mike Snitzer wrote:
> > I've been busy getting DM changes for the 4.2 merge window finalized.
> > As such I haven't connected with others on the team to discuss this
> > issue.
> > 
> > I'll see if we can make time in the next 2 days.  But I also have
> > RHEL-specific kernel deadlines I'm coming up against.
> > 
> > Seems late to be staging this extensive a change for 4.2... are you
> > pushing for this code to land in the 4.2 merge window?  Or do we have
> > time to work this further and target the 4.3 merge?
> > 
> 
> 4.2-rc1 was out.
> Would you have time to work together for 4.3 merge? 

Ping ...

What can I do to move forward?

This patchset not only simplify block layer a lot, it's also a
prerequisite of the direct IO rewrite patches, which I saw 40%
performance improvement for null_blk and 10% improvement for NVMe
drives. I have been fixing bugs for the direct IO patches. I'll post it
once it passes xfstests.

Mike,
Can I have your ACK? Or do you have other test plan?

Thanks,
Ming

> 
> Fio test results(4.1-rc4/rc7) showed no performance regressions
> for HW/SW RAID6 and DM stripe tests.
> http://minggr.net/pub/20150608/fio_results/summary.log
> 
> v5:
>   - rebase on top of 4.2-rc1
>   - reorder patch 6,7
>   - add NeilBrown's ACKs
>   - fix memory leak: free "bio_split" bioset in blk_release_queue()
> 
> v4:
>   - rebase on top of 4.1-rc4
>   - use BIO_POOL_SIZE instead of number 4 for bioset_create()
>   - call blk_queue_split() in blk_mq_make_request()
>   - call blk_queue_split() in zram_make_request()
>   - add patch "block: remove bio_get_nr_vecs()"
>   - remove split code in blkdev_issue_discard()
>   - drop patch "md/raid10: make sync_request_write() call bio_copy_data()".
>     NeilBrown queued it.
>   - drop patch "block: allow __blk_queue_bounce() to handle bios larger than BIO_MAX_PAGES".
>     Will send it seperately
> 
> v3:
>   - rebase on top of 4.1-rc2
>   - support for QUEUE_FLAG_SG_GAPS
>   - update commit logs of patch 2&4
>   - split bio for chunk_aligned_read
> 
> v2: https://lkml.org/lkml/2015/4/28/28
> v1: https://lkml.org/lkml/2014/12/22/128
> 
> This is the 5th attempt of simplifying block layer based on immutable
> biovecs. Immutable biovecs, implemented by Kent Overstreet, have been
> available in mainline since v3.14. Its original goal was actually making
> generic_make_request() accept arbitrarily sized bios, and pushing the
> splitting down to the drivers or wherever it's required. See also
> discussions in the past, [1] [2] [3].
> 
> This will bring not only performance improvements, but also a great amount
> of reduction in code complexity all over the block layer. Performance gain
> is possible due to the fact that bio_add_page() does not have to check
> unnecesary conditions such as queue limits or if biovecs are mergeable.
> Those will be delegated to the driver level. Kent already said that he
> actually benchmarked the impact of this with fio on a micron p320h, which
> showed definitely a positive impact.
> 
> Moreover, this patchset also allows a lot of code to be deleted, mainly
> because of removal of merge_bvec_fn() callbacks. We have been aware that
> it has been always a delicate issue for stacking block drivers (e.g. md
> and bcache) to handle merging bio consistently. This simplication will
> help every individual block driver avoid having such an issue.
> 
> Patches are against 4.2-rc1. These are also available in my git repo at:
> 
>   https://git.kernel.org/cgit/linux/kernel/git/mlin/linux.git/log/?h=block-generic-req
>   git://git.kernel.org/pub/scm/linux/kernel/git/mlin/linux.git block-generic-req
> 
> This patchset is a prerequisite of other consecutive patchsets, e.g.
> multipage biovecs, rewriting plugging, or rewriting direct-IO, which are
> excluded this time. That means, this patchset should not bring any
> regression to end-users.
> 
> Comments are welcome.
> Ming
> 
> [1] https://lkml.org/lkml/2014/11/23/263
> [2] https://lkml.org/lkml/2013/11/25/732
> [3] https://lkml.org/lkml/2014/2/26/618
> 
> Dongsu Park (1):
>       Documentation: update notes in biovecs about arbitrarily sized bios
> 
> Kent Overstreet (8):
>       block: make generic_make_request handle arbitrarily sized bios
>       block: simplify bio_add_page()
>       bcache: remove driver private bio splitting code
>       btrfs: remove bio splitting and merge_bvec_fn() calls
>       md/raid5: get rid of bio_fits_rdev()
>       block: kill merge_bvec_fn() completely
>       fs: use helper bio_add_page() instead of open coding on bi_io_vec
>       block: remove bio_get_nr_vecs()
> 
> Ming Lin (2):
>       block: remove split code in blkdev_issue_discard
>       md/raid5: split bio for chunk_aligned_read
> 
>  Documentation/block/biovecs.txt             |  10 +-
>  block/bio.c                                 | 152 ++++++++++------------------
>  block/blk-core.c                            |  19 ++--
>  block/blk-lib.c                             |  73 +++----------
>  block/blk-merge.c                           | 148 +++++++++++++++++++++++++--
>  block/blk-mq.c                              |   4 +
>  block/blk-settings.c                        |  22 ----
>  block/blk-sysfs.c                           |   3 +
>  drivers/block/drbd/drbd_int.h               |   1 -
>  drivers/block/drbd/drbd_main.c              |   1 -
>  drivers/block/drbd/drbd_req.c               |  37 +------
>  drivers/block/pktcdvd.c                     |  27 +----
>  drivers/block/ps3vram.c                     |   2 +
>  drivers/block/rbd.c                         |  47 ---------
>  drivers/block/rsxx/dev.c                    |   2 +
>  drivers/block/umem.c                        |   2 +
>  drivers/block/zram/zram_drv.c               |   2 +
>  drivers/md/bcache/bcache.h                  |  18 ----
>  drivers/md/bcache/io.c                      | 100 +-----------------
>  drivers/md/bcache/journal.c                 |   4 +-
>  drivers/md/bcache/request.c                 |  16 +--
>  drivers/md/bcache/super.c                   |  32 +-----
>  drivers/md/bcache/util.h                    |   5 +-
>  drivers/md/bcache/writeback.c               |   4 +-
>  drivers/md/dm-cache-target.c                |  21 ----
>  drivers/md/dm-crypt.c                       |  16 ---
>  drivers/md/dm-era-target.c                  |  15 ---
>  drivers/md/dm-flakey.c                      |  16 ---
>  drivers/md/dm-io.c                          |   2 +-
>  drivers/md/dm-linear.c                      |  16 ---
>  drivers/md/dm-log-writes.c                  |  16 ---
>  drivers/md/dm-raid.c                        |  19 ----
>  drivers/md/dm-snap.c                        |  15 ---
>  drivers/md/dm-stripe.c                      |  21 ----
>  drivers/md/dm-table.c                       |   8 --
>  drivers/md/dm-thin.c                        |  31 ------
>  drivers/md/dm-verity.c                      |  16 ---
>  drivers/md/dm.c                             | 129 +----------------------
>  drivers/md/dm.h                             |   2 -
>  drivers/md/linear.c                         |  43 --------
>  drivers/md/md.c                             |  28 +----
>  drivers/md/md.h                             |  12 ---
>  drivers/md/multipath.c                      |  21 ----
>  drivers/md/raid0.c                          |  56 ----------
>  drivers/md/raid0.h                          |   2 -
>  drivers/md/raid1.c                          |  58 +----------
>  drivers/md/raid10.c                         | 121 +---------------------
>  drivers/md/raid5.c                          |  92 ++++++-----------
>  drivers/s390/block/dcssblk.c                |   2 +
>  drivers/s390/block/xpram.c                  |   2 +
>  drivers/staging/lustre/lustre/llite/lloop.c |   2 +
>  fs/btrfs/compression.c                      |   5 +-
>  fs/btrfs/extent_io.c                        |   9 +-
>  fs/btrfs/inode.c                            |   3 +-
>  fs/btrfs/scrub.c                            |  18 +---
>  fs/btrfs/volumes.c                          |  72 -------------
>  fs/buffer.c                                 |   7 +-
>  fs/direct-io.c                              |   2 +-
>  fs/ext4/page-io.c                           |   3 +-
>  fs/ext4/readpage.c                          |   2 +-
>  fs/f2fs/data.c                              |   2 +-
>  fs/gfs2/lops.c                              |   9 +-
>  fs/jfs/jfs_logmgr.c                         |  14 +--
>  fs/logfs/dev_bdev.c                         |   4 +-
>  fs/mpage.c                                  |   4 +-
>  fs/nilfs2/segbuf.c                          |   2 +-
>  fs/xfs/xfs_aops.c                           |   3 +-
>  include/linux/bio.h                         |   1 -
>  include/linux/blkdev.h                      |  13 +--
>  include/linux/device-mapper.h               |   4 -
>  mm/page_io.c                                |   8 +-
>  71 files changed, 340 insertions(+), 1358 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1183997

FromMing Lin <mlin@kernel.org>
Date2015-07-14 23:00 +0200
Message-ID<pMft0-37o-11@gated-at.bofh.it>
In reply to#1182523
On Mon, 2015-07-13 at 11:35 -0400, Mike Snitzer wrote:
> On Mon, Jul 13 2015 at  1:12am -0400,
> Ming Lin <mlin@kernel.org> wrote:
> 
> > On Mon, 2015-07-06 at 00:11 -0700, mlin@kernel.org wrote:
> > > Hi Mike,
> > > 
> > > On Wed, 2015-06-10 at 17:46 -0400, Mike Snitzer wrote:
> > > > I've been busy getting DM changes for the 4.2 merge window finalized.
> > > > As such I haven't connected with others on the team to discuss this
> > > > issue.
> > > > 
> > > > I'll see if we can make time in the next 2 days.  But I also have
> > > > RHEL-specific kernel deadlines I'm coming up against.
> > > > 
> > > > Seems late to be staging this extensive a change for 4.2... are you
> > > > pushing for this code to land in the 4.2 merge window?  Or do we have
> > > > time to work this further and target the 4.3 merge?
> > > > 
> > > 
> > > 4.2-rc1 was out.
> > > Would you have time to work together for 4.3 merge? 
> > 
> > Ping ...
> > 
> > What can I do to move forward?
> 
> You can show further testing.  Particularly that you've covered all the
> edge cases.
> 
> Until someone can produce some perf test results where they are actually
> properly controlling for the splitting, we have no useful information.
> 
> The primary concerns associated with this patchset are:
> 1) In the context of RAID, XFS's use of bio_add_page() used to build up
>    optimal IOs when the underlying block device provides striping info
>    via IO limits.  With this patchset how large will bios become in
>    practice _without_ bio_add_page() being bounded by the underlying IO
>    limits?
> 
> 2) The late splitting that occurs for the (presummably) large bios that
>    are sent down.. how does it cope/perform in the face of very
>    low/fragmented system memory?
> 
> 3) More open-ended comment than question: Linux has evolved to perform
>    well on "enterprise" systems.  We generally don't fall off a cliff on 
>    performance like we used to.  The concern associated with this
>    patchset is that if it goes in without _real_ due-diligence on
>    "enterprise" scale systems and workloads it'll be too late once we
>    notice the problem(s).
> 
> So we really need answers to 1 and 2 above in order to feel better about
> the risks associated 3.
> 
> Alasdair's feedback to you on testing still applies (and hasn't been
> done AFAIK):
> https://www.redhat.com/archives/dm-devel/2015-May/msg00203.html
> 
> Particularly:
> "you might need to instrument the kernels to tell you the sizes of the
> bios being created and the amount of splitting actually happening."
> 
> and
> 
> "You may also want to test systems with a restricted amount of available
> memory to show how the splitting via worker thread performs.  (Again,
> instrument to prove the extent to which the new code is being exercised.)"
> 
> > This patchset not only simplify block layer a lot, it's also a
> > prerequisite of the direct IO rewrite patches, which I saw 40%
> > performance improvement for null_blk and 10% improvement for NVMe
> > drives. I have been fixing bugs for the direct IO patches. I'll post it
> > once it passes xfstests.
> > 
> > Mike,
> > Can I have your ACK? Or do you have other test plan?
> 
> I'm not the only person with concerns.  I share Alasdair's concerns.
> Jeff Moyer is also concerned about the implications of this patchset.
> We're all in favor of this patchset's cleanup _if and only if_ it can be
> proven that we aren't going to be falling off a cliff on performance due
> to some pathological workload (be it under memory pressure or whatever).
> 
> Apologies for not being able to put time to this like I hoped.  But that
> doesn't mean you are off the hook on showing you've done the testing and
> understand the scope and implications of the changes you're pushing for.
> 
> I will do additional review to answer 1 and 2 above.  And Jeff Moyer
> told me he'd test the patchset on one of his testbeds.
> 
> But if you can help answer 1 and 2 above that'd go a long way.

Thanks for the response.
I'm working on more testing.

I'll ask if I have question about the testing.

Thanks,
Ming

> 
> Thanks,
> Mike


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web