* dm-flakey NOT to return -EIO on READ?
@ 2016-07-06 6:33 Akira Hayakawa
2016-07-06 9:52 ` Lukas Herbolt
0 siblings, 1 reply; 12+ messages in thread
From: Akira Hayakawa @ 2016-07-06 6:33 UTC (permalink / raw)
To: device-mapper development
Hi,
I am using dm-flakey to emulate a broken device that should return -EIO on both read and write.
I use the parameter up_interval=0 and down_interval=1.
But when I am dd-ing the flakey device, while write fails, read succeeds.
This isn't the behavior I expect.
Then I looked into the code.
326 static int flakey_end_io(struct dm_target *ti, struct bio *bio, int error)
327 {
328 struct flakey_c *fc = ti->private;
329 struct per_bio_data *pb = dm_per_bio_data(bio, sizeof(struct per_bio_data));
330
331 /*
332 * Corrupt successful READs while in down state.
333 * If flags were specified, only corrupt those that match.
334 */
335 if (fc->corrupt_bio_byte && !error && pb->bio_submitted &&
336 (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw == READ) &&
337 all_corrupt_bio_flags_match(bio, fc))
338 corrupt_bio_data(bio, fc);
339
340 return error;
341 }
When the bio direction is READ and currupt_bio_bytes isn't specified
the READ bio is handled normally right?
I think READ requests should return -EIO if
1. corrupt_bio_bytes isn't specified
2. but the requested is handled during down interval.
as well as WRITE requests:
305 /*
306 * Corrupt matching writes.
307 */
308 if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw == WRITE)) {
309 if (all_corrupt_bio_flags_match(bio, fc))
310 corrupt_bio_data(bio, fc);
311 goto map_bio;
312 }
313
314 /*
315 * By default, error all I/O.
316 */
317 return -EIO;
318 }
This code is similar to what we see in the end_io.
If my understanding is correct, I would like to modify dm-flakey to return -EIO in the end_io.
I would like to request for comments.
Thanks,
- Akira
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dm-flakey NOT to return -EIO on READ?
2016-07-06 6:33 dm-flakey NOT to return -EIO on READ? Akira Hayakawa
@ 2016-07-06 9:52 ` Lukas Herbolt
2016-07-06 12:54 ` Akira Hayakawa
0 siblings, 1 reply; 12+ messages in thread
From: Lukas Herbolt @ 2016-07-06 9:52 UTC (permalink / raw)
To: Akira Hayakawa; +Cc: device-mapper development
[-- Attachment #1.1: Type: text/plain, Size: 2523 bytes --]
Hi,
Yes this part is wrong and reads are not dropped.
I have a patch ready, just have to send it out.
Lukas
On Wed, Jul 6, 2016 at 8:33 AM, Akira Hayakawa <ruby.wktk@gmail.com> wrote:
> Hi,
>
> I am using dm-flakey to emulate a broken device that should return -EIO on
> both read and write.
> I use the parameter up_interval=0 and down_interval=1.
>
> But when I am dd-ing the flakey device, while write fails, read succeeds.
> This isn't the behavior I expect.
>
> Then I looked into the code.
>
> 326 static int flakey_end_io(struct dm_target *ti, struct bio *bio, int
> error)
> 327 {
> 328 struct flakey_c *fc = ti->private;
> 329 struct per_bio_data *pb = dm_per_bio_data(bio, sizeof(struct
> per_bio_data));
> 330
> 331 /*
> 332 * Corrupt successful READs while in down state.
> 333 * If flags were specified, only corrupt those that match.
> 334 */
> 335 if (fc->corrupt_bio_byte && !error && pb->bio_submitted &&
> 336 (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw ==
> READ) &&
> 337 all_corrupt_bio_flags_match(bio, fc))
> 338 corrupt_bio_data(bio, fc);
> 339
> 340 return error;
> 341 }
>
> When the bio direction is READ and currupt_bio_bytes isn't specified
> the READ bio is handled normally right?
>
> I think READ requests should return -EIO if
>
> 1. corrupt_bio_bytes isn't specified
> 2. but the requested is handled during down interval.
>
> as well as WRITE requests:
>
> 305 /*
> 306 * Corrupt matching writes.
> 307 */
> 308 if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw ==
> WRITE)) {
> 309 if (all_corrupt_bio_flags_match(bio, fc))
> 310 corrupt_bio_data(bio, fc);
> 311 goto map_bio;
> 312 }
> 313
> 314 /*
> 315 * By default, error all I/O.
> 316 */
> 317 return -EIO;
> 318 }
>
> This code is similar to what we see in the end_io.
>
> If my understanding is correct, I would like to modify dm-flakey to return
> -EIO in the end_io.
>
> I would like to request for comments.
>
> Thanks,
>
> - Akira
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>
--
Lukas Herbolt
RHCE, RH436, BSc, SSc
Senior Technical Support Engineer
Global Support Services (GSS)
Email: lherbolt@redhat.com
[-- Attachment #1.2: Type: text/html, Size: 4325 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dm-flakey NOT to return -EIO on READ?
2016-07-06 9:52 ` Lukas Herbolt
@ 2016-07-06 12:54 ` Akira Hayakawa
2016-07-17 1:36 ` Akira Hayakawa
0 siblings, 1 reply; 12+ messages in thread
From: Akira Hayakawa @ 2016-07-06 12:54 UTC (permalink / raw)
To: Lukas Herbolt; +Cc: device-mapper development
Thanks,
I want the patch in the main-tree quickly.
Because without it, my tests will not be green.
This is really annoying.
I made a quick reproducer of this problem.
If you are looking for one, this will help.
(You need to install sbt first)
https://github.com/akiradeveloper/writeboost-test-suite
test("flakey (read)") {
Memory(Sector.M(16)) { a =>
Flakey.Table(a, 0, 1).create { b =>
intercept[Exception] {
Shell(s"dd status=none if=${b.bdev.path} iflag=direct of=/dev/null")
}
}
}
}
$ sudo sbt "testOnly dmtest.FlakeyTest"
12:44:00.643 [pool-2-thread-3-ScalaTest-running-FlakeyTest] INFO dmtest - [TEST] flakey (read)
12:44:00.752 [pool-2-thread-3-ScalaTest-running-FlakeyTest] DEBUG dmtest - reload: table=0 32768 flakey /dev/loop0 0 0 1
[info] FlakeyTest:
[info] - flakey (read) *** FAILED ***
[info] Expected exception java.lang.Exception to be thrown, but no exception was thrown. (FlakeyTest.scala:9)
[info] Run completed in 2 seconds, 781 milliseconds.
[info] Total number of tests run: 1
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 0, failed 1, canceled 0, ignored 0, pending 0
- Akira
On 2016/07/06 18:52, Lukas Herbolt wrote:
> Hi,
> Yes this part is wrong and reads are not dropped.
> I have a patch ready, just have to send it out.
>
> Lukas
>
> On Wed, Jul 6, 2016 at 8:33 AM, Akira Hayakawa <ruby.wktk@gmail.com> wrote:
>
>> Hi,
>>
>> I am using dm-flakey to emulate a broken device that should return -EIO on
>> both read and write.
>> I use the parameter up_interval=0 and down_interval=1.
>>
>> But when I am dd-ing the flakey device, while write fails, read succeeds.
>> This isn't the behavior I expect.
>>
>> Then I looked into the code.
>>
>> 326 static int flakey_end_io(struct dm_target *ti, struct bio *bio, int
>> error)
>> 327 {
>> 328 struct flakey_c *fc = ti->private;
>> 329 struct per_bio_data *pb = dm_per_bio_data(bio, sizeof(struct
>> per_bio_data));
>> 330
>> 331 /*
>> 332 * Corrupt successful READs while in down state.
>> 333 * If flags were specified, only corrupt those that match.
>> 334 */
>> 335 if (fc->corrupt_bio_byte && !error && pb->bio_submitted &&
>> 336 (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw ==
>> READ) &&
>> 337 all_corrupt_bio_flags_match(bio, fc))
>> 338 corrupt_bio_data(bio, fc);
>> 339
>> 340 return error;
>> 341 }
>>
>> When the bio direction is READ and currupt_bio_bytes isn't specified
>> the READ bio is handled normally right?
>>
>> I think READ requests should return -EIO if
>>
>> 1. corrupt_bio_bytes isn't specified
>> 2. but the requested is handled during down interval.
>>
>> as well as WRITE requests:
>>
>> 305 /*
>> 306 * Corrupt matching writes.
>> 307 */
>> 308 if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw ==
>> WRITE)) {
>> 309 if (all_corrupt_bio_flags_match(bio, fc))
>> 310 corrupt_bio_data(bio, fc);
>> 311 goto map_bio;
>> 312 }
>> 313
>> 314 /*
>> 315 * By default, error all I/O.
>> 316 */
>> 317 return -EIO;
>> 318 }
>>
>> This code is similar to what we see in the end_io.
>>
>> If my understanding is correct, I would like to modify dm-flakey to return
>> -EIO in the end_io.
>>
>> I would like to request for comments.
>>
>> Thanks,
>>
>> - Akira
>>
>> --
>> dm-devel mailing list
>> dm-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/dm-devel
>>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dm-flakey NOT to return -EIO on READ?
2016-07-06 12:54 ` Akira Hayakawa
@ 2016-07-17 1:36 ` Akira Hayakawa
2016-07-29 6:25 ` Lukas Herbolt
0 siblings, 1 reply; 12+ messages in thread
From: Akira Hayakawa @ 2016-07-17 1:36 UTC (permalink / raw)
To: Lukas Herbolt; +Cc: device-mapper development
> On 2016/07/06 18:52, Lukas Herbolt wrote:
>> Hi,
>> Yes this part is wrong and reads are not dropped.
>> I have a patch ready, just have to send it out.
btw, is this fix going to be included in the next release?
On 2016/07/06 21:54, Akira Hayakawa wrote:
> Thanks,
>
> I want the patch in the main-tree quickly.
> Because without it, my tests will not be green.
> This is really annoying.
>
> I made a quick reproducer of this problem.
> If you are looking for one, this will help.
> (You need to install sbt first)
>
> https://github.com/akiradeveloper/writeboost-test-suite
>
> test("flakey (read)") {
> Memory(Sector.M(16)) { a =>
> Flakey.Table(a, 0, 1).create { b =>
> intercept[Exception] {
> Shell(s"dd status=none if=${b.bdev.path} iflag=direct of=/dev/null")
> }
> }
> }
> }
>
> $ sudo sbt "testOnly dmtest.FlakeyTest"
> 12:44:00.643 [pool-2-thread-3-ScalaTest-running-FlakeyTest] INFO dmtest - [TEST] flakey (read)
> 12:44:00.752 [pool-2-thread-3-ScalaTest-running-FlakeyTest] DEBUG dmtest - reload: table=0 32768 flakey /dev/loop0 0 0 1
> [info] FlakeyTest:
> [info] - flakey (read) *** FAILED ***
> [info] Expected exception java.lang.Exception to be thrown, but no exception was thrown. (FlakeyTest.scala:9)
> [info] Run completed in 2 seconds, 781 milliseconds.
> [info] Total number of tests run: 1
> [info] Suites: completed 1, aborted 0
> [info] Tests: succeeded 0, failed 1, canceled 0, ignored 0, pending 0
>
> - Akira
>
> On 2016/07/06 18:52, Lukas Herbolt wrote:
>> Hi,
>> Yes this part is wrong and reads are not dropped.
>> I have a patch ready, just have to send it out.
>>
>> Lukas
>>
>> On Wed, Jul 6, 2016 at 8:33 AM, Akira Hayakawa <ruby.wktk@gmail.com> wrote:
>>
>>> Hi,
>>>
>>> I am using dm-flakey to emulate a broken device that should return -EIO on
>>> both read and write.
>>> I use the parameter up_interval=0 and down_interval=1.
>>>
>>> But when I am dd-ing the flakey device, while write fails, read succeeds.
>>> This isn't the behavior I expect.
>>>
>>> Then I looked into the code.
>>>
>>> 326 static int flakey_end_io(struct dm_target *ti, struct bio *bio, int
>>> error)
>>> 327 {
>>> 328 struct flakey_c *fc = ti->private;
>>> 329 struct per_bio_data *pb = dm_per_bio_data(bio, sizeof(struct
>>> per_bio_data));
>>> 330
>>> 331 /*
>>> 332 * Corrupt successful READs while in down state.
>>> 333 * If flags were specified, only corrupt those that match.
>>> 334 */
>>> 335 if (fc->corrupt_bio_byte && !error && pb->bio_submitted &&
>>> 336 (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw ==
>>> READ) &&
>>> 337 all_corrupt_bio_flags_match(bio, fc))
>>> 338 corrupt_bio_data(bio, fc);
>>> 339
>>> 340 return error;
>>> 341 }
>>>
>>> When the bio direction is READ and currupt_bio_bytes isn't specified
>>> the READ bio is handled normally right?
>>>
>>> I think READ requests should return -EIO if
>>>
>>> 1. corrupt_bio_bytes isn't specified
>>> 2. but the requested is handled during down interval.
>>>
>>> as well as WRITE requests:
>>>
>>> 305 /*
>>> 306 * Corrupt matching writes.
>>> 307 */
>>> 308 if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw ==
>>> WRITE)) {
>>> 309 if (all_corrupt_bio_flags_match(bio, fc))
>>> 310 corrupt_bio_data(bio, fc);
>>> 311 goto map_bio;
>>> 312 }
>>> 313
>>> 314 /*
>>> 315 * By default, error all I/O.
>>> 316 */
>>> 317 return -EIO;
>>> 318 }
>>>
>>> This code is similar to what we see in the end_io.
>>>
>>> If my understanding is correct, I would like to modify dm-flakey to return
>>> -EIO in the end_io.
>>>
>>> I would like to request for comments.
>>>
>>> Thanks,
>>>
>>> - Akira
>>>
>>> --
>>> dm-devel mailing list
>>> dm-devel@redhat.com
>>> https://www.redhat.com/mailman/listinfo/dm-devel
>>>
>>
>>
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dm-flakey NOT to return -EIO on READ?
2016-07-17 1:36 ` Akira Hayakawa
@ 2016-07-29 6:25 ` Lukas Herbolt
2016-07-29 6:32 ` Akira Hayakawa
0 siblings, 1 reply; 12+ messages in thread
From: Lukas Herbolt @ 2016-07-29 6:25 UTC (permalink / raw)
To: Akira Hayakawa; +Cc: device-mapper development
[-- Attachment #1.1: Type: text/plain, Size: 4786 bytes --]
Hi Akira,
Sorry I was on holiday.
I have patch ready, would you be able to test it before posting it.
Let me know if I should build the kernel for you or just share the source
with you.
Lukas
On Sun, Jul 17, 2016 at 3:36 AM, Akira Hayakawa <ruby.wktk@gmail.com> wrote:
> > On 2016/07/06 18:52, Lukas Herbolt wrote:
> >> Hi,
> >> Yes this part is wrong and reads are not dropped.
> >> I have a patch ready, just have to send it out.
>
> btw, is this fix going to be included in the next release?
>
> On 2016/07/06 21:54, Akira Hayakawa wrote:
> > Thanks,
> >
> > I want the patch in the main-tree quickly.
> > Because without it, my tests will not be green.
> > This is really annoying.
> >
> > I made a quick reproducer of this problem.
> > If you are looking for one, this will help.
> > (You need to install sbt first)
> >
> > https://github.com/akiradeveloper/writeboost-test-suite
> >
> > test("flakey (read)") {
> > Memory(Sector.M(16)) { a =>
> > Flakey.Table(a, 0, 1).create { b =>
> > intercept[Exception] {
> > Shell(s"dd status=none if=${b.bdev.path} iflag=direct
> of=/dev/null")
> > }
> > }
> > }
> > }
> >
> > $ sudo sbt "testOnly dmtest.FlakeyTest"
> > 12:44:00.643 [pool-2-thread-3-ScalaTest-running-FlakeyTest] INFO dmtest
> - [TEST] flakey (read)
> > 12:44:00.752 [pool-2-thread-3-ScalaTest-running-FlakeyTest] DEBUG dmtest
> - reload: table=0 32768 flakey /dev/loop0 0 0 1
> > [info] FlakeyTest:
> > [info] - flakey (read) *** FAILED ***
> > [info] Expected exception java.lang.Exception to be thrown, but no
> exception was thrown. (FlakeyTest.scala:9)
> > [info] Run completed in 2 seconds, 781 milliseconds.
> > [info] Total number of tests run: 1
> > [info] Suites: completed 1, aborted 0
> > [info] Tests: succeeded 0, failed 1, canceled 0, ignored 0, pending 0
> >
> > - Akira
> >
> > On 2016/07/06 18:52, Lukas Herbolt wrote:
> >> Hi,
> >> Yes this part is wrong and reads are not dropped.
> >> I have a patch ready, just have to send it out.
> >>
> >> Lukas
> >>
> >> On Wed, Jul 6, 2016 at 8:33 AM, Akira Hayakawa <ruby.wktk@gmail.com>
> wrote:
> >>
> >>> Hi,
> >>>
> >>> I am using dm-flakey to emulate a broken device that should return
> -EIO on
> >>> both read and write.
> >>> I use the parameter up_interval=0 and down_interval=1.
> >>>
> >>> But when I am dd-ing the flakey device, while write fails, read
> succeeds.
> >>> This isn't the behavior I expect.
> >>>
> >>> Then I looked into the code.
> >>>
> >>> 326 static int flakey_end_io(struct dm_target *ti, struct bio *bio, int
> >>> error)
> >>> 327 {
> >>> 328 struct flakey_c *fc = ti->private;
> >>> 329 struct per_bio_data *pb = dm_per_bio_data(bio,
> sizeof(struct
> >>> per_bio_data));
> >>> 330
> >>> 331 /*
> >>> 332 * Corrupt successful READs while in down state.
> >>> 333 * If flags were specified, only corrupt those that match.
> >>> 334 */
> >>> 335 if (fc->corrupt_bio_byte && !error && pb->bio_submitted &&
> >>> 336 (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw ==
> >>> READ) &&
> >>> 337 all_corrupt_bio_flags_match(bio, fc))
> >>> 338 corrupt_bio_data(bio, fc);
> >>> 339
> >>> 340 return error;
> >>> 341 }
> >>>
> >>> When the bio direction is READ and currupt_bio_bytes isn't specified
> >>> the READ bio is handled normally right?
> >>>
> >>> I think READ requests should return -EIO if
> >>>
> >>> 1. corrupt_bio_bytes isn't specified
> >>> 2. but the requested is handled during down interval.
> >>>
> >>> as well as WRITE requests:
> >>>
> >>> 305 /*
> >>> 306 * Corrupt matching writes.
> >>> 307 */
> >>> 308 if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw ==
> >>> WRITE)) {
> >>> 309 if (all_corrupt_bio_flags_match(bio, fc))
> >>> 310 corrupt_bio_data(bio, fc);
> >>> 311 goto map_bio;
> >>> 312 }
> >>> 313
> >>> 314 /*
> >>> 315 * By default, error all I/O.
> >>> 316 */
> >>> 317 return -EIO;
> >>> 318 }
> >>>
> >>> This code is similar to what we see in the end_io.
> >>>
> >>> If my understanding is correct, I would like to modify dm-flakey to
> return
> >>> -EIO in the end_io.
> >>>
> >>> I would like to request for comments.
> >>>
> >>> Thanks,
> >>>
> >>> - Akira
> >>>
> >>> --
> >>> dm-devel mailing list
> >>> dm-devel@redhat.com
> >>> https://www.redhat.com/mailman/listinfo/dm-devel
> >>>
> >>
> >>
> >>
>
--
Lukas Herbolt
RHCE, RH436, BSc, SSc
Senior Technical Support Engineer
Global Support Services (GSS)
Email: lherbolt@redhat.com
[-- Attachment #1.2: Type: text/html, Size: 8049 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dm-flakey NOT to return -EIO on READ?
2016-07-29 6:25 ` Lukas Herbolt
@ 2016-07-29 6:32 ` Akira Hayakawa
2016-07-29 14:31 ` Lukas Herbolt
0 siblings, 1 reply; 12+ messages in thread
From: Akira Hayakawa @ 2016-07-29 6:32 UTC (permalink / raw)
To: Lukas Herbolt; +Cc: device-mapper development
Hi Lukas,
> I have patch ready, would you be able to test it before posting it.
>
> Let me know if I should build the kernel for you or just share the source
> with you.
OK.
Please share the git tree and specify the branch to test.
Akira
On 2016/07/29 15:25, Lukas Herbolt wrote:
> Hi Akira,
> Sorry I was on holiday.
> I have patch ready, would you be able to test it before posting it.
>
> Let me know if I should build the kernel for you or just share the source
> with you.
>
> Lukas
>
> On Sun, Jul 17, 2016 at 3:36 AM, Akira Hayakawa <ruby.wktk@gmail.com> wrote:
>
>>> On 2016/07/06 18:52, Lukas Herbolt wrote:
>>>> Hi,
>>>> Yes this part is wrong and reads are not dropped.
>>>> I have a patch ready, just have to send it out.
>>
>> btw, is this fix going to be included in the next release?
>>
>> On 2016/07/06 21:54, Akira Hayakawa wrote:
>>> Thanks,
>>>
>>> I want the patch in the main-tree quickly.
>>> Because without it, my tests will not be green.
>>> This is really annoying.
>>>
>>> I made a quick reproducer of this problem.
>>> If you are looking for one, this will help.
>>> (You need to install sbt first)
>>>
>>> https://github.com/akiradeveloper/writeboost-test-suite
>>>
>>> test("flakey (read)") {
>>> Memory(Sector.M(16)) { a =>
>>> Flakey.Table(a, 0, 1).create { b =>
>>> intercept[Exception] {
>>> Shell(s"dd status=none if=${b.bdev.path} iflag=direct
>> of=/dev/null")
>>> }
>>> }
>>> }
>>> }
>>>
>>> $ sudo sbt "testOnly dmtest.FlakeyTest"
>>> 12:44:00.643 [pool-2-thread-3-ScalaTest-running-FlakeyTest] INFO dmtest
>> - [TEST] flakey (read)
>>> 12:44:00.752 [pool-2-thread-3-ScalaTest-running-FlakeyTest] DEBUG dmtest
>> - reload: table=0 32768 flakey /dev/loop0 0 0 1
>>> [info] FlakeyTest:
>>> [info] - flakey (read) *** FAILED ***
>>> [info] Expected exception java.lang.Exception to be thrown, but no
>> exception was thrown. (FlakeyTest.scala:9)
>>> [info] Run completed in 2 seconds, 781 milliseconds.
>>> [info] Total number of tests run: 1
>>> [info] Suites: completed 1, aborted 0
>>> [info] Tests: succeeded 0, failed 1, canceled 0, ignored 0, pending 0
>>>
>>> - Akira
>>>
>>> On 2016/07/06 18:52, Lukas Herbolt wrote:
>>>> Hi,
>>>> Yes this part is wrong and reads are not dropped.
>>>> I have a patch ready, just have to send it out.
>>>>
>>>> Lukas
>>>>
>>>> On Wed, Jul 6, 2016 at 8:33 AM, Akira Hayakawa <ruby.wktk@gmail.com>
>> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I am using dm-flakey to emulate a broken device that should return
>> -EIO on
>>>>> both read and write.
>>>>> I use the parameter up_interval=0 and down_interval=1.
>>>>>
>>>>> But when I am dd-ing the flakey device, while write fails, read
>> succeeds.
>>>>> This isn't the behavior I expect.
>>>>>
>>>>> Then I looked into the code.
>>>>>
>>>>> 326 static int flakey_end_io(struct dm_target *ti, struct bio *bio, int
>>>>> error)
>>>>> 327 {
>>>>> 328 struct flakey_c *fc = ti->private;
>>>>> 329 struct per_bio_data *pb = dm_per_bio_data(bio,
>> sizeof(struct
>>>>> per_bio_data));
>>>>> 330
>>>>> 331 /*
>>>>> 332 * Corrupt successful READs while in down state.
>>>>> 333 * If flags were specified, only corrupt those that match.
>>>>> 334 */
>>>>> 335 if (fc->corrupt_bio_byte && !error && pb->bio_submitted &&
>>>>> 336 (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw ==
>>>>> READ) &&
>>>>> 337 all_corrupt_bio_flags_match(bio, fc))
>>>>> 338 corrupt_bio_data(bio, fc);
>>>>> 339
>>>>> 340 return error;
>>>>> 341 }
>>>>>
>>>>> When the bio direction is READ and currupt_bio_bytes isn't specified
>>>>> the READ bio is handled normally right?
>>>>>
>>>>> I think READ requests should return -EIO if
>>>>>
>>>>> 1. corrupt_bio_bytes isn't specified
>>>>> 2. but the requested is handled during down interval.
>>>>>
>>>>> as well as WRITE requests:
>>>>>
>>>>> 305 /*
>>>>> 306 * Corrupt matching writes.
>>>>> 307 */
>>>>> 308 if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw ==
>>>>> WRITE)) {
>>>>> 309 if (all_corrupt_bio_flags_match(bio, fc))
>>>>> 310 corrupt_bio_data(bio, fc);
>>>>> 311 goto map_bio;
>>>>> 312 }
>>>>> 313
>>>>> 314 /*
>>>>> 315 * By default, error all I/O.
>>>>> 316 */
>>>>> 317 return -EIO;
>>>>> 318 }
>>>>>
>>>>> This code is similar to what we see in the end_io.
>>>>>
>>>>> If my understanding is correct, I would like to modify dm-flakey to
>> return
>>>>> -EIO in the end_io.
>>>>>
>>>>> I would like to request for comments.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> - Akira
>>>>>
>>>>> --
>>>>> dm-devel mailing list
>>>>> dm-devel@redhat.com
>>>>> https://www.redhat.com/mailman/listinfo/dm-devel
>>>>>
>>>>
>>>>
>>>>
>>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dm-flakey NOT to return -EIO on READ?
2016-07-29 6:32 ` Akira Hayakawa
@ 2016-07-29 14:31 ` Lukas Herbolt
2016-07-29 14:35 ` Mike Snitzer
0 siblings, 1 reply; 12+ messages in thread
From: Lukas Herbolt @ 2016-07-29 14:31 UTC (permalink / raw)
To: Akira Hayakawa; +Cc: device-mapper development
[-- Attachment #1.1: Type: text/plain, Size: 5604 bytes --]
- http://people.redhat.com/~lherbolt/dm-flakey/v4.7-dm_flakey.tar.gz
Lukas
On Fri, Jul 29, 2016 at 8:32 AM, Akira Hayakawa <ruby.wktk@gmail.com> wrote:
> Hi Lukas,
>
> > I have patch ready, would you be able to test it before posting it.
> >
> > Let me know if I should build the kernel for you or just share the source
> > with you.
>
> OK.
> Please share the git tree and specify the branch to test.
>
> Akira
>
> On 2016/07/29 15:25, Lukas Herbolt wrote:
> > Hi Akira,
> > Sorry I was on holiday.
> > I have patch ready, would you be able to test it before posting it.
> >
> > Let me know if I should build the kernel for you or just share the source
> > with you.
> >
> > Lukas
> >
> > On Sun, Jul 17, 2016 at 3:36 AM, Akira Hayakawa <ruby.wktk@gmail.com>
> wrote:
> >
> >>> On 2016/07/06 18:52, Lukas Herbolt wrote:
> >>>> Hi,
> >>>> Yes this part is wrong and reads are not dropped.
> >>>> I have a patch ready, just have to send it out.
> >>
> >> btw, is this fix going to be included in the next release?
> >>
> >> On 2016/07/06 21:54, Akira Hayakawa wrote:
> >>> Thanks,
> >>>
> >>> I want the patch in the main-tree quickly.
> >>> Because without it, my tests will not be green.
> >>> This is really annoying.
> >>>
> >>> I made a quick reproducer of this problem.
> >>> If you are looking for one, this will help.
> >>> (You need to install sbt first)
> >>>
> >>> https://github.com/akiradeveloper/writeboost-test-suite
> >>>
> >>> test("flakey (read)") {
> >>> Memory(Sector.M(16)) { a =>
> >>> Flakey.Table(a, 0, 1).create { b =>
> >>> intercept[Exception] {
> >>> Shell(s"dd status=none if=${b.bdev.path} iflag=direct
> >> of=/dev/null")
> >>> }
> >>> }
> >>> }
> >>> }
> >>>
> >>> $ sudo sbt "testOnly dmtest.FlakeyTest"
> >>> 12:44:00.643 [pool-2-thread-3-ScalaTest-running-FlakeyTest] INFO dmtest
> >> - [TEST] flakey (read)
> >>> 12:44:00.752 [pool-2-thread-3-ScalaTest-running-FlakeyTest] DEBUG
> dmtest
> >> - reload: table=0 32768 flakey /dev/loop0 0 0 1
> >>> [info] FlakeyTest:
> >>> [info] - flakey (read) *** FAILED ***
> >>> [info] Expected exception java.lang.Exception to be thrown, but no
> >> exception was thrown. (FlakeyTest.scala:9)
> >>> [info] Run completed in 2 seconds, 781 milliseconds.
> >>> [info] Total number of tests run: 1
> >>> [info] Suites: completed 1, aborted 0
> >>> [info] Tests: succeeded 0, failed 1, canceled 0, ignored 0, pending 0
> >>>
> >>> - Akira
> >>>
> >>> On 2016/07/06 18:52, Lukas Herbolt wrote:
> >>>> Hi,
> >>>> Yes this part is wrong and reads are not dropped.
> >>>> I have a patch ready, just have to send it out.
> >>>>
> >>>> Lukas
> >>>>
> >>>> On Wed, Jul 6, 2016 at 8:33 AM, Akira Hayakawa <ruby.wktk@gmail.com>
> >> wrote:
> >>>>
> >>>>> Hi,
> >>>>>
> >>>>> I am using dm-flakey to emulate a broken device that should return
> >> -EIO on
> >>>>> both read and write.
> >>>>> I use the parameter up_interval=0 and down_interval=1.
> >>>>>
> >>>>> But when I am dd-ing the flakey device, while write fails, read
> >> succeeds.
> >>>>> This isn't the behavior I expect.
> >>>>>
> >>>>> Then I looked into the code.
> >>>>>
> >>>>> 326 static int flakey_end_io(struct dm_target *ti, struct bio *bio,
> int
> >>>>> error)
> >>>>> 327 {
> >>>>> 328 struct flakey_c *fc = ti->private;
> >>>>> 329 struct per_bio_data *pb = dm_per_bio_data(bio,
> >> sizeof(struct
> >>>>> per_bio_data));
> >>>>> 330
> >>>>> 331 /*
> >>>>> 332 * Corrupt successful READs while in down state.
> >>>>> 333 * If flags were specified, only corrupt those that
> match.
> >>>>> 334 */
> >>>>> 335 if (fc->corrupt_bio_byte && !error && pb->bio_submitted
> &&
> >>>>> 336 (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw ==
> >>>>> READ) &&
> >>>>> 337 all_corrupt_bio_flags_match(bio, fc))
> >>>>> 338 corrupt_bio_data(bio, fc);
> >>>>> 339
> >>>>> 340 return error;
> >>>>> 341 }
> >>>>>
> >>>>> When the bio direction is READ and currupt_bio_bytes isn't specified
> >>>>> the READ bio is handled normally right?
> >>>>>
> >>>>> I think READ requests should return -EIO if
> >>>>>
> >>>>> 1. corrupt_bio_bytes isn't specified
> >>>>> 2. but the requested is handled during down interval.
> >>>>>
> >>>>> as well as WRITE requests:
> >>>>>
> >>>>> 305 /*
> >>>>> 306 * Corrupt matching writes.
> >>>>> 307 */
> >>>>> 308 if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw
> ==
> >>>>> WRITE)) {
> >>>>> 309 if (all_corrupt_bio_flags_match(bio, fc))
> >>>>> 310 corrupt_bio_data(bio, fc);
> >>>>> 311 goto map_bio;
> >>>>> 312 }
> >>>>> 313
> >>>>> 314 /*
> >>>>> 315 * By default, error all I/O.
> >>>>> 316 */
> >>>>> 317 return -EIO;
> >>>>> 318 }
> >>>>>
> >>>>> This code is similar to what we see in the end_io.
> >>>>>
> >>>>> If my understanding is correct, I would like to modify dm-flakey to
> >> return
> >>>>> -EIO in the end_io.
> >>>>>
> >>>>> I would like to request for comments.
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> - Akira
> >>>>>
> >>>>> --
> >>>>> dm-devel mailing list
> >>>>> dm-devel@redhat.com
> >>>>> https://www.redhat.com/mailman/listinfo/dm-devel
> >>>>>
> >>>>
> >>>>
> >>>>
> >>
> >
> >
> >
>
--
Lukas Herbolt
RHCE, RH436, BSc, SSc
Senior Technical Support Engineer
Global Support Services (GSS)
Email: lherbolt@redhat.com
[-- Attachment #1.2: Type: text/html, Size: 9768 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dm-flakey NOT to return -EIO on READ?
2016-07-29 14:31 ` Lukas Herbolt
@ 2016-07-29 14:35 ` Mike Snitzer
2016-07-29 17:30 ` Mike Snitzer
0 siblings, 1 reply; 12+ messages in thread
From: Mike Snitzer @ 2016-07-29 14:35 UTC (permalink / raw)
To: Lukas Herbolt; +Cc: device-mapper development, Akira Hayakawa
On Fri, Jul 29 2016 at 10:31am -0400,
Lukas Herbolt <lherbolt@redhat.com> wrote:
> - http://people.redhat.com/~lherbolt/dm-flakey/v4.7-dm_flakey.tar.gz
>
> Lukas
Please post an incremental patch relative to 4.7 or latest Linus
kernel. Don't make people suffer with tarballs of whatever you've
packaged above.
Mike
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dm-flakey NOT to return -EIO on READ?
2016-07-29 14:35 ` Mike Snitzer
@ 2016-07-29 17:30 ` Mike Snitzer
2016-07-29 18:00 ` Lukas Herbolt
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Mike Snitzer @ 2016-07-29 17:30 UTC (permalink / raw)
To: Lukas Herbolt; +Cc: device-mapper development, Akira Hayakawa
On Fri, Jul 29 2016 at 10:35am -0400,
Mike Snitzer <snitzer@redhat.com> wrote:
> On Fri, Jul 29 2016 at 10:31am -0400,
> Lukas Herbolt <lherbolt@redhat.com> wrote:
>
> > - http://people.redhat.com/~lherbolt/dm-flakey/v4.7-dm_flakey.tar.gz
> >
> > Lukas
>
> Please post an incremental patch relative to 4.7 or latest Linus
> kernel. Don't make people suffer with tarballs of whatever you've
> packaged above.
I pulled out your incremental diff. Your proposed fix isn't correct. It
breaks the corrupt_bio_byte feature. We cannot blindly drop reads early
in flakey_map(), like you've proposed, because the corrupt_bio_byte
feature relies on corrupt data being returned on read during the
down_interval.
The issue reported is: reads aren't dropped during the "down_interval".
I do agree that this needs fixing, and it was commit a3998799fb4df ("dm
flakey: add corrupt_bio_byte feature") that caused this regression.
But a fix must not break the corrupt_bio_byte feature.
This fix should resolve the problem, I'll be staging it to go upstream
during the 4.8-rc cycle (Akira, please test to verify your test fails as
expected now):
From: Mike Snitzer <snitzer@redhat.com>
Date: Fri, 29 Jul 2016 13:19:55 -0400
Subject: [PATCH] dm flakey: error READ bios during the down_interval
When the corrupt_bio_byte feature was introduced it caused READ bios to
no longer be errored with -EIO during the down_interval. This had to do
with the complexity of needing to submit READs if the corrupt_bio_byte
feature was used.
Fix it so READ bios are properly errored with -EIO; doing so early in
flakey_map() as long as there isn't a match for the corrupt_bio_byte
feature.
Fixes: a3998799fb4df ("dm flakey: add corrupt_bio_byte feature")
Reported-by: Akira Hayakawa <ruby.wktk@gmail.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Cc: stable@vger.kernel.org
---
drivers/md/dm-flakey.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c
index 29b99fb..19db13e 100644
--- a/drivers/md/dm-flakey.c
+++ b/drivers/md/dm-flakey.c
@@ -289,10 +289,16 @@ static int flakey_map(struct dm_target *ti, struct bio *bio)
pb->bio_submitted = true;
/*
- * Map reads as normal.
+ * Map reads as normal only if corrupt_bio_byte set.
*/
- if (bio_data_dir(bio) == READ)
- goto map_bio;
+ if (bio_data_dir(bio) == READ) {
+ /* If flags were specified, only corrupt those that match. */
+ if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw == READ) &&
+ all_corrupt_bio_flags_match(bio, fc))
+ goto map_bio;
+ else
+ return -EIO;
+ }
/*
* Drop writes?
@@ -330,12 +336,13 @@ static int flakey_end_io(struct dm_target *ti, struct bio *bio, int error)
/*
* Corrupt successful READs while in down state.
- * If flags were specified, only corrupt those that match.
*/
- if (fc->corrupt_bio_byte && !error && pb->bio_submitted &&
- (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw == READ) &&
- all_corrupt_bio_flags_match(bio, fc))
- corrupt_bio_data(bio, fc);
+ if (!error && pb->bio_submitted && (bio_data_dir(bio) == READ)) {
+ if (fc->corrupt_bio_byte)
+ corrupt_bio_data(bio, fc);
+ else
+ return -EIO;
+ }
return error;
}
--
2.7.4 (Apple Git-66)
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: dm-flakey NOT to return -EIO on READ?
2016-07-29 17:30 ` Mike Snitzer
@ 2016-07-29 18:00 ` Lukas Herbolt
2016-07-29 22:55 ` Akira Hayakawa
2016-07-30 0:26 ` Akira Hayakawa
2 siblings, 0 replies; 12+ messages in thread
From: Lukas Herbolt @ 2016-07-29 18:00 UTC (permalink / raw)
To: Mike Snitzer; +Cc: device-mapper development, Akira Hayakawa
[-- Attachment #1.1: Type: text/plain, Size: 4247 bytes --]
Thanks Mike for fixing.
This was my first attempt to fix something in kernel so I wasn't sure about
the patch itself.
- Lukas
On Fri, Jul 29, 2016 at 7:30 PM, Mike Snitzer <snitzer@redhat.com> wrote:
> On Fri, Jul 29 2016 at 10:35am -0400,
> Mike Snitzer <snitzer@redhat.com> wrote:
>
> > On Fri, Jul 29 2016 at 10:31am -0400,
> > Lukas Herbolt <lherbolt@redhat.com> wrote:
> >
> > > - http://people.redhat.com/~lherbolt/dm-flakey/v4.7-dm_flakey.tar.gz
> > >
> > > Lukas
> >
> > Please post an incremental patch relative to 4.7 or latest Linus
> > kernel. Don't make people suffer with tarballs of whatever you've
> > packaged above.
>
> I pulled out your incremental diff. Your proposed fix isn't correct. It
> breaks the corrupt_bio_byte feature. We cannot blindly drop reads early
> in flakey_map(), like you've proposed, because the corrupt_bio_byte
> feature relies on corrupt data being returned on read during the
> down_interval.
>
> The issue reported is: reads aren't dropped during the "down_interval".
>
> I do agree that this needs fixing, and it was commit a3998799fb4df ("dm
> flakey: add corrupt_bio_byte feature") that caused this regression.
>
> But a fix must not break the corrupt_bio_byte feature.
>
> This fix should resolve the problem, I'll be staging it to go upstream
> during the 4.8-rc cycle (Akira, please test to verify your test fails as
> expected now):
>
> From: Mike Snitzer <snitzer@redhat.com>
> Date: Fri, 29 Jul 2016 13:19:55 -0400
> Subject: [PATCH] dm flakey: error READ bios during the down_interval
>
> When the corrupt_bio_byte feature was introduced it caused READ bios to
> no longer be errored with -EIO during the down_interval. This had to do
> with the complexity of needing to submit READs if the corrupt_bio_byte
> feature was used.
>
> Fix it so READ bios are properly errored with -EIO; doing so early in
> flakey_map() as long as there isn't a match for the corrupt_bio_byte
> feature.
>
> Fixes: a3998799fb4df ("dm flakey: add corrupt_bio_byte feature")
> Reported-by: Akira Hayakawa <ruby.wktk@gmail.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> Cc: stable@vger.kernel.org
> ---
> drivers/md/dm-flakey.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c
> index 29b99fb..19db13e 100644
> --- a/drivers/md/dm-flakey.c
> +++ b/drivers/md/dm-flakey.c
> @@ -289,10 +289,16 @@ static int flakey_map(struct dm_target *ti, struct
> bio *bio)
> pb->bio_submitted = true;
>
> /*
> - * Map reads as normal.
> + * Map reads as normal only if corrupt_bio_byte set.
> */
> - if (bio_data_dir(bio) == READ)
> - goto map_bio;
> + if (bio_data_dir(bio) == READ) {
> + /* If flags were specified, only corrupt those
> that match. */
> + if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw ==
> READ) &&
> + all_corrupt_bio_flags_match(bio, fc))
> + goto map_bio;
> + else
> + return -EIO;
> + }
>
> /*
> * Drop writes?
> @@ -330,12 +336,13 @@ static int flakey_end_io(struct dm_target *ti,
> struct bio *bio, int error)
>
> /*
> * Corrupt successful READs while in down state.
> - * If flags were specified, only corrupt those that match.
> */
> - if (fc->corrupt_bio_byte && !error && pb->bio_submitted &&
> - (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw == READ) &&
> - all_corrupt_bio_flags_match(bio, fc))
> - corrupt_bio_data(bio, fc);
> + if (!error && pb->bio_submitted && (bio_data_dir(bio) == READ)) {
> + if (fc->corrupt_bio_byte)
> + corrupt_bio_data(bio, fc);
> + else
> + return -EIO;
> + }
>
> return error;
> }
> --
> 2.7.4 (Apple Git-66)
>
>
--
Lukas Herbolt
RHCE, RH436, BSc, SSc
Senior Technical Support Engineer
Global Support Services (GSS)
Email: lherbolt@redhat.com
[-- Attachment #1.2: Type: text/html, Size: 6565 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dm-flakey NOT to return -EIO on READ?
2016-07-29 17:30 ` Mike Snitzer
2016-07-29 18:00 ` Lukas Herbolt
@ 2016-07-29 22:55 ` Akira Hayakawa
2016-07-30 0:26 ` Akira Hayakawa
2 siblings, 0 replies; 12+ messages in thread
From: Akira Hayakawa @ 2016-07-29 22:55 UTC (permalink / raw)
To: Mike Snitzer, Lukas Herbolt; +Cc: device-mapper development
I just looked at the patch. (not tested yet)
I think the else part in the endio isn't necessary.
> + else
> + return -EIO;
Akira
On 2016/07/30 2:30, Mike Snitzer wrote:
> On Fri, Jul 29 2016 at 10:35am -0400,
> Mike Snitzer <snitzer@redhat.com> wrote:
>
>> On Fri, Jul 29 2016 at 10:31am -0400,
>> Lukas Herbolt <lherbolt@redhat.com> wrote:
>>
>>> - http://people.redhat.com/~lherbolt/dm-flakey/v4.7-dm_flakey.tar.gz
>>>
>>> Lukas
>>
>> Please post an incremental patch relative to 4.7 or latest Linus
>> kernel. Don't make people suffer with tarballs of whatever you've
>> packaged above.
>
> I pulled out your incremental diff. Your proposed fix isn't correct. It
> breaks the corrupt_bio_byte feature. We cannot blindly drop reads early
> in flakey_map(), like you've proposed, because the corrupt_bio_byte
> feature relies on corrupt data being returned on read during the
> down_interval.
>
> The issue reported is: reads aren't dropped during the "down_interval".
>
> I do agree that this needs fixing, and it was commit a3998799fb4df ("dm
> flakey: add corrupt_bio_byte feature") that caused this regression.
>
> But a fix must not break the corrupt_bio_byte feature.
>
> This fix should resolve the problem, I'll be staging it to go upstream
> during the 4.8-rc cycle (Akira, please test to verify your test fails as
> expected now):
>
> From: Mike Snitzer <snitzer@redhat.com>
> Date: Fri, 29 Jul 2016 13:19:55 -0400
> Subject: [PATCH] dm flakey: error READ bios during the down_interval
>
> When the corrupt_bio_byte feature was introduced it caused READ bios to
> no longer be errored with -EIO during the down_interval. This had to do
> with the complexity of needing to submit READs if the corrupt_bio_byte
> feature was used.
>
> Fix it so READ bios are properly errored with -EIO; doing so early in
> flakey_map() as long as there isn't a match for the corrupt_bio_byte
> feature.
>
> Fixes: a3998799fb4df ("dm flakey: add corrupt_bio_byte feature")
> Reported-by: Akira Hayakawa <ruby.wktk@gmail.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> Cc: stable@vger.kernel.org
> ---
> drivers/md/dm-flakey.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c
> index 29b99fb..19db13e 100644
> --- a/drivers/md/dm-flakey.c
> +++ b/drivers/md/dm-flakey.c
> @@ -289,10 +289,16 @@ static int flakey_map(struct dm_target *ti, struct bio *bio)
> pb->bio_submitted = true;
>
> /*
> - * Map reads as normal.
> + * Map reads as normal only if corrupt_bio_byte set.
> */
> - if (bio_data_dir(bio) == READ)
> - goto map_bio;
> + if (bio_data_dir(bio) == READ) {
> + /* If flags were specified, only corrupt those that match. */
> + if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw == READ) &&
> + all_corrupt_bio_flags_match(bio, fc))
> + goto map_bio;
> + else
> + return -EIO;
> + }
>
> /*
> * Drop writes?
> @@ -330,12 +336,13 @@ static int flakey_end_io(struct dm_target *ti, struct bio *bio, int error)
>
> /*
> * Corrupt successful READs while in down state.
> - * If flags were specified, only corrupt those that match.
> */
> - if (fc->corrupt_bio_byte && !error && pb->bio_submitted &&
> - (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw == READ) &&
> - all_corrupt_bio_flags_match(bio, fc))
> - corrupt_bio_data(bio, fc);
> + if (!error && pb->bio_submitted && (bio_data_dir(bio) == READ)) {
> + if (fc->corrupt_bio_byte)
> + corrupt_bio_data(bio, fc);
> + else
> + return -EIO;
> + }
>
> return error;
> }
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: dm-flakey NOT to return -EIO on READ?
2016-07-29 17:30 ` Mike Snitzer
2016-07-29 18:00 ` Lukas Herbolt
2016-07-29 22:55 ` Akira Hayakawa
@ 2016-07-30 0:26 ` Akira Hayakawa
2 siblings, 0 replies; 12+ messages in thread
From: Akira Hayakawa @ 2016-07-30 0:26 UTC (permalink / raw)
To: Mike Snitzer; +Cc: device-mapper development, Lukas Herbolt
Mike,
> This fix should resolve the problem, I'll be staging it to go upstream
> during the 4.8-rc cycle (Akira, please test to verify your test fails as
> expected now):
I tested with the patch and it's now ok.
Akira
On 2016/07/30 2:30, Mike Snitzer wrote:
> On Fri, Jul 29 2016 at 10:35am -0400,
> Mike Snitzer <snitzer@redhat.com> wrote:
>
>> On Fri, Jul 29 2016 at 10:31am -0400,
>> Lukas Herbolt <lherbolt@redhat.com> wrote:
>>
>>> - http://people.redhat.com/~lherbolt/dm-flakey/v4.7-dm_flakey.tar.gz
>>>
>>> Lukas
>>
>> Please post an incremental patch relative to 4.7 or latest Linus
>> kernel. Don't make people suffer with tarballs of whatever you've
>> packaged above.
>
> I pulled out your incremental diff. Your proposed fix isn't correct. It
> breaks the corrupt_bio_byte feature. We cannot blindly drop reads early
> in flakey_map(), like you've proposed, because the corrupt_bio_byte
> feature relies on corrupt data being returned on read during the
> down_interval.
>
> The issue reported is: reads aren't dropped during the "down_interval".
>
> I do agree that this needs fixing, and it was commit a3998799fb4df ("dm
> flakey: add corrupt_bio_byte feature") that caused this regression.
>
> But a fix must not break the corrupt_bio_byte feature.
>
> This fix should resolve the problem, I'll be staging it to go upstream
> during the 4.8-rc cycle (Akira, please test to verify your test fails as
> expected now):
>
> From: Mike Snitzer <snitzer@redhat.com>
> Date: Fri, 29 Jul 2016 13:19:55 -0400
> Subject: [PATCH] dm flakey: error READ bios during the down_interval
>
> When the corrupt_bio_byte feature was introduced it caused READ bios to
> no longer be errored with -EIO during the down_interval. This had to do
> with the complexity of needing to submit READs if the corrupt_bio_byte
> feature was used.
>
> Fix it so READ bios are properly errored with -EIO; doing so early in
> flakey_map() as long as there isn't a match for the corrupt_bio_byte
> feature.
>
> Fixes: a3998799fb4df ("dm flakey: add corrupt_bio_byte feature")
> Reported-by: Akira Hayakawa <ruby.wktk@gmail.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> Cc: stable@vger.kernel.org
> ---
> drivers/md/dm-flakey.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c
> index 29b99fb..19db13e 100644
> --- a/drivers/md/dm-flakey.c
> +++ b/drivers/md/dm-flakey.c
> @@ -289,10 +289,16 @@ static int flakey_map(struct dm_target *ti, struct bio *bio)
> pb->bio_submitted = true;
>
> /*
> - * Map reads as normal.
> + * Map reads as normal only if corrupt_bio_byte set.
> */
> - if (bio_data_dir(bio) == READ)
> - goto map_bio;
> + if (bio_data_dir(bio) == READ) {
> + /* If flags were specified, only corrupt those that match. */
> + if (fc->corrupt_bio_byte && (fc->corrupt_bio_rw == READ) &&
> + all_corrupt_bio_flags_match(bio, fc))
> + goto map_bio;
> + else
> + return -EIO;
> + }
>
> /*
> * Drop writes?
> @@ -330,12 +336,13 @@ static int flakey_end_io(struct dm_target *ti, struct bio *bio, int error)
>
> /*
> * Corrupt successful READs while in down state.
> - * If flags were specified, only corrupt those that match.
> */
> - if (fc->corrupt_bio_byte && !error && pb->bio_submitted &&
> - (bio_data_dir(bio) == READ) && (fc->corrupt_bio_rw == READ) &&
> - all_corrupt_bio_flags_match(bio, fc))
> - corrupt_bio_data(bio, fc);
> + if (!error && pb->bio_submitted && (bio_data_dir(bio) == READ)) {
> + if (fc->corrupt_bio_byte)
> + corrupt_bio_data(bio, fc);
> + else
> + return -EIO;
> + }
>
> return error;
> }
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-07-30 0:26 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-06 6:33 dm-flakey NOT to return -EIO on READ? Akira Hayakawa
2016-07-06 9:52 ` Lukas Herbolt
2016-07-06 12:54 ` Akira Hayakawa
2016-07-17 1:36 ` Akira Hayakawa
2016-07-29 6:25 ` Lukas Herbolt
2016-07-29 6:32 ` Akira Hayakawa
2016-07-29 14:31 ` Lukas Herbolt
2016-07-29 14:35 ` Mike Snitzer
2016-07-29 17:30 ` Mike Snitzer
2016-07-29 18:00 ` Lukas Herbolt
2016-07-29 22:55 ` Akira Hayakawa
2016-07-30 0:26 ` Akira Hayakawa
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.