2011-10-24 11:21:03

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH 1/3] NFSD: Added fault injection

From: Bryan Schumaker <[email protected]>

Fault injection on the NFS server makes it easier to test the client's
state manager and recovery threads. Simulating errors on the server is
easier than finding the right conditions that cause them naturally.

This patch uses debugfs to add a simple framework for fault injection to
the server. This framework is a config option, and can be enabled
through CONFIG_NFSD_FAULT_INJECTION. Assuming you have debugfs mounted
to /sys/debug, a set of files will be created in /sys/debug/nfsd/.
Writing to any of these files will cause the corresponding action and
write a log entry to dmesg.

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

Changes in v4:
- Move fault injection function declarations to a new .h file
- Use the Makefile to selectively compile fault_injection.c
- Add copyright notices to top of files

Changes in v3:
- Code cleanup and better use of generic functions
- Allow the user to input the number of state objects to delete
- Remove "forget_everything()" since forgetting a client is basically
the same thing

Changes in v2:
- Replaced "forget all state owners" with "forget all open owners"
- Include fs/nfsd/fault_inject.c in the patch

Signed-off-by: Bryan Schumaker <[email protected]>
---
fs/nfsd/Kconfig | 10 ++++
fs/nfsd/Makefile | 1 +
fs/nfsd/fault_inject.c | 90 +++++++++++++++++++++++++++++++++++++
fs/nfsd/fault_inject.h | 28 ++++++++++++
fs/nfsd/nfs4state.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/nfsctl.c | 7 +++
6 files changed, 251 insertions(+), 0 deletions(-)
create mode 100644 fs/nfsd/fault_inject.c
create mode 100644 fs/nfsd/fault_inject.h

diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index 10e6366..8df1ea4 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -80,3 +80,13 @@ config NFSD_V4
available from http://linux-nfs.org/.

If unsure, say N.
+
+config NFSD_FAULT_INJECTION
+ bool "NFS server manual fault injection"
+ depends on NFSD_V4 && DEBUG_KERNEL
+ help
+ This option enables support for manually injecting faults
+ into the NFS server. This is intended to be used for
+ testing error recovery on the NFS client.
+
+ If unsure, say N.
diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
index 9b118ee..af32ef0 100644
--- a/fs/nfsd/Makefile
+++ b/fs/nfsd/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_NFSD) += nfsd.o

