IO errors after "block: remove bio_get_nr_vecs()"

classic Classic list List threaded Threaded
45 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: IO errors after "block: remove bio_get_nr_vecs()"

Artem S. Tashkinov
On 2015-12-22 10:38, Kent Overstreet wrote:

> On Tue, Dec 22, 2015 at 05:26:12AM +0000, Junichi Nomura wrote:
>> On 12/22/15 12:59, Kent Overstreet wrote:
>> > reproduced it with 32 bit pae:
>> >
>> >> 1. Exclude memory above 4G line with boot param "max_addr=4G".
>> >
>> > doesn't work - max_addr=1G doesn't work either
>> >
>> >> 2. Disable highmem with "highmem=0".
>> >
>> > works!
>> >
>> >> 3. Try booting 64bit kernel.
>> >
>> > works
>>
>> blk_queue_bio() does split then bounce, which makes the segment
>> counting based on pages before bouncing and could go wrong.
>>
>> What do you think of a patch like this?
>
> Artem, can you give this patch a try?


This patch ostensibly fixes the issue - at least I cannot immediately
reproduce it. You can count me in as "Tested-by: Artem S. Tashkinov"

>
>>
>> --
>> Jun'ichi Nomura, NEC Corporation
>>
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index 5131993b..1d1c3c7 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -1689,8 +1689,6 @@ static blk_qc_t blk_queue_bio(struct
>> request_queue *q, struct bio *bio)
>>   struct request *req;
>>   unsigned int request_count = 0;
>>
>> - blk_queue_split(q, &bio, q->bio_split);
>> -
>>   /*
>>   * low level driver can indicate that it wants pages above a
>>   * certain limit bounced to low memory (ie for highmem, or even
>> @@ -1698,6 +1696,8 @@ static blk_qc_t blk_queue_bio(struct
>> request_queue *q, struct bio *bio)
>>   */
>>   blk_queue_bounce(q, &bio);
>>
>> + blk_queue_split(q, &bio, q->bio_split);
>> +
>>   if (bio_integrity_enabled(bio) && bio_integrity_prep(bio)) {
>>   bio->bi_error = -EIO;
>>   bio_endio(bio);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

Re: IO errors after "block: remove bio_get_nr_vecs()"

Kent Overstreet
On Tue, Dec 22, 2015 at 10:52:37AM +0500, Artem S. Tashkinov wrote:

> On 2015-12-22 10:38, Kent Overstreet wrote:
> >On Tue, Dec 22, 2015 at 05:26:12AM +0000, Junichi Nomura wrote:
> >>On 12/22/15 12:59, Kent Overstreet wrote:
> >>> reproduced it with 32 bit pae:
> >>>
> >>>> 1. Exclude memory above 4G line with boot param "max_addr=4G".
> >>>
> >>> doesn't work - max_addr=1G doesn't work either
> >>>
> >>>> 2. Disable highmem with "highmem=0".
> >>>
> >>> works!
> >>>
> >>>> 3. Try booting 64bit kernel.
> >>>
> >>> works
> >>
> >>blk_queue_bio() does split then bounce, which makes the segment
> >>counting based on pages before bouncing and could go wrong.
> >>
> >>What do you think of a patch like this?
> >
> >Artem, can you give this patch a try?
>
>
> This patch ostensibly fixes the issue - at least I cannot immediately
> reproduce it. You can count me in as "Tested-by: Artem S. Tashkinov"

Let's all contemplate the fact that blk_segment_map_sg() _overrunning the end of
the provided sglist_ was this much of a clusterfuck to debug.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

Re: IO errors after "block: remove bio_get_nr_vecs()"

Artem S. Tashkinov
On 2015-12-22 10:55, Kent Overstreet wrote:

> On Tue, Dec 22, 2015 at 10:52:37AM +0500, Artem S. Tashkinov wrote:
>> On 2015-12-22 10:38, Kent Overstreet wrote:
>> >On Tue, Dec 22, 2015 at 05:26:12AM +0000, Junichi Nomura wrote:
>> >>On 12/22/15 12:59, Kent Overstreet wrote:
>> >>> reproduced it with 32 bit pae:
>> >>>
>> >>>> 1. Exclude memory above 4G line with boot param "max_addr=4G".
>> >>>
>> >>> doesn't work - max_addr=1G doesn't work either
>> >>>
>> >>>> 2. Disable highmem with "highmem=0".
>> >>>
>> >>> works!
>> >>>
>> >>>> 3. Try booting 64bit kernel.
>> >>>
>> >>> works
>> >>
>> >>blk_queue_bio() does split then bounce, which makes the segment
>> >>counting based on pages before bouncing and could go wrong.
>> >>
>> >>What do you think of a patch like this?
>> >
>> >Artem, can you give this patch a try?
>>
>>
>> This patch ostensibly fixes the issue - at least I cannot immediately
>> reproduce it. You can count me in as "Tested-by: Artem S. Tashkinov"
>
> Let's all contemplate the fact that blk_segment_map_sg() _overrunning
> the end of
> the provided sglist_ was this much of a clusterfuck to debug.

 From the look of it this fix has nothing to do with PAE, so then why
only PAE users like me were affected by the original
(b54ffb73cadcdcff9cc1ae0e11f502407e3e2e4c) patch?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

Re: IO errors after "block: remove bio_get_nr_vecs()"

Kent Overstreet
On Tue, Dec 22, 2015 at 10:59:09AM +0500, Artem S. Tashkinov wrote:

> On 2015-12-22 10:55, Kent Overstreet wrote:
> >On Tue, Dec 22, 2015 at 10:52:37AM +0500, Artem S. Tashkinov wrote:
> >>On 2015-12-22 10:38, Kent Overstreet wrote:
> >>>On Tue, Dec 22, 2015 at 05:26:12AM +0000, Junichi Nomura wrote:
> >>>>On 12/22/15 12:59, Kent Overstreet wrote:
> >>>>> reproduced it with 32 bit pae:
> >>>>>
> >>>>>> 1. Exclude memory above 4G line with boot param "max_addr=4G".
> >>>>>
> >>>>> doesn't work - max_addr=1G doesn't work either
> >>>>>
> >>>>>> 2. Disable highmem with "highmem=0".
> >>>>>
> >>>>> works!
> >>>>>
> >>>>>> 3. Try booting 64bit kernel.
> >>>>>
> >>>>> works
> >>>>
> >>>>blk_queue_bio() does split then bounce, which makes the segment
> >>>>counting based on pages before bouncing and could go wrong.
> >>>>
> >>>>What do you think of a patch like this?
> >>>
> >>>Artem, can you give this patch a try?
> >>
> >>
> >>This patch ostensibly fixes the issue - at least I cannot immediately
> >>reproduce it. You can count me in as "Tested-by: Artem S. Tashkinov"
> >
> >Let's all contemplate the fact that blk_segment_map_sg() _overrunning the
> >end of
> >the provided sglist_ was this much of a clusterfuck to debug.
>
> From the look of it this fix has nothing to do with PAE, so then why only
> PAE users like me were affected by the original
> (b54ffb73cadcdcff9cc1ae0e11f502407e3e2e4c) patch?

The amusing thing is that I doubt PAE actually requires bouncing - addressing
limits come from the device, not the cpu.

But evidently in PAE mode, the block layer is in fact bouncing bios. Probably
from some default setting in the queue limits that no one ever looks at.

The whole queue limits design is an atrocity, it leads to exactly this kind of
crap where no one can predict the actual behaviour of any given setup.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Reply | Threaded
Open this post in threaded view
|

Re: IO errors after "block: remove bio_get_nr_vecs()"

Jens Axboe-3
In reply to this post by Junichi Nomura
On 12/21/2015 10:26 PM, Junichi Nomura wrote:

> On 12/22/15 12:59, Kent Overstreet wrote:
>> reproduced it with 32 bit pae:
>>
>>> 1. Exclude memory above 4G line with boot param "max_addr=4G".
>>
>> doesn't work - max_addr=1G doesn't work either
>>
>>> 2. Disable highmem with "highmem=0".
>>
>> works!
>>
>>> 3. Try booting 64bit kernel.
>>
>> works
>
> blk_queue_bio() does split then bounce, which makes the segment
> counting based on pages before bouncing and could go wrong.

Good catch! The blk-mq parts aren't affected by this, the screw up only
happened in the old IO path. I've added this with the appropriate
tested-by from Artem, and CC stable and listed the commit that broke it:

commit 54efd50bfd873e2dbf784e0b21a8027ba4299a3e
Author: Kent Overstreet <[hidden email]>
Date:   Thu Apr 23 22:37:18 2015 -0700

     block: make generic_make_request handle arbitrarily sized bios

Thanks to all involved in nailing this down, it'll go out shortly.

--
Jens Axboe

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