On 10/17/2011 06:18 PM, J. Bruce Fields wrote: > On Fri, Oct 07, 2011 at 01:44:26PM -0400, bjschuma@xxxxxxxxxx wrote: >> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig >> index 10e6366..52fdd1c 100644 >> --- a/fs/nfsd/Kconfig >> +++ b/fs/nfsd/Kconfig >> @@ -80,3 +80,13 @@ config NFSD_V4 >> available from http://linux-nfs.org/. >> >> If unsure, say N. >> + >> +config NFSD_FAULT_INJECTION >> + bool "NFS server manual fault injection" >> + depends on NFSD_V4 && DEBUG_KERNEL >> + help >> + This option enables support for manually injectiong faults > ^^^^^^^^^^ Good catch, this is a weird cross between "injection" and "injecting" > >> diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c >> new file mode 100644 >> index 0000000..9f6815b >> --- /dev/null >> +++ b/fs/nfsd/fault_inject.c >> @@ -0,0 +1,94 @@ >> +/* >> + * Copyright (c) 2011 Bryan Schumaker <bjschuma@xxxxxxxxxx> >> + * >> + * Uses debugfs to create fault injection points for client testing >> + */ >> + >> +#include <linux/types.h> >> +#include <linux/fs.h> >> +#include <linux/debugfs.h> >> +#include <linux/module.h> >> + >> +#include "state.h" >> +#include "fault_inject.h" >> + >> +struct nfsd_fault_inject_op { >> + char *action; >> + char *item; >> + char *file; >> + int file_data; >> + void (*func)(u64); >> +}; >> + >> +#define INJECTION_OP(op_action, op_item, op_func) \ >> +{ \ >> + .action = op_action, \ >> + .item = op_item, \ >> + .file = op_action"_"op_item, \ >> + .func = op_func, \ >> +} >> + >> +static struct nfsd_fault_inject_op inject_ops[] = { >> + INJECTION_OP("forget", "clients", nfsd_forget_clients), >> + INJECTION_OP("forget", "locks", nfsd_forget_locks), >> + INJECTION_OP("forget", "openowners", nfsd_forget_openowners), >> + INJECTION_OP("forget", "delegations", nfsd_forget_delegations), >> + INJECTION_OP("recall", "delegations", nfsd_recall_delegations), > > This is a little clever for my taste.... Could we just do > > static struct nfsd_fault_inject_op inject_ops[] = { > { > .file = "forget_client", > .op = nfsd_forget_clients, > }, > ... > } > > and do away with the separate item and action fields? > > I'd rather be sort of obvious and boring even if it's slightly less > compact. > I was going for compact when I initially wrote this, but I can change it. I have them as separate fields so I can print out slightly different messages based on what is going on. Such as: "NFSD: Server forgetting all clients" or "NFSD: Server recalling at most 4 delegations". >> +}; >> + >> +static long int NUM_INJECT_OPS = sizeof(inject_ops) / sizeof(struct nfsd_fault_inject_op); >> +static struct dentry *debug_dir; >> + >> +static int nfsd_inject_set(void *data, u64 val) >> +{ >> + int i; >> + struct nfsd_fault_inject_op *op; >> + >> + for (i = 0; i < NUM_INJECT_OPS; i++) { >> + op = &inject_ops[i]; >> + if (&op->file_data == data) { > > Huh, OK, so if I understand right, the contents of file_data doesn't > matter, you're just using a pointer to that field as a way to identify > the op array. > > But then couldn't you just pass in a pointer to the op itself: > >> + for (i = 0; i < NUM_INJECT_OPS; i++) { >> + op = &inject_ops[i]; >> + debugfs_create_file(op->file, mode, debug_dir, &op->file_data, &fops_nfsd); > > like: > > debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd); > > and eliminate the file_data field? I've never thought about trying it that way, but it seems fairly straightforward. I'll try it that way and see if it works! > > Patches look OK otherwise on a quick skim, thanks. > > --b. > > >> + } >> + return 0; -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
