All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/3] NFSD: Added fault injection
@ 2011-09-29 18:59 bjschuma
  2011-09-29 18:59 ` [PATCH v3 2/3] NFSD: Added fault injection script bjschuma
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: bjschuma @ 2011-09-29 18:59 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 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       |    3 +-
 fs/nfsd/fault_inject.c |  102 ++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfsctl.c       |    6 +++
 fs/nfsd/nfsd.h         |   12 +++++
 6 files changed, 247 insertions(+), 1 deletions(-)
 create mode 100644 fs/nfsd/fault_inject.c

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..69eae75 100644
--- a/fs/nfsd/Makefile
+++ b/fs/nfsd/Makefile
@@ -5,7 +5,8 @@
 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
+			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
+			   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..2139883
--- /dev/null
+++ b/fs/nfsd/fault_inject.c
@@ -0,0 +1,102 @@
+#ifdef CONFIG_NFSD_FAULT_INJECTION
+
+#include <linux/types.h>
+#include <linux/fs.h>
+#include <linux/debugfs.h>
+#include <linux/module.h>
+
+#include "state.h"
+#include "nfsd.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;
+}
+
+#else /* CONFIG_NFSD_FAULT_INJECTION */
+
+inline void nfsd_fault_inject_cleanup(void)
+{}
+
+inline int nfsd_fault_inject_init(void)
+{
+	return 0;
+}
+
+#endif /* CONFIG_NFSD_FAULT_INJECTION */
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..e2f1b5a 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1130,6 +1130,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 +1164,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 +1179,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);
 }
 
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 58134a2..3ea303d 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -105,11 +105,18 @@ static inline int nfsd_v4client(struct svc_rqst *rq)
 #ifdef CONFIG_NFSD_V4
 extern unsigned int max_delegations;
 int nfs4_state_init(void);
+int nfsd_fault_inject_init(void);
+void nfsd_fault_inject_cleanup(void);
 void nfsd4_free_slabs(void);
 int nfs4_state_start(void);
 void nfs4_state_shutdown(void);
 void nfs4_reset_lease(time_t leasetime);
 int nfs4_reset_recoverydir(char *recdir);
+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
 static inline int nfs4_state_init(void) { return 0; }
 static inline void nfsd4_free_slabs(void) { }
@@ -117,6 +124,11 @@ static inline int nfs4_state_start(void) { return 0; }
 static inline void nfs4_state_shutdown(void) { }
 static inline void nfs4_reset_lease(time_t leasetime) { }
 static inline int nfs4_reset_recoverydir(char *recdir) { return 0; }
+static inline void nfsd_forget_clients(u64) {}
+static inline void nfsd_forget_locks(u64) {}
+static inline void nfsd_forget_openowners(u64) {}
+static inline void nfsd_forget_delegations(u64) {}
+static inline void nfsd_recall_delegations(u64) {}
 #endif
 
 /*
-- 
1.7.6.4


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

* [PATCH v3 2/3] NFSD: Added fault injection script
  2011-09-29 18:59 [PATCH v3 1/3] NFSD: Added fault injection bjschuma
@ 2011-09-29 18:59 ` bjschuma
  2011-09-29 18:59 ` [PATCH v3 3/3] NFSD: Added fault injection documentation bjschuma
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: bjschuma @ 2011-09-29 18:59 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/nfs/inject_fault |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)
 create mode 100644 tools/nfs/inject_fault

diff --git a/tools/nfs/inject_fault b/tools/nfs/inject_fault
new file mode 100644
index 0000000..4869bdd
--- /dev/null
+++ b/tools/nfs/inject_fault
@@ -0,0 +1,45 @@
+#!/bin/bash
+
+# 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.6.4


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

* [PATCH v3 3/3] NFSD: Added fault injection documentation
  2011-09-29 18:59 [PATCH v3 1/3] NFSD: Added fault injection bjschuma
  2011-09-29 18:59 ` [PATCH v3 2/3] NFSD: Added fault injection script bjschuma
@ 2011-09-29 18:59 ` bjschuma
  2011-09-29 19:06 ` [PATCH v3 1/3] NFSD: Added fault injection Chuck Lever
  2011-09-30 14:35 ` J. Bruce Fields
  3 siblings, 0 replies; 16+ messages in thread
From: bjschuma @ 2011-09-29 18:59 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 |   70 +++++++++++++++++++++
 2 files changed, 72 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..b7a4bca
--- /dev/null
+++ b/Documentation/filesystems/nfs/fault_injection.txt
@@ -0,0 +1,70 @@
+
+===============
+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 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 forget_locks 1` as root will instruct the server to forget one
+lock.  Running `inject_faults forget_locks` will instruct the server to forget
+all locks.
-- 
1.7.6.4


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

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-29 18:59 [PATCH v3 1/3] NFSD: Added fault injection bjschuma
  2011-09-29 18:59 ` [PATCH v3 2/3] NFSD: Added fault injection script bjschuma
  2011-09-29 18:59 ` [PATCH v3 3/3] NFSD: Added fault injection documentation bjschuma
@ 2011-09-29 19:06 ` Chuck Lever
  2011-09-29 19:15   ` Bryan Schumaker
  2011-09-29 20:14   ` Bryan Schumaker
  2011-09-30 14:35 ` J. Bruce Fields
  3 siblings, 2 replies; 16+ messages in thread
From: Chuck Lever @ 2011-09-29 19:06 UTC (permalink / raw)
  To: bjschuma; +Cc: bfields, linux-nfs


On Sep 29, 2011, at 2:59 PM, 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.
> 
> 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       |    3 +-
> fs/nfsd/fault_inject.c |  102 ++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/nfsctl.c       |    6 +++
> fs/nfsd/nfsd.h         |   12 +++++
> 6 files changed, 247 insertions(+), 1 deletions(-)
> create mode 100644 fs/nfsd/fault_inject.c
> 
> 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..69eae75 100644
> --- a/fs/nfsd/Makefile
> +++ b/fs/nfsd/Makefile
> @@ -5,7 +5,8 @@
> 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
> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
> +			   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..2139883
> --- /dev/null
> +++ b/fs/nfsd/fault_inject.c
> @@ -0,0 +1,102 @@
> +#ifdef CONFIG_NFSD_FAULT_INJECTION

