All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9 0/3] mmc: make fault injection available for MMC IO
@ 2011-08-19 12:52 ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-08-19 12:52 UTC (permalink / raw)
  To: linux-mmc, linux-kernel, linaro-dev, Linus Walleij, Akinobu Mita
  Cc: Chris Ball, linux-doc, Per Forlin

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(-)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v9 0/3] mmc: make fault injection available for MMC IO
@ 2011-08-19 12:52 ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-08-19 12:52 UTC (permalink / raw)
  To: linux-mmc, linux-kernel, linaro-dev, Linus Walleij, Akinobu Mita
  Cc: Chris Ball, linux-doc, Per Forlin

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(-)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v9 1/3] fault-inject: export fault injection functions
  2011-08-19 12:52 ` Per Forlin
@ 2011-08-19 12:52   ` Per Forlin
  -1 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-08-19 12:52 UTC (permalink / raw)
  To: linux-mmc, linux-kernel, linaro-dev, Linus Walleij, Akinobu Mita
  Cc: Chris Ball, linux-doc, Per Forlin

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 */
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v9 1/3] fault-inject: export fault injection functions
@ 2011-08-19 12:52   ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-08-19 12:52 UTC (permalink / raw)
  To: linux-mmc, linux-kernel, linaro-dev, Linus Walleij, Akinobu Mita
  Cc: Chris Ball, linux-doc, Per Forlin

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 */
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v9 2/3] mmc: core: add random fault injection
@ 2011-08-19 12:52     ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-08-19 12:52 UTC (permalink / raw)
  To: linux-mmc, linux-kernel, linaro-dev, Linus Walleij, Akinobu Mita
  Cc: Chris Ball, linux-doc, Per Forlin

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
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v9 2/3] mmc: core: add random fault injection
@ 2011-08-19 12:52     ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-08-19 12:52 UTC (permalink / raw)
  To: linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linaro-dev-cunTk1MwBs8s++Sfvej+rw, Linus Walleij, Akinobu Mita
  Cc: linux-doc-u79uwXL29TY76Z2rM5mHXA

From: Per Forlin <per.forlin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Acked-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 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
-- 
1.6.3.3

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v9 3/3] fault injection: add documentation on MMC IO fault injection
  2011-08-19 12:52     ` Per Forlin
@ 2011-08-19 12:52       ` Per Forlin
  -1 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-08-19 12:52 UTC (permalink / raw)
  To: linux-mmc, linux-kernel, linaro-dev, Linus Walleij, Akinobu Mita
  Cc: Chris Ball, linux-doc, Per Forlin

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
 -----------------------------------------
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v9 3/3] fault injection: add documentation on MMC IO fault injection
@ 2011-08-19 12:52       ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-08-19 12:52 UTC (permalink / raw)
  To: linux-mmc, linux-kernel, linaro-dev, Linus Walleij, Akinobu Mita
  Cc: Chris Ball, linux-doc, Per Forlin

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
 -----------------------------------------
-- 
1.6.3.3


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
@ 2011-08-19 14:16       ` Linus Walleij
  0 siblings, 0 replies; 20+ messages in thread
From: Linus Walleij @ 2011-08-19 14:16 UTC (permalink / raw)
  To: Per Forlin
  Cc: linux-mmc, linux-kernel, linaro-dev, Akinobu Mita, Chris Ball,
	linux-doc, Per Forlin

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
@ 2011-08-19 14:16       ` Linus Walleij
  0 siblings, 0 replies; 20+ messages in thread
From: Linus Walleij @ 2011-08-19 14:16 UTC (permalink / raw)
  To: Per Forlin
  Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Akinobu Mita

2011/8/19 Per Forlin <per.forlin-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>:

> From: Per Forlin <per.forlin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Acked-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

OK!

Reviewed-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
For the MMC portions in 2/3.

Linus Walleij

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
  2011-08-19 12:52     ` Per Forlin
                       ` (2 preceding siblings ...)
  (?)
@ 2011-09-13 13:12     ` Akinobu Mita
  2011-09-13 14:19         ` Per Forlin
  -1 siblings, 1 reply; 20+ messages in thread
From: Akinobu Mita @ 2011-09-13 13:12 UTC (permalink / raw)
  To: Per Forlin
  Cc: linux-mmc, linux-kernel, linaro-dev, Linus Walleij, Chris Ball,
	linux-doc, Per Forlin

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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
@ 2011-09-13 14:19         ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-09-13 14:19 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: Per Forlin, linux-mmc, linux-kernel, linaro-dev, Linus Walleij,
	Chris Ball, linux-doc

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
@ 2011-09-13 14:19         ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-09-13 14:19 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Per Forlin

