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

Groups > linux.kernel > #1371830 > unrolled thread

[PATCH] block: make sure big bio is splitted into at most 256 bvecs

Back to article view | Back to linux.kernel

Contents

  [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-05 19:50 +0200
    Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Shaohua Li <shli@fb.com> - 2016-04-05 20:30 +0200
      Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Kent Overstreet <kent.overstreet@gmail.com> - 2016-04-06 02:30 +0200
        Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Kent Overstreet <kent.overstreet@gmail.com> - 2016-04-06 02:40 +0200
          Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Kent Overstreet <kent.overstreet@gmail.com> - 2016-04-06 02:50 +0200
            Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-06 03:10 +0200
            Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Shaohua Li <shli@fb.com> - 2016-04-06 03:10 +0200
          Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Shaohua Li <shli@fb.com> - 2016-04-06 02:50 +0200
        Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Shaohua Li <shli@fb.com> - 2016-04-06 02:40 +0200
      Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-06 02:50 +0200
        Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Shaohua Li <shli@fb.com> - 2016-04-06 03:10 +0200
          Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-06 03:20 +0200
    Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Kent Overstreet <kent.overstreet@gmail.com> - 2016-04-06 02:40 +0200
      Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-06 03:00 +0200
        Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Kent Overstreet <kent.overstreet@gmail.com> - 2016-04-06 03:20 +0200
          Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-06 03:30 +0200
            Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Kent Overstreet <kent.overstreet@gmail.com> - 2016-04-06 03:30 +0200
              Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-06 04:00 +0200
                Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Kent Overstreet <kent.overstreet@gmail.com> - 2016-04-06 04:30 +0200
                  Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-06 04:40 +0200
                    Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Kent Overstreet <kent.overstreet@gmail.com> - 2016-04-06 04:50 +0200
                      Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Kent Overstreet <kent.overstreet@gmail.com> - 2016-04-06 05:00 +0200
                      Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-06 05:00 +0200
                  Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Kent Overstreet <kent.overstreet@gmail.com> - 2016-04-06 04:40 +0200
                  Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-06 04:40 +0200
    Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-06 03:10 +0200
      Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Eric Wheeler <bcache@lists.ewheeler.net> - 2016-04-07 03:50 +0200
    Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Eric Wheeler <bcache@lists.ewheeler.net> - 2016-04-07 03:40 +0200
      Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-07 03:50 +0200
        Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Eric Wheeler <bcache@lists.ewheeler.net> - 2016-04-07 04:00 +0200
          Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-07 04:20 +0200
            Re: [PATCH] block: make sure big bio is splitted into at most 256  bvecs Eric Wheeler <bcache@lists.ewheeler.net> - 2016-04-08 01:30 +0200
              Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs Ming Lei <ming.lei@canonical.com> - 2016-04-08 02:30 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1372155 — Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs

On Wed, Apr 06, 2016 at 10:37:05AM +0800, Ming Lei wrote:
> On Wed, Apr 6, 2016 at 10:34 AM, Kent Overstreet
> <kent.overstreet@gmail.com> wrote:
> > On Wed, Apr 06, 2016 at 10:30:22AM +0800, Ming Lei wrote:
> >> Now limit.max_segments is for limiting segments from hw view, one this
> >> segment may include lots of pages/bvecs.
> >>
> >> The current bio_clone() issue is that we can't clone from one bio which
> >> includes more than 256 bvecs, maybe all these 256 bvecs belong to
> >> one same hw segment.
> >
> > So the distinction is purely a post multipage bvec thing?
> 
> Even after multipage bvec is applied, the limit for max bvecs is still needed
> for some cases like bio bounce, in which bio_clone() need to
> clone single page bvec.

s/max bvecs/max pages/?

What I meant is that until we have multipage bvecs, unless I'm missing something
max_segments is exactly what we want. After multipage bvecs, things do get more
complicated I agree.

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

#1372158 — Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs

On Wed, Apr 06, 2016 at 10:51:40AM +0800, Ming Lei wrote:
> On Wed, Apr 6, 2016 at 10:40 AM, Kent Overstreet
> <kent.overstreet@gmail.com> wrote:
> > On Wed, Apr 06, 2016 at 10:37:05AM +0800, Ming Lei wrote:
> >> On Wed, Apr 6, 2016 at 10:34 AM, Kent Overstreet
> >> <kent.overstreet@gmail.com> wrote:
> >> > On Wed, Apr 06, 2016 at 10:30:22AM +0800, Ming Lei wrote:
> >> >> Now limit.max_segments is for limiting segments from hw view, one this
> >> >> segment may include lots of pages/bvecs.
> >> >>
> >> >> The current bio_clone() issue is that we can't clone from one bio which
> >> >> includes more than 256 bvecs, maybe all these 256 bvecs belong to
> >> >> one same hw segment.
> >> >
> >> > So the distinction is purely a post multipage bvec thing?
> >>
> >> Even after multipage bvec is applied, the limit for max bvecs is still needed
> >> for some cases like bio bounce, in which bio_clone() need to
> >> clone single page bvec.
> >
> > s/max bvecs/max pages/?
> 
> Exactly, :-)
> 
> >
> > What I meant is that until we have multipage bvecs, unless I'm missing something
> > max_segments is exactly what we want. After multipage bvecs, things do get more
> 
> Yes, but now we need to fix current issue and backport the fix. Given bio
> bounce isn't easy to fix, not like other users of bio_clone, I think
> we still need
> to apply the max pages limit globally, what do you think of it?

Ugh, yeah bouncing is an issue, with that it's most of the drivers.

Ok, yeah I guess the global limit makes sense for now.

Please add a giant comment explaining why the global limit is there, precisely
what it's needed for and that we'd like to get rid of it.

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

#1372160

On Wed, Apr 6, 2016 at 10:40 AM, Kent Overstreet
<kent.overstreet@gmail.com> wrote:
> On Wed, Apr 06, 2016 at 10:37:05AM +0800, Ming Lei wrote:
>> On Wed, Apr 6, 2016 at 10:34 AM, Kent Overstreet
>> <kent.overstreet@gmail.com> wrote:
>> > On Wed, Apr 06, 2016 at 10:30:22AM +0800, Ming Lei wrote:
>> >> Now limit.max_segments is for limiting segments from hw view, one this
>> >> segment may include lots of pages/bvecs.
>> >>
>> >> The current bio_clone() issue is that we can't clone from one bio which
>> >> includes more than 256 bvecs, maybe all these 256 bvecs belong to
>> >> one same hw segment.
>> >
>> > So the distinction is purely a post multipage bvec thing?
>>
>> Even after multipage bvec is applied, the limit for max bvecs is still needed
>> for some cases like bio bounce, in which bio_clone() need to
>> clone single page bvec.
>
> s/max bvecs/max pages/?