nfsd-y := nfssvc.o nfsctl.o nfsproc.o nfsfh.o vfs.o \
export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o
+nfsd-$(CONFIG_NFSD_FAULT_INJECTION) += fault_inject.o
nfsd-$(CONFIG_NFSD_V2_ACL) += nfs2acl.o
nfsd-$(CONFIG_NFSD_V3) += nfs3proc.o nfs3xdr.o
nfsd-$(CONFIG_NFSD_V3_ACL) += nfs3acl.o
diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
new file mode 100644
index 0000000..1ac4134
--- /dev/null
+++ b/fs/nfsd/fault_inject.c
@@ -0,0 +1,90 @@
+/*
+ * Copyright (c) 2011 Bryan Schumaker <[email protected]>
+ *
+ * Uses debugfs to create fault injection points for client testing
+ */
+
+#include <linux/types.h>
+#include <linux/fs.h>
+#include <linux/debugfs.h>
+#include <linux/module.h>
+
+#include "state.h"
+#include "fault_inject.h"
+
+struct nfsd_fault_inject_op {
+ char *file;
+ void (*func)(u64);
+};
+
+static struct nfsd_fault_inject_op inject_ops[] = {
+ {
+ .file = "forget_clients",
+ .func = nfsd_forget_clients,
+ },
+ {
+ .file = "forget_locks",
+ .func = nfsd_forget_locks,
+ },
+ {
+ .file = "forget_openowners",
+ .func = nfsd_forget_openowners,
+ },
+ {
+ .file = "forget_delegations",
+ .func = nfsd_forget_delegations,
+ },
+ {
+ .file = "recall_delegations",
+ .func = nfsd_recall_delegations,
+ },
+};
+
+static long int NUM_INJECT_OPS = sizeof(inject_ops) / sizeof(struct nfsd_fault_inject_op);
+static struct dentry *debug_dir;
+
+static int nfsd_inject_set(void *op_ptr, u64 val)
+{
+ struct nfsd_fault_inject_op *op = op_ptr;
+
+ if (val == 0)
+ printk(KERN_INFO "NFSD Fault Injection: %s (all)", op->file);
+ else
+ printk(KERN_INFO "NFSD Fault Injection: %s (n = %llu)", op->file, val);
+
+ op->func(val);
+ return 0;
+}
+
+static int nfsd_inject_get(void *data, u64 *val)
+{
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(fops_nfsd, nfsd_inject_get, nfsd_inject_set, "%llu\n");
+
+void nfsd_fault_inject_cleanup(void)
+{
+ debugfs_remove_recursive(debug_dir);
+}
+
+int nfsd_fault_inject_init(void)
+{
+ unsigned int i;
+ struct nfsd_fault_inject_op *op;
+ mode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
+
+ debug_dir = debugfs_create_dir("nfsd", NULL);
+ if (!debug_dir)
+ goto fail;
+
+ for (i = 0; i < NUM_INJECT_OPS; i++) {
+ op = &inject_ops[i];
+ debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd);
+ }
+ return 0;
+
+fail:
+ nfsd_fault_inject_cleanup();
+ return -ENOMEM;
+}
diff --git a/fs/nfsd/fault_inject.h b/fs/nfsd/fault_inject.h
new file mode 100644
index 0000000..90bd057
--- /dev/null
+++ b/fs/nfsd/fault_inject.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2011 Bryan Schumaker <[email protected]>
+ *
+ * Function definitions for fault injection
+ */
+
+#ifndef LINUX_NFSD_FAULT_INJECT_H
+#define LINUX_NFSD_FAULT_INJECT_H
+
+#ifdef CONFIG_NFSD_FAULT_INJECTION
+int nfsd_fault_inject_init(void);
+void nfsd_fault_inject_cleanup(void);
+void nfsd_forget_clients(u64);
+void nfsd_forget_locks(u64);
+void nfsd_forget_openowners(u64);
+void nfsd_forget_delegations(u64);
+void nfsd_recall_delegations(u64);
+#else /* CONFIG_NFSD_FAULT_INJECTION */
+static inline int nfsd_fault_inject_init(void) { return 0; }
+static inline void nfsd_fault_inject_cleanup(void) {}
+static inline void nfsd_forget_clients(u64 num) {}
+static inline void nfsd_forget_locks(u64 num) {}
+static inline void nfsd_forget_openowners(u64 num) {}
+static inline void nfsd_forget_delegations(u64 num) {}
+static inline void nfsd_recall_delegations(u64 num) {}
+#endif /* CONFIG_NFSD_FAULT_INJECTION */
+
+#endif /* LINUX_NFSD_FAULT_INJECT_H */
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 05f4c69..64fa6b0 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4358,6 +4358,121 @@ nfs4_check_open_reclaim(clientid_t *clid)
return nfs4_find_reclaim_client(clid) ? nfs_ok : nfserr_reclaim_bad;
}

+#ifdef CONFIG_NFSD_FAULT_INJECTION
+
+void nfsd_forget_clients(u64 num)
+{
+ struct nfs4_client *clp, *next;
+ int count = 0;
+
+ nfs4_lock_state();
+ list_for_each_entry_safe(clp, next, &client_lru, cl_lru) {
+ nfsd4_remove_clid_dir(clp);
+ expire_client(clp);
+ if (++count == num)
+ break;
+ }
+ nfs4_unlock_state();
+
+ printk(KERN_INFO "NFSD: Forgot %d clients", count);
+}
+
+static void release_lockowner_sop(struct nfs4_stateowner *sop)
+{
+ release_lockowner(lockowner(sop));
+}
+
+static void release_openowner_sop(struct nfs4_stateowner *sop)
+{
+ release_openowner(openowner(sop));
+}
+
+static int nfsd_release_n_owners(u64 num,
+ struct list_head hashtbl[],
+ unsigned int hashtbl_size,
+ void (*release_sop)(struct nfs4_stateowner *))
+{
+ int i, count = 0;
+ struct nfs4_stateowner *sop, *next;
+
+ for (i = 0; i < hashtbl_size; i++) {
+ list_for_each_entry_safe(sop, next, &hashtbl[i], so_strhash) {
+ release_sop(sop);
+ if (++count == num)
+ return count;
+ }
+ }
+ return count;
+}
+
+void nfsd_forget_locks(u64 num)
+{
+ int count;
+
+ nfs4_lock_state();
+ count = nfsd_release_n_owners(num, lock_ownerstr_hashtbl,
+ LOCK_HASH_SIZE, release_lockowner_sop);
+ nfs4_unlock_state();
+
+ printk(KERN_INFO "NFSD: Forgot %d locks", count);
+}
+
+void nfsd_forget_openowners(u64 num)
+{
+ int count;
+
+ nfs4_lock_state();
+ count = nfsd_release_n_owners(num, open_ownerstr_hashtbl,
+ OPEN_OWNER_HASH_SIZE, release_openowner_sop);
+ nfs4_unlock_state();
+
+ printk(KERN_INFO "NFSD: Forgot %d open owners", count);
+}
+
+int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *))
+{
+ int i, count = 0;
+ struct nfs4_file *fp;
+ struct nfs4_delegation *dp, *next;
+
+ for (i = 0; i < FILE_HASH_SIZE; i++) {
+ list_for_each_entry(fp, &file_hashtbl[i], fi_hash) {
+ list_for_each_entry_safe(dp, next, &fp->fi_delegations, dl_perfile) {
+ deleg_func(dp);
+ if (++count == num)
+ return count;
+ }
+ }
+ }
+ return count;
+}
+
+void nfsd_forget_delegations(u64 num)
+{
+ unsigned int count;
+
+ nfs4_lock_state();
+ count = nfsd_process_n_delegations(num, unhash_delegation);
+ nfs4_unlock_state();
+
+ printk(KERN_INFO "NFSD: Forgot %d delegations", count);
+}
+
+void nfsd_recall_delegations(u64 num)
+{
+ unsigned int count;
+
+ nfs4_lock_state();
+ spin_lock(&recall_lock);
+ count = nfsd_process_n_delegations(num, nfsd_break_one_deleg);
+ spin_unlock(&recall_lock);
+ nfs4_unlock_state();
+
+ printk(KERN_INFO "NFSD: Recalled %d delegations", count);
+}
+
+#endif /* CONFIG_NFSD_FAULT_INJECTION */
+
/* initialization to perform at module load time: */