On 13 September 2011 15:12, Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2011/8/19 Per Forlin <per.forlin-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>:
>
>> +#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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
@ 2011-09-13 16:15           ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-09-13 16:15 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: Per Forlin, linux-mmc, linux-kernel, linaro-dev, Linus Walleij,
	Chris Ball, linux-doc

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
@ 2011-09-13 16:15           ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-09-13 16:15 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Per Forlin

Hi Akinobu,

On 13 September 2011 16:19, Per Forlin <per.forlin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 13 September 2011 15:12, Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> 2011/8/19 Per Forlin <per.forlin-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>:
>>
>>> +#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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
@ 2011-09-13 23:37             ` Akinobu Mita
  0 siblings, 0 replies; 20+ messages in thread
From: Akinobu Mita @ 2011-09-13 23:37 UTC (permalink / raw)
  To: Per Forlin
  Cc: Per Forlin, linux-mmc, linux-kernel, linaro-dev, Linus Walleij,
	Chris Ball, linux-doc

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);

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
@ 2011-09-13 23:37             ` Akinobu Mita
  0 siblings, 0 replies; 20+ messages in thread
From: Akinobu Mita @ 2011-09-13 23:37 UTC (permalink / raw)
  To: Per Forlin
  Cc: linaro-dev-cunTk1MwBs8s++Sfvej+rw,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Per Forlin

2011/9/14 Per Forlin <per.forlin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
> Hi Akinobu,
>
> On 13 September 2011 16:19, Per Forlin <per.forlin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On 13 September 2011 15:12, Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> 2011/8/19 Per Forlin <per.forlin-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>:
>>>
>>>> +#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);

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
  2011-09-13 23:37             ` Akinobu Mita
  (?)
@ 2011-09-14  6:51             ` Per Forlin
  2011-09-14  8:04               ` Akinobu Mita
  -1 siblings, 1 reply; 20+ messages in thread
From: Per Forlin @ 2011-09-14  6:51 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linux-mmc, linux-kernel, linaro-dev, Linus Walleij, Chris Ball,
	linux-doc

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
  2011-09-14  6:51             ` Per Forlin
@ 2011-09-14  8:04               ` Akinobu Mita
  2011-09-14  8:29                 ` Per Forlin
  0 siblings, 1 reply; 20+ messages in thread
From: Akinobu Mita @ 2011-09-14  8:04 UTC (permalink / raw)
  To: Per Forlin
  Cc: linux-mmc, linux-kernel, linaro-dev, Linus Walleij, Chris Ball,
	linux-doc

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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v9 2/3] mmc: core: add random fault injection
  2011-09-14  8:04               ` Akinobu Mita
@ 2011-09-14  8:29                 ` Per Forlin
  0 siblings, 0 replies; 20+ messages in thread
From: Per Forlin @ 2011-09-14  8:29 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: linux-mmc, linux-kernel, linaro-dev, Linus Walleij, Chris Ball,
	linux-doc

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2011-09-14  8:29 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-19 12:52 [PATCH v9 0/3] mmc: make fault injection available for MMC IO Per Forlin
2011-08-19 12:52 ` Per Forlin
2011-08-19 12:52 ` [PATCH v9 1/3] fault-inject: export fault injection functions Per Forlin
2011-08-19 12:52   ` Per Forlin
2011-08-19 12:52   ` [PATCH v9 2/3] mmc: core: add random fault injection Per Forlin
2011-08-19 12:52     ` Per Forlin
2011-08-19 12:52     ` [PATCH v9 3/3] fault injection: add documentation on MMC IO " Per Forlin
2011-08-19 12:52       ` Per Forlin
2011-08-19 14:16     ` [PATCH v9 2/3] mmc: core: add random " Linus Walleij
2011-08-19 14:16       ` Linus Walleij
2011-09-13 13:12     ` Akinobu Mita
2011-09-13 14:19       ` Per Forlin
2011-09-13 14:19         ` Per Forlin
2011-09-13 16:15         ` Per Forlin
2011-09-13 16:15           ` Per Forlin
2011-09-13 23:37           ` Akinobu Mita
2011-09-13 23:37             ` Akinobu Mita
2011-09-14  6:51             ` Per Forlin
2011-09-14  8:04               ` Akinobu Mita
2011-09-14  8:29                 ` Per Forlin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.