Exactly, :-)

>
> What I meant is that until we have multipage bvecs, unless I'm missing something
> max_segments is exactly what we want. After multipage bvecs, things do get more

Yes, but now we need to fix current issue and backport the fix. Given bio
bounce isn't easy to fix, not like other users of bio_clone, I think
we still need
to apply the max pages limit globally, what do you think of it?

For multipage bvecs, there may be a long way ahead, :-)

thanks,
Ming

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

#1372152 — Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs

On Wed, Apr 06, 2016 at 10:30:22AM +0800, Ming Lei wrote:
> Now limit.max_segments is for limiting segments from hw view, one this
> segment may include lots of pages/bvecs.
> 
> The current bio_clone() issue is that we can't clone from one bio which
> includes more than 256 bvecs, maybe all these 256 bvecs belong to
> one same hw segment.

So the distinction is purely a post multipage bvec thing?

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

#1372153

On Wed, Apr 6, 2016 at 10:22 AM, Kent Overstreet
<kent.overstreet@gmail.com> wrote:
> On Wed, Apr 06, 2016 at 09:51:02AM +0800, Ming Lei wrote:
>> On Wed, Apr 6, 2016 at 9:28 AM, Kent Overstreet
>> <kent.overstreet@gmail.com> wrote:
>> > On Wed, Apr 06, 2016 at 09:20:59AM +0800, Ming Lei wrote:
>> >> On Wed, Apr 6, 2016 at 9:10 AM, Kent Overstreet
>> >> <kent.overstreet@gmail.com> wrote:
>> >> > On Wed, Apr 06, 2016 at 08:59:31AM +0800, Ming Lei wrote:
>> >> >> On Wed, Apr 6, 2016 at 8:30 AM, Kent Overstreet
>> >> >> <kent.overstreet@gmail.com> wrote:
>> >> >> > On Wed, Apr 06, 2016 at 01:44:06AM +0800, Ming Lei wrote:
>> >> >> >> After arbitrary bio size is supported, the incoming bio may
>> >> >> >> be very big. We have to split the bio into small bios so that
>> >> >> >> each holds at most BIO_MAX_PAGES bvecs for safety reason, such
>> >> >> >> as bio_clone().
>> >> >> >>
>> >> >> >> This patch fixes the following kernel crash:
>> >> >> >
>> >> >> > Ming, let's not do it this way; drivers that don't clone biovecs are the norm -
>> >> >> > instead, md has its own queue limits that it ought to be setting up correctly.
>> >> >>
>> >> >> Except for md, there are also several usages of bio_clone:
>> >> >>
>> >> >>          - drbd
>> >> >>          - osdblk
>> >> >>          - pktcdvd
>> >> >>          - xen-blkfront
>> >> >>          - verify code of bcache
>> >> >>
>> >> >> I don't like bio_clone() too, which can cause trouble to multipage bvecs.
>> >> >>
>> >> >> How about fixing the issue by this simple patch first? Then once we limits
>> >> >> all above queues by max sectors, the global limit can be removed as
>> >> >> mentioned by the comment.
>> >> >
>> >> > just do this:
>> >> >
>> >> > void blk_set_limit_clonable(struct queue_limits *lim)
>> >> > {
>> >> >         lim->max_segments = min(lim->max_segments, BIO_MAX_PAGES);
>> >> > }
>> >>
>> >> As I memtioned it is __not__ correct to use max_segments, and the issue is
>> >> related with max sectors, please see the code of bio_clone_bioset():
>> >
>> > I know how bio_clone_bioset() works but I'm not seeing how that has anything to
>> > do with max sectors. The way it copies the biovec is not going to merge
>> > segments, if the original bio had non full page segments then so is the clone.
>>
>> OK, I see, now it is a totally new limit, and no current queue limit can fit
>> the purpose.
>>
>> Looks we need to introduce the new limit of io_max_vecs, which can be
>> applied into blk_bio_segment_split().
>>
>> But a queue flag should be better than queue limit since it is a 'limit' from
>> software/driver.
>
> Why is max_segments not appropriate?

Now limit.max_segments is for limiting segments from hw view, one this
segment may include lots of pages/bvecs.

The current bio_clone() issue is that we can't clone from one bio which
includes more than 256 bvecs, maybe all these 256 bvecs belong to
one same hw segment.


Thanks,
Ming

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

#1372114