int
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index db34a58..e67f30c 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -17,6 +17,7 @@
#include "idmap.h"
#include "nfsd.h"
#include "cache.h"
+#include "fault_inject.h"

/*
* We have a single directory with several nodes in it.
@@ -1130,6 +1131,9 @@ static int __init init_nfsd(void)
retval = nfs4_state_init(); /* nfs4 locking state */
if (retval)
return retval;
+ retval = nfsd_fault_inject_init(); /* nfsd fault injection controls */
+ if (retval)
+ goto out_cleanup_fault_injection;
nfsd_stat_init(); /* Statistics */
retval = nfsd_reply_cache_init();
if (retval)
@@ -1161,6 +1165,8 @@ out_free_cache:
out_free_stat:
nfsd_stat_shutdown();
nfsd4_free_slabs();
+out_cleanup_fault_injection:
+ nfsd_fault_inject_cleanup();
return retval;
}

@@ -1174,6 +1180,7 @@ static void __exit exit_nfsd(void)
nfsd_lockd_shutdown();
nfsd_idmap_shutdown();
nfsd4_free_slabs();
+ nfsd_fault_inject_cleanup();
unregister_filesystem(&nfsd_fs_type);
}

--
1.7.7



2011-10-24 11:21:06

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH 3/3] NFSD: Added fault injection documentation

From: Bryan Schumaker <[email protected]>

Signed-off-by: Bryan Schumaker <[email protected]>
---
Documentation/filesystems/nfs/00-INDEX | 2 +
Documentation/filesystems/nfs/fault_injection.txt | 69 +++++++++++++++++++++
2 files changed, 71 insertions(+), 0 deletions(-)
create mode 100644 Documentation/filesystems/nfs/fault_injection.txt

