Re: [PATCH v3 2/3] mmc: core: add random fault injection

From: Akinobu Mita
Date: Mon Jul 25 2011 - 11:58:37 EST


2011/7/21 Per Forlin <per.forlin@xxxxxxxxxx>:
> 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.

Looks good to me.

> @@ -82,6 +87,66 @@ static void mmc_flush_scheduled_work(void)
>        flush_workqueue(workqueue);
>  }
>
> +#ifdef CONFIG_FAIL_MMC_REQUEST
> +
> +static DECLARE_FAULT_ATTR(fail_mmc_request);

I think the fail_attr should be defined for each mmc_host like make_it_fail
in struct mmc_host and debugfs entries should also be created in a
subdirectory of mmc host debugfs.

And I know that init_fault_attr_dentries() can only create a
subdirectory in debugfs root directory. But I have a patch which
support for creating it in arbitrary directory. Could you take a look
at this? (Note that this patch is based on mmotm and not yet tested)
From 79fdd83b2af932d6fd155ae918e20572e6784bb8 Mon Sep 17 00:00:00 2001
From: Akinobu Mita <akinobu.mita@xxxxxxxxx>
Date: Mon, 25 Jul 2011 23:51:20 +0900
Subject: [PATCH] fault-injection: support for creating debugfs entries in
arbitrary directory

Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx>
---
Documentation/fault-injection/fault-injection.txt | 3 +--
block/blk-core.c | 6 ++++--
block/blk-timeout.c | 5 ++++-
include/linux/fault-inject.h | 18 +++++-------------
lib/fault-inject.c | 20 +++++++-------------
mm/failslab.c | 14 +++++++-------
mm/page_alloc.c | 13 +++++--------
7 files changed, 33 insertions(+), 46 deletions(-)

diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt
index 7be15e4..dda989e 100644
--- a/Documentation/fault-injection/fault-injection.txt
+++ b/Documentation/fault-injection/fault-injection.txt
@@ -143,8 +143,7 @@ o provide a way to configure fault attributes
failslab, fail_page_alloc, and fail_make_request use this way.
Helper functions:

- init_fault_attr_dentries(entries, attr, name);
- void cleanup_fault_attr_dentries(entries);
+ debugfs_create_fault_attr(name, parent, attr);

- module parameters

diff --git a/block/blk-core.c b/block/blk-core.c
index 07988b4..498c525 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1367,8 +1367,10 @@ static bool should_fail_request(struct hd_struct *part, unsigned int bytes)

static int __init fail_make_request_debugfs(void)
{
- return init_fault_attr_dentries(&fail_make_request,
- "fail_make_request");
+ struct dentry *dir = debugfs_create_fault_attr("fail_make_request",
+ NULL, &fail_make_request);
+
+ return IS_ERR(dir) ? PTR_ERR(dir) : 0;
}

late_initcall(fail_make_request_debugfs);
diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index 4f0c06c..6397e2e 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -28,7 +28,10 @@ int blk_should_fake_timeout(struct request_queue *q)

static int __init fail_io_timeout_debugfs(void)
{
- return init_fault_attr_dentries(&fail_io_timeout, "fail_io_timeout");
+ struct dentry *dir = debugfs_create_fault_attr("fail_io_timeout",
+ NULL, &fail_io_timeout);
+
+ return IS_ERR(dir) ? PTR_ERR(dir) : 0;
}

late_initcall(fail_io_timeout_debugfs);
diff --git a/include/linux/fault-inject.h b/include/linux/fault-inject.h
index a842db6..3f583df 100644
--- a/include/linux/fault-inject.h
+++ b/include/linux/fault-inject.h
@@ -25,10 +25,6 @@ struct fault_attr {
unsigned long reject_end;

unsigned long count;
-
-#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
- struct dentry *dir;
-#endif
};

#define FAULT_ATTR_INITIALIZER { \
@@ -45,19 +41,15 @@ bool should_fail(struct fault_attr *attr, ssize_t size);

#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS

-int init_fault_attr_dentries(struct fault_attr *attr, const char *name);
-void cleanup_fault_attr_dentries(struct fault_attr *attr);
+struct dentry *debugfs_create_fault_attr(const char *name,
+ struct dentry *parent, struct fault_attr *attr);

