[pnfs] [PATCH 6/8] nfsd: dprint operation names

Benny Halevy bhalevy at panasas.com
Tue Jul 1 09:47:37 EDT 2008


On Jun. 30, 2008, 23:26 +0300, "J. Bruce Fields" <bfields at fieldses.org> wrote:
> On Mon, Jun 30, 2008 at 04:22:41PM -0400, bfields wrote:
>> On Mon, Jun 30, 2008 at 11:16:16PM +0300, Benny Halevy wrote:
>>> J. Bruce Fields wrote:
>>>> On Mon, Jun 30, 2008 at 09:08:15PM +0300, Benny Halevy wrote:
>>>>> Signed-off-by: Benny Halevy <bhalevy at panasas.com>
>>>>> ---
>>>>>  fs/nfsd/nfs4proc.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>>  1 files changed, 56 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>>>> index f3226c0..28ae1dc 100644
>>>>> --- a/fs/nfsd/nfs4proc.c
>>>>> +++ b/fs/nfsd/nfs4proc.c
>>>>> @@ -51,6 +51,59 @@
>>>>>  
>>>>>  #define NFSDDBG_FACILITY		NFSDDBG_PROC
>>>>>  
>>>>> +#ifdef OP
>>>>> +#error OP is defined
>>>>> +#endif
>>>>> +#define OP(op) [OP_ ## op] = #op
>>>>> +char *nfsd4_op_names[] = {
>>>>> +	OP(ACCESS),
>>>> I'm not sure how the macro helps--wouldn't
>>>>
>>>> 	"OP_ACCESS"
>>>>
>>>> be just as nice here?
>>> Do you mean, use no macro and code this as
>>> 	[OP_ACCESS] = "OP_ACCESS",
>>>
>>> or define OP as follows?
>>> #define OP(op) [OP_ ## op] = "OP_" #op
>>>
>>> I'm OK either way...
>> Cripes--I missed the need for the index on the left side of the
>> assignment.  OK, makes sense!
>>
>> The #ifdef/#error seems like mild paranoia--we use similarly generic
>> macro names ("PROC") locally elsewhere and it hasn't caused problem--but
>> I don't really have a problem with it if you prefer it that way.
> 
> Hm. If we're sharing nfsd4_ops array with v4 anyway, I wonder if it'd
> make sense just to add an op_name field to the nfsd4_operation struct
> instead of defining a separate array?
> 
> (But if we do that I'd rather avoid the temptation to create another
> macro like PROC()--that confuses everyone who greps for the caller of
> one of these functions.)

This turned out to be quite nice.
See following patch.
And thanks for your comments!

Benny

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index f3226c0..e9d41f8 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -843,10 +843,13 @@ struct nfsd4_operation {
 #define ALLOWED_WITHOUT_FH 1
 /* GETATTR and ops not listed as returning NFS4ERR_MOVED: */
 #define ALLOWED_ON_ABSENT_FS 2
+	char *op_name;
 };
 
 static struct nfsd4_operation nfsd4_ops[];
 
+static inline char *nfsd4_op_name(unsigned opnum);
+
 /*
  * COMPOUND call.
  */
@@ -888,7 +891,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
 	while (!status && resp->opcnt < args->opcnt) {
 		op = &args->ops[resp->opcnt++];
 
-		dprintk("nfsv4 compound op #%d: %d\n", resp->opcnt, op->opnum);
+		dprintk("nfsv4 compound op #%d/%d: %d (%s)\n",
+			resp->opcnt, args->opcnt, op->opnum,
+			nfsd4_op_name(op->opnum));
 
 		/*
 		 * The XDR decode routines may have pre-set op->status;
@@ -963,120 +968,163 @@ out:
 static struct nfsd4_operation nfsd4_ops[OP_RELEASE_LOCKOWNER+1] = {
 	[OP_ACCESS] = {
 		.op_func = (nfsd4op_func)nfsd4_access,
+		.op_name = "OP_ACCESS",
 	},
 	[OP_CLOSE] = {
 		.op_func = (nfsd4op_func)nfsd4_close,
+		.op_name = "OP_CLOSE",
 	},
 	[OP_COMMIT] = {
 		.op_func = (nfsd4op_func)nfsd4_commit,
+		.op_name = "OP_COMMIT",
 	},
 	[OP_CREATE] = {
 		.op_func = (nfsd4op_func)nfsd4_create,
+		.op_name = "OP_CREATE",
 	},
 	[OP_DELEGRETURN] = {
 		.op_func = (nfsd4op_func)nfsd4_delegreturn,
+		.op_name = "OP_DELEGRETURN",
 	},
 	[OP_GETATTR] = {
 		.op_func = (nfsd4op_func)nfsd4_getattr,
 		.op_flags = ALLOWED_ON_ABSENT_FS,
+		.op_name = "OP_GETATTR",
 	},
 	[OP_GETFH] = {
 		.op_func = (nfsd4op_func)nfsd4_getfh,
+		.op_name = "OP_GETFH",
 	},
 	[OP_LINK] = {
 		.op_func = (nfsd4op_func)nfsd4_link,
+		.op_name = "OP_LINK",
 	},
 	[OP_LOCK] = {
 		.op_func = (nfsd4op_func)nfsd4_lock,
+		.op_name = "OP_LOCK",
 	},
 	[OP_LOCKT] = {
 		.op_func = (nfsd4op_func)nfsd4_lockt,
+		.op_name = "OP_LOCKT",
 	},
 	[OP_LOCKU] = {
 		.op_func = (nfsd4op_func)nfsd4_locku,
+		.op_name = "OP_LOCKU",
 	},
 	[OP_LOOKUP] = {
 		.op_func = (nfsd4op_func)nfsd4_lookup,
+		.op_name = "OP_LOOKUP",
 	},
 	[OP_LOOKUPP] = {
 		.op_func = (nfsd4op_func)nfsd4_lookupp,
+		.op_name = "OP_LOOKUPP",
 	},
 	[OP_NVERIFY] = {
 		.op_func = (nfsd4op_func)nfsd4_nverify,
+		.op_name = "OP_NVERIFY",
 	},
 	[OP_OPEN] = {
 		.op_func = (nfsd4op_func)nfsd4_open,
+		.op_name = "OP_OPEN",
 	},
 	[OP_OPEN_CONFIRM] = {
 		.op_func = (nfsd4op_func)nfsd4_open_confirm,
+		.op_name = "OP_OPEN_CONFIRM",
 	},
 	[OP_OPEN_DOWNGRADE] = {
 		.op_func = (nfsd4op_func)nfsd4_open_downgrade,
+		.op_name = "OP_OPEN_DOWNGRADE",
 	},
 	[OP_PUTFH] = {
 		.op_func = (nfsd4op_func)nfsd4_putfh,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_name = "OP_PUTFH",
 	},
 	[OP_PUTPUBFH] = {
-		/* unsupported; just for future reference: */
+		/* unsupported, just for future reference: */
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_name = "OP_PUTPUBFH",
 	},
 	[OP_PUTROOTFH] = {
 		.op_func = (nfsd4op_func)nfsd4_putrootfh,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_name = "OP_PUTROOTFH",
 	},
 	[OP_READ] = {
 		.op_func = (nfsd4op_func)nfsd4_read,
+		.op_name = "OP_READ",
 	},
 	[OP_READDIR] = {
 		.op_func = (nfsd4op_func)nfsd4_readdir,
+		.op_name = "OP_READDIR",
 	},
 	[OP_READLINK] = {
 		.op_func = (nfsd4op_func)nfsd4_readlink,
+		.op_name = "OP_READLINK",
 	},
 	[OP_REMOVE] = {
 		.op_func = (nfsd4op_func)nfsd4_remove,
+		.op_name = "OP_REMOVE",
 	},
 	[OP_RENAME] = {
+		.op_name = "OP_RENAME",
 		.op_func = (nfsd4op_func)nfsd4_rename,
 	},
 	[OP_RENEW] = {
 		.op_func = (nfsd4op_func)nfsd4_renew,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_name = "OP_RENEW",
 	},
 	[OP_RESTOREFH] = {
 		.op_func = (nfsd4op_func)nfsd4_restorefh,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_name = "OP_RESTOREFH",
 	},
 	[OP_SAVEFH] = {
 		.op_func = (nfsd4op_func)nfsd4_savefh,
+		.op_name = "OP_SAVEFH",
 	},
 	[OP_SECINFO] = {
 		.op_func = (nfsd4op_func)nfsd4_secinfo,
+		.op_name = "OP_SECINFO",
 	},
 	[OP_SETATTR] = {
 		.op_func = (nfsd4op_func)nfsd4_setattr,
+		.op_name = "OP_SETATTR",
 	},
 	[OP_SETCLIENTID] = {
 		.op_func = (nfsd4op_func)nfsd4_setclientid,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_name = "OP_SETCLIENTID",
 	},
 	[OP_SETCLIENTID_CONFIRM] = {
 		.op_func = (nfsd4op_func)nfsd4_setclientid_confirm,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_name = "OP_SETCLIENTID_CONFIRM",
 	},
 	[OP_VERIFY] = {
 		.op_func = (nfsd4op_func)nfsd4_verify,
+		.op_name = "OP_VERIFY",
 	},
 	[OP_WRITE] = {
 		.op_func = (nfsd4op_func)nfsd4_write,
+		.op_name = "OP_WRITE",
 	},
 	[OP_RELEASE_LOCKOWNER] = {
 		.op_func = (nfsd4op_func)nfsd4_release_lockowner,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS,
+		.op_name = "OP_RELEASE_LOCKOWNER",
 	},
 };
 
