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

Groups > linux.kernel > #1409133 > unrolled thread

[PATCH v6 0/8] block: prepare for multipage bvecs

Back to article view | Back to linux.kernel

Contents

  [PATCH v6 0/8] block: prepare for multipage bvecs Ming Lei <ming.lei@canonical.com> - 2016-05-30 15:40 +0200
    Re: [PATCH v6 0/8] block: prepare for multipage bvecs Mike Snitzer <snitzer@redhat.com> - 2016-05-31 18:00 +0200
      Re: [PATCH v6 0/8] block: prepare for multipage bvecs Ming Lei <ming.lei@canonical.com> - 2016-06-01 14:40 +0200
        Re: [PATCH v6 0/8] block: prepare for multipage bvecs Christoph Hellwig <hch@infradead.org> - 2016-06-01 15:50 +0200
          Re: [PATCH v6 0/8] block: prepare for multipage bvecs Mike Snitzer <snitzer@redhat.com> - 2016-06-01 16:00 +0200
            Re: [PATCH v6 0/8] block: prepare for multipage bvecs Christoph Hellwig <hch@infradead.org> - 2016-06-01 16:10 +0200
            Re: [PATCH v6 0/8] block: prepare for multipage bvecs Ming Lei <ming.lei@canonical.com> - 2016-06-02 04:20 +0200
    Re: [PATCH v6 0/8] block: prepare for multipage bvecs Christoph Hellwig <hch@infradead.org> - 2016-06-01 15:50 +0200
      Re: [PATCH v6 0/8] block: prepare for multipage bvecs Hannes Reinecke <hare@suse.de> - 2016-06-01 16:00 +0200
        Re: [PATCH v6 0/8] block: prepare for multipage bvecs Mike Snitzer <snitzer@redhat.com> - 2016-06-01 16:00 +0200
    Re: [PATCH v6 0/8] block: prepare for multipage bvecs Jens Axboe <axboe@kernel.dk> - 2016-06-09 18:10 +0200
      Re: [PATCH v6 0/8] block: prepare for multipage bvecs Ming Lei <ming.lei@canonical.com> - 2016-06-10 04:50 +0200

#1409133 — [PATCH v6 0/8] block: prepare for multipage bvecs

Hi,

Interests[1] have been shown in multipage bvecs, so this patchset
try to prepare for the support and do two things:

1) the 1st 4 patches use bvec iterator to implement iterate_bvec(),
then we can drop the non-standard way for iterating bvec, which
can be thought as a good cleanup for lib/iov_iter.c

2) remove BIO_MAX_SECTORS and makre BIO_MAX_SIZE as obsolete, and
now there is only one user for each. Once multipage bvecs is introduced,
one bio may hold lots of sectors, and we should always use sort of
BIO_MAX_VECS which should be introduced in future and is similiar
with current BIO_MAX_PAGES.

The only functional change is iterate_bvec():lib/iov_iter.c

xfstests(-a auto) over loop aio is run for ext4/xfs to verify 
the change and no regression found with this patchset.

V6:
	- rebased on v4.7-rc1
	- add reviewed-by tag
	- mark BIO_MAX_SIZE as obsolete instead of removing because
	dm-tree adds one usage now

V5:
	- use bvec's iterator to figure new base vec address and
	update 'skip' correctly
	- run xfstests(-a auto) on loop aio/dio for verifying
	the change in iterate_bvec(), and no regression reported
	- use stree-ng to trigger heavy swap over swapfile to verify
	change in iterate_bvec() too, looks everything is fine
V4:
	- make xfstests cover xfs
	- rebase on for-next of block tree
V3:
	- include kenrel.h & bug.h in bvec.h for fix comiling failure on arm
	as reported by 0day ktest
	- build test on arm & arm64

V2:
	- rename bvec_iter.h as bvec.h
	- always include bvec.h into blk_types.h as suggested by Christoph

V1:
	- don't move BIO_MAX_* to bvec_iter.h as pointed out by Christoph
	- run xfstests against v4.6-rc1-next-20160329
	- add Reviewed-by
	- for 1,4 and 5, Reviewd-by not added, Christoph still expressed
	'this looks fine to me.'


