From: Bryan Schumaker <[email protected]>
Fault injection on the NFS server makes it easier to test the client's
state manager and recovery threads. Simulating errors on the server is
easier than finding the right conditions that cause them naturally.
This patch uses debugfs to add a simple framework for fault injection to
the server. This framework is a config option, and can be enabled
through CONFIG_NFSD_FAULT_INJECTION. Assuming you have debugfs mounted
to /sys/debug, a set of files will be created in /sys/debug/nfsd/.
Writing to any of these files will cause the corresponding action and
write a log entry to dmesg.
Changes in v4:
- Move fault injection function declarations to a new .h file
- Use the Makefile to selectively compile fault_injection.c
- Add copyright notices to top of files
Changes in v3:
- Code cleanup and better use of generic functions
- Allow the user to input the number of state objects to delete
- Remove "forget_everything()" since forgetting a client is basically
the same thing
Changes in v2:
- Replaced "forget all state owners" with "forget all open owners"
- Include fs/nfsd/fault_inject.c in the patch
Signed-off-by: Bryan Schumaker <[email protected]>
---
fs/nfsd/Kconfig | 10 ++++
fs/nfsd/Makefile | 1 +
fs/nfsd/fault_inject.c | 94 +++++++++++++++++++++++++++++++++++++++
fs/nfsd/fault_inject.h | 28 ++++++++++++
fs/nfsd/nfs4state.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/nfsctl.c | 7 +++
6 files changed, 255 insertions(+), 0 deletions(-)
create mode 100644 fs/nfsd/fault_inject.c
create mode 100644 fs/nfsd/fault_inject.h
diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index 10e6366..52fdd1c 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -80,3 +80,13 @@ config NFSD_V4
available from http://linux-nfs.org/.
If unsure, say N.
+
+config NFSD_FAULT_INJECTION
+ bool "NFS server manual fault injection"
+ depends on NFSD_V4 && DEBUG_KERNEL
+ help
+ This option enables support for manually injectiong faults
+ into the NFS server. This is intended to be used for
+ testing error recovery on the NFS client.
+
+ If unsure, say N.
diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
index 9b118ee..af32ef0 100644
--- a/fs/nfsd/Makefile
+++ b/fs/nfsd/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_NFSD) += nfsd.o
nfsd-y := nfssvc.o nfsctl.o nfsproc.o nfsfh.o vfs.o \
export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o
+nfsd-$(CONFIG_NFSD_FAULT_INJECTION) += fault_inject.o
nfsd-$(CONFIG_NFSD_V2_ACL) += nfs2acl.o
nfsd-$(CONFIG_NFSD_V3) += nfs3proc.o nfs3xdr.o
nfsd-$(CONFIG_NFSD_V3_ACL) += nfs3acl.o
diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
new file mode 100644
index 0000000..9f6815b
--- /dev/null
+++ b/fs/nfsd/fault_inject.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 2011 Bryan Schumaker <[email protected]>
+ *
+ * Uses debugfs to create fault injection points for client testing
+ */
+
+#include <linux/types.h>
+#include <linux/fs.h>
+#include <linux/debugfs.h>
+#include <linux/module.h>
+
+#include "state.h"
+#include "fault_inject.h"
+
+struct nfsd_fault_inject_op {
+ char *action;
+ char *item;
+ char *file;
+ int file_data;
+ void (*func)(u64);
+};
+
+#define INJECTION_OP(op_action, op_item, op_func) \
+{ \
+ .action = op_action, \
+ .item = op_item, \
+ .file = op_action"_"op_item, \
+ .func = op_func, \
+}
+
+static struct nfsd_fault_inject_op inject_ops[] = {
+ INJECTION_OP("forget", "clients", nfsd_forget_clients),
+ INJECTION_OP("forget", "locks", nfsd_forget_locks),
+ INJECTION_OP("forget", "openowners", nfsd_forget_openowners),
+ INJECTION_OP("forget", "delegations", nfsd_forget_delegations),
+ INJECTION_OP("recall", "delegations", nfsd_recall_delegations),
+};
+
+static long int NUM_INJECT_OPS = sizeof(inject_ops) / sizeof(struct nfsd_fault_inject_op);
+static struct dentry *debug_dir;
+
+static int nfsd_inject_set(void *data, u64 val)
+{
+ int i;
+ struct nfsd_fault_inject_op *op;
+
+ for (i = 0; i < NUM_INJECT_OPS; i++) {
+ op = &inject_ops[i];
+ if (&op->file_data == data) {
+ if (val == 0) {
+ printk(KERN_INFO "NFSD: Server %sing all %s",
+ op->action, op->item);
+ } else {
+ printk(KERN_INFO "NFSD: Server %sing at most %llu %s",
+ op->action, val, op->item);
+ }
+ op->func(val);
+ }
+ }
+ return 0;
+}
+
+static int nfsd_inject_get(void *data, u64 *val)
+{
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(fops_nfsd, nfsd_inject_get, nfsd_inject_set, "%llu\n");
+
+void nfsd_fault_inject_cleanup(void)
+{
+ debugfs_remove_recursive(debug_dir);
+}
+
+int nfsd_fault_inject_init(void)
+{
+ unsigned int i;
+ struct nfsd_fault_inject_op *op;
+ mode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
+
+ debug_dir = debugfs_create_dir("nfsd", NULL);
+ if (!debug_dir)
+ goto fail;
+
+ for (i = 0; i < NUM_INJECT_OPS; i++) {
+ op = &inject_ops[i];
+ debugfs_create_file(op->file, mode, debug_dir, &op->file_data, &fops_nfsd);
+ }
+ return 0;
+
+fail:
+ nfsd_fault_inject_cleanup();
+ return -ENOMEM;
+}
diff --git a/fs/nfsd/fault_inject.h b/fs/nfsd/fault_inject.h
new file mode 100644
index 0000000..90bd057
--- /dev/null
+++ b/fs/nfsd/fault_inject.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2011 Bryan Schumaker <[email protected]>
+ *
+ * Function definitions for fault injection
+ */
+
+#ifndef LINUX_NFSD_FAULT_INJECT_H
+#define LINUX_NFSD_FAULT_INJECT_H
+
+#ifdef CONFIG_NFSD_FAULT_INJECTION
+int nfsd_fault_inject_init(void);
+void nfsd_fault_inject_cleanup(void);
+void nfsd_forget_clients(u64);
+void nfsd_forget_locks(u64);
+void nfsd_forget_openowners(u64);
+void nfsd_forget_delegations(u64);
+void nfsd_recall_delegations(u64);
+#else /* CONFIG_NFSD_FAULT_INJECTION */
+static inline int nfsd_fault_inject_init(void) { return 0; }
+static inline void nfsd_fault_inject_cleanup(void) {}
+static inline void nfsd_forget_clients(u64 num) {}
+static inline void nfsd_forget_locks(u64 num) {}
+static inline void nfsd_forget_openowners(u64 num) {}
+static inline void nfsd_forget_delegations(u64 num) {}
+static inline void nfsd_recall_delegations(u64 num) {}
+#endif /* CONFIG_NFSD_FAULT_INJECTION */
+
+#endif /* LINUX_NFSD_FAULT_INJECT_H */
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 05f4c69..64fa6b0 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4358,6 +4358,121 @@ nfs4_check_open_reclaim(clientid_t *clid)
return nfs4_find_reclaim_client(clid) ? nfs_ok : nfserr_reclaim_bad;
}
+#ifdef CONFIG_NFSD_FAULT_INJECTION
+
+void nfsd_forget_clients(u64 num)
+{
+ struct nfs4_client *clp, *next;
+ int count = 0;
+
+ nfs4_lock_state();
+ list_for_each_entry_safe(clp, next, &client_lru, cl_lru) {
+ nfsd4_remove_clid_dir(clp);
+ expire_client(clp);
+ if (++count == num)
+ break;
+ }
+ nfs4_unlock_state();
+
+ printk(KERN_INFO "NFSD: Forgot %d clients", count);
+}
+
+static void release_lockowner_sop(struct nfs4_stateowner *sop)
+{
+ release_lockowner(lockowner(sop));
+}
+
+static void release_openowner_sop(struct nfs4_stateowner *sop)
+{
+ release_openowner(openowner(sop));
+}
+
+static int nfsd_release_n_owners(u64 num,
+ struct list_head hashtbl[],
+ unsigned int hashtbl_size,
+ void (*release_sop)(struct nfs4_stateowner *))
+{
+ int i, count = 0;
+ struct nfs4_stateowner *sop, *next;
+
+ for (i = 0; i < hashtbl_size; i++) {
+ list_for_each_entry_safe(sop, next, &hashtbl[i], so_strhash) {
+ release_sop(sop);
+ if (++count == num)
+ return count;
+ }
+ }
+ return count;
+}
+
+void nfsd_forget_locks(u64 num)
+{
+ int count;
+
+ nfs4_lock_state();
+ count = nfsd_release_n_owners(num, lock_ownerstr_hashtbl,
+ LOCK_HASH_SIZE, release_lockowner_sop);
+ nfs4_unlock_state();
+
+ printk(KERN_INFO "NFSD: Forgot %d locks", count);
+}
+
+void nfsd_forget_openowners(u64 num)
+{
+ int count;
+
+ nfs4_lock_state();
+ count = nfsd_release_n_owners(num, open_ownerstr_hashtbl,
+ OPEN_OWNER_HASH_SIZE, release_openowner_sop);
+ nfs4_unlock_state();
+
+ printk(KERN_INFO "NFSD: Forgot %d open owners", count);
+}
+
+int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *))
+{
+ int i, count = 0;
+ struct nfs4_file *fp;
+ struct nfs4_delegation *dp, *next;
+
+ for (i = 0; i < FILE_HASH_SIZE; i++) {
+ list_for_each_entry(fp, &file_hashtbl[i], fi_hash) {
+ list_for_each_entry_safe(dp, next, &fp->fi_delegations, dl_perfile) {
+ deleg_func(dp);
+ if (++count == num)
+ return count;
+ }
+ }
+ }
+ return count;
+}
+
+void nfsd_forget_delegations(u64 num)
+{
+ unsigned int count;
+
+ nfs4_lock_state();
+ count = nfsd_process_n_delegations(num, unhash_delegation);
+ nfs4_unlock_state();
+
+ printk(KERN_INFO "NFSD: Forgot %d delegations", count);
+}
+
+void nfsd_recall_delegations(u64 num)
+{
+ unsigned int count;
+
+ nfs4_lock_state();
+ spin_lock(&recall_lock);
+ count = nfsd_process_n_delegations(num, nfsd_break_one_deleg);
+ spin_unlock(&recall_lock);
+ nfs4_unlock_state();
+
+ printk(KERN_INFO "NFSD: Recalled %d delegations", count);
+}
+
+#endif /* CONFIG_NFSD_FAULT_INJECTION */
+
/* initialization to perform at module load time: */
int
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index db34a58..e67f30c 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -17,6 +17,7 @@
#include "idmap.h"
#include "nfsd.h"
#include "cache.h"
+#include "fault_inject.h"
/*
* We have a single directory with several nodes in it.
@@ -1130,6 +1131,9 @@ static int __init init_nfsd(void)
retval = nfs4_state_init(); /* nfs4 locking state */
if (retval)
return retval;
+ retval = nfsd_fault_inject_init(); /* nfsd fault injection controls */
+ if (retval)
+ goto out_cleanup_fault_injection;
nfsd_stat_init(); /* Statistics */
retval = nfsd_reply_cache_init();
if (retval)
@@ -1161,6 +1165,8 @@ out_free_cache:
out_free_stat:
nfsd_stat_shutdown();
nfsd4_free_slabs();
+out_cleanup_fault_injection:
+ nfsd_fault_inject_cleanup();
return retval;
}
@@ -1174,6 +1180,7 @@ static void __exit exit_nfsd(void)
nfsd_lockd_shutdown();
nfsd_idmap_shutdown();
nfsd4_free_slabs();
+ nfsd_fault_inject_cleanup();
unregister_filesystem(&nfsd_fs_type);
}
--
1.7.7
On Mon, Oct 17, 2011 at 06:57:18PM -0400, Bryan Schumaker wrote:
> On 10/17/2011 06:18 PM, J. Bruce Fields wrote:
> > On Fri, Oct 07, 2011 at 01:44:26PM -0400, [email protected] wrote:
> >> +#define INJECTION_OP(op_action, op_item, op_func) \
> >> +{ \
> >> + .action = op_action, \
> >> + .item = op_item, \
> >> + .file = op_action"_"op_item, \
> >> + .func = op_func, \
> >> +}
> >> +
> >> +static struct nfsd_fault_inject_op inject_ops[] = {
> >> + INJECTION_OP("forget", "clients", nfsd_forget_clients),
> >> + INJECTION_OP("forget", "locks", nfsd_forget_locks),
> >> + INJECTION_OP("forget", "openowners", nfsd_forget_openowners),
> >> + INJECTION_OP("forget", "delegations", nfsd_forget_delegations),
> >> + INJECTION_OP("recall", "delegations", nfsd_recall_delegations),
> >
> > This is a little clever for my taste.... Could we just do
> >
> > static struct nfsd_fault_inject_op inject_ops[] = {
> > {
> > .file = "forget_client",
> > .op = nfsd_forget_clients,
> > },
> > ...
> > }
> >
> > and do away with the separate item and action fields?
> >
> > I'd rather be sort of obvious and boring even if it's slightly less
> > compact.
> >
> I was going for compact when I initially wrote this, but I can change it. I have them as separate fields so I can print out slightly different messages based on what is going on. Such as: "NFSD: Server forgetting all clients" or "NFSD: Server recalling at most 4 delegations".
Even
{ .file = "forget_client", .op=nfsd_forget_clients },
{ ... }
would be fine by me and still pretty compact.
And log messages are probably a good idea but I don't think they have to
be beautiful--"NFSD: recall_delegations(4)" would do fine.
--b.
>
> >> +};
> >> +
> >> +static long int NUM_INJECT_OPS = sizeof(inject_ops) / sizeof(struct nfsd_fault_inject_op);
> >> +static struct dentry *debug_dir;
> >> +
> >> +static int nfsd_inject_set(void *data, u64 val)
> >> +{
> >> + int i;
> >> + struct nfsd_fault_inject_op *op;
> >> +
> >> + for (i = 0; i < NUM_INJECT_OPS; i++) {
> >> + op = &inject_ops[i];
> >> + if (&op->file_data == data) {
> >
> > Huh, OK, so if I understand right, the contents of file_data doesn't
> > matter, you're just using a pointer to that field as a way to identify
> > the op array.
> >
> > But then couldn't you just pass in a pointer to the op itself:
> >
> >> + for (i = 0; i < NUM_INJECT_OPS; i++) {
> >> + op = &inject_ops[i];
> >> + debugfs_create_file(op->file, mode, debug_dir, &op->file_data, &fops_nfsd);
> >
> > like:
> >
> > debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd);
> >
> > and eliminate the file_data field?
>
> I've never thought about trying it that way, but it seems fairly straightforward. I'll try it that way and see if it works!
> >
> > Patches look OK otherwise on a quick skim, thanks.
> >
> > --b.
> >
> >
> >> + }
> >> + return 0;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 07, 2011 at 01:44:26PM -0400, [email protected] wrote:
> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> index 10e6366..52fdd1c 100644
> --- a/fs/nfsd/Kconfig
> +++ b/fs/nfsd/Kconfig
> @@ -80,3 +80,13 @@ config NFSD_V4
> available from http://linux-nfs.org/.
>
> If unsure, say N.
> +
> +config NFSD_FAULT_INJECTION
> + bool "NFS server manual fault injection"
> + depends on NFSD_V4 && DEBUG_KERNEL
> + help
> + This option enables support for manually injectiong faults
^^^^^^^^^^
> diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
> new file mode 100644
> index 0000000..9f6815b
> --- /dev/null
> +++ b/fs/nfsd/fault_inject.c
> @@ -0,0 +1,94 @@
> +/*
> + * Copyright (c) 2011 Bryan Schumaker <[email protected]>
> + *
> + * Uses debugfs to create fault injection points for client testing
> + */
> +
> +#include <linux/types.h>
> +#include <linux/fs.h>
> +#include <linux/debugfs.h>
> +#include <linux/module.h>
> +
> +#include "state.h"
> +#include "fault_inject.h"
> +
> +struct nfsd_fault_inject_op {
> + char *action;
> + char *item;
> + char *file;
> + int file_data;
> + void (*func)(u64);
> +};
> +
> +#define INJECTION_OP(op_action, op_item, op_func) \
> +{ \
> + .action = op_action, \
> + .item = op_item, \
> + .file = op_action"_"op_item, \
> + .func = op_func, \
> +}
> +
> +static struct nfsd_fault_inject_op inject_ops[] = {
> + INJECTION_OP("forget", "clients", nfsd_forget_clients),
> + INJECTION_OP("forget", "locks", nfsd_forget_locks),
> + INJECTION_OP("forget", "openowners", nfsd_forget_openowners),
> + INJECTION_OP("forget", "delegations", nfsd_forget_delegations),
> + INJECTION_OP("recall", "delegations", nfsd_recall_delegations),
This is a little clever for my taste.... Could we just do
static struct nfsd_fault_inject_op inject_ops[] = {
{
.file = "forget_client",
.op = nfsd_forget_clients,
},
...
}
and do away with the separate item and action fields?
I'd rather be sort of obvious and boring even if it's slightly less
compact.
> +};
> +
> +static long int NUM_INJECT_OPS = sizeof(inject_ops) / sizeof(struct nfsd_fault_inject_op);
> +static struct dentry *debug_dir;
> +
> +static int nfsd_inject_set(void *data, u64 val)
> +{
> + int i;
> + struct nfsd_fault_inject_op *op;
> +
> + for (i = 0; i < NUM_INJECT_OPS; i++) {
> + op = &inject_ops[i];
> + if (&op->file_data == data) {
Huh, OK, so if I understand right, the contents of file_data doesn't
matter, you're just using a pointer to that field as a way to identify
the op array.
But then couldn't you just pass in a pointer to the op itself:
> + for (i = 0; i < NUM_INJECT_OPS; i++) {
> + op = &inject_ops[i];
> + debugfs_create_file(op->file, mode, debug_dir, &op->file_data, &fops_nfsd);
like:
debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd);
and eliminate the file_data field?
Patches look OK otherwise on a quick skim, thanks.
--b.
> + }
> + return 0;
On 10/17/2011 06:18 PM, J. Bruce Fields wrote:
> On Fri, Oct 07, 2011 at 01:44:26PM -0400, [email protected] wrote:
>> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
>> index 10e6366..52fdd1c 100644
>> --- a/fs/nfsd/Kconfig
>> +++ b/fs/nfsd/Kconfig
>> @@ -80,3 +80,13 @@ config NFSD_V4
>> available from http://linux-nfs.org/.
>>
>> If unsure, say N.
>> +
>> +config NFSD_FAULT_INJECTION
>> + bool "NFS server manual fault injection"
>> + depends on NFSD_V4 && DEBUG_KERNEL
>> + help
>> + This option enables support for manually injectiong faults
> ^^^^^^^^^^
Good catch, this is a weird cross between "injection" and "injecting"
>
>> diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
>> new file mode 100644
>> index 0000000..9f6815b
>> --- /dev/null
>> +++ b/fs/nfsd/fault_inject.c
>> @@ -0,0 +1,94 @@
>> +/*
>> + * Copyright (c) 2011 Bryan Schumaker <[email protected]>
>> + *
>> + * Uses debugfs to create fault injection points for client testing
>> + */
>> +
>> +#include <linux/types.h>
>> +#include <linux/fs.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/module.h>
>> +
>> +#include "state.h"
>> +#include "fault_inject.h"
>> +
>> +struct nfsd_fault_inject_op {
>> + char *action;
>> + char *item;
>> + char *file;
>> + int file_data;
>> + void (*func)(u64);
>> +};
>> +
>> +#define INJECTION_OP(op_action, op_item, op_func) \
>> +{ \
>> + .action = op_action, \
>> + .item = op_item, \
>> + .file = op_action"_"op_item, \
>> + .func = op_func, \
>> +}
>> +
>> +static struct nfsd_fault_inject_op inject_ops[] = {
>> + INJECTION_OP("forget", "clients", nfsd_forget_clients),
>> + INJECTION_OP("forget", "locks", nfsd_forget_locks),
>> + INJECTION_OP("forget", "openowners", nfsd_forget_openowners),
>> + INJECTION_OP("forget", "delegations", nfsd_forget_delegations),
>> + INJECTION_OP("recall", "delegations", nfsd_recall_delegations),
>
> This is a little clever for my taste.... Could we just do
>
> static struct nfsd_fault_inject_op inject_ops[] = {
> {
> .file = "forget_client",
> .op = nfsd_forget_clients,
> },
> ...
> }
>
> and do away with the separate item and action fields?
>
> I'd rather be sort of obvious and boring even if it's slightly less
> compact.
>
I was going for compact when I initially wrote this, but I can change it. I have them as separate fields so I can print out slightly different messages based on what is going on. Such as: "NFSD: Server forgetting all clients" or "NFSD: Server recalling at most 4 delegations".
>> +};
>> +
>> +static long int NUM_INJECT_OPS = sizeof(inject_ops) / sizeof(struct nfsd_fault_inject_op);
>> +static struct dentry *debug_dir;
>> +
>> +static int nfsd_inject_set(void *data, u64 val)
>> +{
>> + int i;
>> + struct nfsd_fault_inject_op *op;
>> +
>> + for (i = 0; i < NUM_INJECT_OPS; i++) {
>> + op = &inject_ops[i];
>> + if (&op->file_data == data) {
>
> Huh, OK, so if I understand right, the contents of file_data doesn't
> matter, you're just using a pointer to that field as a way to identify
> the op array.
>
> But then couldn't you just pass in a pointer to the op itself:
>
>> + for (i = 0; i < NUM_INJECT_OPS; i++) {
>> + op = &inject_ops[i];
>> + debugfs_create_file(op->file, mode, debug_dir, &op->file_data, &fops_nfsd);
>
> like:
>
> debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd);
>
> and eliminate the file_data field?
I've never thought about trying it that way, but it seems fairly straightforward. I'll try it that way and see if it works!
>
> Patches look OK otherwise on a quick skim, thanks.
>
> --b.
>
>
>> + }
>> + return 0;
On Mon, Oct 24, 2011 at 07:20:57AM -0400, [email protected] wrote:
...
> diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
> new file mode 100644
> index 0000000..1ac4134
> --- /dev/null
> +++ b/fs/nfsd/fault_inject.c
> @@ -0,0 +1,90 @@
...
> +int nfsd_fault_inject_init(void)
> +{
> + unsigned int i;
> + struct nfsd_fault_inject_op *op;
> + mode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
> +
> + debug_dir = debugfs_create_dir("nfsd", NULL);
> + if (!debug_dir)
> + goto fail;
> +
> + for (i = 0; i < NUM_INJECT_OPS; i++) {
> + op = &inject_ops[i];
> + debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd);
I think you need to check the return value and "goto fail" on NULL.
(Do you also need to put the returned dentry on success? Checking other
callers.... No, I guess not, OK.)
> + }
> + return 0;
> +
> +fail:
> + nfsd_fault_inject_cleanup();
> + return -ENOMEM;
> +}
...
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index db34a58..e67f30c 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
...
> @@ -1130,6 +1131,9 @@ static int __init init_nfsd(void)
> retval = nfs4_state_init(); /* nfs4 locking state */
> if (retval)
> return retval;
> + retval = nfsd_fault_inject_init(); /* nfsd fault injection controls */
> + if (retval)
> + goto out_cleanup_fault_injection;
> nfsd_stat_init(); /* Statistics */
> retval = nfsd_reply_cache_init();
> if (retval)
> @@ -1161,6 +1165,8 @@ out_free_cache:
> out_free_stat:
> nfsd_stat_shutdown();
> nfsd4_free_slabs();
> +out_cleanup_fault_injection:
> + nfsd_fault_inject_cleanup();
Check the cleanup here again.... The slabs are allocated in state_init,
so you need to do that on nfsd_fault_inject_init, but you *don't* need
the fault_inject_cleanup in that case, since fault_inject_init cleanup
cleans up after itself--that's needed only on later failures.
--b.
> return retval;
> }
>
> @@ -1174,6 +1180,7 @@ static void __exit exit_nfsd(void)
> nfsd_lockd_shutdown();
> nfsd_idmap_shutdown();
> nfsd4_free_slabs();
> + nfsd_fault_inject_cleanup();
> unregister_filesystem(&nfsd_fs_type);
> }
>
> --
> 1.7.7
>
From: Bryan Schumaker <[email protected]>
Signed-off-by: Bryan Schumaker <[email protected]>
---
Documentation/filesystems/nfs/00-INDEX | 2 +
Documentation/filesystems/nfs/fault_injection.txt | 69 +++++++++++++++++++++
2 files changed, 71 insertions(+), 0 deletions(-)
create mode 100644 Documentation/filesystems/nfs/fault_injection.txt
diff --git a/Documentation/filesystems/nfs/00-INDEX b/Documentation/filesystems/nfs/00-INDEX
index a57e124..1716874 100644
--- a/Documentation/filesystems/nfs/00-INDEX
+++ b/Documentation/filesystems/nfs/00-INDEX
@@ -2,6 +2,8 @@
- this file (nfs-related documentation).
Exporting
- explanation of how to make filesystems exportable.
+fault_injection.txt
+ - information for using fault injection on the server
knfsd-stats.txt
- statistics which the NFS server makes available to user space.
nfs.txt
diff --git a/Documentation/filesystems/nfs/fault_injection.txt b/Documentation/filesystems/nfs/fault_injection.txt
new file mode 100644
index 0000000..426d166
--- /dev/null
+++ b/Documentation/filesystems/nfs/fault_injection.txt
@@ -0,0 +1,69 @@
+
+Fault Injection
+===============
+Fault injection is a method for forcing errors that may not normally occur, or
+may be difficult to reproduce. Forcing these errors in a controlled environment
+can help the developer find and fix bugs before their code is shipped in a
+production system. Injecting an error on the Linux NFS server will allow us to
+observe how the client reacts and if it manages to recover its state correctly.
+
+NFSD_FAULT_INJECTION must be selected when configuring the kernel to use this
+feature.
+
+
+Using Fault Injection
+=====================
+On the client, mount the fault injection server through NFS v4.0+ and do some
+work over NFS (open files, take locks, ...).
+
+On the server, mount the debugfs filesystem to <debug_dir> and ls
+<debug_dir>/nfsd. This will show a list of files that will be used for
+injecting faults on the NFS server. As root, write a number n to the file
+corresponding to the action you want the server to take. The server will then
+process the first n items it finds. So if you want to forget 5 locks, echo '5'
+to <debug_dir>/nfsd/forget_locks. A value of 0 will tell the server to forget
+all corresponding items. A log message will be created containing the number
+of items forgotten (check dmesg).
+
+Go back to work on the client and check if the client recovered from the error
+correctly.
+
+
+Available Faults
+================
+forget_clients:
+ The NFS server keeps a list of clients that have placed a mount call. If
+ this list is cleared, the server will have no knowledge of who the client
+ is, forcing the client to reauthenticate with the server.
+
+forget_openowners:
+ The NFS server keeps a list of what files are currently opened and who
+ they were opened by. Clearing this list will force the client to reopen
+ its files.
+
+forget_locks:
+ The NFS server keeps a list of what files are currently locked in the VFS.
+ Clearing this list will force the client to reclaim its locks (files are
+ unlocked through the VFS as they are cleared from this list).
+
+forget_delegations:
+ A delegation is used to assure the client that a file, or part of a file,
+ has not changed since the delegation was awarded. Clearing this list will
+ force the client to reaquire its delegation before accessing the file
+ again.
+
+recall_delegations:
+ Delegations can be recalled by the server when another client attempts to
+ access a file. This test will notify the client that its delegation has
+ been revoked, forcing the client to reaquire the delegation before using
+ the file again.
+
+
+tools/nfs/inject_faults.sh script
+=================================
+This script has been created to ease the fault injection process. This script
+will detect the mounted debugfs directory and write to the files located there
+based on the arguments passed by the user. For example, running
+`inject_faults.sh forget_locks 1` as root will instruct the server to forget
+one lock. Running `inject_faults forget_locks` will instruct the server to
+forgetall locks.
--
1.7.7
From: Bryan Schumaker <[email protected]>
This script provides a convenient way to use the NFSD fault injection
framework. Fault injection writes to dmesg using the KERN_INFO flag, so
this script will compare the before and after output of `dmesg` to show
the user what happened
Signed-off-by: Bryan Schumaker <[email protected]>
---
tools/nfsd/inject_fault.sh | 49 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 49 insertions(+), 0 deletions(-)
create mode 100755 tools/nfsd/inject_fault.sh
diff --git a/tools/nfsd/inject_fault.sh b/tools/nfsd/inject_fault.sh
new file mode 100755
index 0000000..06a399a
--- /dev/null
+++ b/tools/nfsd/inject_fault.sh
@@ -0,0 +1,49 @@
+#!/bin/bash
+#
+# Copyright (c) 2011 Bryan Schumaker <[email protected]>
+#
+# Script for easier NFSD fault injection
+
+# Check that debugfs has been mounted
+DEBUGFS=`cat /proc/mounts | grep debugfs`
+if [ "$DEBUGFS" == "" ]; then
+ echo "debugfs does not appear to be mounted!"
+ echo "Please mount debugfs and try again"
+ exit 1
+fi
+
+# Check that the fault injection directory exists
+DEBUGDIR=`echo $DEBUGFS | awk '{print $2}'`/nfsd
+if [ ! -d "$DEBUGDIR" ]; then
+ echo "$DEBUGDIR does not exist"
+ echo "Check that your .config selects CONFIG_NFSD_FAULT_INJECTION"
+ exit 1
+fi
+
+function help()
+{
+ echo "Usage $0 injection_type [count]"
+ echo ""
+ echo "Injection types are:"
+ ls $DEBUGDIR
+ exit 1
+}
+
+if [ $# == 0 ]; then
+ help
+elif [ ! -f $DEBUGDIR/$1 ]; then
+ help
+elif [ $# != 2 ]; then
+ COUNT=0
+else
+ COUNT=$2
+fi
+
+BEFORE=`mktemp`
+AFTER=`mktemp`
+dmesg > $BEFORE
+echo $COUNT > $DEBUGDIR/$1
+dmesg > $AFTER
+# Capture lines that only exist in the $AFTER file
+diff $BEFORE $AFTER | grep ">"
+rm -f $BEFORE $AFTER
--
1.7.7
On Fri 28 Oct 2011 05:24:50 PM EDT, J. Bruce Fields wrote:
> On Mon, Oct 24, 2011 at 07:20:57AM -0400, [email protected] wrote:
> ...
>> diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
>> new file mode 100644
>> index 0000000..1ac4134
>> --- /dev/null
>> +++ b/fs/nfsd/fault_inject.c
>> @@ -0,0 +1,90 @@
> ...
>> +int nfsd_fault_inject_init(void)
>> +{
>> + unsigned int i;
>> + struct nfsd_fault_inject_op *op;
>> + mode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
>> +
>> + debug_dir = debugfs_create_dir("nfsd", NULL);
>> + if (!debug_dir)
>> + goto fail;
>> +
>> + for (i = 0; i < NUM_INJECT_OPS; i++) {
>> + op = &inject_ops[i];
>> + debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd);
>
> I think you need to check the return value and "goto fail" on NULL.
Makes sense. Thanks for catching that!
>
> (Do you also need to put the returned dentry on success? Checking other
> callers.... No, I guess not, OK.)
>
>> + }
>> + return 0;
>> +
>> +fail:
>> + nfsd_fault_inject_cleanup();
>> + return -ENOMEM;
>> +}
> ...
>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>> index db34a58..e67f30c 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
> ...
>> @@ -1130,6 +1131,9 @@ static int __init init_nfsd(void)
>> retval = nfs4_state_init(); /* nfs4 locking state */
>> if (retval)
>> return retval;
>> + retval = nfsd_fault_inject_init(); /* nfsd fault injection controls */
>> + if (retval)
>> + goto out_cleanup_fault_injection;
>> nfsd_stat_init(); /* Statistics */
>> retval = nfsd_reply_cache_init();
>> if (retval)
>> @@ -1161,6 +1165,8 @@ out_free_cache:
>> out_free_stat:
>> nfsd_stat_shutdown();
>> nfsd4_free_slabs();
>> +out_cleanup_fault_injection:
>> + nfsd_fault_inject_cleanup();
>
> Check the cleanup here again.... The slabs are allocated in state_init,
> so you need to do that on nfsd_fault_inject_init, but you *don't* need
> the fault_inject_cleanup in that case, since fault_inject_init cleanup
> cleans up after itself--that's needed only on later failures.
Would it be better to change the fault_inject_init() function so it
doesn't clean up after itself? This would keep it consistent with the
other nfsd init functions. I didn't realize that slabs were
initialized in the state_init() function, I'll fix that.
Thanks for the review!
- Bryan
>
> --b.
>
>> return retval;
>> }
>>
>> @@ -1174,6 +1180,7 @@ static void __exit exit_nfsd(void)
>> nfsd_lockd_shutdown();
>> nfsd_idmap_shutdown();
>> nfsd4_free_slabs();
>> + nfsd_fault_inject_cleanup();
>> unregister_filesystem(&nfsd_fs_type);
>> }
>>
>> --
>> 1.7.7
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html