diff --git a/Documentation/filesystems/nfs/00-INDEX b/Documentation/filesystems/nfs/00-INDEX
index a57e124..1716874 100644
--- a/Documentation/filesystems/nfs/00-INDEX
+++ b/Documentation/filesystems/nfs/00-INDEX
@@ -2,6 +2,8 @@
- this file (nfs-related documentation).
Exporting
- explanation of how to make filesystems exportable.
+fault_injection.txt
+ - information for using fault injection on the server
knfsd-stats.txt
- statistics which the NFS server makes available to user space.
nfs.txt
diff --git a/Documentation/filesystems/nfs/fault_injection.txt b/Documentation/filesystems/nfs/fault_injection.txt
new file mode 100644
index 0000000..426d166
--- /dev/null
+++ b/Documentation/filesystems/nfs/fault_injection.txt
@@ -0,0 +1,69 @@
+
+Fault Injection
+===============
+Fault injection is a method for forcing errors that may not normally occur, or
+may be difficult to reproduce. Forcing these errors in a controlled environment
+can help the developer find and fix bugs before their code is shipped in a
+production system. Injecting an error on the Linux NFS server will allow us to
+observe how the client reacts and if it manages to recover its state correctly.
+
+NFSD_FAULT_INJECTION must be selected when configuring the kernel to use this
+feature.
+
+
+Using Fault Injection
+=====================
+On the client, mount the fault injection server through NFS v4.0+ and do some
+work over NFS (open files, take locks, ...).
+
+On the server, mount the debugfs filesystem to <debug_dir> and ls
+<debug_dir>/nfsd. This will show a list of files that will be used for
+injecting faults on the NFS server. As root, write a number n to the file
+corresponding to the action you want the server to take. The server will then
+process the first n items it finds. So if you want to forget 5 locks, echo '5'
+to <debug_dir>/nfsd/forget_locks. A value of 0 will tell the server to forget
+all corresponding items. A log message will be created containing the number
+of items forgotten (check dmesg).
+
+Go back to work on the client and check if the client recovered from the error
+correctly.
+
+
+Available Faults
+================
+forget_clients:
+ The NFS server keeps a list of clients that have placed a mount call. If
+ this list is cleared, the server will have no knowledge of who the client
+ is, forcing the client to reauthenticate with the server.
+
+forget_openowners:
+ The NFS server keeps a list of what files are currently opened and who
+ they were opened by. Clearing this list will force the client to reopen
+ its files.
+
+forget_locks:
+ The NFS server keeps a list of what files are currently locked in the VFS.
+ Clearing this list will force the client to reclaim its locks (files are
+ unlocked through the VFS as they are cleared from this list).
+
+forget_delegations:
+ A delegation is used to assure the client that a file, or part of a file,
+ has not changed since the delegation was awarded. Clearing this list will
+ force the client to reaquire its delegation before accessing the file
+ again.
+
+recall_delegations:
+ Delegations can be recalled by the server when another client attempts to
+ access a file. This test will notify the client that its delegation has
+ been revoked, forcing the client to reaquire the delegation before using
+ the file again.
+
+
+tools/nfs/inject_faults.sh script
+=================================
+This script has been created to ease the fault injection process. This script
+will detect the mounted debugfs directory and write to the files located there
+based on the arguments passed by the user. For example, running
+`inject_faults.sh forget_locks 1` as root will instruct the server to forget
+one lock. Running `inject_faults forget_locks` will instruct the server to
+forgetall locks.
--
1.7.7


2011-10-24 11:21:04

by Anna Schumaker

[permalink] [raw]
Subject: [PATCH 2/3] NFSD: Added fault injection script

From: Bryan Schumaker <[email protected]>

This script provides a convenient way to use the NFSD fault injection
framework. Fault injection writes to dmesg using the KERN_INFO flag, so
this script will compare the before and after output of `dmesg` to show
the user what happened

Signed-off-by: Bryan Schumaker <[email protected]>
---
tools/nfsd/inject_fault.sh | 49 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 49 insertions(+), 0 deletions(-)
create mode 100755 tools/nfsd/inject_fault.sh

