linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] NFSD: Added fault injection
@ 2011-10-07 17:44 bjschuma
  2011-10-17 22:18 ` J. Bruce Fields
  0 siblings, 1 reply; 14+ messages in thread
From: bjschuma @ 2011-10-07 17:44 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, Bryan Schumaker

From: Bryan Schumaker <bjschuma@netapp.com>

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 <bjschuma@netapp.com>
---
 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 <bjschuma@netapp.com>
+ *
+ * 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 <bjschuma@netapp.com>
+ *
+ * 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


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

* Re: [PATCH 1/3] NFSD: Added fault injection
  2011-10-07 17:44 bjschuma
@ 2011-10-17 22:18 ` J. Bruce Fields
  2011-10-17 22:57   ` Bryan Schumaker
  0 siblings, 1 reply; 14+ messages in thread
From: J. Bruce Fields @ 2011-10-17 22:18 UTC (permalink / raw)
  To: bjschuma; +Cc: linux-nfs

On Fri, Oct 07, 2011 at 01:44:26PM -0400, bjschuma@netapp.com 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 <bjschuma@netapp.com>
> + *
> + * 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;

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

* Re: [PATCH 1/3] NFSD: Added fault injection
  2011-10-17 22:18 ` J. Bruce Fields
@ 2011-10-17 22:57   ` Bryan Schumaker
  2011-10-18 17:43     ` J. Bruce Fields
  0 siblings, 1 reply; 14+ messages in thread
From: Bryan Schumaker @ 2011-10-17 22:57 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: linux-nfs

On 10/17/2011 06:18 PM, J. Bruce Fields wrote:
> On Fri, Oct 07, 2011 at 01:44:26PM -0400, bjschuma@netapp.com 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 <bjschuma@netapp.com>
>> + *
>> + * 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;


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

* Re: [PATCH 1/3] NFSD: Added fault injection
  2011-10-17 22:57   ` Bryan Schumaker
@ 2011-10-18 17:43     ` J. Bruce Fields
  0 siblings, 0 replies; 14+ messages in thread
From: J. Bruce Fields @ 2011-10-18 17:43 UTC (permalink / raw)
  To: Bryan Schumaker; +Cc: linux-nfs

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, bjschuma@netapp.com 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 1/3] NFSD: Added fault injection
@ 2011-10-24 11:20 bjschuma
  2011-10-24 11:20 ` [PATCH 2/3] NFSD: Added fault injection script bjschuma
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: bjschuma @ 2011-10-24 11:20 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, Bryan Schumaker

From: Bryan Schumaker <bjschuma@netapp.com>

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 v5:
- Fill out inject_ops[] array without a macro
- Simplify debug messages and file creation

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 <bjschuma@netapp.com>
---
 fs/nfsd/Kconfig        |   10 ++++
 fs/nfsd/Makefile       |    1 +
 fs/nfsd/fault_inject.c |   90 +++++++++++++++++++++++++++++++++++++
 fs/nfsd/fault_inject.h |   28 ++++++++++++
 fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfsctl.c       |    7 +++
 6 files changed, 251 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..8df1ea4 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 injecting 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..1ac4134
--- /dev/null
+++ b/fs/nfsd/fault_inject.c
@@ -0,0 +1,90 @@
+/*
+ * Copyright (c) 2011 Bryan Schumaker <bjschuma@netapp.com>
+ *
+ * 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 *file;
+	void (*func)(u64);
+};
+
+static struct nfsd_fault_inject_op inject_ops[] = {
+	{
+		.file   = "forget_clients",
+		.func   = nfsd_forget_clients,
+	},
+	{
+		.file   = "forget_locks",
+		.func   = nfsd_forget_locks,
+	},
+	{
+		.file   = "forget_openowners",
+		.func   = nfsd_forget_openowners,
+	},
+	{
+		.file   = "forget_delegations",
+		.func   = nfsd_forget_delegations,
+	},
+	{
+		.file   = "recall_delegations",
+		.func   = 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 *op_ptr, u64 val)
+{
+	struct nfsd_fault_inject_op *op = op_ptr;
+
+	if (val == 0)
+		printk(KERN_INFO "NFSD Fault Injection: %s (all)", op->file);
+	else
+		printk(KERN_INFO "NFSD Fault Injection: %s (n = %llu)", op->file, val);
+
+	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, &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 <bjschuma@netapp.com>
+ *
+ * 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


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

* [PATCH 2/3] NFSD: Added fault injection script
  2011-10-24 11:20 [PATCH 1/3] NFSD: Added fault injection bjschuma
@ 2011-10-24 11:20 ` bjschuma
  2011-10-24 11:20 ` [PATCH 3/3] NFSD: Added fault injection documentation bjschuma
  2011-10-28 21:24 ` [PATCH 1/3] NFSD: Added fault injection J. Bruce Fields
  2 siblings, 0 replies; 14+ messages in thread
