The first version of this patch is a part of mmc non-blocking v9 patchset: [PATCH v9 11/12] mmc: core: add random fault injection
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
Per Forlin (3): fault-inject: make fault injection available for modules mmc: core: add random fault injection fault injection: add documentation on MMC IO fault injection
Documentation/fault-injection/fault-injection.txt | 5 ++ drivers/mmc/core/core.c | 58 +++++++++++++++++++++ drivers/mmc/core/debugfs.c | 5 ++ include/linux/mmc/host.h | 3 + lib/Kconfig.debug | 11 ++++ lib/fault-inject.c | 2 + 6 files changed, 84 insertions(+), 0 deletions(-)
export symbols should_fail() and init_fault_attr_dentries() in order to make modules use the fault injection functionality
Signed-off-by: Per Forlin per.forlin@linaro.org --- 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 7e65af7..cd28364 100644 --- a/lib/fault-inject.c +++ b/lib/fault-inject.c @@ -131,6 +131,7 @@ bool should_fail(struct fault_attr *attr, ssize_t size)
return true; } +EXPORT_SYMBOL(should_fail);
#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
@@ -311,5 +312,6 @@ fail: cleanup_fault_attr_dentries(attr); return -ENOMEM; } +EXPORT_SYMBOL(init_fault_attr_dentries);
#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
On 07/19/2011 02:31 PM, Per Forlin wrote:
export symbols should_fail() and init_fault_attr_dentries() in order to make modules use the fault injection functionality
Signed-off-by: Per Forlinper.forlin@linaro.org
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 7e65af7..cd28364 100644 --- a/lib/fault-inject.c +++ b/lib/fault-inject.c @@ -131,6 +131,7 @@ bool should_fail(struct fault_attr *attr, ssize_t size)
return true; } +EXPORT_SYMBOL(should_fail);
Should this be EXPORT_SYMBOL_GPL()?
#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
@@ -311,5 +312,6 @@ fail: cleanup_fault_attr_dentries(attr); return -ENOMEM; } +EXPORT_SYMBOL(init_fault_attr_dentries);
and here too?
#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
Hi James,
On 20 July 2011 01:44, J Freyensee james_p_freyensee@linux.intel.com wrote:
On 07/19/2011 02:31 PM, Per Forlin wrote:
export symbols should_fail() and init_fault_attr_dentries() in order to make modules use the fault injection functionality
Signed-off-by: Per Forlinper.forlin@linaro.org
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 7e65af7..cd28364 100644 --- a/lib/fault-inject.c +++ b/lib/fault-inject.c @@ -131,6 +131,7 @@ bool should_fail(struct fault_attr *attr, ssize_t size)
return true; } +EXPORT_SYMBOL(should_fail);
Should this be EXPORT_SYMBOL_GPL()?
I forgot to consider this. GPL is fine with me. I'll update.
#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
@@ -311,5 +312,6 @@ fail: cleanup_fault_attr_dentries(attr); return -ENOMEM; } +EXPORT_SYMBOL(init_fault_attr_dentries);
and here too?
dito.
#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
-- J (James/Jay) Freyensee Storage Technology Group Intel Corporation
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 --- drivers/mmc/core/core.c | 58 ++++++++++++++++++++++++++++++++++++++++++++ drivers/mmc/core/debugfs.c | 5 ++++ include/linux/mmc/host.h | 3 ++ lib/Kconfig.debug | 11 ++++++++ 4 files changed, 77 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index ab36c7b..3f822b4 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -23,6 +23,8 @@ #include <linux/log2.h> #include <linux/regulator/consumer.h> #include <linux/pm_runtime.h> +#include <linux/fault-inject.h> +#include <linux/random.h>
#include <linux/mmc/card.h> #include <linux/mmc/host.h> @@ -82,6 +84,58 @@ static void mmc_flush_scheduled_work(void) flush_workqueue(workqueue); }
+#ifdef CONFIG_FAIL_MMC_REQUEST + +static DECLARE_FAULT_ATTR(fail_mmc_request); + +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); + +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 || !host->make_it_fail || + !should_fail(&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; +} + +static int __init fail_mmc_request_debugfs(void) +{ + return init_fault_attr_dentries(&fail_mmc_request, + "fail_mmc_request"); +} + +#else /* CONFIG_FAIL_MMC_REQUEST */ + +static void mmc_should_fail_request(struct mmc_host *host, + struct mmc_request *mrq) +{ +} + +static int __init fail_mmc_request_debugfs(void) +{ + return 0; +} +#endif /* CONFIG_FAIL_MMC_REQUEST */ + + /** * mmc_request_done - finish processing an MMC request * @host: MMC host which completed request @@ -108,6 +162,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", @@ -2064,6 +2120,8 @@ static int __init mmc_init(void) if (ret) goto unregister_host_class;
+ fail_mmc_request_debugfs(); + return 0;
unregister_host_class: diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c index 998797e..588e76f 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -188,6 +188,11 @@ void mmc_add_host_debugfs(struct mmc_host *host) root, &host->clk_delay)) goto err_node; #endif +#ifdef CONFIG_FAIL_MMC_REQUEST + if (!debugfs_create_u8("make-it-fail", S_IRUSR | S_IWUSR, + root, &host->make_it_fail)) + goto err_node; +#endif return;
err_node: diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 771455f..250b46d 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -303,6 +303,9 @@ struct mmc_host {
struct mmc_async_req *areq; /* active async req */
+#ifdef CONFIG_FAIL_MMC_REQUEST + u8 make_it_fail; +#endif unsigned long private[0] ____cacheline_aligned; };
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index c768bcd..c2d1423 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1057,6 +1057,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 07/19/2011 02:31 PM, Per Forlin 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.
Signed-off-by: Per Forlinper.forlin@linaro.org
drivers/mmc/core/core.c | 58 ++++++++++++++++++++++++++++++++++++++++++++ drivers/mmc/core/debugfs.c | 5 ++++ include/linux/mmc/host.h | 3 ++ lib/Kconfig.debug | 11 ++++++++ 4 files changed, 77 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index ab36c7b..3f822b4 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -23,6 +23,8 @@ #include<linux/log2.h> #include<linux/regulator/consumer.h> #include<linux/pm_runtime.h> +#include<linux/fault-inject.h> +#include<linux/random.h>
As a suggestion, would you want to also use '#ifdef CONFIG_FAIL_MMC_REQUEST' for fault-inject.h and random.h? If they are not used in a non-debug linux kernel configuration, could this possibly cause a little extra code bloat if they are a part of a production Linux kernel compile/configuration?
#include<linux/mmc/card.h> #include<linux/mmc/host.h> @@ -82,6 +84,58 @@ static void mmc_flush_scheduled_work(void) flush_workqueue(workqueue); }
+#ifdef CONFIG_FAIL_MMC_REQUEST
+static DECLARE_FAULT_ATTR(fail_mmc_request);
+static int __init setup_fail_mmc_request(char *str)
Function comment header somewhere (here or the .h file?)
+{
- return setup_fault_attr(&fail_mmc_request, str);
+} +__setup("fail_mmc_request=", setup_fail_mmc_request);
+static void mmc_should_fail_request(struct mmc_host *host,
struct mmc_request *mrq)+{
Function comment header somewhere (here or the .h file)?
- struct mmc_command *cmd = mrq->cmd;
- struct mmc_data *data = mrq->data;
- static const int data_errors[] = {
-ETIMEDOUT,-EILSEQ,-EIO,- };
- if (!data)
return;
Should 'if (!cmd)' also be checked or is this guaranteed to always have a valid value for this function (and if there are certain commands in 'struct mmc_command *cmd = mrq->cmd' that will not work with this function then that is something that should be documented in the function comment header)?
- if (cmd->error || data->error || !host->make_it_fail ||
!should_fail(&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;
+}
+static int __init fail_mmc_request_debugfs(void) +{
- return init_fault_attr_dentries(&fail_mmc_request,
"fail_mmc_request");+}
+#else /* CONFIG_FAIL_MMC_REQUEST */
+static void mmc_should_fail_request(struct mmc_host *host,
struct mmc_request *mrq)+{ +}
+static int __init fail_mmc_request_debugfs(void) +{
- return 0;
+} +#endif /* CONFIG_FAIL_MMC_REQUEST */
- /**
- mmc_request_done - finish processing an MMC request
- @host: MMC host which completed request
@@ -108,6 +162,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",
@@ -2064,6 +2120,8 @@ static int __init mmc_init(void) if (ret) goto unregister_host_class;
fail_mmc_request_debugfs();
return 0;
unregister_host_class:
diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c index 998797e..588e76f 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -188,6 +188,11 @@ void mmc_add_host_debugfs(struct mmc_host *host) root,&host->clk_delay)) goto err_node; #endif +#ifdef CONFIG_FAIL_MMC_REQUEST
- if (!debugfs_create_u8("make-it-fail", S_IRUSR | S_IWUSR,
root,&host->make_it_fail))goto err_node;+#endif return;
err_node: diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 771455f..250b46d 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -303,6 +303,9 @@ struct mmc_host {
struct mmc_async_req *areq; /* active async req */
+#ifdef CONFIG_FAIL_MMC_REQUEST
- u8 make_it_fail;
+#endif unsigned long private[0] ____cacheline_aligned; };
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index c768bcd..c2d1423 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1057,6 +1057,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 isuseful to test the error handling in the mmc block deviceand to test how the mmc host driver handles retries fromthe block device.- config FAULT_INJECTION_DEBUG_FS bool "Debugfs entries for fault-injection capabilities" depends on FAULT_INJECTION&& SYSFS&& DEBUG_FS
Hi James,
On 20 July 2011 02:04, J Freyensee james_p_freyensee@linux.intel.com wrote:
On 07/19/2011 02:31 PM, Per Forlin 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.
Signed-off-by: Per Forlinper.forlin@linaro.org
drivers/mmc/core/core.c | 58 ++++++++++++++++++++++++++++++++++++++++++++ drivers/mmc/core/debugfs.c | 5 ++++ include/linux/mmc/host.h | 3 ++ lib/Kconfig.debug | 11 ++++++++ 4 files changed, 77 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index ab36c7b..3f822b4 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -23,6 +23,8 @@ #include<linux/log2.h> #include<linux/regulator/consumer.h> #include<linux/pm_runtime.h> +#include<linux/fault-inject.h> +#include<linux/random.h>
As a suggestion, would you want to also use '#ifdef CONFIG_FAIL_MMC_REQUEST' for fault-inject.h and random.h? If they are not used in a non-debug linux kernel configuration, could this possibly cause a little extra code bloat if they are a part of a production Linux kernel compile/configuration?
I try to avoid ifdefs in general but I see your point. I accept your comment and add the ifdef.
#include<linux/mmc/card.h> #include<linux/mmc/host.h> @@ -82,6 +84,58 @@ static void mmc_flush_scheduled_work(void) flush_workqueue(workqueue); }
+#ifdef CONFIG_FAIL_MMC_REQUEST
+static DECLARE_FAULT_ATTR(fail_mmc_request);
+static int __init setup_fail_mmc_request(char *str)
Function comment header somewhere (here or the .h file?)
+{
- return setup_fault_attr(&fail_mmc_request, str);
+} +__setup("fail_mmc_request=", setup_fail_mmc_request);
+static void mmc_should_fail_request(struct mmc_host *host,
- struct mmc_request *mrq)
+{
Function comment header somewhere (here or the .h file)?
I will do.
- struct mmc_command *cmd = mrq->cmd;
- struct mmc_data *data = mrq->data;
- static const int data_errors[] = {
- -ETIMEDOUT,
- -EILSEQ,
- -EIO,
- };
- if (!data)
- return;
Should 'if (!cmd)' also be checked or is this guaranteed to always have a valid value for this function (and if there are certain commands in 'struct mmc_command *cmd = mrq->cmd' that will not work with this function then that is something that should be documented in the function comment header)?
cmd will always be set from where the mmc_should_fail_request() is called. This code only inject data errors. For commands with no data struct there will be no error inject. This is a choice I have made. It is possible to extend this by injecting errors for cmds without data too but it would require an additional flag in order to control inject errors on data and cmd separately. I will add this information to the function description.
- if (cmd->error || data->error || !host->make_it_fail ||
- !should_fail(&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; +}
+static int __init fail_mmc_request_debugfs(void) +{
- return init_fault_attr_dentries(&fail_mmc_request,
- "fail_mmc_request");
+}
+#else /* CONFIG_FAIL_MMC_REQUEST */
+static void mmc_should_fail_request(struct mmc_host *host,
- struct mmc_request *mrq)
+{ +}
+static int __init fail_mmc_request_debugfs(void) +{
- return 0;
+} +#endif /* CONFIG_FAIL_MMC_REQUEST */
/** * mmc_request_done - finish processing an MMC request * @host: MMC host which completed request @@ -108,6 +162,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", @@ -2064,6 +2120,8 @@ static int __init mmc_init(void) if (ret) goto unregister_host_class;
- fail_mmc_request_debugfs();
return 0;
unregister_host_class: diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c index 998797e..588e76f 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -188,6 +188,11 @@ void mmc_add_host_debugfs(struct mmc_host *host) root,&host->clk_delay)) goto err_node; #endif +#ifdef CONFIG_FAIL_MMC_REQUEST
- if (!debugfs_create_u8("make-it-fail", S_IRUSR | S_IWUSR,
- root,&host->make_it_fail))
- goto err_node;
+#endif return;
err_node: diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 771455f..250b46d 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -303,6 +303,9 @@ struct mmc_host {
struct mmc_async_req *areq; /* active async req */
+#ifdef CONFIG_FAIL_MMC_REQUEST
- u8 make_it_fail;
+#endif unsigned long private[0] ____cacheline_aligned; };
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index c768bcd..c2d1423 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1057,6 +1057,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
-- J (James/Jay) Freyensee Storage Technology Group Intel Corporation
Add description on how to enable random fault injection for MMC IO
Signed-off-by: Per Forlin per.forlin@linaro.org --- Documentation/fault-injection/fault-injection.txt | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt index 7be15e4..27eede4 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 + /sys/kernel/debug/mmc0/make-it-fail + Configure fault-injection capabilities behavior -----------------------------------------------
-
J Freyensee -
Per Forlin