linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Make fault injection available for MMC IO
@ 2011-07-19 21:31 Per Forlin
  2011-07-19 21:31 ` [PATCH v2 1/3] fault-inject: make fault injection available for modules Per Forlin
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Per Forlin @ 2011-07-19 21:31 UTC (permalink / raw)
  To: linaro-dev, Nicolas Pitre, linux-arm-kernel, linux-kernel,
	linux-mmc, linux-doc, Venkatraman S, Linus Walleij,
	Kyungmin Park, Arnd Bergmann, Sourav Poddar, Chris Ball,
	Akinobu Mita
  Cc: Randy Dunlap, Per Forlin

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

-- 
1.7.4.1


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

* [PATCH v2 1/3] fault-inject: make fault injection available for modules
  2011-07-19 21:31 [PATCH v2 0/3] Make fault injection available for MMC IO Per Forlin
@ 2011-07-19 21:31 ` Per Forlin
  2011-07-19 23:44   ` J Freyensee
  2011-07-19 21:31 ` [PATCH v2 2/3] mmc: core: add random fault injection Per Forlin
  2011-07-19 21:31 ` [PATCH v2 3/3] fault injection: add documentation on MMC IO " Per Forlin
  2 siblings, 1 reply; 8+ messages in thread
From: Per Forlin @ 2011-07-19 21:31 UTC (permalink / raw)
  To: linaro-dev, Nicolas Pitre, linux-arm-kernel, linux-kernel,
	linux-mmc, linux-doc, Venkatraman S, Linus Walleij,
	Kyungmin Park, Arnd Bergmann, Sourav Poddar, Chris Ball,
	Akinobu Mita
  Cc: Randy Dunlap, Per Forlin

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


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

* [PATCH v2 2/3] mmc: core: add random fault injection
  2011-07-19 21:31 [PATCH v2 0/3] Make fault injection available for MMC IO Per Forlin
  2011-07-19 21:31 ` [PATCH v2 1/3] fault-inject: make fault injection available for modules Per Forlin
@ 2011-07-19 21:31 ` Per Forlin
  2011-07-20  0:04   ` J Freyensee
  2011-07-19 21:31 ` [PATCH v2 3/3] fault injection: add documentation on MMC IO " Per Forlin
  2 siblings, 1 reply; 8+ messages in thread
From: Per Forlin @ 2011-07-19 21:31 UTC (permalink / raw)
  To: linaro-dev, Nicolas Pitre, linux-arm-kernel, linux-kernel,
	linux-mmc, linux-doc, Venkatraman S, Linus Walleij,
	Kyungmin Park, Arnd Bergmann, Sourav Poddar, Chris Ball,
	Akinobu Mita
  Cc: Randy Dunlap, Per Forlin

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


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

* [PATCH v2 3/3] fault injection: add documentation on MMC IO fault injection
  2011-07-19 21:31 [PATCH v2 0/3] Make fault injection available for MMC IO Per Forlin
  2011-07-19 21:31 ` [PATCH v2 1/3] fault-inject: make fault injection available for modules Per Forlin
  2011-07-19 21:31 ` [PATCH v2 2/3] mmc: core: add random fault injection Per Forlin
@ 2011-07-19 21:31 ` Per Forlin
  2 siblings, 0 replies; 8+ messages in thread
From: Per Forlin @ 2011-07-19 21:31 UTC (permalink / raw)
  To: linaro-dev, Nicolas Pitre, linux-arm-kernel, linux-kernel,
	linux-mmc, linux-doc, Venkatraman S, Linus Walleij,
	Kyungmin Park, Arnd Bergmann, Sourav Poddar, Chris Ball,
	Akinobu Mita
  Cc: Randy Dunlap, Per Forlin

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


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

* Re: [PATCH v2 1/3] fault-inject: make fault injection available for modules
  2011-07-19 21:31 ` [PATCH v2 1/3] fault-inject: make fault injection available for modules Per Forlin
@ 2011-07-19 23:44   ` J Freyensee
  2011-07-20 21:50     ` Per Forlin
  0 siblings, 1 reply; 8+ messages in thread
From: J Freyensee @ 2011-07-19 23:44 UTC (permalink / raw)
  To: Per Forlin
  Cc: linaro-dev, Nicolas Pitre, linux-arm-kernel, linux-kernel,
	linux-mmc, linux-doc, Venkatraman S, Linus Walleij,
	Kyungmin Park, Arnd Bergmann, Sourav Poddar, Chris Ball,
	Akinobu Mita, Randy Dunlap

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

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


-- 
J (James/Jay) Freyensee
Storage Technology Group
Intel Corporation

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

* Re: [PATCH v2 2/3] mmc: core: add random fault injection
  2011-07-19 21:31 ` [PATCH v2 2/3] mmc: core: add random fault injection Per Forlin
@ 2011-07-20  0:04   ` J Freyensee
  2011-07-20 21:49     ` Per Forlin
  0 siblings, 1 reply; 8+ messages in thread
From: J Freyensee @ 2011-07-20  0:04 UTC (permalink / raw)
  To: Per Forlin
  Cc: linaro-dev, Nicolas Pitre, linux-arm-kernel, linux-kernel,
	linux-mmc, linux-doc, Venkatraman S, Linus Walleij,
	Kyungmin Park, Arnd Bergmann, Sourav Poddar, Chris Ball,
	Akinobu Mita, Randy Dunlap

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

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

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

* Re: [PATCH v2 2/3] mmc: core: add random fault injection
  2011-07-20  0:04   ` J Freyensee
@ 2011-07-20 21:49     ` Per Forlin
  0 siblings, 0 replies; 8+ messages in thread
From: Per Forlin @ 2011-07-20 21:49 UTC (permalink / raw)
  To: J Freyensee
  Cc: linaro-dev, Nicolas Pitre, linux-arm-kernel, linux-kernel,
	linux-mmc, linux-doc, Venkatraman S, Linus Walleij,
	Kyungmin Park, Arnd Bergmann, Sourav Poddar, Chris Ball,
	Akinobu Mita, Randy Dunlap

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

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

* Re: [PATCH v2 1/3] fault-inject: make fault injection available for modules
  2011-07-19 23:44   ` J Freyensee
@ 2011-07-20 21:50     ` Per Forlin
  0 siblings, 0 replies; 8+ messages in thread
From: Per Forlin @ 2011-07-20 21:50 UTC (permalink / raw)
  To: J Freyensee
  Cc: linaro-dev, Nicolas Pitre, linux-arm-kernel, linux-kernel,
	linux-mmc, linux-doc, Venkatraman S, Linus Walleij,
	Kyungmin Park, Arnd Bergmann, Sourav Poddar, Chris Ball,
	Akinobu Mita, Randy Dunlap

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

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

end of thread, other threads:[~2011-07-20 21:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-19 21:31 [PATCH v2 0/3] Make fault injection available for MMC IO Per Forlin
2011-07-19 21:31 ` [PATCH v2 1/3] fault-inject: make fault injection available for modules Per Forlin
2011-07-19 23:44   ` J Freyensee
2011-07-20 21:50     ` Per Forlin
2011-07-19 21:31 ` [PATCH v2 2/3] mmc: core: add random fault injection Per Forlin
2011-07-20  0:04   ` J Freyensee
2011-07-20 21:49     ` Per Forlin
2011-07-19 21:31 ` [PATCH v2 3/3] fault injection: add documentation on MMC IO " Per Forlin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).