diff --git a/tools/nfsd/inject_fault.sh b/tools/nfsd/inject_fault.sh
new file mode 100755
index 0000000..06a399a
--- /dev/null
+++ b/tools/nfsd/inject_fault.sh
@@ -0,0 +1,49 @@
+#!/bin/bash
+#
+# Copyright (c) 2011 Bryan Schumaker <[email protected]>
+#
+# Script for easier NFSD fault injection
+
+# Check that debugfs has been mounted
+DEBUGFS=`cat /proc/mounts | grep debugfs`
+if [ "$DEBUGFS" == "" ]; then
+ echo "debugfs does not appear to be mounted!"
+ echo "Please mount debugfs and try again"
+ exit 1
+fi
+
+# Check that the fault injection directory exists
+DEBUGDIR=`echo $DEBUGFS | awk '{print $2}'`/nfsd
+if [ ! -d "$DEBUGDIR" ]; then
+ echo "$DEBUGDIR does not exist"
+ echo "Check that your .config selects CONFIG_NFSD_FAULT_INJECTION"
+ exit 1
+fi
+
+function help()
+{
+ echo "Usage $0 injection_type [count]"
+ echo ""
+ echo "Injection types are:"
+ ls $DEBUGDIR
+ exit 1
+}
+
+if [ $# == 0 ]; then
+ help
+elif [ ! -f $DEBUGDIR/$1 ]; then
+ help
+elif [ $# != 2 ]; then
+ COUNT=0
+else
+ COUNT=$2
+fi
+
+BEFORE=`mktemp`
+AFTER=`mktemp`
+dmesg > $BEFORE
+echo $COUNT > $DEBUGDIR/$1
+dmesg > $AFTER
+# Capture lines that only exist in the $AFTER file
+diff $BEFORE $AFTER | grep ">"
+rm -f $BEFORE $AFTER
--
1.7.7


2011-11-04 21:30:59

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/3] NFSD: Added fault injection

On Tue, Nov 01, 2011 at 01:35:21PM -0400, [email protected] wrote:
> From: Bryan Schumaker <[email protected]>
>
> Fault injection on the NFS server makes it easier to test the client's
> state manager and recovery threads. Simulating errors on the server is
> easier than finding the right conditions that cause them naturally.
>
> This patch uses debugfs to add a simple framework for fault injection to
> the server. This framework is a config option, and can be enabled
> through CONFIG_NFSD_FAULT_INJECTION. Assuming you have debugfs mounted
> to /sys/debug, a set of files will be created in /sys/debug/nfsd/.
> Writing to any of these files will cause the corresponding action and
> write a log entry to dmesg.

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

Oh, just one request for next time:

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

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

--b.

>
> Changes in v5:
> - Fill out inject_ops[] array without a macro
> - Simplify debug messages and file creation
>
> Changes in v4:
> - Move fault injection function declarations to a new .h file
> - Use the Makefile to selectively compile fault_injection.c
> - Add copyright notices to top of files
>
> Changes in v3:
> - Code cleanup and better use of generic functions
> - Allow the user to input the number of state objects to delete
> - Remove "forget_everything()" since forgetting a client is basically
> the same thing
>
> Changes in v2:
> - Replaced "forget all state owners" with "forget all open owners"
> - Include fs/nfsd/fault_inject.c in the patch
>
> Signed-off-by: Bryan Schumaker <[email protected]>
> ---
> fs/nfsd/Kconfig | 10 ++++
> fs/nfsd/Makefile | 1 +
> fs/nfsd/fault_inject.c | 91 ++++++++++++++++++++++++++++++++++++++
> fs/nfsd/fault_inject.h | 28 ++++++++++++
> fs/nfsd/nfs4state.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/nfsctl.c | 7 +++
> 6 files changed, 252 insertions(+), 0 deletions(-)
> create mode 100644 fs/nfsd/fault_inject.c
> create mode 100644 fs/nfsd/fault_inject.h
>
> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
> index 10e6366..8df1ea4 100644
> --- a/fs/nfsd/Kconfig
> +++ b/fs/nfsd/Kconfig
> @@ -80,3 +80,13 @@ config NFSD_V4
> available from http://linux-nfs.org/.
>
> If unsure, say N.
> +
> +config NFSD_FAULT_INJECTION
> + bool "NFS server manual fault injection"
> + depends on NFSD_V4 && DEBUG_KERNEL
> + help
> + This option enables support for manually injecting faults
> + into the NFS server. This is intended to be used for
> + testing error recovery on the NFS client.
> +
> + If unsure, say N.
> diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile
> index 9b118ee..af32ef0 100644
> --- a/fs/nfsd/Makefile
> +++ b/fs/nfsd/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_NFSD) += nfsd.o
>
> nfsd-y := nfssvc.o nfsctl.o nfsproc.o nfsfh.o vfs.o \
> export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o
> +nfsd-$(CONFIG_NFSD_FAULT_INJECTION) += fault_inject.o
> nfsd-$(CONFIG_NFSD_V2_ACL) += nfs2acl.o
> nfsd-$(CONFIG_NFSD_V3) += nfs3proc.o nfs3xdr.o
> nfsd-$(CONFIG_NFSD_V3_ACL) += nfs3acl.o
> diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
> new file mode 100644
> index 0000000..ce7f075
> --- /dev/null
> +++ b/fs/nfsd/fault_inject.c
> @@ -0,0 +1,91 @@
> +/*
> + * Copyright (c) 2011 Bryan Schumaker <[email protected]>
> + *
> + * Uses debugfs to create fault injection points for client testing
> + */
> +
> +#include <linux/types.h>
> +#include <linux/fs.h>
> +#include <linux/debugfs.h>
> +#include <linux/module.h>
> +
> +#include "state.h"
> +#include "fault_inject.h"
> +
> +struct nfsd_fault_inject_op {
> + char *file;
> + void (*func)(u64);
> +};
> +
> +static struct nfsd_fault_inject_op inject_ops[] = {
> + {
> + .file = "forget_clients",
> + .func = nfsd_forget_clients,
> + },
> + {
> + .file = "forget_locks",
> + .func = nfsd_forget_locks,
> + },
> + {
> + .file = "forget_openowners",
> + .func = nfsd_forget_openowners,
> + },
> + {
> + .file = "forget_delegations",
> + .func = nfsd_forget_delegations,
> + },
> + {
> + .file = "recall_delegations",
> + .func = nfsd_recall_delegations,
> + },
> +};
> +
> +static long int NUM_INJECT_OPS = sizeof(inject_ops) / sizeof(struct nfsd_fault_inject_op);
> +static struct dentry *debug_dir;
> +
> +static int nfsd_inject_set(void *op_ptr, u64 val)
> +{
> + struct nfsd_fault_inject_op *op = op_ptr;
> +
> + if (val == 0)
> + printk(KERN_INFO "NFSD Fault Injection: %s (all)", op->file);
> + else
> + printk(KERN_INFO "NFSD Fault Injection: %s (n = %llu)", op->file, val);
> +
> + op->func(val);
> + return 0;
> +}
> +
> +static int nfsd_inject_get(void *data, u64 *val)
> +{
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(fops_nfsd, nfsd_inject_get, nfsd_inject_set, "%llu\n");
> +
> +void nfsd_fault_inject_cleanup(void)
> +{
> + debugfs_remove_recursive(debug_dir);
> +}
> +
> +int nfsd_fault_inject_init(void)
> +{
> + unsigned int i;
> + struct nfsd_fault_inject_op *op;
> + mode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
> +
> + debug_dir = debugfs_create_dir("nfsd", NULL);
> + if (!debug_dir)
> + goto fail;
> +
> + for (i = 0; i < NUM_INJECT_OPS; i++) {
> + op = &inject_ops[i];
> + if (!debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd))
> + goto fail;
> + }
> + return 0;
> +
> +fail:
> + nfsd_fault_inject_cleanup();
> + return -ENOMEM;
> +}
> diff --git a/fs/nfsd/fault_inject.h b/fs/nfsd/fault_inject.h
> new file mode 100644
> index 0000000..90bd057
> --- /dev/null
> +++ b/fs/nfsd/fault_inject.h
> @@ -0,0 +1,28 @@
> +/*
> + * Copyright (c) 2011 Bryan Schumaker <[email protected]>
> + *
> + * Function definitions for fault injection
> + */
> +
> +#ifndef LINUX_NFSD_FAULT_INJECT_H
> +#define LINUX_NFSD_FAULT_INJECT_H
> +
> +#ifdef CONFIG_NFSD_FAULT_INJECTION
> +int nfsd_fault_inject_init(void);
> +void nfsd_fault_inject_cleanup(void);
> +void nfsd_forget_clients(u64);
> +void nfsd_forget_locks(u64);
> +void nfsd_forget_openowners(u64);
> +void nfsd_forget_delegations(u64);
> +void nfsd_recall_delegations(u64);
> +#else /* CONFIG_NFSD_FAULT_INJECTION */
> +static inline int nfsd_fault_inject_init(void) { return 0; }
> +static inline void nfsd_fault_inject_cleanup(void) {}
> +static inline void nfsd_forget_clients(u64 num) {}
> +static inline void nfsd_forget_locks(u64 num) {}
> +static inline void nfsd_forget_openowners(u64 num) {}
> +static inline void nfsd_forget_delegations(u64 num) {}
> +static inline void nfsd_recall_delegations(u64 num) {}
> +#endif /* CONFIG_NFSD_FAULT_INJECTION */
> +
> +#endif /* LINUX_NFSD_FAULT_INJECT_H */
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 47e94e3..3c3762e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4394,6 +4394,121 @@ nfs4_check_open_reclaim(clientid_t *clid)
> return nfs4_find_reclaim_client(clid) ? nfs_ok : nfserr_reclaim_bad;
> }
>
> +#ifdef CONFIG_NFSD_FAULT_INJECTION
> +
> +void nfsd_forget_clients(u64 num)
> +{
> + struct nfs4_client *clp, *next;
> + int count = 0;
> +
> + nfs4_lock_state();
> + list_for_each_entry_safe(clp, next, &client_lru, cl_lru) {
> + nfsd4_remove_clid_dir(clp);
> + expire_client(clp);
> + if (++count == num)
> + break;
> + }
> + nfs4_unlock_state();
> +
> + printk(KERN_INFO "NFSD: Forgot %d clients", count);
> +}
> +
> +static void release_lockowner_sop(struct nfs4_stateowner *sop)
> +{
> + release_lockowner(lockowner(sop));
> +}
> +
> +static void release_openowner_sop(struct nfs4_stateowner *sop)
> +{
> + release_openowner(openowner(sop));
> +}
> +
> +static int nfsd_release_n_owners(u64 num,
> + struct list_head hashtbl[],
> + unsigned int hashtbl_size,
> + void (*release_sop)(struct nfs4_stateowner *))
> +{
> + int i, count = 0;
> + struct nfs4_stateowner *sop, *next;
> +
> + for (i = 0; i < hashtbl_size; i++) {
> + list_for_each_entry_safe(sop, next, &hashtbl[i], so_strhash) {
> + release_sop(sop);
> + if (++count == num)
> + return count;
> + }
> + }
> + return count;
> +}
> +
> +void nfsd_forget_locks(u64 num)
> +{
> + int count;
> +
> + nfs4_lock_state();
> + count = nfsd_release_n_owners(num, lock_ownerstr_hashtbl,
> + LOCK_HASH_SIZE, release_lockowner_sop);
> + nfs4_unlock_state();
> +
> + printk(KERN_INFO "NFSD: Forgot %d locks", count);
> +}
> +
> +void nfsd_forget_openowners(u64 num)
> +{
> + int count;
> +
> + nfs4_lock_state();
> + count = nfsd_release_n_owners(num, open_ownerstr_hashtbl,
> + OPEN_OWNER_HASH_SIZE, release_openowner_sop);
> + nfs4_unlock_state();
> +
> + printk(KERN_INFO "NFSD: Forgot %d open owners", count);
> +}
> +
> +int nfsd_process_n_delegations(u64 num, void (*deleg_func)(struct nfs4_delegation *))
> +{
> + int i, count = 0;
> + struct nfs4_file *fp;
> + struct nfs4_delegation *dp, *next;
> +
> + for (i = 0; i < FILE_HASH_SIZE; i++) {
> + list_for_each_entry(fp, &file_hashtbl[i], fi_hash) {
> + list_for_each_entry_safe(dp, next, &fp->fi_delegations, dl_perfile) {
> + deleg_func(dp);
> + if (++count == num)
> + return count;
> + }
> + }
> + }
> + return count;
> +}
> +
> +void nfsd_forget_delegations(u64 num)
> +{
> + unsigned int count;
> +
> + nfs4_lock_state();
> + count = nfsd_process_n_delegations(num, unhash_delegation);
> + nfs4_unlock_state();
> +
> + printk(KERN_INFO "NFSD: Forgot %d delegations", count);
> +}
> +
> +void nfsd_recall_delegations(u64 num)
> +{
> + unsigned int count;
> +
> + nfs4_lock_state();
> + spin_lock(&recall_lock);
> + count = nfsd_process_n_delegations(num, nfsd_break_one_deleg);
> + spin_unlock(&recall_lock);
> + nfs4_unlock_state();
> +
> + printk(KERN_INFO "NFSD: Recalled %d delegations", count);
> +}
> +
> +#endif /* CONFIG_NFSD_FAULT_INJECTION */
> +
> /* initialization to perform at module load time: */
>
> int
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index db34a58..34c841a 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -17,6 +17,7 @@
> #include "idmap.h"
> #include "nfsd.h"
> #include "cache.h"
> +#include "fault_inject.h"
>
> /*
> * We have a single directory with several nodes in it.
> @@ -1130,6 +1131,9 @@ static int __init init_nfsd(void)
> retval = nfs4_state_init(); /* nfs4 locking state */
> if (retval)
> return retval;
> + retval = nfsd_fault_inject_init(); /* nfsd fault injection controls */
> + if (retval)
> + goto out_free_slabs;
> nfsd_stat_init(); /* Statistics */
> retval = nfsd_reply_cache_init();
> if (retval)
> @@ -1160,6 +1164,8 @@ out_free_cache:
> nfsd_reply_cache_shutdown();
> out_free_stat:
> nfsd_stat_shutdown();
> + nfsd_fault_inject_cleanup();
> +out_free_slabs:
> nfsd4_free_slabs();
> return retval;
> }
> @@ -1174,6 +1180,7 @@ static void __exit exit_nfsd(void)
> nfsd_lockd_shutdown();
> nfsd_idmap_shutdown();
> nfsd4_free_slabs();
> + nfsd_fault_inject_cleanup();
> unregister_filesystem(&nfsd_fs_type);
> }
>
> --
> 1.7.7
>

