From: Per Forlin per.forlin@linaro.org
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. v9 - remove ifdef around include files and inline empty function, comments from Linus Walleij.
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 | 41 +++++++++++++++++++++ drivers/mmc/core/debugfs.c | 25 +++++++++++++ include/linux/mmc/host.h | 5 +++ lib/Kconfig.debug | 11 ++++++ lib/fault-inject.c | 2 + 6 files changed, 91 insertions(+), 1 deletions(-)
From: Per Forlin per.forlin@linaro.org
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 */
From: Per Forlin per.forlin@linaro.org
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 | 41 +++++++++++++++++++++++++++++++++++++++++ drivers/mmc/core/debugfs.c | 25 +++++++++++++++++++++++++ include/linux/mmc/host.h | 5 +++++ lib/Kconfig.debug | 11 +++++++++++ 4 files changed, 82 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 91a0a74..d704dfa 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -24,6 +24,8 @@ #include <linux/regulator/consumer.h> #include <linux/pm_runtime.h> #include <linux/suspend.h> +#include <linux/fault-inject.h> +#include <linux/random.h>
#include <linux/mmc/card.h> #include <linux/mmc/host.h> @@ -83,6 +85,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 inline 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 +148,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 998797e..5acd707 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -12,6 +12,7 @@ #include <linux/seq_file.h> #include <linux/slab.h> #include <linux/stat.h> +#include <linux/fault-inject.h>
#include <linux/mmc/card.h> #include <linux/mmc/host.h> @@ -158,6 +159,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");
@@ -188,6 +206,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 1d09562..4c4bddf 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -12,6 +12,7 @@
#include <linux/leds.h> #include <linux/sched.h> +#include <linux/fault-inject.h>
#include <linux/mmc/core.h> #include <linux/mmc/pm.h> @@ -302,6 +303,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 c0cb9c4..1c7dbbf 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1070,6 +1070,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
From: Per Forlin per.forlin@linaro.org
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 -----------------------------------------
2011/8/19 Per Forlin per.forlin@stericsson.com:
From: Per Forlin per.forlin@linaro.org
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
OK!
Reviewed-by: Linus Walleij linus.walleij@linaro.org For the MMC portions in 2/3.
Linus Walleij
2011/8/19 Per Forlin per.forlin@stericsson.com:
+#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 */
You attempt to enable __setup() only when mmc_core is built into the kernel. Does it really work? I cannot find any drivers using "KERNEL" macro.
You can use module_param_cb() instead of __setup() without #ifdef KERNEL. When mmc_core is built into the kernel, you can specify the parameter with "mmc_core.fail_mmc_request=..."
Sorry for pointing that out too late.
On 13 September 2011 15:12, Akinobu Mita akinobu.mita@gmail.com wrote:
2011/8/19 Per Forlin per.forlin@stericsson.com:
+#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 */
You attempt to enable __setup() only when mmc_core is built into the kernel. Does it really work? I cannot find any drivers using "KERNEL" macro.
Your right it doesn't work. I think I change from ifndef MODULE to ifdef KERNEL at one point. It should be "ifndef MODULE"
You can use module_param_cb() instead of __setup() without #ifdef KERNEL. When mmc_core is built into the kernel, you can specify the parameter with "mmc_core.fail_mmc_request=..."
Thanks, this is the proper way to do it.
Sorry for pointing that out too late.
I think this patch is queued for 3.2 so there should be time to fix it still.
Thanks again, Per
Hi Akinobu,
On 13 September 2011 16:19, Per Forlin per.forlin@linaro.org wrote:
On 13 September 2011 15:12, Akinobu Mita akinobu.mita@gmail.com wrote:
2011/8/19 Per Forlin per.forlin@stericsson.com:
+#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 */
You attempt to enable __setup() only when mmc_core is built into the kernel. Does it really work? I cannot find any drivers using "KERNEL" macro.
Your right it doesn't work. I think I change from ifndef MODULE to ifdef KERNEL at one point. It should be "ifndef MODULE"
You can use module_param_cb() instead of __setup() without #ifdef KERNEL. When mmc_core is built into the kernel, you can specify the parameter with "mmc_core.fail_mmc_request=..."
I am considering using module_param() with perm = 0 (not visible in sysfs). The purpose of the param is to set fault attributes during kerne boot time or module load time. After the kernel boot all can be set under debug fs, therefore no need to make the module param visible.
What do you think about this? I have not tested it yet. ---------------------------------------------------------- +++ b/drivers/mmc/core/debugfs.c @@ -20,6 +20,14 @@ #include "core.h" #include "mmc_ops.h"
+#ifdef CONFIG_FAIL_MMC_REQUEST + +static DECLARE_FAULT_ATTR(fail_mmc_default_attr); +static char *fail_mmc_request; +module_param(fail_mmc_request, charp, 0); + +#endif /* CONFIG_FAIL_MMC_REQUEST */ + /* The debugfs functions are optimized away when CONFIG_DEBUG_FS isn't set. */ static int mmc_ios_show(struct seq_file *s, void *data) { @@ -159,23 +167,6 @@ 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");
@@ -207,7 +198,9 @@ void mmc_add_host_debugfs(struct mmc_host *host) goto err_node; #endif #ifdef CONFIG_FAIL_MMC_REQUEST - host->fail_mmc_request = fail_mmc_request; + if (fail_mmc_request) + setup_fault_attr(&fail_mmc_default_attr, fail_mmc_request); + host->fail_mmc_request = fail_mmc_default_attr; if (IS_ERR(fault_create_debugfs_attr("fail_mmc_request", root, &host->fail_mmc_request))) ---------------------------------------------------------- It's only necessary to call setup_fault_attr() once for all hosts. Here it's called one time for each host. I think it's ok since the routine is small and used for debugging purposes. I could use a static bool to indicate whether setup_fault_attr() has already been issued. + if (fail_mmc_request && !setup_fault_attr_done)
Regards, Per
2011/9/14 Per Forlin per.forlin@linaro.org:
Hi Akinobu,
On 13 September 2011 16:19, Per Forlin per.forlin@linaro.org wrote:
On 13 September 2011 15:12, Akinobu Mita akinobu.mita@gmail.com wrote:
2011/8/19 Per Forlin per.forlin@stericsson.com:
+#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 */
You attempt to enable __setup() only when mmc_core is built into the kernel. Does it really work? I cannot find any drivers using "KERNEL" macro.
Your right it doesn't work. I think I change from ifndef MODULE to ifdef KERNEL at one point. It should be "ifndef MODULE"
It's simple and I like this solution.
module_param is more complicated than this. Also the parameter is only usefull when when mmc_core is built into the kernel (it's useless when mmc_core is built as a module).
You can use module_param_cb() instead of __setup() without #ifdef KERNEL. When mmc_core is built into the kernel, you can specify the parameter with "mmc_core.fail_mmc_request=..."
I am considering using module_param() with perm = 0 (not visible in sysfs). The purpose of the param is to set fault attributes during kerne boot time or module load time. After the kernel boot all can be set under debug fs, therefore no need to make the module param visible.
What do you think about this? I have not tested it yet.
...
It's only necessary to call setup_fault_attr() once for all hosts. Here it's called one time for each host. I think it's ok since the routine is small and used for debugging purposes. I could use a static bool to indicate whether setup_fault_attr() has already been issued.
- if (fail_mmc_request && !setup_fault_attr_done)
module_param_cb() doesn't work as you expected? (Although I suggested to use #ifdef MODULE instead of #ifdef KERNEL, I'm just asking out of curiosity)
static int fail_mmc_request_param_set(const char *val, const struct kernel_param *kp) { setup_fault_attr(&fail_mmc_request, val); return 0; }
static const struct kernel_param_ops fail_mmc_request_param_ops = { .set = fail_mmc_request_param_set };
module_param_cb(fail_mmc_request, &fail_mmc_request_param_ops, &fail_mmc_request, 0);
On 14 September 2011 01:37, Akinobu Mita akinobu.mita@gmail.com wrote:
2011/9/14 Per Forlin per.forlin@linaro.org:
Hi Akinobu,
On 13 September 2011 16:19, Per Forlin per.forlin@linaro.org wrote:
On 13 September 2011 15:12, Akinobu Mita akinobu.mita@gmail.com wrote:
2011/8/19 Per Forlin per.forlin@stericsson.com:
+#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 */
You attempt to enable __setup() only when mmc_core is built into the kernel. Does it really work? I cannot find any drivers using "KERNEL" macro.
Your right it doesn't work. I think I change from ifndef MODULE to ifdef KERNEL at one point. It should be "ifndef MODULE"
It's simple and I like this solution.
It's simple and the patch would be just two lines. The reason for changing my mind is that it may be useful to be able to pass the fault injection attributes even when mmc_core is a module.
module_param is more complicated than this. Also the parameter is only usefull when when mmc_core is built into the kernel (it's useless when mmc_core is built as a module).
If you want to enable fault injection for the mmc_core module at load time (during mmc initialisation) the param must be used. modprobe mmc_core fail_request=1,1,1,1 As soon as the module is loaded there is no need for the module param anymore.
You can use module_param_cb() instead of __setup() without #ifdef KERNEL. When mmc_core is built into the kernel, you can specify the parameter with "mmc_core.fail_mmc_request=..."
I am considering using module_param() with perm = 0 (not visible in sysfs). The purpose of the param is to set fault attributes during kerne boot time or module load time. After the kernel boot all can be set under debug fs, therefore no need to make the module param visible.
What do you think about this? I have not tested it yet.
...
It's only necessary to call setup_fault_attr() once for all hosts. Here it's called one time for each host. I think it's ok since the routine is small and used for debugging purposes. I could use a static bool to indicate whether setup_fault_attr() has already been issued.
- if (fail_mmc_request && !setup_fault_attr_done)
module_param_cb() doesn't work as you expected?
I made a silly mistake thinking the set() hook would only be issued if setting the param via sysfs. I turned out that I just mistyped the boot-param name, sorry. I now have a working test with module_param_cb() implemented.
Thanks, Per
2011/9/14 Per Forlin per.forlin@linaro.org:
It's simple and the patch would be just two lines. The reason for changing my mind is that it may be useful to be able to pass the fault injection attributes even when mmc_core is a module.
module_param is more complicated than this. Also the parameter is only usefull when when mmc_core is built into the kernel (it's useless when mmc_core is built as a module).
If you want to enable fault injection for the mmc_core module at load time (during mmc initialisation) the param must be used. modprobe mmc_core fail_request=1,1,1,1 As soon as the module is loaded there is no need for the module param anymore.
OK, I agree with you. The module parameter is the only way to enable mmc fault injection if CONFIG_FAULT_INJECTION_DEBUG_FS is disabled.
On 14 September 2011 10:04, Akinobu Mita akinobu.mita@gmail.com wrote:
2011/9/14 Per Forlin per.forlin@linaro.org:
It's simple and the patch would be just two lines. The reason for changing my mind is that it may be useful to be able to pass the fault injection attributes even when mmc_core is a module.
module_param is more complicated than this. Also the parameter is only usefull when when mmc_core is built into the kernel (it's useless when mmc_core is built as a module).
If you want to enable fault injection for the mmc_core module at load time (during mmc initialisation) the param must be used. modprobe mmc_core fail_request=1,1,1,1 As soon as the module is loaded there is no need for the module param anymore.
OK, I agree with you. The module parameter is the only way to enable mmc fault injection if CONFIG_FAULT_INJECTION_DEBUG_FS is disabled.
This is true as well. My point is that if using CONFIG_FAULT_INJECTION_DEBUG_FS the fault attributes can't be set until after the mmc module initialisation. One may want to test the error handling during the mmc initialisation. I'll send out a version v2 using module_param_cb().
Thanks again, Per
-
Akinobu Mita -
Linus Walleij -
Per Forlin -
Per Forlin