Ming Lei (8):
  block: move bvec iterator into include/linux/bvec.h
  block: move two bvec structure into bvec.h
  block: mark 1st parameter of bvec_iter_advance as const
  iov_iter: use bvec iterator to implement iterate_bvec()
  fs: xfs: replace BIO_MAX_SECTORS with BIO_MAX_PAGES
  block: bio: remove BIO_MAX_SECTORS
  block: drbd: avoid to use BIO_MAX_SIZE
  block: mark BIO_MAX_SIZE as obsolete

 drivers/block/drbd/drbd_int.h |  4 +-
 fs/xfs/xfs_buf.c              |  4 +-
 include/linux/bio.h           | 58 ++++----------------------
 include/linux/blk_types.h     | 22 +---------
 include/linux/bvec.h          | 96 +++++++++++++++++++++++++++++++++++++++++++
 lib/iov_iter.c                | 45 +++++++-------------
 6 files changed, 122 insertions(+), 107 deletions(-)
 create mode 100644 include/linux/bvec.h

-- 
1.9.1

[toc] | [next] | [standalone]

#1410386

On Mon, May 30 2016 at  9:34am -0400,
Ming Lei <ming.lei@canonical.com> wrote:

> 
> Hi,
> 
> Interests[1] have been shown in multipage bvecs, so this patchset
> try to prepare for the support and do two things:
> 
> 1) the 1st 4 patches use bvec iterator to implement iterate_bvec(),
> then we can drop the non-standard way for iterating bvec, which
> can be thought as a good cleanup for lib/iov_iter.c
> 
> 2) remove BIO_MAX_SECTORS and makre BIO_MAX_SIZE as obsolete, and
> now there is only one user for each. Once multipage bvecs is introduced,
> one bio may hold lots of sectors, and we should always use sort of
> BIO_MAX_VECS which should be introduced in future and is similiar
> with current BIO_MAX_PAGES.
> 
> The only functional change is iterate_bvec():lib/iov_iter.c
> 
> xfstests(-a auto) over loop aio is run for ext4/xfs to verify 
> the change and no regression found with this patchset.
> 
> V6:
> 	- rebased on v4.7-rc1
> 	- add reviewed-by tag
> 	- mark BIO_MAX_SIZE as obsolete instead of removing because
> 	dm-tree adds one usage now

Not sure what you're referring to here with "dm-tree" (since "dm-tree"
doesn't exist).  But only direct user of "BIO_MAX_SIZE" in DM appears to
be dm-crypt.c.  Maybe you've identified some indirect use of
BIO_MAX_SIZE?

[toc] | [prev] | [next] | [standalone]

#1411155

On Tue, May 31, 2016 at 11:53 PM, Mike Snitzer <snitzer@redhat.com> wrote:
> On Mon, May 30 2016 at  9:34am -0400,
> Ming Lei <ming.lei@canonical.com> wrote:
>
>>
>> Hi,
>>
>> Interests[1] have been shown in multipage bvecs, so this patchset
>> try to prepare for the support and do two things:
>>
>> 1) the 1st 4 patches use bvec iterator to implement iterate_bvec(),
>> then we can drop the non-standard way for iterating bvec, which
>> can be thought as a good cleanup for lib/iov_iter.c
>>
>> 2) remove BIO_MAX_SECTORS and makre BIO_MAX_SIZE as obsolete, and
>> now there is only one user for each. Once multipage bvecs is introduced,
>> one bio may hold lots of sectors, and we should always use sort of
>> BIO_MAX_VECS which should be introduced in future and is similiar
>> with current BIO_MAX_PAGES.
>>
>> The only functional change is iterate_bvec():lib/iov_iter.c
>>
>> xfstests(-a auto) over loop aio is run for ext4/xfs to verify
>> the change and no regression found with this patchset.
>>
>> V6:
>>       - rebased on v4.7-rc1
>>       - add reviewed-by tag
>>       - mark BIO_MAX_SIZE as obsolete instead of removing because
>>       dm-tree adds one usage now
>
> Not sure what you're referring to here with "dm-tree" (since "dm-tree"
> doesn't exist).  But only direct user of "BIO_MAX_SIZE" in DM appears to