2011-11-01 14:17:47

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/3] NFSD: Added fault injection

On Tue, Nov 01, 2011 at 10:08:37AM -0400, Bryan Schumaker wrote:
> On Fri 28 Oct 2011 05:24:50 PM EDT, J. Bruce Fields wrote:
> > On Mon, Oct 24, 2011 at 07:20:57AM -0400, [email protected] wrote:
> > ...
> >> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> >> index db34a58..e67f30c 100644
> >> --- a/fs/nfsd/nfsctl.c
> >> +++ b/fs/nfsd/nfsctl.c
> > ...
> >> @@ -1130,6 +1131,9 @@ static int __init init_nfsd(void)
> >> retval = nfs4_state_init(); /* nfs4 locking state */
> >> if (retval)
> >> return retval;
> >> + retval = nfsd_fault_inject_init(); /* nfsd fault injection controls */
> >> + if (retval)
> >> + goto out_cleanup_fault_injection;
> >> nfsd_stat_init(); /* Statistics */
> >> retval = nfsd_reply_cache_init();
> >> if (retval)
> >> @@ -1161,6 +1165,8 @@ out_free_cache:
> >> out_free_stat:
> >> nfsd_stat_shutdown();
> >> nfsd4_free_slabs();
> >> +out_cleanup_fault_injection:
> >> + nfsd_fault_inject_cleanup();
> >
> > Check the cleanup here again.... The slabs are allocated in state_init,
> > so you need to do that on nfsd_fault_inject_init, but you *don't* need
> > the fault_inject_cleanup in that case, since fault_inject_init cleanup
> > cleans up after itself--that's needed only on later failures.
>
> Would it be better to change the fault_inject_init() function so it
> doesn't clean up after itself? This would keep it consistent with the
> other nfsd init functions.