+static inline char *
+nfsd4_op_name(unsigned opnum)
+{
+	if (opnum < ARRAY_SIZE(nfsd4_ops))
+		return nfsd4_ops[opnum].op_name;
+	return "unknown_operation";
+}
+
 #define nfs4svc_decode_voidargs		NULL
 #define nfs4svc_release_void		NULL
 #define nfsd4_voidres			nfsd4_voidargs
-- 
1.5.6.GIT


> 
> --b.
> 
>> --b.
>>
>>> Benny
>>>
>>>> --b.
>>>>
>>>>> +	OP(CLOSE),
>>>>> +	OP(COMMIT),
>>>>> +	OP(CREATE),
>>>>> +	OP(DELEGPURGE),
>>>>> +	OP(DELEGRETURN),
>>>>> +	OP(GETATTR),
>>>>> +	OP(GETFH),
>>>>> +	OP(LINK),
>>>>> +	OP(LOCK),
>>>>> +	OP(LOCKT),
>>>>> +	OP(LOCKU),
>>>>> +	OP(LOOKUP),
>>>>> +	OP(LOOKUPP),
>>>>> +	OP(NVERIFY),
>>>>> +	OP(OPEN),
>>>>> +	OP(OPENATTR),
>>>>> +	OP(OPEN_CONFIRM),
>>>>> +	OP(OPEN_DOWNGRADE),
>>>>> +	OP(PUTFH),
>>>>> +	OP(PUTPUBFH),
>>>>> +	OP(PUTROOTFH),
>>>>> +	OP(READ),
>>>>> +	OP(READDIR),
>>>>> +	OP(READLINK),
>>>>> +	OP(REMOVE),
>>>>> +	OP(RENAME),
>>>>> +	OP(RENEW),
>>>>> +	OP(RESTOREFH),
>>>>> +	OP(SAVEFH),
>>>>> +	OP(SECINFO),
>>>>> +	OP(SETATTR),
>>>>> +	OP(SETCLIENTID),
>>>>> +	OP(SETCLIENTID_CONFIRM),
>>>>> +	OP(VERIFY),
>>>>> +	OP(WRITE),
>>>>> +	OP(RELEASE_LOCKOWNER),
>>>>> +};
>>>>> +#undef OP
>>>>> +
>>>>> +static inline char *
>>>>> +nfsd4_op_name(unsigned opnum)
>>>>> +{
>>>>> +	if (opnum < ARRAY_SIZE(nfsd4_op_names))
>>>>> +		return nfsd4_op_names[opnum];
>>>>> +	return "unknown_operation";
>>>>> +}
>>>>> +
>>>>>  static inline void
>>>>>  fh_dup2(struct svc_fh *dst, struct svc_fh *src)
>>>>>  {
>>>>> @@ -888,7 +941,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
>>>>>  	while (!status && resp->opcnt < args->opcnt) {
>>>>>  		op = &args->ops[resp->opcnt++];
>>>>>  
>>>>> -		dprintk("nfsv4 compound op #%d: %d\n", resp->opcnt, op->opnum);
>>>>> +		dprintk("nfsv4 compound op #%d/%d: %d (%s)\n",
>>>>> +			resp->opcnt, args->opcnt, op->opnum,
>>>>> +			nfsd4_op_name(op->opnum));
>>>>>  
>>>>>  		/*
>>>>>  		 * The XDR decode routines may have pre-set op->status;
>>>>> -- 
>>>>> 1.5.6.GIT
>>>>>



More information about the pNFS mailing list