Akinobu Mita reported that the boot option for mmc fault injection is never compiled in due to a fauly "ifdef KERNEL" that is never set. A correct ifdef would be "ifndef MODULE". This patch set adds a module_param_cb() and thereby no ifndef MODULE is needed. Using a module_param makes it possible to pass default fault attributes for external modules too.
This patch set is for 3.2 Change log: v2 - use module_param_cb() to set default fault attributes - fix spelling of documentation in patch #3
Per Forlin (3): fault-inject: export setup_fault_attr() mmc: add module param to set fault injection attributes fault-injection: update documentation with the mmc module param
Documentation/fault-injection/fault-injection.txt | 2 +- drivers/mmc/core/debugfs.c | 38 +++++++++++---------- lib/fault-inject.c | 3 +- 3 files changed, 23 insertions(+), 20 deletions(-)
mmc_core module needs to use setup_fault_attr() in order to set fault injection attributes during module load time.
Signed-off-by: Per Forlin per.forlin@linaro.org --- lib/fault-inject.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/lib/fault-inject.c b/lib/fault-inject.c index 328d433..4f75540 100644 --- a/lib/fault-inject.c +++ b/lib/fault-inject.c @@ -14,7 +14,7 @@ * setup_fault_attr() is a helper function for various __setup handlers, so it * returns 0 on error, because that is what __setup handlers do. */ -int __init setup_fault_attr(struct fault_attr *attr, char *str) +int setup_fault_attr(struct fault_attr *attr, char *str) { unsigned long probability; unsigned long interval; @@ -36,6 +36,7 @@ int __init setup_fault_attr(struct fault_attr *attr, char *str)
return 1; } +EXPORT_SYMBOL_GPL(setup_fault_attr);
static void fail_dump(struct fault_attr *attr) {
Replace setup("fail_mmc_request") and faulty "ifdef KERNEL" with a module_param_cb(). The module param mmc_core.fail_request may be used to set the fault injection attributes during boot time or module load time.
Signed-off-by: Per Forlin per.forlin@linaro.org --- drivers/mmc/core/debugfs.c | 38 ++++++++++++++++++++------------------ 1 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c index 5acd707..d80e234 100644 --- a/drivers/mmc/core/debugfs.c +++ b/drivers/mmc/core/debugfs.c @@ -20,6 +20,25 @@ #include "core.h" #include "mmc_ops.h"
+#ifdef CONFIG_FAIL_MMC_REQUEST + +static DECLARE_FAULT_ATTR(fail_default_attr); +static char *fail_request; +static int fail_mmc_request_param_set(const char *val, + const struct kernel_param *kp) +{ + setup_fault_attr(&fail_default_attr, (char *) val); + return 0; +} + +static const struct kernel_param_ops fail_mmc_request_param_ops = { + .set = fail_mmc_request_param_set +}; +module_param_cb(fail_request, &fail_mmc_request_param_ops, + &fail_request, 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 +178,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 +209,7 @@ 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; + host->fail_mmc_request = fail_default_attr; if (IS_ERR(fault_create_debugfs_attr("fail_mmc_request", root, &host->fail_mmc_request)))
2011/9/14 Per Forlin per.forlin@linaro.org:
+#ifdef CONFIG_FAIL_MMC_REQUEST
+static DECLARE_FAULT_ATTR(fail_default_attr); +static char *fail_request;
This is not used anymore and ...
+static int fail_mmc_request_param_set(const char *val,
- const struct kernel_param *kp)
+{
- setup_fault_attr(&fail_default_attr, (char *) val);
- return 0;
+}
+static const struct kernel_param_ops fail_mmc_request_param_ops = {
- .set = fail_mmc_request_param_set
+}; +module_param_cb(fail_request, &fail_mmc_request_param_ops,
- &fail_request, 0);
you can change this third parameter to NULL.
+#endif /* CONFIG_FAIL_MMC_REQUEST */
On 14 September 2011 12:05, Akinobu Mita akinobu.mita@gmail.com wrote:
2011/9/14 Per Forlin per.forlin@linaro.org:
+#ifdef CONFIG_FAIL_MMC_REQUEST
+static DECLARE_FAULT_ATTR(fail_default_attr); +static char *fail_request;
This is not used anymore and ...
Yes of course. Will remove it.
+static int fail_mmc_request_param_set(const char *val,
- const struct kernel_param *kp)
+{
- setup_fault_attr(&fail_default_attr, (char *) val);
- return 0;
+}
+static const struct kernel_param_ops fail_mmc_request_param_ops = {
- .set = fail_mmc_request_param_set
+}; +module_param_cb(fail_request, &fail_mmc_request_param_ops,
- &fail_request, 0);
you can change this third parameter to NULL.
Absolutely..
Thanks, Per
On 14 September 2011 12:18, Per Forlin per.forlin@linaro.org wrote:
On 14 September 2011 12:05, Akinobu Mita akinobu.mita@gmail.com wrote:
2011/9/14 Per Forlin per.forlin@linaro.org:
+#ifdef CONFIG_FAIL_MMC_REQUEST
+static DECLARE_FAULT_ATTR(fail_default_attr); +static char *fail_request;
This is not used anymore and ...
Yes of course. Will remove it.
+static int fail_mmc_request_param_set(const char *val,
- const struct kernel_param *kp)
+{
- setup_fault_attr(&fail_default_attr, (char *) val);
I am thinking of returning failure here if setup_fault_attr() fails. if (setup_fault_attr(&fail_default_attr, (char *) val) == 0) return -EINVAL;
There will be a printk(KERN_WARNING "FAULT_INJECTION: failed to parse arguments) it setup() fails. Is it too harsh to return failure?
Regards, Per
On 14 September 2011 12:38, Per Forlin per.forlin@linaro.org wrote:
On 14 September 2011 12:18, Per Forlin per.forlin@linaro.org wrote:
On 14 September 2011 12:05, Akinobu Mita akinobu.mita@gmail.com wrote:
2011/9/14 Per Forlin per.forlin@linaro.org:
+#ifdef CONFIG_FAIL_MMC_REQUEST
+static DECLARE_FAULT_ATTR(fail_default_attr); +static char *fail_request;
This is not used anymore and ...
Yes of course. Will remove it.
+static int fail_mmc_request_param_set(const char *val,
- const struct kernel_param *kp)
+{
- setup_fault_attr(&fail_default_attr, (char *) val);
I am thinking of returning failure here if setup_fault_attr() fails. if (setup_fault_attr(&fail_default_attr, (char *) val) == 0) return -EINVAL;
There will be a printk(KERN_WARNING "FAULT_INJECTION: failed to parse arguments) it setup() fails. Is it too harsh to return failure?
If error is returned here the kernel prints: "invalid for parameter `mmc_core.fail_request'" This piece of information is a reason for returning error on failure.
Regards, Per
2011/9/14 Per Forlin per.forlin@linaro.org:
+static int fail_mmc_request_param_set(const char *val,
- const struct kernel_param *kp)
+{
- setup_fault_attr(&fail_default_attr, (char *) val);
I am thinking of returning failure here if setup_fault_attr() fails. if (setup_fault_attr(&fail_default_attr, (char *) val) == 0) return -EINVAL;
It's good idea and it also prevents from loading the module if invalid parameter is specified.
There will be a printk(KERN_WARNING "FAULT_INJECTION: failed to parse arguments) it setup() fails. Is it too harsh to return failure?
Yes. I'll try to fix in the future.
Signed-off-by: Per Forlin per.forlin@linaro.org --- Documentation/fault-injection/fault-injection.txt | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt index 70f924e..ba4be8b 100644 --- a/Documentation/fault-injection/fault-injection.txt +++ b/Documentation/fault-injection/fault-injection.txt @@ -121,7 +121,7 @@ use the boot option: failslab= fail_page_alloc= fail_make_request= - fail_mmc_request=<interval>,<probability>,<space>,<times> + mmc_core.fail_request=<interval>,<probability>,<space>,<times>
How to add new fault injection capability -----------------------------------------
-
Akinobu Mita -
Per Forlin