2006-08-23 11:35:38

by Akinobu Mita

[permalink] [raw]
Subject: [patch 4/5] fail-injection capability for disk IO

This patch provides fail-injection capability for disk IO.

Boot option:

fail_make_request=<probability>,<interval>,<times>,<space>

<probability>

specifies how often it should fail in percent.

<interval>

specifies the interval of failures.

<times>

specifies how many times failures may happen at most.

<space>

specifies the size of free space where disk IO can be issued
safely in bytes.

Example:

fail_make_request=100,10,-1,0

generic_make_request() fails once per 10 times.

Cc: Jens Axboe <[email protected]>
Signed-off-by: Akinobu Mita <[email protected]>

block/ll_rw_blk.c | 19 +++++++++++++++++++
include/linux/should_fail.h | 4 ++++
lib/Kconfig.debug | 7 +++++++
3 files changed, 30 insertions(+)

Index: work-failmalloc/block/ll_rw_blk.c
===================================================================
--- work-failmalloc.orig/block/ll_rw_blk.c
+++ work-failmalloc/block/ll_rw_blk.c
@@ -28,6 +28,7 @@
#include <linux/interrupt.h>
#include <linux/cpu.h>
#include <linux/blktrace_api.h>
+#include <linux/should_fail.h>

/*
* for max sense size
@@ -2993,6 +2994,21 @@ static void handle_bad_sector(struct bio
set_bit(BIO_EOF, &bio->bi_flags);
}

+#ifdef CONFIG_FAIL_MAKE_REQUEST
+
+static DEFINE_SHOULD_FAIL(fail_make_request_data);
+
+static int __init setup_fail_make_request(char *str)
+{
+ return setup_should_fail(&fail_make_request_data, str);
+}
+__setup("fail_make_request=", setup_fail_make_request);
+
+struct should_fail_data *fail_make_request = &fail_make_request_data;
+EXPORT_SYMBOL_GPL(fail_make_request);
+
+#endif
+
/**
* generic_make_request: hand a buffer to its device driver for I/O
* @bio: The bio describing the location in memory and on the device.
@@ -3077,6 +3093,9 @@ end_io:
if (unlikely(test_bit(QUEUE_FLAG_DEAD, &q->queue_flags)))
goto end_io;

+ if (should_fail(fail_make_request, bio->bi_size))
+ goto end_io;
+
/*
* If this device has partitions, remap block n
* of partition p to block n+start(p) of the disk.
Index: work-failmalloc/lib/Kconfig.debug
===================================================================
--- work-failmalloc.orig/lib/Kconfig.debug
+++ work-failmalloc/lib/Kconfig.debug
@@ -386,3 +386,10 @@ config FAIL_PAGE_ALLOC
help
This option provides fault-injection capabilitiy for alloc_pages().

+config FAIL_MAKE_REQUEST
+ bool "fault-injection capabilitiy for disk IO"
+ depends on DEBUG_KERNEL
+ select SHOULD_FAIL
+ help
+ This option provides fault-injection capabilitiy to disk IO.
+
Index: work-failmalloc/include/linux/should_fail.h
===================================================================
--- work-failmalloc.orig/include/linux/should_fail.h
+++ work-failmalloc/include/linux/should_fail.h
@@ -43,6 +43,10 @@ extern struct should_fail_data *failslab
extern struct should_fail_data *fail_page_alloc;
#endif

+#ifdef CONFIG_FAIL_MAKE_REQUEST
+extern struct should_fail_data *fail_make_request;
+#endif
+
#else

#define should_fail(data, size) (0)

--


2006-08-23 12:01:37

by Jens Axboe

[permalink] [raw]
Subject: Re: [patch 4/5] fail-injection capability for disk IO

On Wed, Aug 23 2006, Akinobu Mita wrote:
> This patch provides fail-injection capability for disk IO.
>
> Boot option:
>
> fail_make_request=<probability>,<interval>,<times>,<space>
>
> <probability>
>
> specifies how often it should fail in percent.
>
> <interval>
>
> specifies the interval of failures.
>
> <times>
>
> specifies how many times failures may happen at most.
>
> <space>
>
> specifies the size of free space where disk IO can be issued
> safely in bytes.
>
> Example:
>
> fail_make_request=100,10,-1,0
>
> generic_make_request() fails once per 10 times.

Hmm dunno, seems a pretty useless feature to me. Wouldn't it make a lot
more sense to do this per-queue instead of a global entity?

--
Jens Axboe

2006-08-23 12:07:13

by Andi Kleen

[permalink] [raw]
Subject: Re: [patch 4/5] fail-injection capability for disk IO

Akinobu Mita <[email protected]> writes:
> * @bio: The bio describing the location in memory and on the device.
> @@ -3077,6 +3093,9 @@ end_io:
> if (unlikely(test_bit(QUEUE_FLAG_DEAD, &q->queue_flags)))
> goto end_io;
>
> + if (should_fail(fail_make_request, bio->bi_size))
> + goto end_io;

AFAIK it is reasonably easy to write stacking block drivers.
I think I would prefer a stackable driver instead of this hook.

-Andi

2006-08-23 12:08:12

by Jens Axboe

[permalink] [raw]
Subject: Re: [patch 4/5] fail-injection capability for disk IO

On Wed, Aug 23 2006, Andi Kleen wrote:
> Akinobu Mita <[email protected]> writes:
> > * @bio: The bio describing the location in memory and on the device.
> > @@ -3077,6 +3093,9 @@ end_io:
> > if (unlikely(test_bit(QUEUE_FLAG_DEAD, &q->queue_flags)))
> > goto end_io;
> >
> > + if (should_fail(fail_make_request, bio->bi_size))
> > + goto end_io;
>
> AFAIK it is reasonably easy to write stacking block drivers.
> I think I would prefer a stackable driver instead of this hook.

But that makes it more tricky to setup a test, since you have to change
from using /dev/sda (for example) to /dev/stacked-driver.

--
Jens Axboe

2006-08-23 17:27:57

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch 4/5] fail-injection capability for disk IO

On Wed, 23 Aug 2006 14:03:55 +0200
Jens Axboe <[email protected]> wrote:

> On Wed, Aug 23 2006, Akinobu Mita wrote:
> > This patch provides fail-injection capability for disk IO.
> >
> > Boot option:
> >
> > fail_make_request=<probability>,<interval>,<times>,<space>
> >
> > <probability>
> >
> > specifies how often it should fail in percent.
> >
> > <interval>
> >
> > specifies the interval of failures.
> >
> > <times>
> >
> > specifies how many times failures may happen at most.
> >
> > <space>
> >
> > specifies the size of free space where disk IO can be issued
> > safely in bytes.
> >
> > Example:
> >
> > fail_make_request=100,10,-1,0
> >
> > generic_make_request() fails once per 10 times.
>
> Hmm dunno, seems a pretty useless feature to me.

We need it. What is the FS/VFS/VM behaviour in the presence of IO
errors? Nobody knows, because we rarely test it. Those few times where
people _do_ test it (the hard way), bad things tend to happen. reiserfs
(for example) likes to go wobble, wobble, wobble, BUG.

> Wouldn't it make a lot
> more sense to do this per-queue instead of a global entity?

Yes, I think so. /sys/block/sda/sda2/make-it-fail.

2006-08-23 17:58:58

by Jens Axboe

[permalink] [raw]
Subject: Re: [patch 4/5] fail-injection capability for disk IO

On Wed, Aug 23 2006, Andrew Morton wrote:
> On Wed, 23 Aug 2006 14:03:55 +0200
> Jens Axboe <[email protected]> wrote:
>
> > On Wed, Aug 23 2006, Akinobu Mita wrote:
> > > This patch provides fail-injection capability for disk IO.
> > >
> > > Boot option:
> > >
> > > fail_make_request=<probability>,<interval>,<times>,<space>
> > >
> > > <probability>
> > >
> > > specifies how often it should fail in percent.
> > >
> > > <interval>
> > >
> > > specifies the interval of failures.
> > >
> > > <times>
> > >
> > > specifies how many times failures may happen at most.
> > >
> > > <space>
> > >
> > > specifies the size of free space where disk IO can be issued
> > > safely in bytes.
> > >
> > > Example:
> > >
> > > fail_make_request=100,10,-1,0
> > >
> > > generic_make_request() fails once per 10 times.
> >
> > Hmm dunno, seems a pretty useless feature to me.
>
> We need it. What is the FS/VFS/VM behaviour in the presence of IO
> errors? Nobody knows, because we rarely test it. Those few times where
> people _do_ test it (the hard way), bad things tend to happen. reiserfs
> (for example) likes to go wobble, wobble, wobble, BUG.

You misunderstood me - a global parameter is useless, as it makes it
pretty impossible for people to use this for any sort of testing (unless
it's very specialized). I didn't say a feature to test io errors was
useless!

> > Wouldn't it make a lot
> > more sense to do this per-queue instead of a global entity?
>
> Yes, I think so. /sys/block/sda/sda2/make-it-fail.

Precisely.

--
Jens Axboe

2006-08-23 18:17:28

by Ric Wheeler

[permalink] [raw]
Subject: Re: [patch 4/5] fail-injection capability for disk IO

Jens Axboe wrote:
> On Wed, Aug 23 2006, Andrew Morton wrote:
>
>>On Wed, 23 Aug 2006 14:03:55 +0200
>>Jens Axboe <[email protected]> wrote:
>>
>>
>>>On Wed, Aug 23 2006, Akinobu Mita wrote:
>>>
>>>>This patch provides fail-injection capability for disk IO.
>>>>
>>>>Boot option:
>>>>
>>>> fail_make_request=<probability>,<interval>,<times>,<space>
>>>>
>>>> <probability>
>>>>
>>>> specifies how often it should fail in percent.
>>>>
>>>> <interval>
>>>>
>>>> specifies the interval of failures.
>>>>
>>>> <times>
>>>>
>>>> specifies how many times failures may happen at most.
>>>>
>>>> <space>
>>>>
>>>> specifies the size of free space where disk IO can be issued
>>>> safely in bytes.
>>>>
>>>>Example:
>>>>
>>>> fail_make_request=100,10,-1,0
>>>>
>>>>generic_make_request() fails once per 10 times.
>>>
>>>Hmm dunno, seems a pretty useless feature to me.
>>
>>We need it. What is the FS/VFS/VM behaviour in the presence of IO
>>errors? Nobody knows, because we rarely test it. Those few times where
>>people _do_ test it (the hard way), bad things tend to happen. reiserfs
>>(for example) likes to go wobble, wobble, wobble, BUG.
>
>
> You misunderstood me - a global parameter is useless, as it makes it
> pretty impossible for people to use this for any sort of testing (unless
> it's very specialized). I didn't say a feature to test io errors was
> useless!
>
>
>>>Wouldn't it make a lot
>>>more sense to do this per-queue instead of a global entity?
>>
>>Yes, I think so. /sys/block/sda/sda2/make-it-fail.
>
>
> Precisely.
>

I think that this is very useful for testing file systems.

What this will miss is the error path through the lower levels of the IO
path (i.e., the libata/SCSI error handling confusion that Mark Lord has
been working on patches for would need some error injection at or below
the libata level).

We currently test this whole path with either weird fault injection gear
to hit the s-ata bus or the old fashion pile of moderately flaky disks
that we try hard not to fix or totally kill.

It would be really useful to get something (target mode SW disk? libata
or other low level error injection?) to test this whole path in software...

ric


2006-08-23 18:22:31

by Hans Reiser

[permalink] [raw]
Subject: Re: [patch 4/5] fail-injection capability for disk IO

Andrew Morton wrote:
> On Wed, 23 Aug 2006 14:03:55 +0200
> Jens Axboe <[email protected]> wrote:
>
>
>> On Wed, Aug 23 2006, Akinobu Mita wrote:
>>
>>> This patch provides fail-injection capability for disk IO.
>>>
>>> Boot option:
>>>
>>> fail_make_request=<probability>,<interval>,<times>,<space>
>>>
>>> <probability>
>>>
>>> specifies how often it should fail in percent.
>>>
>>> <interval>
>>>
>>> specifies the interval of failures.
>>>
>>> <times>
>>>
>>> specifies how many times failures may happen at most.
>>>
>>> <space>
>>>
>>> specifies the size of free space where disk IO can be issued
>>> safely in bytes.
>>>
>>> Example:
>>>
>>> fail_make_request=100,10,-1,0
>>>
>>> generic_make_request() fails once per 10 times.
>>>
>> Hmm dunno, seems a pretty useless feature to me.
>>
>
> We need it. What is the FS/VFS/VM behaviour in the presence of IO
> errors? Nobody knows, because we rarely test it. Those few times where
> people _do_ test it (the hard way), bad things tend to happen. reiserfs
> (for example) likes to go wobble, wobble, wobble, BUG.
>
The iron folks tested it, and we did better than other FS's. That said,
it seems like a valuable feature to me.

2006-08-23 18:24:13

by Jens Axboe

[permalink] [raw]
Subject: Re: [patch 4/5] fail-injection capability for disk IO

On Wed, Aug 23 2006, Ric Wheeler wrote:
> Jens Axboe wrote:
> >On Wed, Aug 23 2006, Andrew Morton wrote:
> >
> >>On Wed, 23 Aug 2006 14:03:55 +0200
> >>Jens Axboe <[email protected]> wrote:
> >>
> >>
> >>>On Wed, Aug 23 2006, Akinobu Mita wrote:
> >>>
> >>>>This patch provides fail-injection capability for disk IO.
> >>>>
> >>>>Boot option:
> >>>>
> >>>> fail_make_request=<probability>,<interval>,<times>,<space>
> >>>>
> >>>> <probability>
> >>>>
> >>>> specifies how often it should fail in percent.
> >>>>
> >>>> <interval>
> >>>>
> >>>> specifies the interval of failures.
> >>>>
> >>>> <times>
> >>>>
> >>>> specifies how many times failures may happen at most.
> >>>>
> >>>> <space>
> >>>>
> >>>> specifies the size of free space where disk IO can be issued
> >>>> safely in bytes.
> >>>>
> >>>>Example:
> >>>>
> >>>> fail_make_request=100,10,-1,0
> >>>>
> >>>>generic_make_request() fails once per 10 times.
> >>>
> >>>Hmm dunno, seems a pretty useless feature to me.
> >>
> >>We need it. What is the FS/VFS/VM behaviour in the presence of IO
> >>errors? Nobody knows, because we rarely test it. Those few times where
> >>people _do_ test it (the hard way), bad things tend to happen. reiserfs
> >>(for example) likes to go wobble, wobble, wobble, BUG.
> >
> >
> >You misunderstood me - a global parameter is useless, as it makes it
> >pretty impossible for people to use this for any sort of testing (unless
> >it's very specialized). I didn't say a feature to test io errors was
> >useless!
> >
> >
> >>>Wouldn't it make a lot
> >>>more sense to do this per-queue instead of a global entity?
> >>
> >>Yes, I think so. /sys/block/sda/sda2/make-it-fail.
> >
> >
> >Precisely.
> >
>
> I think that this is very useful for testing file systems.
>
> What this will miss is the error path through the lower levels of the IO
> path (i.e., the libata/SCSI error handling confusion that Mark Lord has
> been working on patches for would need some error injection at or below
> the libata level).
>
> We currently test this whole path with either weird fault injection gear
> to hit the s-ata bus or the old fashion pile of moderately flaky disks
> that we try hard not to fix or totally kill.
>
> It would be really useful to get something (target mode SW disk? libata
> or other low level error injection?) to test this whole path in software...

Yes, this approach only tests the layer(s) above the device. To simulate
hardware failure or timeouts, I _think_ scsi_debug can already help you
quite a bit. If not, it should be easy enough to extend do add these
sorts of things.

--
Jens Axboe

2006-08-23 19:34:49

by Mario 'BitKoenig' Holbe

[permalink] [raw]
Subject: Re: [patch 4/5] fail-injection capability for disk IO

Jens Axboe <[email protected]> wrote:
> On Wed, Aug 23 2006, Andi Kleen wrote:
>> I think I would prefer a stackable driver instead of this hook.

I second this, preferrably a device-mapper target similar to dm-error.

> But that makes it more tricky to setup a test, since you have to change
> from using /dev/sda (for example) to /dev/stacked-driver.

Do you really think somebody would run such tests on otherwise normally
used devices?


regards
Mario
--
There are two major products that come from Berkeley: LSD and UNIX.
We don't believe this to be a coincidence. -- Jeremy S. Anderson

2006-08-23 19:43:24

by Ric Wheeler

[permalink] [raw]
Subject: Re: [patch 4/5] fail-injection capability for disk IO

Mario 'BitKoenig' Holbe wrote:
> Jens Axboe <[email protected]> wrote:
>
>>On Wed, Aug 23 2006, Andi Kleen wrote:
>>
>>>I think I would prefer a stackable driver instead of this hook.
>
>
> I second this, preferrably a device-mapper target similar to dm-error.
>
>
>>But that makes it more tricky to setup a test, since you have to change
>>from using /dev/sda (for example) to /dev/stacked-driver.
>
>
> Do you really think somebody would run such tests on otherwise normally
> used devices?
>

We certainly run this kind of tests on a routine basis - before we ship
a kernel to our installed field, we need to verify that it will handle
disk IO errors correctly.

In our case, the tests are run on a farm of machines that get pxe'ed to
a specific image, tested (usually by sticking in a disk known to be bad
enough to cause reliable errors ;-)) and then we watch to see that the
errors do not cause hangs, etc.

Having a requirement to change our standard image (sda ->
stacked-driver) would not be impossible, but would be less convenient...

ric