change log: v2 - Resolve build issue in mmc core.c due to multiple init_module by removing the fault inject module. - Export fault injection functions to make them available for modules - Update fault injection documentation on MMC IO v3 - add function descriptions in core.c - use export GPL for fault injection functions v4 - make the fault_attr per host. This prepares for upcoming patch from Akinobu that adds support for creating debugfs entries in arbitrary directory. v5 - Make use of fault_create_debugfs_attr() in Akinobu's patch "fault-injection: add ability to export fault_attr in...". v6 - Fix typo in commit message in patch "export fault injection functions" v7 - Don't compile in boot param setup function if mmc-core is built as module. v8 - Update fault injection documentation. Add fail_mmc_request to boot option section.
Per Forlin (3): fault-inject: export fault injection functions mmc: core: add random fault injection fault injection: add documentation on MMC IO fault injection
Documentation/fault-injection/fault-injection.txt | 8 +++- drivers/mmc/core/core.c | 44 +++++++++++++++++++++ drivers/mmc/core/debugfs.c | 27 +++++++++++++ include/linux/mmc/host.h | 7 +++ lib/Kconfig.debug | 11 +++++ lib/fault-inject.c | 2 + 6 files changed, 98 insertions(+), 1 deletions(-)
export symbols should_fail() and fault_create_debugfs_attr() in order to let modules utilize the fault injection
Signed-off-by: Per Forlin per.forlin@linaro.org Acked-by: Akinobu Mita akinobu.mita@gmail.com --- lib/fault-inject.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/lib/fault-inject.c b/lib/fault-inject.c index f193b77..328d433 100644 --- a/lib/fault-inject.c +++ b/lib/fault-inject.c @@ -130,6 +130,7 @@ bool should_fail(struct fault_attr *attr, ssize_t size)
return true; } +EXPORT_SYMBOL_GPL(should_fail);
#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
@@ -243,5 +244,6 @@ fail:
return ERR_PTR(-ENOMEM); } +EXPORT_SYMBOL_GPL(fault_create_debugfs_attr);
#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
This adds support to inject data errors after a completed host transfer. The mmc core will return error even though the host transfer is successful. This simple fault injection proved to be very useful to test the non-blocking error handling in the mmc_blk_issue_rw_rq(). Random faults can also test how the host driver handles pre_req() and post_req() in case of errors.
Signed-off-by: Per Forlin per.forlin@linaro.org Acked-by: Akinobu Mita akinobu.mita@gmail.com --- drivers/mmc/core/core.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ drivers/mmc/core/debugfs.c | 27 +++++++++++++++++++++++++++ include/linux/mmc/host.h | 7 +++++++ lib/Kconfig.debug | 11 +++++++++++ 4 files changed, 89 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 89bdeae..a4996b0 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -25,6 +25,11 @@ #include <linux/pm_runtime.h> #include <linux/suspend.h>
+#ifdef CONFIG_FAIL_MMC_REQUEST +#include <linux/fault-inject.h> +#include <linux/random.h> +#endif + #include <linux/mmc/card.h> #include <linux/mmc/host.h> #include <linux/mmc/mmc.h> @@ -83,6 +88,43 @@ static void mmc_flush_scheduled_work(void) flush_workqueue(workqueue); }
+#ifdef CONFIG_FAIL_MMC_REQUEST + +/* + * Internal function. Inject random data errors. + * If mmc_data is NULL no errors are injected. + */ +static void mmc_should_fail_request(struct mmc_host *host, + struct mmc_request *mrq) +{ + struct mmc_command *cmd = mrq->cmd; + struct mmc_data *data = mrq->data; + static const int data_errors[] = { + -ETIMEDOUT, + -EILSEQ, + -EIO, + }; + + if (!data) + return; + + if (cmd->error || data->error || + !should_fail(&host->fail_mmc_request, data->blksz * data->blocks)) + return; + + data->error = data_errors[random32() % ARRAY_SIZE(data_errors)]; + data->bytes_xfered = (random32() % (data->bytes_xfered >> 9)) << 9; +} + +#else /* CONFIG_FAIL_MMC_REQUEST */ + +static void mmc_should_fail_request(struct mmc_host *host, + struct mmc_request *mrq) +{ +} + +#endif /* CONFIG_FAIL_MMC_REQUEST */ + /** * mmc_request_done - finish processing an MMC request * @host: MMC host which completed request @@ -109,6 +151,8 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq) cmd->error = 0; host->ops->request(host, mrq); } else { + mmc_should_fail_request(host, mrq); + led_trigger_event(host->led, LED_OFF);
pr_debug("%s: req done (CMD%u): %d: %08x %08x %08x %08x\n", diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c index f573753..189581d 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -13,6 +13,9 @@ #include <linux/seq_file.h> #include <linux/slab.h> #include <linux/stat.h> +#ifdef CONFIG_FAIL_MMC_REQUEST +#include <linux/fault-inject.h> +#endif
#include <linux/mmc/card.h> #include <linux/mmc/host.h> @@ -159,6 +162,23 @@ static int mmc_clock_opt_set(void *data, u64 val) return 0; }
+#ifdef CONFIG_FAIL_MMC_REQUEST + +static DECLARE_FAULT_ATTR(fail_mmc_request); + +#ifdef KERNEL +/* + * Internal function. Pass the boot param fail_mmc_request to + * the setup fault injection attributes routine. + */ +static int __init setup_fail_mmc_request(char *str) +{ + return setup_fault_attr(&fail_mmc_request, str); +} +__setup("fail_mmc_request=", setup_fail_mmc_request); +#endif /* KERNEL */ +#endif /* CONFIG_FAIL_MMC_REQUEST */ + DEFINE_SIMPLE_ATTRIBUTE(mmc_clock_fops, mmc_clock_opt_get, mmc_clock_opt_set, "%llu\n");
@@ -189,6 +209,13 @@ void mmc_add_host_debugfs(struct mmc_host *host) root, &host->clk_delay)) goto err_node; #endif +#ifdef CONFIG_FAIL_MMC_REQUEST + host->fail_mmc_request = fail_mmc_request; + if (IS_ERR(fault_create_debugfs_attr("fail_mmc_request", + root, + &host->fail_mmc_request))) + goto err_node; +#endif return;
err_node: diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 0f83858..ee472fe 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -12,6 +12,9 @@
#include <linux/leds.h> #include <linux/sched.h> +#ifdef CONFIG_FAIL_MMC_REQUEST +#include <linux/fault-inject.h> +#endif
#include <linux/mmc/core.h> #include <linux/mmc/pm.h> @@ -304,6 +307,10 @@ struct mmc_host {
struct mmc_async_req *areq; /* active async req */
+#ifdef CONFIG_FAIL_MMC_REQUEST + struct fault_attr fail_mmc_request; +#endif + unsigned long private[0] ____cacheline_aligned; };
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 47879c7..ebff0c9 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1090,6 +1090,17 @@ config FAIL_IO_TIMEOUT Only works with drivers that use the generic timeout handling, for others it wont do anything.
+config FAIL_MMC_REQUEST + bool "Fault-injection capability for MMC IO" + select DEBUG_FS + depends on FAULT_INJECTION && MMC + help + Provide fault-injection capability for MMC IO. + This will make the mmc core return data errors. This is + useful to test the error handling in the mmc block device + and to test how the mmc host driver handles retries from + the block device. + config FAULT_INJECTION_DEBUG_FS bool "Debugfs entries for fault-injection capabilities" depends on FAULT_INJECTION && SYSFS && DEBUG_FS
On Tue, Aug 9, 2011 at 2:07 PM, Per Forlin per.forlin@linaro.org wrote:
This adds support to inject data errors after a completed host transfer. The mmc core will return error even though the host transfer is successful. This simple fault injection proved to be very useful to test the non-blocking error handling in the mmc_blk_issue_rw_rq(). Random faults can also test how the host driver handles pre_req() and post_req() in case of errors.
Good idea!
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 89bdeae..a4996b0 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -25,6 +25,11 @@ #include <linux/pm_runtime.h> #include <linux/suspend.h>
+#ifdef CONFIG_FAIL_MMC_REQUEST +#include <linux/fault-inject.h> +#include <linux/random.h> +#endif
You don't need to #ifdef around the #include <> stuff, and if you do, something is wrong with those headers. It's just a bunch of defines that aren't used in some circumstances. Stack them with the others, simply, just #ifdef the code below.
@@ -83,6 +88,43 @@ static void mmc_flush_scheduled_work(void) flush_workqueue(workqueue); }
+#ifdef CONFIG_FAIL_MMC_REQUEST
+/*
- Internal function. Inject random data errors.
- If mmc_data is NULL no errors are injected.
- */
+static void mmc_should_fail_request(struct mmc_host *host,
- struct mmc_request *mrq)
+{
- struct mmc_command *cmd = mrq->cmd;
- struct mmc_data *data = mrq->data;
- static const int data_errors[] = {
- -ETIMEDOUT,
- -EILSEQ,
- -EIO,
- };
- if (!data)
- return;
- if (cmd->error || data->error ||
- !should_fail(&host->fail_mmc_request, data->blksz * data->blocks))
- return;
- data->error = data_errors[random32() % ARRAY_SIZE(data_errors)];
- data->bytes_xfered = (random32() % (data->bytes_xfered >> 9)) << 9;
+}
+#else /* CONFIG_FAIL_MMC_REQUEST */
+static void mmc_should_fail_request(struct mmc_host *host,
- struct mmc_request *mrq)
Should be "static inline" so we know it will be folded in and nullified by the compiler, lots of kernel code use that pattern.
diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c index f573753..189581d 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -13,6 +13,9 @@ #include <linux/seq_file.h> #include <linux/slab.h> #include <linux/stat.h> +#ifdef CONFIG_FAIL_MMC_REQUEST +#include <linux/fault-inject.h> +#endif
No #ifdef:ing...
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 0f83858..ee472fe 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -12,6 +12,9 @@
#include <linux/leds.h> #include <linux/sched.h> +#ifdef CONFIG_FAIL_MMC_REQUEST +#include <linux/fault-inject.h> +#endif
Neither here...
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 47879c7..ebff0c9 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug
I'm contemplating if we should create drivers/mmc/Kconfig.debug and stash this in there instead, i.e. also move out MMC_DEBUG from drivers/mmc/Kconfig and add to that?
It seems more apropriate to select this from the MMC subsystem. However the core of fault injection is in lib/
So maybe a simple:
config FAIL_MMC_REQUEST bool select FAULT_INJECTION
That can then be selected by a debug option in the MMC subsystem? I fear it may be hard to find this otherwise...
(NB: I have very little clue how the Kconfig.debug files get sourced into the Kbuild so I might be misguided...)
@@ -1090,6 +1090,17 @@ config FAIL_IO_TIMEOUT Only works with drivers that use the generic timeout handling, for others it wont do anything.
+config FAIL_MMC_REQUEST
- bool "Fault-injection capability for MMC IO"
- select DEBUG_FS
- depends on FAULT_INJECTION && MMC
Isn't:
depends on MMC select FAULT_INJECTION
Simpler to use? Now you have to select fault injection first to even see this option right?
Apart from this it looks fine.
Thanks, Linus Walleij
On 19 August 2011 13:40, Linus Walleij linus.walleij@linaro.org wrote:
On Tue, Aug 9, 2011 at 2:07 PM, Per Forlin per.forlin@linaro.org wrote:
This adds support to inject data errors after a completed host transfer. The mmc core will return error even though the host transfer is successful. This simple fault injection proved to be very useful to test the non-blocking error handling in the mmc_blk_issue_rw_rq(). Random faults can also test how the host driver handles pre_req() and post_req() in case of errors.
Good idea!
Thanks.
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 89bdeae..a4996b0 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -25,6 +25,11 @@ #include <linux/pm_runtime.h> #include <linux/suspend.h>
+#ifdef CONFIG_FAIL_MMC_REQUEST +#include <linux/fault-inject.h> +#include <linux/random.h> +#endif
You don't need to #ifdef around the #include <> stuff, and if you do, something is wrong with those headers. It's just a bunch of defines that aren't used in some circumstances. Stack them with the others, simply, just #ifdef the code below.
I added them after suggestion from J Freyensee. I am also in favor of no ifdefs here. I'll remove them in the next patchset unless James has any strong objections.
@@ -83,6 +88,43 @@ static void mmc_flush_scheduled_work(void) flush_workqueue(workqueue); }
+#ifdef CONFIG_FAIL_MMC_REQUEST
+/*
- Internal function. Inject random data errors.
- If mmc_data is NULL no errors are injected.
- */
+static void mmc_should_fail_request(struct mmc_host *host,
- struct mmc_request *mrq)
+{
- struct mmc_command *cmd = mrq->cmd;
- struct mmc_data *data = mrq->data;
- static const int data_errors[] = {
- -ETIMEDOUT,
- -EILSEQ,
- -EIO,
- };
- if (!data)
- return;
- if (cmd->error || data->error ||
- !should_fail(&host->fail_mmc_request, data->blksz * data->blocks))
- return;
- data->error = data_errors[random32() % ARRAY_SIZE(data_errors)];
- data->bytes_xfered = (random32() % (data->bytes_xfered >> 9)) << 9;
+}
+#else /* CONFIG_FAIL_MMC_REQUEST */
+static void mmc_should_fail_request(struct mmc_host *host,
- struct mmc_request *mrq)
Should be "static inline" so we know it will be folded in and nullified by the compiler, lots of kernel code use that pattern.
I'll fix.
diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c index f573753..189581d 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -13,6 +13,9 @@ #include <linux/seq_file.h> #include <linux/slab.h> #include <linux/stat.h> +#ifdef CONFIG_FAIL_MMC_REQUEST +#include <linux/fault-inject.h> +#endif
No #ifdef:ing...
I'll remove it.
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 0f83858..ee472fe 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -12,6 +12,9 @@
#include <linux/leds.h> #include <linux/sched.h> +#ifdef CONFIG_FAIL_MMC_REQUEST +#include <linux/fault-inject.h> +#endif
Neither here...
dito
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 47879c7..ebff0c9 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug
I'm contemplating if we should create drivers/mmc/Kconfig.debug and stash this in there instead, i.e. also move out MMC_DEBUG from drivers/mmc/Kconfig and add to that?
It seems more apropriate to select this from the MMC subsystem. However the core of fault injection is in lib/
So maybe a simple:
config FAIL_MMC_REQUEST bool select FAULT_INJECTION
That can then be selected by a debug option in the MMC subsystem? I fear it may be hard to find this otherwise...
(NB: I have very little clue how the Kconfig.debug files get sourced into the Kbuild so I might be misguided...)
The FAIL_MMC_REQUEST sits right next to the rest of the fail injection functions.
config FAILSLAB depends on FAULT_INJECTION depends on SLAB || SLUB
config FAIL_PAGE_ALLOC depends on FAULT_INJECTION
config FAIL_MAKE_REQUEST depends on FAULT_INJECTION && BLOCK
config FAIL_IO_TIMEOUT depends on FAULT_INJECTION && BLOCK
config FAIL_MMC_REQUEST select DEBUG_FS depends on FAULT_INJECTION && MMC
I think the proper place is to have it here together with the rest.
@@ -1090,6 +1090,17 @@ config FAIL_IO_TIMEOUT Only works with drivers that use the generic timeout handling, for others it wont do anything.
+config FAIL_MMC_REQUEST
- bool "Fault-injection capability for MMC IO"
- select DEBUG_FS
- depends on FAULT_INJECTION && MMC
Isn't:
depends on MMC select FAULT_INJECTION
Simpler to use? Now you have to select fault injection first to even see this option right?
In menuconfig you have to select FAULT_INJECTION first, then you can choose from a list of available fault injection options. I don't see any real reason for treating FAIL_MMC_REQUEST different from the rest.
Thanks for your comments. /Per
Add description on how to enable random fault injection for MMC IO
Signed-off-by: Per Forlin per.forlin@linaro.org Acked-by: Akinobu Mita akinobu.mita@gmail.com --- Documentation/fault-injection/fault-injection.txt | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt index 82a5d25..70f924e 100644 --- a/Documentation/fault-injection/fault-injection.txt +++ b/Documentation/fault-injection/fault-injection.txt @@ -21,6 +21,11 @@ o fail_make_request /sys/block/<device>/make-it-fail or /sys/block/<device>/<partition>/make-it-fail. (generic_make_request())
+o fail_mmc_request + + injects MMC data errors on devices permitted by setting + debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request + Configure fault-injection capabilities behavior -----------------------------------------------
@@ -115,7 +120,8 @@ use the boot option:
failslab= fail_page_alloc= - fail_make_request=<interval>,<probability>,<space>,<times> + fail_make_request= + fail_mmc_request=<interval>,<probability>,<space>,<times>
How to add new fault injection capability -----------------------------------------
Hi Chris,
It's no longer necessary to merge this through the mm-tree since Akinobu's patch "fault-injection: add ability to export fault_attr in arbitrary directory" is in mainline. Chris, would you mind merging the fault-injection patches in this patchset to mmc-next once the mmc part of this patchset is acked and accepted?
Regards, Per
On 9 August 2011 14:07, Per Forlin per.forlin@linaro.org wrote:
change log: v2 - Resolve build issue in mmc core.c due to multiple init_module by removing the fault inject module. - Export fault injection functions to make them available for modules - Update fault injection documentation on MMC IO v3 - add function descriptions in core.c - use export GPL for fault injection functions v4 - make the fault_attr per host. This prepares for upcoming patch from Akinobu that adds support for creating debugfs entries in arbitrary directory. v5 - Make use of fault_create_debugfs_attr() in Akinobu's patch "fault-injection: add ability to export fault_attr in...". v6 - Fix typo in commit message in patch "export fault injection functions" v7 - Don't compile in boot param setup function if mmc-core is built as module. v8 - Update fault injection documentation. Add fail_mmc_request to boot option section.
Per Forlin (3): fault-inject: export fault injection functions mmc: core: add random fault injection fault injection: add documentation on MMC IO fault injection
Documentation/fault-injection/fault-injection.txt | 8 +++- drivers/mmc/core/core.c | 44 +++++++++++++++++++++ drivers/mmc/core/debugfs.c | 27 +++++++++++++ include/linux/mmc/host.h | 7 +++ lib/Kconfig.debug | 11 +++++ lib/fault-inject.c | 2 + 6 files changed, 98 insertions(+), 1 deletions(-)
-- 1.7.4.1
Hi Per,
On Fri, Aug 19 2011, Per Forlin wrote:
Hi Chris,
It's no longer necessary to merge this through the mm-tree since Akinobu's patch "fault-injection: add ability to export fault_attr in arbitrary directory" is in mainline. Chris, would you mind merging the fault-injection patches in this patchset to mmc-next once the mmc part of this patchset is acked and accepted?
That's fine -- merged to mmc-next for 3.2 now, with Linus W's review.
Thanks,
- Chris.
-
Chris Ball -
Linus Walleij -
Per Forlin