The usual practice is to conditionally compile this source file via a Makefile variable, not by wrapping the whole source file in an ifdef.  You can see examples of this in the Makefile hunk above.

General comment, though: would it make sense to add fault injection points to lockd as well?

> +
> +#include <linux/types.h>
> +#include <linux/fs.h>
> +#include <linux/debugfs.h>
> +#include <linux/module.h>
> +
> +#include "state.h"
> +#include "nfsd.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;
> +}
> +
> +#else /* CONFIG_NFSD_FAULT_INJECTION */
> +
> +inline void nfsd_fault_inject_cleanup(void)
> +{}
> +
> +inline int nfsd_fault_inject_init(void)
> +{
> +	return 0;
> +}
> +
> +#endif /* CONFIG_NFSD_FAULT_INJECTION */
> 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..e2f1b5a 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1130,6 +1130,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 +1164,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 +1179,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);
> }
> 
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 58134a2..3ea303d 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -105,11 +105,18 @@ static inline int nfsd_v4client(struct svc_rqst *rq)
> #ifdef CONFIG_NFSD_V4
> extern unsigned int max_delegations;
> int nfs4_state_init(void);
> +int nfsd_fault_inject_init(void);
> +void nfsd_fault_inject_cleanup(void);
> void nfsd4_free_slabs(void);
> int nfs4_state_start(void);
> void nfs4_state_shutdown(void);
> void nfs4_reset_lease(time_t leasetime);
> int nfs4_reset_recoverydir(char *recdir);
> +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
> static inline int nfs4_state_init(void) { return 0; }
> static inline void nfsd4_free_slabs(void) { }
> @@ -117,6 +124,11 @@ static inline int nfs4_state_start(void) { return 0; }
> static inline void nfs4_state_shutdown(void) { }
> static inline void nfs4_reset_lease(time_t leasetime) { }
> static inline int nfs4_reset_recoverydir(char *recdir) { return 0; }
> +static inline void nfsd_forget_clients(u64) {}
> +static inline void nfsd_forget_locks(u64) {}
> +static inline void nfsd_forget_openowners(u64) {}
> +static inline void nfsd_forget_delegations(u64) {}
> +static inline void nfsd_recall_delegations(u64) {}
> #endif
> 
> /*
> -- 
> 1.7.6.4
> 
> --
> 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

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





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

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-29 19:06 ` [PATCH v3 1/3] NFSD: Added fault injection Chuck Lever
@ 2011-09-29 19:15   ` Bryan Schumaker
  2011-09-29 19:22     ` Chuck Lever
  2011-09-29 20:14   ` Bryan Schumaker
  1 sibling, 1 reply; 16+ messages in thread
From: Bryan Schumaker @ 2011-09-29 19:15 UTC (permalink / raw)
  To: Chuck Lever; +Cc: bfields, linux-nfs

On 09/29/2011 03:06 PM, Chuck Lever wrote:
> 
> On Sep 29, 2011, at 2:59 PM, 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.
>>
>> 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       |    3 +-
>> fs/nfsd/fault_inject.c |  102 ++++++++++++++++++++++++++++++++++++++++++
>> fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
>> fs/nfsd/nfsctl.c       |    6 +++
>> fs/nfsd/nfsd.h         |   12 +++++
>> 6 files changed, 247 insertions(+), 1 deletions(-)
>> create mode 100644 fs/nfsd/fault_inject.c
>>
>> 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..69eae75 100644
>> --- a/fs/nfsd/Makefile
>> +++ b/fs/nfsd/Makefile
>> @@ -5,7 +5,8 @@
>> 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
>> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
>> +			   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..2139883
>> --- /dev/null
>> +++ b/fs/nfsd/fault_inject.c
>> @@ -0,0 +1,102 @@
>> +#ifdef CONFIG_NFSD_FAULT_INJECTION
> 
> The usual practice is to conditionally compile this source file via a Makefile variable, not by wrapping the whole source file in an ifdef.  You can see examples of this in the Makefile hunk above.
Yeah, I see that now (although I don't know how I missed it earlier).  Thanks!

> 
> General comment, though: would it make sense to add fault injection points to lockd as well?
That would probably be the easiest way to force lock recovery over v2 and v3, which sounds useful to me.  I think I already have it on a todo list somewhere...

- Bryan

> 
>> +
>> +#include <linux/types.h>
>> +#include <linux/fs.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/module.h>
>> +
>> +#include "state.h"
>> +#include "nfsd.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;
>> +}
>> +
>> +#else /* CONFIG_NFSD_FAULT_INJECTION */
>> +
>> +inline void nfsd_fault_inject_cleanup(void)
>> +{}
>> +
>> +inline int nfsd_fault_inject_init(void)
>> +{
>> +	return 0;
>> +}
>> +
>> +#endif /* CONFIG_NFSD_FAULT_INJECTION */
>> 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..e2f1b5a 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
>> @@ -1130,6 +1130,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 +1164,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 +1179,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);
>> }
>>
>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>> index 58134a2..3ea303d 100644
>> --- a/fs/nfsd/nfsd.h
>> +++ b/fs/nfsd/nfsd.h
>> @@ -105,11 +105,18 @@ static inline int nfsd_v4client(struct svc_rqst *rq)
>> #ifdef CONFIG_NFSD_V4
>> extern unsigned int max_delegations;
>> int nfs4_state_init(void);
>> +int nfsd_fault_inject_init(void);
>> +void nfsd_fault_inject_cleanup(void);
>> void nfsd4_free_slabs(void);
>> int nfs4_state_start(void);
>> void nfs4_state_shutdown(void);
>> void nfs4_reset_lease(time_t leasetime);
>> int nfs4_reset_recoverydir(char *recdir);
>> +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
>> static inline int nfs4_state_init(void) { return 0; }
>> static inline void nfsd4_free_slabs(void) { }
>> @@ -117,6 +124,11 @@ static inline int nfs4_state_start(void) { return 0; }
>> static inline void nfs4_state_shutdown(void) { }
>> static inline void nfs4_reset_lease(time_t leasetime) { }
>> static inline int nfs4_reset_recoverydir(char *recdir) { return 0; }
>> +static inline void nfsd_forget_clients(u64) {}
>> +static inline void nfsd_forget_locks(u64) {}
>> +static inline void nfsd_forget_openowners(u64) {}
>> +static inline void nfsd_forget_delegations(u64) {}
>> +static inline void nfsd_recall_delegations(u64) {}
>> #endif
>>
>> /*
>> -- 
>> 1.7.6.4
>>
>> --
>> 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] 16+ messages in thread

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-29 19:15   ` Bryan Schumaker
@ 2011-09-29 19:22     ` Chuck Lever
  2011-09-29 19:26       ` Bryan Schumaker
  0 siblings, 1 reply; 16+ messages in thread
From: Chuck Lever @ 2011-09-29 19:22 UTC (permalink / raw)
  To: Bryan Schumaker; +Cc: bfields, linux-nfs


On Sep 29, 2011, at 3:15 PM, Bryan Schumaker wrote:

> On 09/29/2011 03:06 PM, Chuck Lever wrote:
>> 
>> On Sep 29, 2011, at 2:59 PM, 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.
>>> 
>>> 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       |    3 +-
>>> fs/nfsd/fault_inject.c |  102 ++++++++++++++++++++++++++++++++++++++++++
>>> fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> fs/nfsd/nfsctl.c       |    6 +++
>>> fs/nfsd/nfsd.h         |   12 +++++
>>> 6 files changed, 247 insertions(+), 1 deletions(-)
>>> create mode 100644 fs/nfsd/fault_inject.c
>>> 
>>> 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..69eae75 100644
>>> --- a/fs/nfsd/Makefile
>>> +++ b/fs/nfsd/Makefile
>>> @@ -5,7 +5,8 @@
>>> 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
>>> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
>>> +			   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..2139883
>>> --- /dev/null
>>> +++ b/fs/nfsd/fault_inject.c
>>> @@ -0,0 +1,102 @@
>>> +#ifdef CONFIG_NFSD_FAULT_INJECTION
>> 
>> The usual practice is to conditionally compile this source file via a Makefile variable, not by wrapping the whole source file in an ifdef.  You can see examples of this in the Makefile hunk above.
> Yeah, I see that now (although I don't know how I missed it earlier).  Thanks!
> 
>> 
>> General comment, though: would it make sense to add fault injection points to lockd as well?
> That would probably be the easiest way to force lock recovery over v2 and v3, which sounds useful to me.

Useful, yes; but unfortunately that's not as simple as it sounds.  If you just want to hook into the reboot recovery code in fs/lockd/host.c, you need an additional parameter (which is managed by statd) to narrow the lock clearing to a single client.  Now, if you're thinking of clearing all locks and triggering a grace period, that might be easier to do.

>  I think I already have it on a todo list somewhere...
> 
> - Bryan
> 
>> 
>>> +
>>> +#include <linux/types.h>
>>> +#include <linux/fs.h>
>>> +#include <linux/debugfs.h>
>>> +#include <linux/module.h>
>>> +
>>> +#include "state.h"
>>> +#include "nfsd.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;
>>> +}
>>> +
>>> +#else /* CONFIG_NFSD_FAULT_INJECTION */
>>> +
>>> +inline void nfsd_fault_inject_cleanup(void)
>>> +{}
>>> +
>>> +inline int nfsd_fault_inject_init(void)
>>> +{
>>> +	return 0;
>>> +}
>>> +
>>> +#endif /* CONFIG_NFSD_FAULT_INJECTION */
>>> 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..e2f1b5a 100644
>>> --- a/fs/nfsd/nfsctl.c
>>> +++ b/fs/nfsd/nfsctl.c
>>> @@ -1130,6 +1130,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 +1164,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 +1179,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);
>>> }
>>> 
>>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>>> index 58134a2..3ea303d 100644
>>> --- a/fs/nfsd/nfsd.h
>>> +++ b/fs/nfsd/nfsd.h
>>> @@ -105,11 +105,18 @@ static inline int nfsd_v4client(struct svc_rqst *rq)
>>> #ifdef CONFIG_NFSD_V4
>>> extern unsigned int max_delegations;
>>> int nfs4_state_init(void);
>>> +int nfsd_fault_inject_init(void);
>>> +void nfsd_fault_inject_cleanup(void);
>>> void nfsd4_free_slabs(void);
>>> int nfs4_state_start(void);
>>> void nfs4_state_shutdown(void);
>>> void nfs4_reset_lease(time_t leasetime);
>>> int nfs4_reset_recoverydir(char *recdir);
>>> +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
>>> static inline int nfs4_state_init(void) { return 0; }
>>> static inline void nfsd4_free_slabs(void) { }
>>> @@ -117,6 +124,11 @@ static inline int nfs4_state_start(void) { return 0; }
>>> static inline void nfs4_state_shutdown(void) { }
>>> static inline void nfs4_reset_lease(time_t leasetime) { }
>>> static inline int nfs4_reset_recoverydir(char *recdir) { return 0; }
>>> +static inline void nfsd_forget_clients(u64) {}
>>> +static inline void nfsd_forget_locks(u64) {}
>>> +static inline void nfsd_forget_openowners(u64) {}
>>> +static inline void nfsd_forget_delegations(u64) {}
>>> +static inline void nfsd_recall_delegations(u64) {}
>>> #endif
>>> 
>>> /*
>>> -- 
>>> 1.7.6.4
>>> 
>>> --
>>> 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
>> 
> 

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





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

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-29 19:22     ` Chuck Lever
@ 2011-09-29 19:26       ` Bryan Schumaker
  0 siblings, 0 replies; 16+ messages in thread
From: Bryan Schumaker @ 2011-09-29 19:26 UTC (permalink / raw)
  To: Chuck Lever; +Cc: bfields, linux-nfs

On 09/29/2011 03:22 PM, Chuck Lever wrote:
> 
> On Sep 29, 2011, at 3:15 PM, Bryan Schumaker wrote:
> 
>> On 09/29/2011 03:06 PM, Chuck Lever wrote:
>>>
>>> On Sep 29, 2011, at 2:59 PM, 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.
>>>>
>>>> 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       |    3 +-
>>>> fs/nfsd/fault_inject.c |  102 ++++++++++++++++++++++++++++++++++++++++++
>>>> fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>> fs/nfsd/nfsctl.c       |    6 +++
>>>> fs/nfsd/nfsd.h         |   12 +++++
>>>> 6 files changed, 247 insertions(+), 1 deletions(-)
>>>> create mode 100644 fs/nfsd/fault_inject.c
>>>>
>>>> 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..69eae75 100644
>>>> --- a/fs/nfsd/Makefile
>>>> +++ b/fs/nfsd/Makefile
>>>> @@ -5,7 +5,8 @@
>>>> 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
>>>> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
>>>> +			   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..2139883
>>>> --- /dev/null
>>>> +++ b/fs/nfsd/fault_inject.c
>>>> @@ -0,0 +1,102 @@
>>>> +#ifdef CONFIG_NFSD_FAULT_INJECTION
>>>
>>> The usual practice is to conditionally compile this source file via a Makefile variable, not by wrapping the whole source file in an ifdef.  You can see examples of this in the Makefile hunk above.
>> Yeah, I see that now (although I don't know how I missed it earlier).  Thanks!
>>
>>>
>>> General comment, though: would it make sense to add fault injection points to lockd as well?
>> That would probably be the easiest way to force lock recovery over v2 and v3, which sounds useful to me.
> 
> Useful, yes; but unfortunately that's not as simple as it sounds.  If you just want to hook into the reboot recovery code in fs/lockd/host.c, you need an additional parameter (which is managed by statd) to narrow the lock clearing to a single client.  Now, if you're thinking of clearing all locks and triggering a grace period, that might be easier to do.
I'm not sure how the users could specify a client to use to clear locks, so that would make it harder to narrow to one client.  I'll probably mimic what I do with v4 and clear the first n locks that I find, and let users specify the value of n.

> 
>>  I think I already have it on a todo list somewhere...
>>
>> - Bryan
>>
>>>
>>>> +
>>>> +#include <linux/types.h>
>>>> +#include <linux/fs.h>
>>>> +#include <linux/debugfs.h>
>>>> +#include <linux/module.h>
>>>> +
>>>> +#include "state.h"
>>>> +#include "nfsd.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;
>>>> +}
>>>> +
>>>> +#else /* CONFIG_NFSD_FAULT_INJECTION */
>>>> +
>>>> +inline void nfsd_fault_inject_cleanup(void)
>>>> +{}
>>>> +
>>>> +inline int nfsd_fault_inject_init(void)
>>>> +{
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +#endif /* CONFIG_NFSD_FAULT_INJECTION */
>>>> 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..e2f1b5a 100644
>>>> --- a/fs/nfsd/nfsctl.c
>>>> +++ b/fs/nfsd/nfsctl.c
>>>> @@ -1130,6 +1130,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 +1164,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 +1179,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);
>>>> }
>>>>
>>>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>>>> index 58134a2..3ea303d 100644
>>>> --- a/fs/nfsd/nfsd.h
>>>> +++ b/fs/nfsd/nfsd.h
>>>> @@ -105,11 +105,18 @@ static inline int nfsd_v4client(struct svc_rqst *rq)
>>>> #ifdef CONFIG_NFSD_V4
>>>> extern unsigned int max_delegations;
>>>> int nfs4_state_init(void);
>>>> +int nfsd_fault_inject_init(void);
>>>> +void nfsd_fault_inject_cleanup(void);
>>>> void nfsd4_free_slabs(void);
>>>> int nfs4_state_start(void);
>>>> void nfs4_state_shutdown(void);
>>>> void nfs4_reset_lease(time_t leasetime);
>>>> int nfs4_reset_recoverydir(char *recdir);
>>>> +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
>>>> static inline int nfs4_state_init(void) { return 0; }
>>>> static inline void nfsd4_free_slabs(void) { }
>>>> @@ -117,6 +124,11 @@ static inline int nfs4_state_start(void) { return 0; }
>>>> static inline void nfs4_state_shutdown(void) { }
>>>> static inline void nfs4_reset_lease(time_t leasetime) { }
>>>> static inline int nfs4_reset_recoverydir(char *recdir) { return 0; }
>>>> +static inline void nfsd_forget_clients(u64) {}
>>>> +static inline void nfsd_forget_locks(u64) {}
>>>> +static inline void nfsd_forget_openowners(u64) {}
>>>> +static inline void nfsd_forget_delegations(u64) {}
>>>> +static inline void nfsd_recall_delegations(u64) {}
>>>> #endif
>>>>
>>>> /*
>>>> -- 
>>>> 1.7.6.4
>>>>
>>>> --
>>>> 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] 16+ messages in thread

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-29 19:06 ` [PATCH v3 1/3] NFSD: Added fault injection Chuck Lever
  2011-09-29 19:15   ` Bryan Schumaker
@ 2011-09-29 20:14   ` Bryan Schumaker
  2011-09-30 14:28     ` Chuck Lever
  1 sibling, 1 reply; 16+ messages in thread
From: Bryan Schumaker @ 2011-09-29 20:14 UTC (permalink / raw)
  To: Chuck Lever; +Cc: bfields, linux-nfs

On 09/29/2011 03:06 PM, Chuck Lever wrote:
> 
> On Sep 29, 2011, at 2:59 PM, bjschuma@netapp.com wrote:
> 
>> From: Bryan Schumaker <bjschuma@netapp.com>
>>
...
>> diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
>> index 9b118ee..69eae75 100644
>> --- a/fs/nfsd/Makefile
>> +++ b/fs/nfsd/Makefile
>> @@ -5,7 +5,8 @@
>> 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
>> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
>> +			   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..2139883
>> --- /dev/null
>> +++ b/fs/nfsd/fault_inject.c
>> @@ -0,0 +1,102 @@
>> +#ifdef CONFIG_NFSD_FAULT_INJECTION
> 
> The usual practice is to conditionally compile this source file via a Makefile variable, not by wrapping the whole source file in an ifdef.  You can see examples of this in the Makefile hunk above.
> 

How's this?  I moved the selectively compile bit to the makefile and created a fault_inject.h for the empty init() and cleanup() functions.  I moved the rest of the fault injection function declarations there, too, to keep them all together.

- Bryan

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

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 |   88 ++++++++++++++++++++++++++++++++++++
 fs/nfsd/fault_inject.h |   22 +++++++++
 fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfsctl.c       |    7 +++
 6 files changed, 243 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..8b6ec8d
--- /dev/null
+++ b/fs/nfsd/fault_inject.c
@@ -0,0 +1,88 @@
+#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..5b003a9
--- /dev/null
+++ b/fs/nfsd/fault_inject.h
@@ -0,0 +1,22 @@
+#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.6.4

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

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-29 20:14   ` Bryan Schumaker
@ 2011-09-30 14:28     ` Chuck Lever
  2011-09-30 14:30       ` J. Bruce Fields
  2011-09-30 14:38       ` Bryan Schumaker
  0 siblings, 2 replies; 16+ messages in thread
From: Chuck Lever @ 2011-09-30 14:28 UTC (permalink / raw)
  To: Bryan Schumaker; +Cc: bfields, linux-nfs


On Sep 29, 2011, at 4:14 PM, Bryan Schumaker wrote:

> On 09/29/2011 03:06 PM, Chuck Lever wrote:
>> 
>> On Sep 29, 2011, at 2:59 PM, bjschuma@netapp.com wrote:
>> 
>>> From: Bryan Schumaker <bjschuma@netapp.com>
>>> 
> ...
>>> diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
>>> index 9b118ee..69eae75 100644
>>> --- a/fs/nfsd/Makefile
>>> +++ b/fs/nfsd/Makefile
>>> @@ -5,7 +5,8 @@
>>> 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
>>> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
>>> +			   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..2139883
>>> --- /dev/null
>>> +++ b/fs/nfsd/fault_inject.c
>>> @@ -0,0 +1,102 @@
>>> +#ifdef CONFIG_NFSD_FAULT_INJECTION
>> 
>> The usual practice is to conditionally compile this source file via a Makefile variable, not by wrapping the whole source file in an ifdef.  You can see examples of this in the Makefile hunk above.
>> 
> 
> How's this?  I moved the selectively compile bit to the makefile and created a fault_inject.h for the empty init() and cleanup() functions.  I moved the rest of the fault injection function declarations there, too, to keep them all together.

OK.  How about adding documenting block comments for the new files?  Does NetApp require you to add a copyright notice at the top of new files?

Also, I wonder if it would be better if you added "default N" for the new CONFIG option.

> - Bryan
> 
> 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
> 
> 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 |   88 ++++++++++++++++++++++++++++++++++++
> fs/nfsd/fault_inject.h |   22 +++++++++
> fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/nfsctl.c       |    7 +++
> 6 files changed, 243 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..8b6ec8d
> --- /dev/null
> +++ b/fs/nfsd/fault_inject.c
> @@ -0,0 +1,88 @@
> +#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..5b003a9
> --- /dev/null
> +++ b/fs/nfsd/fault_inject.h
> @@ -0,0 +1,22 @@
> +#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.6.4

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





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

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-30 14:28     ` Chuck Lever
@ 2011-09-30 14:30       ` J. Bruce Fields
  2011-09-30 14:38       ` Bryan Schumaker
  1 sibling, 0 replies; 16+ messages in thread
From: J. Bruce Fields @ 2011-09-30 14:30 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Bryan Schumaker, linux-nfs

On Fri, Sep 30, 2011 at 10:28:02AM -0400, Chuck Lever wrote:
> 
> On Sep 29, 2011, at 4:14 PM, Bryan Schumaker wrote:
> 
> > On 09/29/2011 03:06 PM, Chuck Lever wrote:
> >> 
> >> On Sep 29, 2011, at 2:59 PM, bjschuma@netapp.com wrote:
> >> 
> >>> From: Bryan Schumaker <bjschuma@netapp.com>
> >>> 
> > ...
> >>> diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
> >>> index 9b118ee..69eae75 100644
> >>> --- a/fs/nfsd/Makefile
> >>> +++ b/fs/nfsd/Makefile
> >>> @@ -5,7 +5,8 @@
> >>> 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
> >>> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
> >>> +			   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..2139883
> >>> --- /dev/null
> >>> +++ b/fs/nfsd/fault_inject.c
> >>> @@ -0,0 +1,102 @@
> >>> +#ifdef CONFIG_NFSD_FAULT_INJECTION
> >> 
> >> The usual practice is to conditionally compile this source file via a Makefile variable, not by wrapping the whole source file in an ifdef.  You can see examples of this in the Makefile hunk above.
> >> 
> > 
> > How's this?  I moved the selectively compile bit to the makefile and created a fault_inject.h for the empty init() and cleanup() functions.  I moved the rest of the fault injection function declarations there, too, to keep them all together.
> 
> OK.  How about adding documenting block comments for the new files?  Does NetApp require you to add a copyright notice at the top of new files?

You probably do want that, but just keep it to a minimum and don't
include the filename in the block comment.

--b.

> Also, I wonder if it would be better if you added "default N" for the
> new CONFIG option.


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

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-29 18:59 [PATCH v3 1/3] NFSD: Added fault injection bjschuma
                   ` (2 preceding siblings ...)
  2011-09-29 19:06 ` [PATCH v3 1/3] NFSD: Added fault injection Chuck Lever
@ 2011-09-30 14:35 ` J. Bruce Fields
  2011-09-30 14:54   ` Bryan Schumaker
  3 siblings, 1 reply; 16+ messages in thread
From: J. Bruce Fields @ 2011-09-30 14:35 UTC (permalink / raw)
  To: bjschuma; +Cc: linux-nfs

On Thu, Sep 29, 2011 at 02:59:23PM -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.

You also might look at the unlock_ip interface that Wendy Cheng added a
few years ago. The fact that it doesn't remove nfsv4 state is really a
bug, it should be blowing away any relevant v4 clients as the same time.

(Though note the IP in question is a server IP, not a client IP.)

Is this the forget_n_clients scenario really a useful one to test?
I'd've thought that the client would need to deal with the entire client
diseappearing (due to network partition, for example), but not normally
individual open owners.

--b.

> 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 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       |    3 +-
>  fs/nfsd/fault_inject.c |  102 ++++++++++++++++++++++++++++++++++++++++++
>  fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/nfsd/nfsctl.c       |    6 +++
>  fs/nfsd/nfsd.h         |   12 +++++
>  6 files changed, 247 insertions(+), 1 deletions(-)
>  create mode 100644 fs/nfsd/fault_inject.c
> 
> 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..69eae75 100644
> --- a/fs/nfsd/Makefile
> +++ b/fs/nfsd/Makefile
> @@ -5,7 +5,8 @@
>  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
> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
> +			   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..2139883
> --- /dev/null
> +++ b/fs/nfsd/fault_inject.c
> @@ -0,0 +1,102 @@
> +#ifdef CONFIG_NFSD_FAULT_INJECTION
> +
> +#include <linux/types.h>
> +#include <linux/fs.h>
> +#include <linux/debugfs.h>
> +#include <linux/module.h>
> +
> +#include "state.h"
> +#include "nfsd.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;
> +}
> +
> +#else /* CONFIG_NFSD_FAULT_INJECTION */
> +
> +inline void nfsd_fault_inject_cleanup(void)
> +{}
> +
> +inline int nfsd_fault_inject_init(void)
> +{
> +	return 0;
> +}
> +
> +#endif /* CONFIG_NFSD_FAULT_INJECTION */
> 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..e2f1b5a 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1130,6 +1130,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 +1164,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 +1179,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);
>  }
>  
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 58134a2..3ea303d 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -105,11 +105,18 @@ static inline int nfsd_v4client(struct svc_rqst *rq)
>  #ifdef CONFIG_NFSD_V4
>  extern unsigned int max_delegations;
>  int nfs4_state_init(void);
> +int nfsd_fault_inject_init(void);
> +void nfsd_fault_inject_cleanup(void);
>  void nfsd4_free_slabs(void);
>  int nfs4_state_start(void);
>  void nfs4_state_shutdown(void);
>  void nfs4_reset_lease(time_t leasetime);
>  int nfs4_reset_recoverydir(char *recdir);
> +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
>  static inline int nfs4_state_init(void) { return 0; }
>  static inline void nfsd4_free_slabs(void) { }
> @@ -117,6 +124,11 @@ static inline int nfs4_state_start(void) { return 0; }
>  static inline void nfs4_state_shutdown(void) { }
>  static inline void nfs4_reset_lease(time_t leasetime) { }
>  static inline int nfs4_reset_recoverydir(char *recdir) { return 0; }
> +static inline void nfsd_forget_clients(u64) {}
> +static inline void nfsd_forget_locks(u64) {}
> +static inline void nfsd_forget_openowners(u64) {}
> +static inline void nfsd_forget_delegations(u64) {}
> +static inline void nfsd_recall_delegations(u64) {}
>  #endif
>  
>  /*
> -- 
> 1.7.6.4
> 

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

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-30 14:28     ` Chuck Lever
  2011-09-30 14:30       ` J. Bruce Fields
@ 2011-09-30 14:38       ` Bryan Schumaker
  2011-09-30 14:57         ` Chuck Lever
  1 sibling, 1 reply; 16+ messages in thread
From: Bryan Schumaker @ 2011-09-30 14:38 UTC (permalink / raw)
  To: Chuck Lever; +Cc: bfields, linux-nfs

On 09/30/2011 10:28 AM, Chuck Lever wrote:
> 
> On Sep 29, 2011, at 4:14 PM, Bryan Schumaker wrote:
> 
>> On 09/29/2011 03:06 PM, Chuck Lever wrote:
>>>
>>> On Sep 29, 2011, at 2:59 PM, bjschuma@netapp.com wrote:
>>>
>>>> From: Bryan Schumaker <bjschuma@netapp.com>
>>>>
>> ...
>>>> diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
>>>> index 9b118ee..69eae75 100644
>>>> --- a/fs/nfsd/Makefile
>>>> +++ b/fs/nfsd/Makefile
>>>> @@ -5,7 +5,8 @@
>>>> 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
>>>> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
>>>> +			   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..2139883
>>>> --- /dev/null
>>>> +++ b/fs/nfsd/fault_inject.c
>>>> @@ -0,0 +1,102 @@
>>>> +#ifdef CONFIG_NFSD_FAULT_INJECTION
>>>
>>> The usual practice is to conditionally compile this source file via a Makefile variable, not by wrapping the whole source file in an ifdef.  You can see examples of this in the Makefile hunk above.
>>>
>>
>> How's this?  I moved the selectively compile bit to the makefile and created a fault_inject.h for the empty init() and cleanup() functions.  I moved the rest of the fault injection function declarations there, too, to keep them all together.
> 
> OK.  How about adding documenting block comments for the new files?  Does NetApp require you to add a copyright notice at the top of new files?

I have no idea if I'm required, but I can if it's needed.
> 
> Also, I wonder if it would be better if you added "default N" for the new CONFIG option.

What would trigger the default option?  Right now I control this by writing a number into the debugfs file.  I suppose reading from the file could trigger the default case.

> 
>> - Bryan
>>
>> 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
>>
>> 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 |   88 ++++++++++++++++++++++++++++++++++++
>> fs/nfsd/fault_inject.h |   22 +++++++++
>> fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
>> fs/nfsd/nfsctl.c       |    7 +++
>> 6 files changed, 243 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..8b6ec8d
>> --- /dev/null
>> +++ b/fs/nfsd/fault_inject.c
>> @@ -0,0 +1,88 @@
>> +#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..5b003a9
>> --- /dev/null
>> +++ b/fs/nfsd/fault_inject.h
>> @@ -0,0 +1,22 @@
>> +#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.6.4
> 


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

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

On 09/30/2011 10:35 AM, J. Bruce Fields wrote:
> On Thu, Sep 29, 2011 at 02:59:23PM -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.
> 
> You also might look at the unlock_ip interface that Wendy Cheng added a
> few years ago. The fact that it doesn't remove nfsv4 state is really a
> bug, it should be blowing away any relevant v4 clients as the same time.
> 
> (Though note the IP in question is a server IP, not a client IP.)

I'll take a look.  Thanks for the warning that it looks for a server IP, but would passing the client IP be more useful?

> 
> Is this the forget_n_clients scenario really a useful one to test?
> I'd've thought that the client would need to deal with the entire client
> diseappearing (due to network partition, for example), but not normally
> individual open owners.

It might not be a normal case, but it still could be interesting to see how the client recovers.

I was mostly looking at what state the server tracks and deleting what I could find.
> 
> --b.
> 
>> 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 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       |    3 +-
>>  fs/nfsd/fault_inject.c |  102 ++++++++++++++++++++++++++++++++++++++++++
>>  fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  fs/nfsd/nfsctl.c       |    6 +++
>>  fs/nfsd/nfsd.h         |   12 +++++
>>  6 files changed, 247 insertions(+), 1 deletions(-)
>>  create mode 100644 fs/nfsd/fault_inject.c
>>
>> 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..69eae75 100644
>> --- a/fs/nfsd/Makefile
>> +++ b/fs/nfsd/Makefile
>> @@ -5,7 +5,8 @@
>>  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
>> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
>> +			   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..2139883
>> --- /dev/null
>> +++ b/fs/nfsd/fault_inject.c
>> @@ -0,0 +1,102 @@
>> +#ifdef CONFIG_NFSD_FAULT_INJECTION
>> +
>> +#include <linux/types.h>
>> +#include <linux/fs.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/module.h>
>> +
>> +#include "state.h"
>> +#include "nfsd.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;
>> +}
>> +
>> +#else /* CONFIG_NFSD_FAULT_INJECTION */
>> +
>> +inline void nfsd_fault_inject_cleanup(void)
>> +{}
>> +
>> +inline int nfsd_fault_inject_init(void)
>> +{
>> +	return 0;
>> +}
>> +
>> +#endif /* CONFIG_NFSD_FAULT_INJECTION */
>> 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..e2f1b5a 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
>> @@ -1130,6 +1130,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 +1164,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 +1179,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);
>>  }
>>  
>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>> index 58134a2..3ea303d 100644
>> --- a/fs/nfsd/nfsd.h
>> +++ b/fs/nfsd/nfsd.h
>> @@ -105,11 +105,18 @@ static inline int nfsd_v4client(struct svc_rqst *rq)
>>  #ifdef CONFIG_NFSD_V4
>>  extern unsigned int max_delegations;
>>  int nfs4_state_init(void);
>> +int nfsd_fault_inject_init(void);
>> +void nfsd_fault_inject_cleanup(void);
>>  void nfsd4_free_slabs(void);
>>  int nfs4_state_start(void);
>>  void nfs4_state_shutdown(void);
>>  void nfs4_reset_lease(time_t leasetime);
>>  int nfs4_reset_recoverydir(char *recdir);
>> +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
>>  static inline int nfs4_state_init(void) { return 0; }
>>  static inline void nfsd4_free_slabs(void) { }
>> @@ -117,6 +124,11 @@ static inline int nfs4_state_start(void) { return 0; }
>>  static inline void nfs4_state_shutdown(void) { }
>>  static inline void nfs4_reset_lease(time_t leasetime) { }
>>  static inline int nfs4_reset_recoverydir(char *recdir) { return 0; }
>> +static inline void nfsd_forget_clients(u64) {}
>> +static inline void nfsd_forget_locks(u64) {}
>> +static inline void nfsd_forget_openowners(u64) {}
>> +static inline void nfsd_forget_delegations(u64) {}
>> +static inline void nfsd_recall_delegations(u64) {}
>>  #endif
>>  
>>  /*
>> -- 
>> 1.7.6.4
>>


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

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-30 14:38       ` Bryan Schumaker
@ 2011-09-30 14:57         ` Chuck Lever
  2011-09-30 15:05           ` Bryan Schumaker
  0 siblings, 1 reply; 16+ messages in thread
From: Chuck Lever @ 2011-09-30 14:57 UTC (permalink / raw)
  To: Bryan Schumaker; +Cc: bfields, linux-nfs


On Sep 30, 2011, at 10:38 AM, Bryan Schumaker wrote:

> On 09/30/2011 10:28 AM, Chuck Lever wrote:
>> 
>> On Sep 29, 2011, at 4:14 PM, Bryan Schumaker wrote:
>> 
>>> On 09/29/2011 03:06 PM, Chuck Lever wrote:
>>>> 
>>>> On Sep 29, 2011, at 2:59 PM, bjschuma@netapp.com wrote:
>>>> 
>>>>> From: Bryan Schumaker <bjschuma@netapp.com>
>>>>> 
>>> ...
>>>>> diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
>>>>> index 9b118ee..69eae75 100644
>>>>> --- a/fs/nfsd/Makefile
>>>>> +++ b/fs/nfsd/Makefile
>>>>> @@ -5,7 +5,8 @@
>>>>> 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
>>>>> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
>>>>> +			   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..2139883
>>>>> --- /dev/null
>>>>> +++ b/fs/nfsd/fault_inject.c
>>>>> @@ -0,0 +1,102 @@
>>>>> +#ifdef CONFIG_NFSD_FAULT_INJECTION
>>>> 
>>>> The usual practice is to conditionally compile this source file via a Makefile variable, not by wrapping the whole source file in an ifdef.  You can see examples of this in the Makefile hunk above.
>>>> 
>>> 
>>> How's this?  I moved the selectively compile bit to the makefile and created a fault_inject.h for the empty init() and cleanup() functions.  I moved the rest of the fault injection function declarations there, too, to keep them all together.
>> 
>> OK.  How about adding documenting block comments for the new files?  Does NetApp require you to add a copyright notice at the top of new files?
> 
> I have no idea if I'm required, but I can if it's needed.

Ask Trond or Beepy.

>> Also, I wonder if it would be better if you added "default N" for the new CONFIG option.
> 
> What would trigger the default option?  Right now I control this by writing a number into the debugfs file.  I suppose reading from the file could trigger the default case.

See below.

> 
>> 
>>> - Bryan
>>> 
>>> 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
>>> 
>>> 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 |   88 ++++++++++++++++++++++++++++++++++++
>>> fs/nfsd/fault_inject.h |   22 +++++++++
>>> fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> fs/nfsd/nfsctl.c       |    7 +++
>>> 6 files changed, 243 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

        default N

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

I'm not sure that would even work.  But the idea is to prevent "make oldconfig" from even asking.  It should just be N unless it's explicitly set via the menu or an architecture config file.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





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

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-30 14:57         ` Chuck Lever
@ 2011-09-30 15:05           ` Bryan Schumaker
  0 siblings, 0 replies; 16+ messages in thread
From: Bryan Schumaker @ 2011-09-30 15:05 UTC (permalink / raw)
  To: Chuck Lever; +Cc: bfields, linux-nfs

On 09/30/2011 10:57 AM, Chuck Lever wrote:
> 
> On Sep 30, 2011, at 10:38 AM, Bryan Schumaker wrote:
> 
>> On 09/30/2011 10:28 AM, Chuck Lever wrote:
>>>
>>> On Sep 29, 2011, at 4:14 PM, Bryan Schumaker wrote:
>>>
>>>> On 09/29/2011 03:06 PM, Chuck Lever wrote:
>>>>>
>>>>> On Sep 29, 2011, at 2:59 PM, bjschuma@netapp.com wrote:
>>>>>
>>>>>> From: Bryan Schumaker <bjschuma@netapp.com>
>>>>>>
>>>> ...
>>>>>> diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
>>>>>> index 9b118ee..69eae75 100644
>>>>>> --- a/fs/nfsd/Makefile
>>>>>> +++ b/fs/nfsd/Makefile
>>>>>> @@ -5,7 +5,8 @@
>>>>>> 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
>>>>>> +			   export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \
>>>>>> +			   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..2139883
>>>>>> --- /dev/null
>>>>>> +++ b/fs/nfsd/fault_inject.c
>>>>>> @@ -0,0 +1,102 @@
>>>>>> +#ifdef CONFIG_NFSD_FAULT_INJECTION
>>>>>
>>>>> The usual practice is to conditionally compile this source file via a Makefile variable, not by wrapping the whole source file in an ifdef.  You can see examples of this in the Makefile hunk above.
>>>>>
>>>>
>>>> How's this?  I moved the selectively compile bit to the makefile and created a fault_inject.h for the empty init() and cleanup() functions.  I moved the rest of the fault injection function declarations there, too, to keep them all together.
>>>
>>> OK.  How about adding documenting block comments for the new files?  Does NetApp require you to add a copyright notice at the top of new files?
>>
>> I have no idea if I'm required, but I can if it's needed.
> 
> Ask Trond or Beepy.
> 
>>> Also, I wonder if it would be better if you added "default N" for the new CONFIG option.
>>
>> What would trigger the default option?  Right now I control this by writing a number into the debugfs file.  I suppose reading from the file could trigger the default case.
> 
> See below.

Oh!  I get what you mean now.  I thought you meant "n" as in the number of things to delete, and not default the feature to off.  Yeah, I can change that.

> 
>>
>>>
>>>> - Bryan
>>>>
>>>> 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
>>>>
>>>> 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 |   88 ++++++++++++++++++++++++++++++++++++
>>>> fs/nfsd/fault_inject.h |   22 +++++++++
>>>> fs/nfsd/nfs4state.c    |  115 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>> fs/nfsd/nfsctl.c       |    7 +++
>>>> 6 files changed, 243 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
> 
>         default N
> 
>>>> +	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.
> 
> I'm not sure that would even work.  But the idea is to prevent "make oldconfig" from even asking.  It should just be N unless it's explicitly set via the menu or an architecture config file.
> 


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

* Re: [PATCH v3 1/3] NFSD: Added fault injection
  2011-09-30 14:54   ` Bryan Schumaker
@ 2011-09-30 15:05     ` J. Bruce Fields
  0 siblings, 0 replies; 16+ messages in thread
From: J. Bruce Fields @ 2011-09-30 15:05 UTC (permalink / raw)
  To: Bryan Schumaker; +Cc: linux-nfs

On Fri, Sep 30, 2011 at 10:54:23AM -0400, Bryan Schumaker wrote:
> On 09/30/2011 10:35 AM, J. Bruce Fields wrote:
> > On Thu, Sep 29, 2011 at 02:59:23PM -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.
> > 
> > You also might look at the unlock_ip interface that Wendy Cheng added a
> > few years ago. The fact that it doesn't remove nfsv4 state is really a
> > bug, it should be blowing away any relevant v4 clients as the same time.
> > 
> > (Though note the IP in question is a server IP, not a client IP.)
> 
> I'll take a look.  Thanks for the warning that it looks for a server IP, but would passing the client IP be more useful?

It was designed for people using floating IP's to make their server look
like multiple servers, who want to shut down just the client's using one
of the floating IP's.

> > Is this the forget_n_clients scenario really a useful one to test?
> > I'd've thought that the client would need to deal with the entire client
> > diseappearing (due to network partition, for example), but not normally
> > individual open owners.
> 
> It might not be a normal case, but it still could be interesting to see how the client recovers.
> 
> I was mostly looking at what state the server tracks and deleting what I could find.

I think it would be more interesting to find out what typical failure
scenarios are and then figure out how to simulate them.

--b.

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

end of thread, other threads:[~2011-09-30 15:06 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-29 18:59 [PATCH v3 1/3] NFSD: Added fault injection bjschuma
2011-09-29 18:59 ` [PATCH v3 2/3] NFSD: Added fault injection script bjschuma
2011-09-29 18:59 ` [PATCH v3 3/3] NFSD: Added fault injection documentation bjschuma
2011-09-29 19:06 ` [PATCH v3 1/3] NFSD: Added fault injection Chuck Lever
2011-09-29 19:15   ` Bryan Schumaker
2011-09-29 19:22     ` Chuck Lever
2011-09-29 19:26       ` Bryan Schumaker
2011-09-29 20:14   ` Bryan Schumaker
2011-09-30 14:28     ` Chuck Lever
2011-09-30 14:30       ` J. Bruce Fields
2011-09-30 14:38       ` Bryan Schumaker
2011-09-30 14:57         ` Chuck Lever
2011-09-30 15:05           ` Bryan Schumaker
2011-09-30 14:35 ` J. Bruce Fields
2011-09-30 14:54   ` Bryan Schumaker
2011-09-30 15:05     ` J. Bruce Fields

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