From: bjschuma @ 2011-10-24 11:20 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, Bryan Schumaker

From: Bryan Schumaker <bjschuma@netapp.com>

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 <bjschuma@netapp.com>
---
 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 <bjschuma@netapp.com>
+#
+# 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


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

* [PATCH 3/3] NFSD: Added fault injection documentation
  2011-10-24 11:20 [PATCH 1/3] NFSD: Added fault injection bjschuma
  2011-10-24 11:20 ` [PATCH 2/3] NFSD: Added fault injection script bjschuma
@ 2011-10-24 11:20 ` bjschuma
  2011-10-28 21:24 ` [PATCH 1/3] NFSD: Added fault injection J. Bruce Fields
  2 siblings, 0 replies; 14+ messages in thread
From: bjschuma @ 2011-10-24 11:20 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, Bryan Schumaker

From: Bryan Schumaker <bjschuma@netapp.com>

Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
---
 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


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

* Re: [PATCH 1/3] NFSD: Added fault injection
  2011-10-24 11:20 [PATCH 1/3] NFSD: Added fault injection bjschuma
  2011-10-24 11:20 ` [PATCH 2/3] NFSD: Added fault injection script bjschuma
  2011-10-24 11:20 ` [PATCH 3/3] NFSD: Added fault injection documentation bjschuma
@ 2011-10-28 21:24 ` J. Bruce Fields
  2011-11-01 14:08   ` Bryan Schumaker
  2 siblings, 1 reply; 14+ messages in thread
From: J. Bruce Fields @ 2011-10-28 21:24 UTC (permalink / raw)
  To: bjschuma; +Cc: linux-nfs

On Mon, Oct 24, 2011 at 07:20:57AM -0400, bjschuma@netapp.com 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
> 

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

* Re: [PATCH 1/3] NFSD: Added fault injection
  2011-10-28 21:24 ` [PATCH 1/3] NFSD: Added fault injection J. Bruce Fields
@ 2011-11-01 14:08   ` Bryan Schumaker
  2011-11-01 14:17     ` J. Bruce Fields
  0 siblings, 1 reply; 14+ messages in thread
From: Bryan Schumaker @ 2011-11-01 14:08 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: linux-nfs

On Fri 28 Oct 2011 05:24:50 PM EDT, J. Bruce Fields wrote:
> On Mon, Oct 24, 2011 at 07:20:57AM -0400, bjschuma@netapp.com 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

* Re: [PATCH 1/3] NFSD: Added fault injection
  2011-11-01 14:08   ` Bryan Schumaker
@ 2011-11-01 14:17     ` J. Bruce Fields
  2011-11-01 14:22       ` Bryan Schumaker
  0 siblings, 1 reply; 14+ messages in thread
From: J. Bruce Fields @ 2011-11-01 14:17 UTC (permalink / raw)
  To: Bryan Schumaker; +Cc: linux-nfs

On Tue, Nov 01, 2011 at 10:08:37AM -0400, Bryan Schumaker wrote:
> On Fri 28 Oct 2011 05:24:50 PM EDT, J. Bruce Fields wrote:
> > On Mon, Oct 24, 2011 at 07:20:57AM -0400, bjschuma@netapp.com wrote:
> > ...
> >> 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 don't think so.  The pattern here is pretty common throughout the
kernel; it's approximately:

	do_thing1();
	err = do_thing2();
	if (err)
		goto undo_thing1;
	err = do_thing3();
	if (err)
		goto undo_thing2;
	...

Functions are no-ops on failure, and the caller's just responsible for
cleaning up previous stuff.

> I didn't realize that slabs were initialized in the state_init()
> function, I'll fix that.

I'll admit that's confusing.  Any suggestion to make it clearer welcome.

--b.

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

* Re: [PATCH 1/3] NFSD: Added fault injection
  2011-11-01 14:17     ` J. Bruce Fields
@ 2011-11-01 14:22       ` Bryan Schumaker
  2011-11-01 14:35         ` J. Bruce Fields
  0 siblings, 1 reply; 14+ messages in thread
From: Bryan Schumaker @ 2011-11-01 14:22 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: linux-nfs