Looks it is from crypto tree:

   git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6

> be dm-crypt.c.  Maybe you've identified some indirect use of
> BIO_MAX_SIZE?

I mean the recently introduced BIO_MAX_SIZE in -next tree:

https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/md/dm-crypt.c?id=4ed89c97b0706477b822ea2182827640c0cec486

> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]

#1411235

On Wed, Jun 01, 2016 at 08:38:41PM +0800, Ming Lei wrote:
> > be dm-crypt.c.  Maybe you've identified some indirect use of
> > BIO_MAX_SIZE?
> 
> I mean the recently introduced BIO_MAX_SIZE in -next tree:
> 
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/md/dm-crypt.c?id=4ed89c97b0706477b822ea2182827640c0cec486

The crazy bcache bios striking back once again.  I really think it's
harmful having a _MAX value and then having a minor driver
reinterpreting it and sending larger ones.  Until we can lift the
maximum limit in general nad have common code exercise it we really need
to stop bcache from sending these instead of littering the tree with
workarounds.

[toc] | [prev] | [next] | [standalone]

#1411241

On Wed, Jun 01 2016 at  9:44am -0400,
Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, Jun 01, 2016 at 08:38:41PM +0800, Ming Lei wrote:
> > > be dm-crypt.c.  Maybe you've identified some indirect use of
> > > BIO_MAX_SIZE?
> > 
> > I mean the recently introduced BIO_MAX_SIZE in -next tree:
> > 
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/md/dm-crypt.c?id=4ed89c97b0706477b822ea2182827640c0cec486
> 
> The crazy bcache bios striking back once again.  I really think it's
> harmful having a _MAX value and then having a minor driver
> reinterpreting it and sending larger ones.  Until we can lift the
> maximum limit in general nad have common code exercise it we really need
> to stop bcache from sending these instead of littering the tree with
> workarounds.

So should I not push this type of fix to Linus now?  I was going to send
the above commit and this one to him this week:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.7&id=57b3001b240629ecc5266d28c845e23ca5f11719

Instead, should bcache be made to not do what it is doing?

[toc] | [prev] | [next] | [standalone]

#1411257

On Wed, Jun 01, 2016 at 09:51:51AM -0400, Mike Snitzer wrote:
> So should I not push this type of fix to Linus now?  I was going to send
> the above commit and this one to him this week:
> https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.7&id=57b3001b240629ecc5266d28c845e23ca5f11719
> 
> Instead, should bcache be made to not do what it is doing?

I think that's much preferably.  Otherwise we'll get a constant trickles
of patches like that whenever someone tries bcache on top of another
stacking driver.

[toc] | [prev] | [next] | [standalone]

#1411765

On Wed, Jun 1, 2016 at 9:51 PM, Mike Snitzer <snitzer@redhat.com> wrote:
> On Wed, Jun 01 2016 at  9:44am -0400,
> Christoph Hellwig <hch@infradead.org> wrote:
>
>> On Wed, Jun 01, 2016 at 08:38:41PM +0800, Ming Lei wrote:
>> > > be dm-crypt.c.  Maybe you've identified some indirect use of
>> > > BIO_MAX_SIZE?
>> >
>> > I mean the recently introduced BIO_MAX_SIZE in -next tree:
>> >
>> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/md/dm-crypt.c?id=4ed89c97b0706477b822ea2182827640c0cec486
>>
>> The crazy bcache bios striking back once again.  I really think it's
>> harmful having a _MAX value and then having a minor driver
>> reinterpreting it and sending larger ones.  Until we can lift the
>> maximum limit in general nad have common code exercise it we really need
>> to stop bcache from sending these instead of littering the tree with
>> workarounds.
>
> So should I not push this type of fix to Linus now?  I was going to send
> the above commit and this one to him this week:
> https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.7&id=57b3001b240629ecc5266d28c845e23ca5f11719

I don't suggest this kind of fix.