I don't think so. The pattern here is pretty common throughout the
kernel; it's approximately:

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

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

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

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

--b.

2011-11-01 14:22:32

by Anna Schumaker

[permalink] [raw]
Subject: Re: [PATCH 1/3] NFSD: Added fault injection

On Tue 01 Nov 2011 10:17:46 AM EDT, J. Bruce Fields wrote:
> On Tue, Nov 01, 2011 at 10:08:37AM -0400, Bryan Schumaker wrote:
>> On Fri 28 Oct 2011 05:24:50 PM EDT, J. Bruce Fields wrote:
>>> On Mon, Oct 24, 2011 at 07:20:57AM -0400, [email protected] wrote:
>>> ...
>>>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>>>> index db34a58..e67f30c 100644
>>>> --- a/fs/nfsd/nfsctl.c
>>>> +++ b/fs/nfsd/nfsctl.c
>>> ...
>>>> @@ -1130,6 +1131,9 @@ static int __init init_nfsd(void)
>>>> retval = nfs4_state_init(); /* nfs4 locking state */
>>>> if (retval)
>>>> return retval;
>>>> + retval = nfsd_fault_inject_init(); /* nfsd fault injection controls */
>>>> + if (retval)
>>>> + goto out_cleanup_fault_injection;
>>>> nfsd_stat_init(); /* Statistics */
>>>> retval = nfsd_reply_cache_init();
>>>> if (retval)
>>>> @@ -1161,6 +1165,8 @@ out_free_cache:
>>>> out_free_stat:
>>>> nfsd_stat_shutdown();
>>>> nfsd4_free_slabs();
>>>> +out_cleanup_fault_injection:
>>>> + nfsd_fault_inject_cleanup();
>>>
>>> Check the cleanup here again.... The slabs are allocated in state_init,
>>> so you need to do that on nfsd_fault_inject_init, but you *don't* need
>>> the fault_inject_cleanup in that case, since fault_inject_init cleanup
>>> cleans up after itself--that's needed only on later failures.
>>
>> Would it be better to change the fault_inject_init() function so it
>> doesn't clean up after itself? This would keep it consistent with the
>> other nfsd init functions.
>
> I don't think so. The pattern here is pretty common throughout the
> kernel; it's approximately:
>
> do_thing1();
> err = do_thing2();
> if (err)
> goto undo_thing1;
> err = do_thing3();
> if (err)
> goto undo_thing2;
> ...
>
> Functions are no-ops on failure, and the caller's just responsible for
> cleaning up previous stuff.

Ok, I won't change that.

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

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



2011-11-01 14:35:39

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/3] NFSD: Added fault injection

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

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

--b.