On Wed, Apr 6, 2016 at 1:44 AM, Ming Lei <ming.lei@canonical.com> wrote:
> After arbitrary bio size is supported, the incoming bio may
> be very big. We have to split the bio into small bios so that
> each holds at most BIO_MAX_PAGES bvecs for safety reason, such
> as bio_clone().
>
> This patch fixes the following kernel crash:
>
>> [  172.660142] BUG: unable to handle kernel NULL pointer dereference at
>> 0000000000000028
>> [  172.660229] IP: [<ffffffff811e53b4>] bio_trim+0xf/0x2a
>> [  172.660289] PGD 7faf3e067 PUD 7f9279067 PMD 0
>> [  172.660399] Oops: 0000 [#1] SMP
>> [...]
>> [  172.664780] Call Trace:
>> [  172.664813]  [<ffffffffa007f3be>] ? raid1_make_request+0x2e8/0xad7 [raid1]
>> [  172.664846]  [<ffffffff811f07da>] ? blk_queue_split+0x377/0x3d4
>> [  172.664880]  [<ffffffffa005fb5f>] ? md_make_request+0xf6/0x1e9 [md_mod]
>> [  172.664912]  [<ffffffff811eb860>] ? generic_make_request+0xb5/0x155
>> [  172.664947]  [<ffffffffa0445c89>] ? prio_io+0x85/0x95 [bcache]
>> [  172.664981]  [<ffffffffa0448252>] ? register_cache_set+0x355/0x8d0 [bcache]
>> [  172.665016]  [<ffffffffa04497d3>] ? register_bcache+0x1006/0x1174 [bcache]
>
> Fixes: 54efd50(block: make generic_make_request handle arbitrarily sized bios)
> Reported-by: Sebastian Roesner <sroesner-kernelorg@roesner-online.de>
> Reported-by: Eric Wheeler <bcache@lists.ewheeler.net>
> Cc: stable@vger.kernel.org (4.2+)
> Cc: Shaohua Li <shli@fb.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
> I can reproduce the issue and verify the fix by the following approach:
>         - create one raid1 over two virtio-blk
>         - build bcache device over the above raid1 and another cache device.
>         - set cache mode as writeback
>         - run random write over ext4 on the bcache device
>         - then the crash can be triggered

For anyone who is interested in issue/fix, forget to mention:

       The bucket size should be set as bigger than 1M during making bcache.
       In my test, the bucket size is 2M.

Thanks,
Ming

>
>  block/blk-merge.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 2613531..9a8651f 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -79,6 +79,18 @@ static inline unsigned get_max_io_size(struct request_queue *q,
>         /* aligned to logical block size */
>         sectors &= ~(mask >> 9);
>
> +       /*
> +        * With arbitrary bio size, the incoming bio may be very big.
> +        * We have to split the bio into small bios so that each holds
> +        * at most BIO_MAX_PAGES bvecs for safety reason, such as
> +        * bio_clone().
> +        *
> +        * In the future, the limit might be converted into per-queue
> +        * flag.
> +        */
> +       sectors = min_t(unsigned, sectors, BIO_MAX_PAGES <<
> +                       (PAGE_CACHE_SHIFT - 9));
> +
>         return sectors;
>  }
>
> --
> 1.9.1
>

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

#1373009 — Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs

On Wed, 6 Apr 2016, Ming Lei wrote:

> On Wed, Apr 6, 2016 at 1:44 AM, Ming Lei <ming.lei@canonical.com> wrote:
> > After arbitrary bio size is supported, the incoming bio may
> > be very big. We have to split the bio into small bios so that
> > each holds at most BIO_MAX_PAGES bvecs for safety reason, such
> > as bio_clone().
> >
> > This patch fixes the following kernel crash:
> >
> >> [  172.660142] BUG: unable to handle kernel NULL pointer dereference at
> >> 0000000000000028
> >> [  172.660229] IP: [<ffffffff811e53b4>] bio_trim+0xf/0x2a
> >> [  172.660289] PGD 7faf3e067 PUD 7f9279067 PMD 0
> >> [  172.660399] Oops: 0000 [#1] SMP
> >> [...]
> >> [  172.664780] Call Trace:
> >> [  172.664813]  [<ffffffffa007f3be>] ? raid1_make_request+0x2e8/0xad7 [raid1]
> >> [  172.664846]  [<ffffffff811f07da>] ? blk_queue_split+0x377/0x3d4
> >> [  172.664880]  [<ffffffffa005fb5f>] ? md_make_request+0xf6/0x1e9 [md_mod]
> >> [  172.664912]  [<ffffffff811eb860>] ? generic_make_request+0xb5/0x155
> >> [  172.664947]  [<ffffffffa0445c89>] ? prio_io+0x85/0x95 [bcache]
> >> [  172.664981]  [<ffffffffa0448252>] ? register_cache_set+0x355/0x8d0 [bcache]
> >> [  172.665016]  [<ffffffffa04497d3>] ? register_bcache+0x1006/0x1174 [bcache]
> >
> > Fixes: 54efd50(block: make generic_make_request handle arbitrarily sized bios)
> > Reported-by: Sebastian Roesner <sroesner-kernelorg@roesner-online.de>
> > Reported-by: Eric Wheeler <bcache@lists.ewheeler.net>
> > Cc: stable@vger.kernel.org (4.2+)
> > Cc: Shaohua Li <shli@fb.com>
> > Signed-off-by: Ming Lei <ming.lei@canonical.com>
> > ---
> > I can reproduce the issue and verify the fix by the following approach:
> >         - create one raid1 over two virtio-blk
> >         - build bcache device over the above raid1 and another cache device.
> >         - set cache mode as writeback
> >         - run random write over ext4 on the bcache device
> >         - then the crash can be triggered
> 
> For anyone who is interested in issue/fix, forget to mention:
> 
>        The bucket size should be set as bigger than 1M during making bcache.
>        In my test, the bucket size is 2M.

Does the bucket size dictate the ideal cached data size, or is it just an 
optimization for erase block boundaries on the SSD?  

Are reads/writes smaller than the bucket size still cached effectively, or 
does a 2MB bucket slurp up 2MB of backing data along with it?  

For example, if 64k is our ideal IO size, should we use 64k buckets?

--
Eric Wheeler

> 
> Thanks,
> Ming
> 
> >
> >  block/blk-merge.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/block/blk-merge.c b/block/blk-merge.c
> > index 2613531..9a8651f 100644
> > --- a/block/blk-merge.c
> > +++ b/block/blk-merge.c
> > @@ -79,6 +79,18 @@ static inline unsigned get_max_io_size(struct request_queue *q,
> >         /* aligned to logical block size */
> >         sectors &= ~(mask >> 9);
> >
> > +       /*
> > +        * With arbitrary bio size, the incoming bio may be very big.
> > +        * We have to split the bio into small bios so that each holds
> > +        * at most BIO_MAX_PAGES bvecs for safety reason, such as
> > +        * bio_clone().
> > +        *
> > +        * In the future, the limit might be converted into per-queue
> > +        * flag.
> > +        */
> > +       sectors = min_t(unsigned, sectors, BIO_MAX_PAGES <<
> > +                       (PAGE_CACHE_SHIFT - 9));
> > +
> >         return sectors;
> >  }
> >
> > --
> > 1.9.1
> >
> 

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

#1373005 — Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs

On Wed, 6 Apr 2016, Ming Lei wrote:

> After arbitrary bio size is supported, the incoming bio may
> be very big. We have to split the bio into small bios so that
> each holds at most BIO_MAX_PAGES bvecs for safety reason, such
> as bio_clone().
> 
> This patch fixes the following kernel crash:
> 
> > [  172.660142] BUG: unable to handle kernel NULL pointer dereference at
> > 0000000000000028
> > [  172.660229] IP: [<ffffffff811e53b4>] bio_trim+0xf/0x2a
> > [  172.660289] PGD 7faf3e067 PUD 7f9279067 PMD 0
> > [  172.660399] Oops: 0000 [#1] SMP
> > [...]
> > [  172.664780] Call Trace:
> > [  172.664813]  [<ffffffffa007f3be>] ? raid1_make_request+0x2e8/0xad7 [raid1]
> > [  172.664846]  [<ffffffff811f07da>] ? blk_queue_split+0x377/0x3d4
> > [  172.664880]  [<ffffffffa005fb5f>] ? md_make_request+0xf6/0x1e9 [md_mod]
> > [  172.664912]  [<ffffffff811eb860>] ? generic_make_request+0xb5/0x155
> > [  172.664947]  [<ffffffffa0445c89>] ? prio_io+0x85/0x95 [bcache]
> > [  172.664981]  [<ffffffffa0448252>] ? register_cache_set+0x355/0x8d0 [bcache]
> > [  172.665016]  [<ffffffffa04497d3>] ? register_bcache+0x1006/0x1174 [bcache]
> 
> Fixes: 54efd50(block: make generic_make_request handle arbitrarily sized bios)
> Reported-by: Sebastian Roesner <sroesner-kernelorg@roesner-online.de>
> Reported-by: Eric Wheeler <bcache@lists.ewheeler.net>
> Cc: stable@vger.kernel.org (4.2+)

Ming Lei,

get_max_io_size doesn't appear until 4.5 based on a quick LXR check so we 
won't see it in stable I don't think.

It would be nice to see this fixed in 4.1 (if affected there).  Is there 
another place this could be applied to be a bit more backward compatible?

-Eric


--
Eric Wheeler

	

> Cc: Shaohua Li <shli@fb.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
> I can reproduce the issue and verify the fix by the following approach:
> 	- create one raid1 over two virtio-blk 
> 	- build bcache device over the above raid1 and another cache device.
> 	- set cache mode as writeback
> 	- run random write over ext4 on the bcache device
> 	- then the crash can be triggered
> 
>  block/blk-merge.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 2613531..9a8651f 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -79,6 +79,18 @@ static inline unsigned get_max_io_size(struct request_queue *q,
>  	/* aligned to logical block size */
>  	sectors &= ~(mask >> 9);
>  
> +	/*
> +	 * With arbitrary bio size, the incoming bio may be very big.
> +	 * We have to split the bio into small bios so that each holds
> +	 * at most BIO_MAX_PAGES bvecs for safety reason, such as
> +	 * bio_clone().
> +	 *
> +	 * In the future, the limit might be converted into per-queue
> +	 * flag.
> +	 */
> +	sectors = min_t(unsigned, sectors, BIO_MAX_PAGES <<
> +			(PAGE_CACHE_SHIFT - 9));
> +
>  	return sectors;
>  }
>  
> -- 
> 1.9.1
> 
> 

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

#1373011

On Thu, Apr 7, 2016 at 9:36 AM, Eric Wheeler <bcache@lists.ewheeler.net> wrote:
> On Wed, 6 Apr 2016, Ming Lei wrote:
>
>> After arbitrary bio size is supported, the incoming bio may
>> be very big. We have to split the bio into small bios so that
>> each holds at most BIO_MAX_PAGES bvecs for safety reason, such
>> as bio_clone().
>>
>> This patch fixes the following kernel crash:
>>
>> > [  172.660142] BUG: unable to handle kernel NULL pointer dereference at
>> > 0000000000000028
>> > [  172.660229] IP: [<ffffffff811e53b4>] bio_trim+0xf/0x2a
>> > [  172.660289] PGD 7faf3e067 PUD 7f9279067 PMD 0
>> > [  172.660399] Oops: 0000 [#1] SMP
>> > [...]
>> > [  172.664780] Call Trace:
>> > [  172.664813]  [<ffffffffa007f3be>] ? raid1_make_request+0x2e8/0xad7 [raid1]
>> > [  172.664846]  [<ffffffff811f07da>] ? blk_queue_split+0x377/0x3d4
>> > [  172.664880]  [<ffffffffa005fb5f>] ? md_make_request+0xf6/0x1e9 [md_mod]
>> > [  172.664912]  [<ffffffff811eb860>] ? generic_make_request+0xb5/0x155
>> > [  172.664947]  [<ffffffffa0445c89>] ? prio_io+0x85/0x95 [bcache]
>> > [  172.664981]  [<ffffffffa0448252>] ? register_cache_set+0x355/0x8d0 [bcache]
>> > [  172.665016]  [<ffffffffa04497d3>] ? register_bcache+0x1006/0x1174 [bcache]
>>
>> Fixes: 54efd50(block: make generic_make_request handle arbitrarily sized bios)
>> Reported-by: Sebastian Roesner <sroesner-kernelorg@roesner-online.de>
>> Reported-by: Eric Wheeler <bcache@lists.ewheeler.net>
>> Cc: stable@vger.kernel.org (4.2+)
>
> Ming Lei,
>
> get_max_io_size doesn't appear until 4.5 based on a quick LXR check so we
> won't see it in stable I don't think.
>
> It would be nice to see this fixed in 4.1 (if affected there).  Is there

The issue should be introduced to v4.3 via 54efd50

> another place this could be applied to be a bit more backward compatible?

The v1 needn't change to get_max_io_size(), and it should be simple enough
to backport to previous stables, please try it:

http://marc.info/?l=linux-block&m=145991422422927&w=2

Thanks,

>
>> Cc: Shaohua Li <shli@fb.com>
>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> ---
>> I can reproduce the issue and verify the fix by the following approach:
>>       - create one raid1 over two virtio-blk
>>       - build bcache device over the above raid1 and another cache device.
>>       - set cache mode as writeback
>>       - run random write over ext4 on the bcache device
>>       - then the crash can be triggered
>>
>>  block/blk-merge.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/block/blk-merge.c b/block/blk-merge.c
>> index 2613531..9a8651f 100644
>> --- a/block/blk-merge.c
>> +++ b/block/blk-merge.c
>> @@ -79,6 +79,18 @@ static inline unsigned get_max_io_size(struct request_queue *q,
>>       /* aligned to logical block size */
>>       sectors &= ~(mask >> 9);
>>
>> +     /*
>> +      * With arbitrary bio size, the incoming bio may be very big.
>> +      * We have to split the bio into small bios so that each holds
>> +      * at most BIO_MAX_PAGES bvecs for safety reason, such as
>> +      * bio_clone().
>> +      *
>> +      * In the future, the limit might be converted into per-queue
>> +      * flag.
>> +      */
>> +     sectors = min_t(unsigned, sectors, BIO_MAX_PAGES <<
>> +                     (PAGE_CACHE_SHIFT - 9));
>> +
>>       return sectors;
>>  }
>>
>> --
>> 1.9.1
>>
>>
> --
> 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]

#1373021 — Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs

On Thu, 7 Apr 2016, Ming Lei wrote:

> On Thu, Apr 7, 2016 at 9:36 AM, Eric Wheeler <bcache@lists.ewheeler.net> wrote:
> > On Wed, 6 Apr 2016, Ming Lei wrote:
> >
> >> After arbitrary bio size is supported, the incoming bio may
> >> be very big. We have to split the bio into small bios so that
> >> each holds at most BIO_MAX_PAGES bvecs for safety reason, such
> >> as bio_clone().
> >>
> >> This patch fixes the following kernel crash:
> >>
> >> > [  172.660142] BUG: unable to handle kernel NULL pointer dereference at
> >> > 0000000000000028
> >> > [  172.660229] IP: [<ffffffff811e53b4>] bio_trim+0xf/0x2a
> >> > [  172.660289] PGD 7faf3e067 PUD 7f9279067 PMD 0
> >> > [  172.660399] Oops: 0000 [#1] SMP
> >> > [...]
> >> > [  172.664780] Call Trace:
> >> > [  172.664813]  [<ffffffffa007f3be>] ? raid1_make_request+0x2e8/0xad7 [raid1]
> >> > [  172.664846]  [<ffffffff811f07da>] ? blk_queue_split+0x377/0x3d4
> >> > [  172.664880]  [<ffffffffa005fb5f>] ? md_make_request+0xf6/0x1e9 [md_mod]
> >> > [  172.664912]  [<ffffffff811eb860>] ? generic_make_request+0xb5/0x155
> >> > [  172.664947]  [<ffffffffa0445c89>] ? prio_io+0x85/0x95 [bcache]
> >> > [  172.664981]  [<ffffffffa0448252>] ? register_cache_set+0x355/0x8d0 [bcache]
> >> > [  172.665016]  [<ffffffffa04497d3>] ? register_bcache+0x1006/0x1174 [bcache]
> >>
> >> Fixes: 54efd50(block: make generic_make_request handle arbitrarily sized bios)
> >> Reported-by: Sebastian Roesner <sroesner-kernelorg@roesner-online.de>
> >> Reported-by: Eric Wheeler <bcache@lists.ewheeler.net>
> >> Cc: stable@vger.kernel.org (4.2+)
> >
> > Ming Lei,
> >
> > get_max_io_size doesn't appear until 4.5 based on a quick LXR check so we
> > won't see it in stable I don't think.
> >
> > It would be nice to see this fixed in 4.1 (if affected there).  Is there
> 
> The issue should be introduced to v4.3 via 54efd50
> 
> > another place this could be applied to be a bit more backward compatible?
> 
> The v1 needn't change to get_max_io_size(), and it should be simple enough
> to backport to previous stables, please try it:
> 
> http://marc.info/?l=linux-block&m=145991422422927&w=2

V1 changes blk_bio_segment_split() which doesn't exist until 4.3.

How might you port this to v4.1.y?

--
Eric Wheeler


> 
> Thanks,
> 
> >
> >> Cc: Shaohua Li <shli@fb.com>
> >> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> >> ---
> >> I can reproduce the issue and verify the fix by the following approach:
> >>       - create one raid1 over two virtio-blk
> >>       - build bcache device over the above raid1 and another cache device.
> >>       - set cache mode as writeback
> >>       - run random write over ext4 on the bcache device
> >>       - then the crash can be triggered
> >>
> >>  block/blk-merge.c | 12 ++++++++++++
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/block/blk-merge.c b/block/blk-merge.c
> >> index 2613531..9a8651f 100644
> >> --- a/block/blk-merge.c
> >> +++ b/block/blk-merge.c
> >> @@ -79,6 +79,18 @@ static inline unsigned get_max_io_size(struct request_queue *q,
> >>       /* aligned to logical block size */
> >>       sectors &= ~(mask >> 9);
> >>
> >> +     /*
> >> +      * With arbitrary bio size, the incoming bio may be very big.
> >> +      * We have to split the bio into small bios so that each holds
> >> +      * at most BIO_MAX_PAGES bvecs for safety reason, such as
> >> +      * bio_clone().
> >> +      *
> >> +      * In the future, the limit might be converted into per-queue
> >> +      * flag.
> >> +      */
> >> +     sectors = min_t(unsigned, sectors, BIO_MAX_PAGES <<
> >> +                     (PAGE_CACHE_SHIFT - 9));
> >> +
> >>       return sectors;
> >>  }
> >>
> >> --
> >> 1.9.1
> >>
> >>
> > --
> > 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]

#1373024

On Thu, Apr 7, 2016 at 9:56 AM, Eric Wheeler <bcache@lists.ewheeler.net> wrote:
> On Thu, 7 Apr 2016, Ming Lei wrote:
>
>> On Thu, Apr 7, 2016 at 9:36 AM, Eric Wheeler <bcache@lists.ewheeler.net> wrote:
>> > On Wed, 6 Apr 2016, Ming Lei wrote:
>> >
>> >> After arbitrary bio size is supported, the incoming bio may
>> >> be very big. We have to split the bio into small bios so that
>> >> each holds at most BIO_MAX_PAGES bvecs for safety reason, such
>> >> as bio_clone().
>> >>
>> >> This patch fixes the following kernel crash:
>> >>
>> >> > [  172.660142] BUG: unable to handle kernel NULL pointer dereference at
>> >> > 0000000000000028
>> >> > [  172.660229] IP: [<ffffffff811e53b4>] bio_trim+0xf/0x2a
>> >> > [  172.660289] PGD 7faf3e067 PUD 7f9279067 PMD 0
>> >> > [  172.660399] Oops: 0000 [#1] SMP
>> >> > [...]
>> >> > [  172.664780] Call Trace:
>> >> > [  172.664813]  [<ffffffffa007f3be>] ? raid1_make_request+0x2e8/0xad7 [raid1]
>> >> > [  172.664846]  [<ffffffff811f07da>] ? blk_queue_split+0x377/0x3d4
>> >> > [  172.664880]  [<ffffffffa005fb5f>] ? md_make_request+0xf6/0x1e9 [md_mod]
>> >> > [  172.664912]  [<ffffffff811eb860>] ? generic_make_request+0xb5/0x155
>> >> > [  172.664947]  [<ffffffffa0445c89>] ? prio_io+0x85/0x95 [bcache]
>> >> > [  172.664981]  [<ffffffffa0448252>] ? register_cache_set+0x355/0x8d0 [bcache]
>> >> > [  172.665016]  [<ffffffffa04497d3>] ? register_bcache+0x1006/0x1174 [bcache]
>> >>
>> >> Fixes: 54efd50(block: make generic_make_request handle arbitrarily sized bios)
>> >> Reported-by: Sebastian Roesner <sroesner-kernelorg@roesner-online.de>
>> >> Reported-by: Eric Wheeler <bcache@lists.ewheeler.net>
>> >> Cc: stable@vger.kernel.org (4.2+)
>> >
>> > Ming Lei,
>> >
>> > get_max_io_size doesn't appear until 4.5 based on a quick LXR check so we
>> > won't see it in stable I don't think.
>> >
>> > It would be nice to see this fixed in 4.1 (if affected there).  Is there
>>
>> The issue should be introduced to v4.3 via 54efd50
>>
>> > another place this could be applied to be a bit more backward compatible?
>>
>> The v1 needn't change to get_max_io_size(), and it should be simple enough
>> to backport to previous stables, please try it:
>>
>> http://marc.info/?l=linux-block&m=145991422422927&w=2
>
> V1 changes blk_bio_segment_split() which doesn't exist until 4.3.
>
> How might you port this to v4.1.y?

Can you see the issue with v4.1?

You mentioned there are three reports:

> [2014-02-04] https://bugzilla.redhat.com/show_bug.cgi?id=1061339
> [2016-01-13] http://www.spinics.net/lists/linux-bcache/msg03335.html
             https://bugzilla.kernel.org/show_bug.cgi?id=110771
> [2016-03-25] http://thread.gmane.org/gmane.linux.kernel.bcache.devel/3607 [this thread]
             https://bugzilla.kernel.org/show_bug.cgi?id=114871

The first one has been fixed by '8423ae3 block: Fix cloning of
discard/write same bios', as mentioned in bugzilla.

The other two are reported on v4.4 and v4.5.

If you look at the patch description, it is just needed for 4.3+.

Or I am wrong?


Thanks,

>
> --
> Eric Wheeler
>
>
>>
>> Thanks,
>>
>> >
>> >> Cc: Shaohua Li <shli@fb.com>
>> >> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> >> ---
>> >> I can reproduce the issue and verify the fix by the following approach:
>> >>       - create one raid1 over two virtio-blk
>> >>       - build bcache device over the above raid1 and another cache device.
>> >>       - set cache mode as writeback
>> >>       - run random write over ext4 on the bcache device
>> >>       - then the crash can be triggered
>> >>
>> >>  block/blk-merge.c | 12 ++++++++++++
>> >>  1 file changed, 12 insertions(+)
>> >>
>> >> diff --git a/block/blk-merge.c b/block/blk-merge.c
>> >> index 2613531..9a8651f 100644
>> >> --- a/block/blk-merge.c
>> >> +++ b/block/blk-merge.c
>> >> @@ -79,6 +79,18 @@ static inline unsigned get_max_io_size(struct request_queue *q,
>> >>       /* aligned to logical block size */
>> >>       sectors &= ~(mask >> 9);
>> >>
>> >> +     /*
>> >> +      * With arbitrary bio size, the incoming bio may be very big.
>> >> +      * We have to split the bio into small bios so that each holds
>> >> +      * at most BIO_MAX_PAGES bvecs for safety reason, such as
>> >> +      * bio_clone().
>> >> +      *
>> >> +      * In the future, the limit might be converted into per-queue
>> >> +      * flag.
>> >> +      */
>> >> +     sectors = min_t(unsigned, sectors, BIO_MAX_PAGES <<
>> >> +                     (PAGE_CACHE_SHIFT - 9));
>> >> +
>> >>       return sectors;
>> >>  }
>> >>
>> >> --
>> >> 1.9.1
>> >>
>> >>
>> > --
>> > 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]

#1373830 — Re: [PATCH] block: make sure big bio is splitted into at most 256 bvecs

On Thu, 7 Apr 2016, Ming Lei wrote:

> On Thu, Apr 7, 2016 at 9:56 AM, Eric Wheeler <bcache@lists.ewheeler.net> wrote:
> > On Thu, 7 Apr 2016, Ming Lei wrote:
> >
> >> On Thu, Apr 7, 2016 at 9:36 AM, Eric Wheeler <bcache@lists.ewheeler.net> wrote:
> >> > On Wed, 6 Apr 2016, Ming Lei wrote:
> >> >
> >> >> After arbitrary bio size is supported, the incoming bio may
> >> >> be very big. We have to split the bio into small bios so that
> >> >> each holds at most BIO_MAX_PAGES bvecs for safety reason, such
> >> >> as bio_clone().
> >> >>
> >> >> This patch fixes the following kernel crash:
> >> >>
> >> >> > [  172.660142] BUG: unable to handle kernel NULL pointer dereference at
> >> >> > 0000000000000028
> >> >> > [  172.660229] IP: [<ffffffff811e53b4>] bio_trim+0xf/0x2a
> >> >> > [  172.660289] PGD 7faf3e067 PUD 7f9279067 PMD 0
> >> >> > [  172.660399] Oops: 0000 [#1] SMP
> >> >> > [...]
> >> >> > [  172.664780] Call Trace:
> >> >> > [  172.664813]  [<ffffffffa007f3be>] ? raid1_make_request+0x2e8/0xad7 [raid1]
> >> >> > [  172.664846]  [<ffffffff811f07da>] ? blk_queue_split+0x377/0x3d4
> >> >> > [  172.664880]  [<ffffffffa005fb5f>] ? md_make_request+0xf6/0x1e9 [md_mod]
> >> >> > [  172.664912]  [<ffffffff811eb860>] ? generic_make_request+0xb5/0x155
> >> >> > [  172.664947]  [<ffffffffa0445c89>] ? prio_io+0x85/0x95 [bcache]
> >> >> > [  172.664981]  [<ffffffffa0448252>] ? register_cache_set+0x355/0x8d0 [bcache]
> >> >> > [  172.665016]  [<ffffffffa04497d3>] ? register_bcache+0x1006/0x1174 [bcache]
> >> >>
> >> >> Fixes: 54efd50(block: make generic_make_request handle arbitrarily sized bios)
> >> >> Reported-by: Sebastian Roesner <sroesner-kernelorg@roesner-online.de>
> >> >> Reported-by: Eric Wheeler <bcache@lists.ewheeler.net>
> >> >> Cc: stable@vger.kernel.org (4.2+)
> >> >
> >> > Ming Lei,
> >> >
> >> > get_max_io_size doesn't appear until 4.5 based on a quick LXR check so we
> >> > won't see it in stable I don't think.
> >> >
> >> > It would be nice to see this fixed in 4.1 (if affected there).  Is there
> >>
> >> The issue should be introduced to v4.3 via 54efd50
> >>
> >> > another place this could be applied to be a bit more backward compatible?
> >>
> >> The v1 needn't change to get_max_io_size(), and it should be simple enough
> >> to backport to previous stables, please try it:
> >>
> >> http://marc.info/?l=linux-block&m=145991422422927&w=2
> >
> > V1 changes blk_bio_segment_split() which doesn't exist until 4.3.
> >
> > How might you port this to v4.1.y?
> 
> Can you see the issue with v4.1?
> 
> You mentioned there are three reports:
> 
> > [2014-02-04] https://bugzilla.redhat.com/show_bug.cgi?id=1061339
> > [2016-01-13] http://www.spinics.net/lists/linux-bcache/msg03335.html
>              https://bugzilla.kernel.org/show_bug.cgi?id=110771
> > [2016-03-25] http://thread.gmane.org/gmane.linux.kernel.bcache.devel/3607 [this thread]
>              https://bugzilla.kernel.org/show_bug.cgi?id=114871
> 
> The first one has been fixed by '8423ae3 block: Fix cloning of
> discard/write same bios', as mentioned in bugzilla.
> 
> The other two are reported on v4.4 and v4.5.
> 
> If you look at the patch description, it is just needed for 4.3+.


Oh, that could be---but just to be sure:

I had thought perhaps this was an old issue since the first mention of 
this backtrace (but not bcache) was in 3.14 back in 2014 based on this 
post:
  https://bugzilla.redhat.com/show_bug.cgi?id=1061339

Is this relevant?

--
Eric Wheeler



> 
> Or I am wrong?
> 
> 
> Thanks,
> 
> >
> > --
> > Eric Wheeler
> >
> >
> >>
> >> Thanks,
> >>
> >> >
> >> >> Cc: Shaohua Li <shli@fb.com>
> >> >> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> >> >> ---
> >> >> I can reproduce the issue and verify the fix by the following approach:
> >> >>       - create one raid1 over two virtio-blk
> >> >>       - build bcache device over the above raid1 and another cache device.
> >> >>       - set cache mode as writeback
> >> >>       - run random write over ext4 on the bcache device
> >> >>       - then the crash can be triggered
> >> >>
> >> >>  block/blk-merge.c | 12 ++++++++++++
> >> >>  1 file changed, 12 insertions(+)
> >> >>
> >> >> diff --git a/block/blk-merge.c b/block/blk-merge.c
> >> >> index 2613531..9a8651f 100644
> >> >> --- a/block/blk-merge.c
> >> >> +++ b/block/blk-merge.c
> >> >> @@ -79,6 +79,18 @@ static inline unsigned get_max_io_size(struct request_queue *q,
> >> >>       /* aligned to logical block size */
> >> >>       sectors &= ~(mask >> 9);
> >> >>
> >> >> +     /*
> >> >> +      * With arbitrary bio size, the incoming bio may be very big.
> >> >> +      * We have to split the bio into small bios so that each holds
> >> >> +      * at most BIO_MAX_PAGES bvecs for safety reason, such as
> >> >> +      * bio_clone().
> >> >> +      *
> >> >> +      * In the future, the limit might be converted into per-queue
> >> >> +      * flag.
> >> >> +      */
> >> >> +     sectors = min_t(unsigned, sectors, BIO_MAX_PAGES <<
> >> >> +                     (PAGE_CACHE_SHIFT - 9));
> >> >> +
> >> >>       return sectors;
> >> >>  }
> >> >>
> >> >> --
> >> >> 1.9.1
> >> >>
> >> >>
> >> > --
> >> > 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]

#1373861

On Fri, Apr 8, 2016 at 7:29 AM, Eric Wheeler <bcache@lists.ewheeler.net> wrote:
> On Thu, 7 Apr 2016, Ming Lei wrote:
>
>> On Thu, Apr 7, 2016 at 9:56 AM, Eric Wheeler <bcache@lists.ewheeler.net> wrote:
>> > On Thu, 7 Apr 2016, Ming Lei wrote:
>> >
>> >> On Thu, Apr 7, 2016 at 9:36 AM, Eric Wheeler <bcache@lists.ewheeler.net> wrote:
>> >> > On Wed, 6 Apr 2016, Ming Lei wrote:
>> >> >
>> >> >> After arbitrary bio size is supported, the incoming bio may
>> >> >> be very big. We have to split the bio into small bios so that
>> >> >> each holds at most BIO_MAX_PAGES bvecs for safety reason, such
>> >> >> as bio_clone().
>> >> >>
>> >> >> This patch fixes the following kernel crash:
>> >> >>
>> >> >> > [  172.660142] BUG: unable to handle kernel NULL pointer dereference at
>> >> >> > 0000000000000028
>> >> >> > [  172.660229] IP: [<ffffffff811e53b4>] bio_trim+0xf/0x2a
>> >> >> > [  172.660289] PGD 7faf3e067 PUD 7f9279067 PMD 0
>> >> >> > [  172.660399] Oops: 0000 [#1] SMP
>> >> >> > [...]
>> >> >> > [  172.664780] Call Trace:
>> >> >> > [  172.664813]  [<ffffffffa007f3be>] ? raid1_make_request+0x2e8/0xad7 [raid1]
>> >> >> > [  172.664846]  [<ffffffff811f07da>] ? blk_queue_split+0x377/0x3d4
>> >> >> > [  172.664880]  [<ffffffffa005fb5f>] ? md_make_request+0xf6/0x1e9 [md_mod]
>> >> >> > [  172.664912]  [<ffffffff811eb860>] ? generic_make_request+0xb5/0x155
>> >> >> > [  172.664947]  [<ffffffffa0445c89>] ? prio_io+0x85/0x95 [bcache]
>> >> >> > [  172.664981]  [<ffffffffa0448252>] ? register_cache_set+0x355/0x8d0 [bcache]
>> >> >> > [  172.665016]  [<ffffffffa04497d3>] ? register_bcache+0x1006/0x1174 [bcache]
>> >> >>
>> >> >> Fixes: 54efd50(block: make generic_make_request handle arbitrarily sized bios)
>> >> >> Reported-by: Sebastian Roesner <sroesner-kernelorg@roesner-online.de>
>> >> >> Reported-by: Eric Wheeler <bcache@lists.ewheeler.net>
>> >> >> Cc: stable@vger.kernel.org (4.2+)
>> >> >
>> >> > Ming Lei,
>> >> >
>> >> > get_max_io_size doesn't appear until 4.5 based on a quick LXR check so we
>> >> > won't see it in stable I don't think.
>> >> >
>> >> > It would be nice to see this fixed in 4.1 (if affected there).  Is there
>> >>
>> >> The issue should be introduced to v4.3 via 54efd50
>> >>
>> >> > another place this could be applied to be a bit more backward compatible?
>> >>
>> >> The v1 needn't change to get_max_io_size(), and it should be simple enough
>> >> to backport to previous stables, please try it:
>> >>
>> >> http://marc.info/?l=linux-block&m=145991422422927&w=2
>> >
>> > V1 changes blk_bio_segment_split() which doesn't exist until 4.3.
>> >
>> > How might you port this to v4.1.y?
>>
>> Can you see the issue with v4.1?
>>
>> You mentioned there are three reports:
>>
>> > [2014-02-04] https://bugzilla.redhat.com/show_bug.cgi?id=1061339
>> > [2016-01-13] http://www.spinics.net/lists/linux-bcache/msg03335.html
>>              https://bugzilla.kernel.org/show_bug.cgi?id=110771
>> > [2016-03-25] http://thread.gmane.org/gmane.linux.kernel.bcache.devel/3607 [this thread]
>>              https://bugzilla.kernel.org/show_bug.cgi?id=114871
>>
>> The first one has been fixed by '8423ae3 block: Fix cloning of
>> discard/write same bios', as mentioned in bugzilla.
>>
>> The other two are reported on v4.4 and v4.5.
>>
>> If you look at the patch description, it is just needed for 4.3+.
>
>
> Oh, that could be---but just to be sure:
>
> I had thought perhaps this was an old issue since the first mention of
> this backtrace (but not bcache) was in 3.14 back in 2014 based on this
> post:
>   https://bugzilla.redhat.com/show_bug.cgi?id=1061339
>
> Is this relevant?

Eric,

That is another bio_clone() issue, which was fixed by commit
'8423ae3 block: Fix cloning of discard/write same bios'.

If you did read the bugzillar comment and my reply, you should
get the answer already.

I am sorry for having to answer your same question again and again...

>
> --
> Eric Wheeler
>
>
>
>>
>> Or I am wrong?
>>
>>
>> Thanks,
>>
>> >
>> > --
>> > Eric Wheeler
>> >
>> >
>> >>
>> >> Thanks,
>> >>
>> >> >
>> >> >> Cc: Shaohua Li <shli@fb.com>
>> >> >> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> >> >> ---
>> >> >> I can reproduce the issue and verify the fix by the following approach:
>> >> >>       - create one raid1 over two virtio-blk
>> >> >>       - build bcache device over the above raid1 and another cache device.
>> >> >>       - set cache mode as writeback
>> >> >>       - run random write over ext4 on the bcache device
>> >> >>       - then the crash can be triggered
>> >> >>
>> >> >>  block/blk-merge.c | 12 ++++++++++++
>> >> >>  1 file changed, 12 insertions(+)
>> >> >>
>> >> >> diff --git a/block/blk-merge.c b/block/blk-merge.c
>> >> >> index 2613531..9a8651f 100644
>> >> >> --- a/block/blk-merge.c
>> >> >> +++ b/block/blk-merge.c
>> >> >> @@ -79,6 +79,18 @@ static inline unsigned get_max_io_size(struct request_queue *q,
>> >> >>       /* aligned to logical block size */
>> >> >>       sectors &= ~(mask >> 9);
>> >> >>
>> >> >> +     /*
>> >> >> +      * With arbitrary bio size, the incoming bio may be very big.
>> >> >> +      * We have to split the bio into small bios so that each holds
>> >> >> +      * at most BIO_MAX_PAGES bvecs for safety reason, such as
>> >> >> +      * bio_clone().
>> >> >> +      *
>> >> >> +      * In the future, the limit might be converted into per-queue
>> >> >> +      * flag.
>> >> >> +      */
>> >> >> +     sectors = min_t(unsigned, sectors, BIO_MAX_PAGES <<
>> >> >> +                     (PAGE_CACHE_SHIFT - 9));
>> >> >> +
>> >> >>       return sectors;
>> >> >>  }
>> >> >>
>> >> >> --
>> >> >> 1.9.1
>> >> >>
>> >> >>
>> >> > --
>> >> > 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
>> >>
>>
> --
> 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]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web