That fix should be in bcache or blk-core instead of other places.
Last time, I sent out the following fix([PATCH v1] block: make sure big bio is
splitted into at most 256 bvecs):

     https://lkml.org/lkml/2016/4/5/1046

But forget to clear the flag of REQ_NOMERGE for this case.

If Jens doesn't mind doing such thing in block-core, I can post v2
for this issue.

>
> Instead, should bcache be made to not do what it is doing?

It depends on if bcache is the only user of this kind of usage.
In theory, any driver can use external bvec table.

Thanks,
Ming

[toc] | [prev] | [next] | [standalone]

#1411234

These patches look good on their own.  They might be an easier sell
just as bio cleanups :)

[toc] | [prev] | [next] | [standalone]

#1411244

On 06/01/2016 03:43 PM, Christoph Hellwig wrote:
> These patches look good on their own.  They might be an easier sell
> just as bio cleanups :)

Fully agree. I've seen (some) improvements with those patches, so
I'd prefer to have them.

You can add:

Tested-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[toc] | [prev] | [next] | [standalone]

#1411246

On Wed, Jun 01 2016 at  9:53am -0400,
Hannes Reinecke <hare@suse.de> wrote:

> On 06/01/2016 03:43 PM, Christoph Hellwig wrote:
> > These patches look good on their own.  They might be an easier sell
> > just as bio cleanups :)
> 
> Fully agree. I've seen (some) improvements with those patches, so
> I'd prefer to have them.

Can you be more specific on what improvements you've seen?

[toc] | [prev] | [next] | [standalone]

#1418457

On 05/30/2016 07:34 AM, Ming Lei wrote:
> Hi,
>
> Interests[1] have been shown in multipage bvecs, so this patchset
> try to prepare for the support and do two things:
>
> 1) the 1st 4 patches use bvec iterator to implement iterate_bvec(),
> then we can drop the non-standard way for iterating bvec, which
> can be thought as a good cleanup for lib/iov_iter.c
>
> 2) remove BIO_MAX_SECTORS and makre BIO_MAX_SIZE as obsolete, and
> now there is only one user for each. Once multipage bvecs is introduced,
> one bio may hold lots of sectors, and we should always use sort of
> BIO_MAX_VECS which should be introduced in future and is similiar
> with current BIO_MAX_PAGES.
>
> The only functional change is iterate_bvec():lib/iov_iter.c
>
> xfstests(-a auto) over loop aio is run for ext4/xfs to verify
> the change and no regression found with this patchset.

Added patches 1-7, still waiting for an answer on my reply to #8.

-- 
Jens Axboe

[toc] | [prev] | [next] | [standalone]

#1419001

On Fri, Jun 10, 2016 at 12:06 AM, Jens Axboe <axboe@kernel.dk> wrote:
> On 05/30/2016 07:34 AM, Ming Lei wrote:
>>
>> Hi,
>>
>> Interests[1] have been shown in multipage bvecs, so this patchset
>> try to prepare for the support and do two things:
>>
>> 1) the 1st 4 patches use bvec iterator to implement iterate_bvec(),
>> then we can drop the non-standard way for iterating bvec, which
>> can be thought as a good cleanup for lib/iov_iter.c
>>
>> 2) remove BIO_MAX_SECTORS and makre BIO_MAX_SIZE as obsolete, and
>> now there is only one user for each. Once multipage bvecs is introduced,
>> one bio may hold lots of sectors, and we should always use sort of
>> BIO_MAX_VECS which should be introduced in future and is similiar
>> with current BIO_MAX_PAGES.
>>
>> The only functional change is iterate_bvec():lib/iov_iter.c
>>
>> xfstests(-a auto) over loop aio is run for ext4/xfs to verify
>> the change and no regression found with this patchset.
>
>
> Added patches 1-7, still waiting for an answer on my reply to #8.

Thanks for adding patches 1-7, and now I just found the added
BIO_MAX_SIZE usage in dm-crypt.c is droped from next, so it is
safe to kill it now.

I will send a new patch #8 against for-4.8/core.

Thanks,
Ming

>
> --
> Jens Axboe
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web