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

Groups > linux.kernel > #1410386 > unrolled thread

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

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 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

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

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] | [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] | [standalone]

Back to top | Article view | linux.kernel

csiph-web