#else /* CONFIG_FAULT_INJECTION_DEBUG_FS */

-static inline int init_fault_attr_dentries(struct fault_attr *attr,
- const char *name)
-{
- return -ENODEV;
-}
-
-static inline void cleanup_fault_attr_dentries(struct fault_attr *attr)
+static inline struct dentry *debugfs_create_fault_attr(const char *name,
+ struct dentry *parent, struct fault_attr *attr);
{
+ return ERR_PTR(-ENODEV);
}

#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
diff --git a/lib/fault-inject.c b/lib/fault-inject.c
index da61bb5..95399e7 100644
--- a/lib/fault-inject.c
+++ b/lib/fault-inject.c
@@ -192,21 +192,15 @@ static struct dentry *debugfs_create_atomic_t(const char *name, mode_t mode,
return debugfs_create_file(name, mode, parent, value, &fops_atomic_t);
}

-void cleanup_fault_attr_dentries(struct fault_attr *attr)
-{
- debugfs_remove_recursive(attr->dir);
-}
-
-int init_fault_attr_dentries(struct fault_attr *attr, const char *name)
+struct dentry *debugfs_create_fault_attr(const char *name,
+ struct dentry *parent, struct fault_attr *attr)
{
mode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
struct dentry *dir;

- dir = debugfs_create_dir(name, NULL);
+ dir = debugfs_create_dir(name, parent);
if (!dir)
- return -ENOMEM;
-
- attr->dir = dir;
+ return ERR_PTR(-ENOMEM);

if (!debugfs_create_ul("probability", mode, dir, &attr->probability))
goto fail;
@@ -238,11 +232,11 @@ int init_fault_attr_dentries(struct fault_attr *attr, const char *name)

#endif /* CONFIG_FAULT_INJECTION_STACKTRACE_FILTER */

- return 0;
+ return dir;
fail:
- debugfs_remove_recursive(attr->dir);
+ debugfs_remove_recursive(dir);

- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
}

#endif /* CONFIG_FAULT_INJECTION_DEBUG_FS */
diff --git a/mm/failslab.c b/mm/failslab.c
index 1ce58c2..59a3093 100644
--- a/mm/failslab.c
+++ b/mm/failslab.c
@@ -34,23 +34,23 @@ __setup("failslab=", setup_failslab);
#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
static int __init failslab_debugfs_init(void)
{
+ struct dentry *dir;
mode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
- int err;

- err = init_fault_attr_dentries(&failslab.attr, "failslab");
- if (err)
- return err;
+ dir = debugfs_create_fault_attr("failslab", NULL, &failslab.attr);
+ if (IS_ERR(dir))
+ return PTR_ERR(dir);

- if (!debugfs_create_bool("ignore-gfp-wait", mode, failslab.attr.dir,
+ if (!debugfs_create_bool("ignore-gfp-wait", mode, dir,
&failslab.ignore_gfp_wait))
goto fail;
- if (!debugfs_create_bool("cache-filter", mode, failslab.attr.dir,
+ if (!debugfs_create_bool("cache-filter", mode, dir,
&failslab.cache_filter))
goto fail;

return 0;
fail:
- cleanup_fault_attr_dentries(&failslab.attr);
+ debugfs_remove_recursive(dir);

return -ENOMEM;
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ed59a2e..00cf04f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1409,14 +1409,11 @@ static int __init fail_page_alloc_debugfs(void)
{
mode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
struct dentry *dir;
- int err;

- err = init_fault_attr_dentries(&fail_page_alloc.attr,
- "fail_page_alloc");
- if (err)
- return err;
-
- dir = fail_page_alloc.attr.dir;
+ dir = debugfs_create_fault_attr("fail_page_alloc", NULL,
+ &fail_page_alloc.attr);
+ if (IS_ERR(dir))
+ return PTR_ERR(dir);

if (!debugfs_create_bool("ignore-gfp-wait", mode, dir,
&fail_page_alloc.ignore_gfp_wait))
@@ -1430,7 +1427,7 @@ static int __init fail_page_alloc_debugfs(void)

return 0;
fail:
- cleanup_fault_attr_dentries(&fail_page_alloc.attr);
+ debugfs_remove_recursive(dir);

return -ENOMEM;
}
--
1.7.4.4