On Tue 01 Nov 2011 10:17:46 AM EDT, J. Bruce Fields wrote:
> On Tue, Nov 01, 2011 at 10:08:37AM -0400, Bryan Schumaker wrote:
>> On Fri 28 Oct 2011 05:24:50 PM EDT, J. Bruce Fields wrote:
>>> On Mon, Oct 24, 2011 at 07:20:57AM -0400, bjschuma@netapp.com wrote:
>>> ...
>>>> 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 don't think so.  The pattern here is pretty common throughout the
> kernel; it's approximately:
>
> 	do_thing1();
> 	err = do_thing2();
> 	if (err)
> 		goto undo_thing1;
> 	err = do_thing3();
> 	if (err)
> 		goto undo_thing2;
> 	...
>
> Functions are no-ops on failure, and the caller's just responsible for
> cleaning up previous stuff.

Ok, I won't change that.

>
>> I didn't realize that slabs were initialized in the state_init()
>> function, I'll fix that.
>
> I'll admit that's confusing.  Any suggestion to make it clearer welcome.

Sure.  The comment after the call to nfs4_state_init() is "nfs4 locking 
state".  Maybe this could be changed to "nfs4 locking state and slabs"? 
 I can make this change and send it as a separate patch if you would 
like.
>
> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

* Re: [PATCH 1/3] NFSD: Added fault injection
  2011-11-01 14:22       ` Bryan Schumaker
@ 2011-11-01 14:35         ` J. Bruce Fields
  0 siblings, 0 replies; 14+ messages in thread
From: J. Bruce Fields @ 2011-11-01 14:35 UTC (permalink / raw)
  To: Bryan Schumaker; +Cc: linux-nfs

On Tue, Nov 01, 2011 at 10:22:30AM -0400, Bryan Schumaker wrote:
> On Tue 01 Nov 2011 10:17:46 AM EDT, J. Bruce Fields wrote:
> >> I didn't realize that slabs were initialized in the state_init()
> >> function, I'll fix that.
> >
> > I'll admit that's confusing.  Any suggestion to make it clearer welcome.
> 
> Sure.  The comment after the call to nfs4_state_init() is "nfs4 locking 
> state".  Maybe this could be changed to "nfs4 locking state and slabs"? 
>  I can make this change and send it as a separate patch if you would 
> like.

If there was a way to fix the code itself to make it clearer, that's
usually better--a comment is a workaround.

--b.

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

* [PATCH 1/3] NFSD: Added fault injection
@ 2011-11-01 17:35 bjschuma
  2011-11-04 21:30 ` J. Bruce Fields
  0 siblings, 1 reply; 14+ messages in thread
From: bjschuma @ 2011-11-01 17:35 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs, Bryan Schumaker

From: Bryan Schumaker <bjschuma@netapp.com>

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 v6:
- Check the return code when creating debugfs files
- Clean up fault injection before freeing slabs

Changes in v5:
- Fill out inject_ops[] array without a macro
- Simplify debug messages and file creation

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 <bjschuma@netapp.com>
---
 fs/nfsd/Kconfig        |   10 ++++
 fs/nfsd/Makefile       |    1 +
 fs/nfsd/fault_inject.c |   91 ++++++++++++++++++++++++++++++++++++++
 fs/nfsd/fault_inject.h |   28 ++++++++++++
 fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfsctl.c       |    7 +++
 6 files changed, 252 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..8df1ea4 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 injecting 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..ce7f075
--- /dev/null
+++ b/fs/nfsd/fault_inject.c
@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2011 Bryan Schumaker <bjschuma@netapp.com>
+ *
+ * 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 *file;
+	void (*func)(u64);
+};
+
+static struct nfsd_fault_inject_op inject_ops[] = {
+	{
+		.file   = "forget_clients",
+		.func   = nfsd_forget_clients,
+	},
+	{
+		.file   = "forget_locks",
+		.func   = nfsd_forget_locks,
+	},
+	{
+		.file   = "forget_openowners",
+		.func   = nfsd_forget_openowners,
+	},
+	{
+		.file   = "forget_delegations",
+		.func   = nfsd_forget_delegations,
+	},
+	{
+		.file   = "recall_delegations",
+		.func   = 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 *op_ptr, u64 val)
+{
+	struct nfsd_fault_inject_op *op = op_ptr;
+
+	if (val == 0)
+		printk(KERN_INFO "NFSD Fault Injection: %s (all)", op->file);
+	else
+		printk(KERN_INFO "NFSD Fault Injection: %s (n = %llu)", op->file, val);
+
+	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];
+		if (!debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd))
+			goto fail;
+	}
+	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 <bjschuma@netapp.com>
+ *
+ * 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 47e94e3..3c3762e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4394,6 +4394,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..34c841a 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_free_slabs;
 	nfsd_stat_init();	/* Statistics */
 	retval = nfsd_reply_cache_init();
 	if (retval)
@@ -1160,6 +1164,8 @@ out_free_cache:
 	nfsd_reply_cache_shutdown();
 out_free_stat:
 	nfsd_stat_shutdown();
+	nfsd_fault_inject_cleanup();
+out_free_slabs:
 	nfsd4_free_slabs();
 	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


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

* Re: [PATCH 1/3] NFSD: Added fault injection
  2011-11-01 17:35 bjschuma
@ 2011-11-04 21:30 ` J. Bruce Fields
  0 siblings, 0 replies; 14+ messages in thread
