* loading scsi_debug with data integrity hits BUG_ON
@ 2014-02-19 13:22 Akinobu Mita
2014-02-20 1:10 ` Nicholas A. Bellinger
0 siblings, 1 reply; 10+ messages in thread
From: Akinobu Mita @ 2014-02-19 13:22 UTC (permalink / raw)
To: linux-scsi, Nicholas Bellinger, Martin K. Petersen, Jens Axboe,
Christoph Hellwig
After commit 5837c80e870b ("bio-integrity: Fix bio_integrity_verify
segment start bug"), loading scsi_debug with data integrity enabled
(for example modprobe scsi_debug dif=1 dix=1) starts hitting BUG_ON()
at bio_integrity_verify():
BUG_ON(total > bio->bi_integrity->bip_iter.bi_size);
Because bip_iter.bi_size has already been set to zero at this point
as a result of consuming bip_iter.
I can resolve this issue by removing the BUG_ON(). But I would like
to ask whether we should preserve an alternative assert, or not
before submitting the patch that simply removing the line.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: loading scsi_debug with data integrity hits BUG_ON
2014-02-19 13:22 loading scsi_debug with data integrity hits BUG_ON Akinobu Mita
@ 2014-02-20 1:10 ` Nicholas A. Bellinger
2014-02-20 13:44 ` Akinobu Mita
2014-02-20 19:16 ` Martin K. Petersen
0 siblings, 2 replies; 10+ messages in thread
From: Nicholas A. Bellinger @ 2014-02-20 1:10 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-scsi, Martin K. Petersen, Jens Axboe, Christoph Hellwig,
Sagi Grimberg, kmo
Hi Akinobu,
On Wed, 2014-02-19 at 22:22 +0900, Akinobu Mita wrote:
> After commit 5837c80e870b ("bio-integrity: Fix bio_integrity_verify
> segment start bug"), loading scsi_debug with data integrity enabled
> (for example modprobe scsi_debug dif=1 dix=1) starts hitting BUG_ON()
> at bio_integrity_verify():
>
> BUG_ON(total > bio->bi_integrity->bip_iter.bi_size);
>
> Because bip_iter.bi_size has already been set to zero at this point
> as a result of consuming bip_iter.
>
> I can resolve this issue by removing the BUG_ON(). But I would like
> to ask whether we should preserve an alternative assert, or not
> before submitting the patch that simply removing the line.
So commit 5837c80e870b just allowed bio_integrity_verify() to actually
process protection information for READs again, which had been broken
since v3.10 code.
The bug your hitting is specific to v3.14-rc1, and was introduced with
the following change to support immutable biovecs:
commit d57a5f7c6605f15f3b5134837e68b448a7cea88e
Author: Kent Overstreet <kmo@daterainc.com>
Date: Sat Nov 23 17:20:16 2013 -0800
bio-integrity: Convert to bvec_iter
Given that there is no (easy) way to ascertain what the original value
of bio_integrity->bip_iter.bi_size was post bio_integrity_advance(),
dropping this BUG_ON() probably makes the most sense.
MKP + Jens, care to ACK + pickup..?
--nab
>From 847ab029fe0d5ed9b54a1f1507c46025f6c3f9e0 Mon Sep 17 00:00:00 2001
From: Nicholas Bellinger <nab@linux-iscsi.org>
Date: Thu, 20 Feb 2014 00:52:01 +0000
Subject: [PATCH] bio-integrity: Drop bio_integrity_verify BUG_ON in post
bip->bip_iter world
Given that bip->bip_iter.bi_size is decremented after bio_advance() ->
bio_integrity_advance() is called, the BUG_ON() in bio_integrity_verify()
ends up tripping in v3.14-rc1 code with the advent of immutable biovecs
in:
commit d57a5f7c6605f15f3b5134837e68b448a7cea88e
Author: Kent Overstreet <kmo@daterainc.com>
Date: Sat Nov 23 17:20:16 2013 -0800
bio-integrity: Convert to bvec_iter
Given that there is no easy way to ascertain the original bi_size
value, go ahead and drop this BUG_ON().
Reported-by: Sagi Grimberg <sagig@dev.mellanox.co.il>
Reported-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Kent Overstreet <kmo@daterainc.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
fs/bio-integrity.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
index 0bad24d..47ed160 100644
--- a/fs/bio-integrity.c
+++ b/fs/bio-integrity.c
@@ -476,7 +476,6 @@ static int bio_integrity_verify(struct bio *bio)
sector += sectors;
prot_buf += sectors * bi->tuple_size;
total += sectors * bi->tuple_size;
- BUG_ON(total > bio->bi_integrity->bip_iter.bi_size);
kunmap_atomic(kaddr);
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: loading scsi_debug with data integrity hits BUG_ON
2014-02-20 1:10 ` Nicholas A. Bellinger
@ 2014-02-20 13:44 ` Akinobu Mita
2014-02-20 19:20 ` Nicholas A. Bellinger
2014-02-20 19:16 ` Martin K. Petersen
1 sibling, 1 reply; 10+ messages in thread
From: Akinobu Mita @ 2014-02-20 13:44 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: linux-scsi, Martin K. Petersen, Jens Axboe, Christoph Hellwig,
Sagi Grimberg, kmo
Thanks for the patch. I have just one nitpicking.
> diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
> index 0bad24d..47ed160 100644
> --- a/fs/bio-integrity.c
> +++ b/fs/bio-integrity.c
> @@ -476,7 +476,6 @@ static int bio_integrity_verify(struct bio *bio)
> sector += sectors;
> prot_buf += sectors * bi->tuple_size;
> total += sectors * bi->tuple_size;
> - BUG_ON(total > bio->bi_integrity->bip_iter.bi_size);
The variable 'total' is only used for the BUG_ON. So it can be removed.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: loading scsi_debug with data integrity hits BUG_ON
2014-02-20 1:10 ` Nicholas A. Bellinger
2014-02-20 13:44 ` Akinobu Mita
@ 2014-02-20 19:16 ` Martin K. Petersen
2014-02-20 19:28 ` Nicholas A. Bellinger
1 sibling, 1 reply; 10+ messages in thread
From: Martin K. Petersen @ 2014-02-20 19:16 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Akinobu Mita, linux-scsi, Martin K. Petersen, Jens Axboe,
Christoph Hellwig, Sagi Grimberg, kmo
>>>>> "nab" == Nicholas A Bellinger <nab@linux-iscsi.org> writes:
nab> Given that there is no (easy) way to ascertain what the original
nab> value of bio_integrity->bip_iter.bi_size was post
nab> bio_integrity_advance(), dropping this BUG_ON() probably makes the
nab> most sense.
nab> MKP + Jens, care to ACK + pickup..?
Please remove total as suggested by Akinobu.
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: loading scsi_debug with data integrity hits BUG_ON
2014-02-20 13:44 ` Akinobu Mita
@ 2014-02-20 19:20 ` Nicholas A. Bellinger
0 siblings, 0 replies; 10+ messages in thread
From: Nicholas A. Bellinger @ 2014-02-20 19:20 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-scsi, Martin K. Petersen, Jens Axboe, Christoph Hellwig,
Sagi Grimberg, kmo
On Thu, 2014-02-20 at 22:44 +0900, Akinobu Mita wrote:
> Thanks for the patch. I have just one nitpicking.
>
> > diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
> > index 0bad24d..47ed160 100644
> > --- a/fs/bio-integrity.c
> > +++ b/fs/bio-integrity.c
> > @@ -476,7 +476,6 @@ static int bio_integrity_verify(struct bio *bio)
> > sector += sectors;
> > prot_buf += sectors * bi->tuple_size;
> > total += sectors * bi->tuple_size;
> > - BUG_ON(total > bio->bi_integrity->bip_iter.bi_size);
>
> The variable 'total' is only used for the BUG_ON. So it can be removed.
Duh, yes of course..
MKP + Jens, any concerns wrt dropping this BUG_ON for v3.14-rc code..?
--nab
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: loading scsi_debug with data integrity hits BUG_ON
2014-02-20 19:16 ` Martin K. Petersen
@ 2014-02-20 19:28 ` Nicholas A. Bellinger
2014-02-21 23:56 ` Jens Axboe
2014-02-24 22:15 ` Muthu Kumar
0 siblings, 2 replies; 10+ messages in thread
From: Nicholas A. Bellinger @ 2014-02-20 19:28 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Akinobu Mita, linux-scsi, Jens Axboe, Christoph Hellwig,
Sagi Grimberg, kmo
On Thu, 2014-02-20 at 14:16 -0500, Martin K. Petersen wrote:
> >>>>> "nab" == Nicholas A Bellinger <nab@linux-iscsi.org> writes:
>
> nab> Given that there is no (easy) way to ascertain what the original
> nab> value of bio_integrity->bip_iter.bi_size was post
> nab> bio_integrity_advance(), dropping this BUG_ON() probably makes the
> nab> most sense.
>
> nab> MKP + Jens, care to ACK + pickup..?
>
> Please remove total as suggested by Akinobu.
>
> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
>
Thanks. Here's an updated patch for Jen's to pickup.
--nab
>From 6bd5636cd46fe8e11de9bdecc26acac14a494f18 Mon Sep 17 00:00:00 2001
From: Nicholas Bellinger <nab@linux-iscsi.org>
Date: Thu, 20 Feb 2014 00:52:01 +0000
Subject: [PATCH] bio-integrity: Drop bio_integrity_verify BUG_ON in post
bip->bip_iter world
Given that bip->bip_iter.bi_size is decremented after bio_advance() ->
bio_integrity_advance() is called, the BUG_ON() in bio_integrity_verify()
ends up tripping in v3.14-rc1 code with the advent of immutable biovecs
in:
commit d57a5f7c6605f15f3b5134837e68b448a7cea88e
Author: Kent Overstreet <kmo@daterainc.com>
Date: Sat Nov 23 17:20:16 2013 -0800
bio-integrity: Convert to bvec_iter
Given that there is no easy way to ascertain the original bi_size
value, go ahead and drop this BUG_ON().
Reported-by: Sagi Grimberg <sagig@dev.mellanox.co.il>
Reported-by: Akinobu Mita <akinobu.mita@gmail.com>
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Kent Overstreet <kmo@daterainc.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
fs/bio-integrity.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
index 0bad24d..76e0116 100644
--- a/fs/bio-integrity.c
+++ b/fs/bio-integrity.c
@@ -449,11 +449,10 @@ static int bio_integrity_verify(struct bio *bio)
struct blk_integrity_exchg bix;
struct bio_vec *bv;
sector_t sector = bio->bi_integrity->bip_iter.bi_sector;
- unsigned int sectors, total, ret;
+ unsigned int sectors, ret = 0;
void *prot_buf = bio->bi_integrity->bip_buf;
int i;
- ret = total = 0;
bix.disk_name = bio->bi_bdev->bd_disk->disk_name;
bix.sector_size = bi->sector_size;
@@ -475,8 +474,6 @@ static int bio_integrity_verify(struct bio *bio)
sectors = bv->bv_len / bi->sector_size;
sector += sectors;
prot_buf += sectors * bi->tuple_size;
- total += sectors * bi->tuple_size;
- BUG_ON(total > bio->bi_integrity->bip_iter.bi_size);
kunmap_atomic(kaddr);
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: loading scsi_debug with data integrity hits BUG_ON
2014-02-20 19:28 ` Nicholas A. Bellinger
@ 2014-02-21 23:56 ` Jens Axboe
2014-02-24 22:15 ` Muthu Kumar
1 sibling, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2014-02-21 23:56 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Martin K. Petersen, Akinobu Mita, linux-scsi, Christoph Hellwig,
Sagi Grimberg, kmo
On Thu, Feb 20 2014, Nicholas A. Bellinger wrote:
> On Thu, 2014-02-20 at 14:16 -0500, Martin K. Petersen wrote:
> > >>>>> "nab" == Nicholas A Bellinger <nab@linux-iscsi.org> writes:
> >
> > nab> Given that there is no (easy) way to ascertain what the original
> > nab> value of bio_integrity->bip_iter.bi_size was post
> > nab> bio_integrity_advance(), dropping this BUG_ON() probably makes the
> > nab> most sense.
> >
> > nab> MKP + Jens, care to ACK + pickup..?
> >
> > Please remove total as suggested by Akinobu.
> >
> > Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
> >
>
> Thanks. Here's an updated patch for Jen's to pickup.
Got it, thanks!
--
Jens Axboe
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: loading scsi_debug with data integrity hits BUG_ON
2014-02-20 19:28 ` Nicholas A. Bellinger
2014-02-21 23:56 ` Jens Axboe
@ 2014-02-24 22:15 ` Muthu Kumar
2014-02-24 22:37 ` Jens Axboe
1 sibling, 1 reply; 10+ messages in thread
From: Muthu Kumar @ 2014-02-24 22:15 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: Martin K. Petersen, Akinobu Mita, linux-scsi, Jens Axboe,
Christoph Hellwig, Sagi Grimberg, Kent Overstreet
NAB,
On Thu, Feb 20, 2014 at 11:28 AM, Nicholas A. Bellinger
<nab@linux-iscsi.org> wrote:
> On Thu, 2014-02-20 at 14:16 -0500, Martin K. Petersen wrote:
>> >>>>> "nab" == Nicholas A Bellinger <nab@linux-iscsi.org> writes:
>>
>> nab> Given that there is no (easy) way to ascertain what the original
>> nab> value of bio_integrity->bip_iter.bi_size was post
>> nab> bio_integrity_advance(), dropping this BUG_ON() probably makes the
>> nab> most sense.
>>
>> nab> MKP + Jens, care to ACK + pickup..?
>>
>> Please remove total as suggested by Akinobu.
>>
>> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
>>
>
> Thanks. Here's an updated patch for Jen's to pickup.
>
> --nab
>
> From 6bd5636cd46fe8e11de9bdecc26acac14a494f18 Mon Sep 17 00:00:00 2001
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> Date: Thu, 20 Feb 2014 00:52:01 +0000
> Subject: [PATCH] bio-integrity: Drop bio_integrity_verify BUG_ON in post
> bip->bip_iter world
>
> Given that bip->bip_iter.bi_size is decremented after bio_advance() ->
> bio_integrity_advance() is called, the BUG_ON() in bio_integrity_verify()
> ends up tripping in v3.14-rc1 code with the advent of immutable biovecs
> in:
>
> commit d57a5f7c6605f15f3b5134837e68b448a7cea88e
> Author: Kent Overstreet <kmo@daterainc.com>
> Date: Sat Nov 23 17:20:16 2013 -0800
>
> bio-integrity: Convert to bvec_iter
>
> Given that there is no easy way to ascertain the original bi_size
> value, go ahead and drop this BUG_ON().
>
> Reported-by: Sagi Grimberg <sagig@dev.mellanox.co.il>
> Reported-by: Akinobu Mita <akinobu.mita@gmail.com>
> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Kent Overstreet <kmo@daterainc.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
> fs/bio-integrity.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
> index 0bad24d..76e0116 100644
> --- a/fs/bio-integrity.c
> +++ b/fs/bio-integrity.c
> @@ -449,11 +449,10 @@ static int bio_integrity_verify(struct bio *bio)
> struct blk_integrity_exchg bix;
> struct bio_vec *bv;
> sector_t sector = bio->bi_integrity->bip_iter.bi_sector;
> - unsigned int sectors, total, ret;
> + unsigned int sectors, ret = 0;
> void *prot_buf = bio->bi_integrity->bip_buf;
> int i;
>
> - ret = total = 0;
Here you removed "ret = 0" as well. So if bio_for_each_segment_all()
is not entered, then we return junk from bio_integrity_verify().
Regards,
Muthu
> bix.disk_name = bio->bi_bdev->bd_disk->disk_name;
> bix.sector_size = bi->sector_size;
>
> @@ -475,8 +474,6 @@ static int bio_integrity_verify(struct bio *bio)
> sectors = bv->bv_len / bi->sector_size;
> sector += sectors;
> prot_buf += sectors * bi->tuple_size;
> - total += sectors * bi->tuple_size;
> - BUG_ON(total > bio->bi_integrity->bip_iter.bi_size);
>
> kunmap_atomic(kaddr);
> }
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: loading scsi_debug with data integrity hits BUG_ON
2014-02-24 22:15 ` Muthu Kumar
@ 2014-02-24 22:37 ` Jens Axboe
2014-02-24 23:01 ` Muthu Kumar
0 siblings, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2014-02-24 22:37 UTC (permalink / raw)
To: Muthu Kumar, Nicholas A. Bellinger
Cc: Martin K. Petersen, Akinobu Mita, linux-scsi, Christoph Hellwig,
Sagi Grimberg, Kent Overstreet
On 2014-02-24 14:15, Muthu Kumar wrote:
> NAB,
>
> On Thu, Feb 20, 2014 at 11:28 AM, Nicholas A. Bellinger
> <nab@linux-iscsi.org> wrote:
>> On Thu, 2014-02-20 at 14:16 -0500, Martin K. Petersen wrote:
>>>>>>>> "nab" == Nicholas A Bellinger <nab@linux-iscsi.org> writes:
>>>
>>> nab> Given that there is no (easy) way to ascertain what the original
>>> nab> value of bio_integrity->bip_iter.bi_size was post
>>> nab> bio_integrity_advance(), dropping this BUG_ON() probably makes the
>>> nab> most sense.
>>>
>>> nab> MKP + Jens, care to ACK + pickup..?
>>>
>>> Please remove total as suggested by Akinobu.
>>>
>>> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
>>>
>>
>> Thanks. Here's an updated patch for Jen's to pickup.
>>
>> --nab
>>
>> From 6bd5636cd46fe8e11de9bdecc26acac14a494f18 Mon Sep 17 00:00:00 2001
>> From: Nicholas Bellinger <nab@linux-iscsi.org>
>> Date: Thu, 20 Feb 2014 00:52:01 +0000
>> Subject: [PATCH] bio-integrity: Drop bio_integrity_verify BUG_ON in post
>> bip->bip_iter world
>>
>> Given that bip->bip_iter.bi_size is decremented after bio_advance() ->
>> bio_integrity_advance() is called, the BUG_ON() in bio_integrity_verify()
>> ends up tripping in v3.14-rc1 code with the advent of immutable biovecs
>> in:
>>
>> commit d57a5f7c6605f15f3b5134837e68b448a7cea88e
>> Author: Kent Overstreet <kmo@daterainc.com>
>> Date: Sat Nov 23 17:20:16 2013 -0800
>>
>> bio-integrity: Convert to bvec_iter
>>
>> Given that there is no easy way to ascertain the original bi_size
>> value, go ahead and drop this BUG_ON().
>>
>> Reported-by: Sagi Grimberg <sagig@dev.mellanox.co.il>
>> Reported-by: Akinobu Mita <akinobu.mita@gmail.com>
>> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
>> Cc: Jens Axboe <axboe@kernel.dk>
>> Cc: Kent Overstreet <kmo@daterainc.com>
>> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
>> ---
>> fs/bio-integrity.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
>> index 0bad24d..76e0116 100644
>> --- a/fs/bio-integrity.c
>> +++ b/fs/bio-integrity.c
>> @@ -449,11 +449,10 @@ static int bio_integrity_verify(struct bio *bio)
>> struct blk_integrity_exchg bix;
>> struct bio_vec *bv;
>> sector_t sector = bio->bi_integrity->bip_iter.bi_sector;
>> - unsigned int sectors, total, ret;
>> + unsigned int sectors, ret = 0;
>> void *prot_buf = bio->bi_integrity->bip_buf;
>> int i;
>>
>> - ret = total = 0;
>
>
> Here you removed "ret = 0" as well. So if bio_for_each_segment_all()
> is not entered, then we return junk from bio_integrity_verify().
The ret = 0 was just moved up to the definition.
--
Jens Axboe
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: loading scsi_debug with data integrity hits BUG_ON
2014-02-24 22:37 ` Jens Axboe
@ 2014-02-24 23:01 ` Muthu Kumar
0 siblings, 0 replies; 10+ messages in thread
From: Muthu Kumar @ 2014-02-24 23:01 UTC (permalink / raw)
To: Jens Axboe
Cc: Nicholas A. Bellinger, Martin K. Petersen, Akinobu Mita,
linux-scsi, Christoph Hellwig, Sagi Grimberg, Kent Overstreet
yep... silly me ;)
On Mon, Feb 24, 2014 at 2:37 PM, Jens Axboe <axboe@kernel.dk> wrote:
> On 2014-02-24 14:15, Muthu Kumar wrote:
>>
>> NAB,
>>
>> On Thu, Feb 20, 2014 at 11:28 AM, Nicholas A. Bellinger
>> <nab@linux-iscsi.org> wrote:
>>>
>>> On Thu, 2014-02-20 at 14:16 -0500, Martin K. Petersen wrote:
>>>>>>>>>
>>>>>>>>> "nab" == Nicholas A Bellinger <nab@linux-iscsi.org> writes:
>>>>
>>>>
>>>> nab> Given that there is no (easy) way to ascertain what the original
>>>> nab> value of bio_integrity->bip_iter.bi_size was post
>>>> nab> bio_integrity_advance(), dropping this BUG_ON() probably makes the
>>>> nab> most sense.
>>>>
>>>> nab> MKP + Jens, care to ACK + pickup..?
>>>>
>>>> Please remove total as suggested by Akinobu.
>>>>
>>>> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
>>>>
>>>
>>> Thanks. Here's an updated patch for Jen's to pickup.
>>>
>>> --nab
>>>
>>> From 6bd5636cd46fe8e11de9bdecc26acac14a494f18 Mon Sep 17 00:00:00 2001
>>> From: Nicholas Bellinger <nab@linux-iscsi.org>
>>> Date: Thu, 20 Feb 2014 00:52:01 +0000
>>> Subject: [PATCH] bio-integrity: Drop bio_integrity_verify BUG_ON in post
>>> bip->bip_iter world
>>>
>>> Given that bip->bip_iter.bi_size is decremented after bio_advance() ->
>>> bio_integrity_advance() is called, the BUG_ON() in bio_integrity_verify()
>>> ends up tripping in v3.14-rc1 code with the advent of immutable biovecs
>>> in:
>>>
>>> commit d57a5f7c6605f15f3b5134837e68b448a7cea88e
>>> Author: Kent Overstreet <kmo@daterainc.com>
>>> Date: Sat Nov 23 17:20:16 2013 -0800
>>>
>>> bio-integrity: Convert to bvec_iter
>>>
>>> Given that there is no easy way to ascertain the original bi_size
>>> value, go ahead and drop this BUG_ON().
>>>
>>> Reported-by: Sagi Grimberg <sagig@dev.mellanox.co.il>
>>> Reported-by: Akinobu Mita <akinobu.mita@gmail.com>
>>> Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
>>> Cc: Jens Axboe <axboe@kernel.dk>
>>> Cc: Kent Overstreet <kmo@daterainc.com>
>>> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
>>> ---
>>> fs/bio-integrity.c | 5 +----
>>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
>>> index 0bad24d..76e0116 100644
>>> --- a/fs/bio-integrity.c
>>> +++ b/fs/bio-integrity.c
>>> @@ -449,11 +449,10 @@ static int bio_integrity_verify(struct bio *bio)
>>> struct blk_integrity_exchg bix;
>>> struct bio_vec *bv;
>>> sector_t sector = bio->bi_integrity->bip_iter.bi_sector;
>>> - unsigned int sectors, total, ret;
>>> + unsigned int sectors, ret = 0;
>>> void *prot_buf = bio->bi_integrity->bip_buf;
>>> int i;
>>>
>>> - ret = total = 0;
>>
>>
>>
>> Here you removed "ret = 0" as well. So if bio_for_each_segment_all()
>> is not entered, then we return junk from bio_integrity_verify().
>
>
> The ret = 0 was just moved up to the definition.
>
> --
> Jens Axboe
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-02-24 23:01 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-19 13:22 loading scsi_debug with data integrity hits BUG_ON Akinobu Mita
2014-02-20 1:10 ` Nicholas A. Bellinger
2014-02-20 13:44 ` Akinobu Mita
2014-02-20 19:20 ` Nicholas A. Bellinger
2014-02-20 19:16 ` Martin K. Petersen
2014-02-20 19:28 ` Nicholas A. Bellinger
2014-02-21 23:56 ` Jens Axboe
2014-02-24 22:15 ` Muthu Kumar
2014-02-24 22:37 ` Jens Axboe
2014-02-24 23:01 ` Muthu Kumar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.