From: J. Bruce Fields @ 2011-11-04 21:30 UTC (permalink / raw)
  To: bjschuma; +Cc: linux-nfs

On Tue, Nov 01, 2011 at 01:35:21PM -0400, bjschuma@netapp.com wrote:
> From: Bryan Schumaker <bjschuma@netapp.com>
> 
> 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.

These look good to me, thanks; applying for 3.3.

Oh, just one request for next time:

> Changes in v6:
> - Check the return code when creating debugfs files
> - Clean up fault injection before freeing slabs

In general we don't want this sort of thing in the final changelog.
Better to write the changelog as if this is how you meant to do it all
along, and add this sort of meta-changelog to a coverletter if it's
needed.  I'll cut it out.

--b.

> 
> Changes in v5:
> - Fill out inject_ops[] array without a macro
> - Simplify debug messages and file creation
> 
> 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 <bjschuma@netapp.com>
> ---
>  fs/nfsd/Kconfig        |   10 ++++
>  fs/nfsd/Makefile       |    1 +
>  fs/nfsd/fault_inject.c |   91 ++++++++++++++++++++++++++++++++++++++
>  fs/nfsd/fault_inject.h |   28 ++++++++++++
>  fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/nfsd/nfsctl.c       |    7 +++
>  6 files changed, 252 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..8df1ea4 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 injecting 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..ce7f075
> --- /dev/null
> +++ b/fs/nfsd/fault_inject.c
> @@ -0,0 +1,91 @@
> +/*
> + * Copyright (c) 2011 Bryan Schumaker <bjschuma@netapp.com>
> + *
> + * 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 *file;
> +	void (*func)(u64);
> +};
> +
> +static struct nfsd_fault_inject_op inject_ops[] = {
> +	{
> +		.file   = "forget_clients",
> +		.func   = nfsd_forget_clients,
> +	},
> +	{
> +		.file   = "forget_locks",
> +		.func   = nfsd_forget_locks,
> +	},
> +	{
> +		.file   = "forget_openowners",
> +		.func   = nfsd_forget_openowners,
> +	},
> +	{
> +		.file   = "forget_delegations",
> +		.func   = nfsd_forget_delegations,
> +	},
> +	{
> +		.file   = "recall_delegations",
> +		.func   = 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 *op_ptr, u64 val)
> +{
> +	struct nfsd_fault_inject_op *op = op_ptr;
> +
> +	if (val == 0)
> +		printk(KERN_INFO "NFSD Fault Injection: %s (all)", op->file);
> +	else
> +		printk(KERN_INFO "NFSD Fault Injection: %s (n = %llu)", op->file, val);
> +
> +	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];
> +		if (!debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd))
> +			goto fail;
> +	}
> +	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 <bjschuma@netapp.com>
> + *
> + * 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 47e94e3..3c3762e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4394,6 +4394,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..34c841a 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_free_slabs;
>  	nfsd_stat_init();	/* Statistics */
>  	retval = nfsd_reply_cache_init();
>  	if (retval)
> @@ -1160,6 +1164,8 @@ out_free_cache:
>  	nfsd_reply_cache_shutdown();
>  out_free_stat:
>  	nfsd_stat_shutdown();
> +	nfsd_fault_inject_cleanup();
> +out_free_slabs:
>  	nfsd4_free_slabs();
>  	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
> 

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

end of thread, other threads:[~2011-11-04 21:30 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-24 11:20 [PATCH 1/3] NFSD: Added fault injection bjschuma
2011-10-24 11:20 ` [PATCH 2/3] NFSD: Added fault injection script bjschuma
2011-10-24 11:20 ` [PATCH 3/3] NFSD: Added fault injection documentation bjschuma
2011-10-28 21:24 ` [PATCH 1/3] NFSD: Added fault injection J. Bruce Fields
2011-11-01 14:08   ` Bryan Schumaker
2011-11-01 14:17     ` J. Bruce Fields
2011-11-01 14:22       ` Bryan Schumaker
2011-11-01 14:35         ` J. Bruce Fields
  -- strict thread matches above, loose matches on Subject: below --
2011-11-01 17:35 bjschuma
2011-11-04 21:30 ` J. Bruce Fields
2011-10-07 17:44 bjschuma
2011-10-17 22:18 ` J. Bruce Fields
2011-10-17 22:57   ` Bryan Schumaker
2011-10-18 17:43     ` J. Bruce Fields

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