2007-05-18 17:45:58

by Tom Tucker

[permalink] [raw]
Subject: [RFC, PATCH 9/15] knfsd: add RDMA debug class to rpc_debug bitfield


Add a bit to the rpc_debug bit field for RDMA transport
debug messages and sysctl definitions for configuration
and statistics files in /proc/sys/sunrpc/svc_rdma.

Signed-off-by: Tom Tucker <[email protected]>
---

include/linux/sunrpc/debug.h | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
index fc3a5dd..9bf7381 100644
--- a/include/linux/sunrpc/debug.h
+++ b/include/linux/sunrpc/debug.h
@@ -24,6 +24,7 @@ #define RPCDBG_SVCSOCK 0x0100
#define RPCDBG_SVCDSP 0x0200
#define RPCDBG_MISC 0x0400
#define RPCDBG_CACHE 0x0800
+#define RPCDBG_RDMA 0x1000
#define RPCDBG_ALL 0x7fff

#ifdef __KERNEL__
@@ -93,6 +94,20 @@ enum {
CTL_RDMA_MAXINLINEWRITE,
CTL_RDMA_WRITEPADDING,
CTL_RDMA_MEMREG,
+ CTL_SVCRDMA,
+ CTL_RDMA_MAX_REQUESTS,
+ CTL_RDMA_MAX_REQ_SIZE,
+ CTL_RDMA_ORD,
+ CTL_RDMA_READ_THROTTLE,
+ CTL_RDMA_STAT_RECV,
+ CTL_RDMA_STAT_READ,
+ CTL_RDMA_STAT_WRITE,
+ CTL_RDMA_STAT_SQ_STARVE,
+ CTL_RDMA_STAT_RQ_STARVE,
+ CTL_RDMA_STAT_RQ_POLL,
+ CTL_RDMA_STAT_RQ_PROD,
+ CTL_RDMA_STAT_SQ_POLL,
+ CTL_RDMA_STAT_SQ_PROD
};

#endif /* _LINUX_SUNRPC_DEBUG_H_ */


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs


2007-05-18 18:57:48

by Tom Tucker

[permalink] [raw]
Subject: Re: [RFC, PATCH 9/15] knfsd: add RDMA debug class to rpc_debug bitfield

On Fri, 2007-05-18 at 14:52 -0400, Chuck Lever wrote:
> Hi Tom-
>
> On the client side, I added a generic debug flag for transports, not a
> specific flag for sockets. Would you consider doing the same here?

Right. That's what I should do here too.

Thanks,
>
> Tom Tucker wrote:
> > Add a bit to the rpc_debug bit field for RDMA transport
> > debug messages and sysctl definitions for configuration
> > and statistics files in /proc/sys/sunrpc/svc_rdma.
> >
> > Signed-off-by: Tom Tucker <[email protected]>
> > ---
> >
> > include/linux/sunrpc/debug.h | 15 +++++++++++++++
> > 1 files changed, 15 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> > index fc3a5dd..9bf7381 100644
> > --- a/include/linux/sunrpc/debug.h
> > +++ b/include/linux/sunrpc/debug.h
> > @@ -24,6 +24,7 @@ #define RPCDBG_SVCSOCK 0x0100
> > #define RPCDBG_SVCDSP 0x0200
> > #define RPCDBG_MISC 0x0400
> > #define RPCDBG_CACHE 0x0800
> > +#define RPCDBG_RDMA 0x1000
> > #define RPCDBG_ALL 0x7fff
> >
> > #ifdef __KERNEL__
> > @@ -93,6 +94,20 @@ enum {
> > CTL_RDMA_MAXINLINEWRITE,
> > CTL_RDMA_WRITEPADDING,
> > CTL_RDMA_MEMREG,
> > + CTL_SVCRDMA,
> > + CTL_RDMA_MAX_REQUESTS,
> > + CTL_RDMA_MAX_REQ_SIZE,
> > + CTL_RDMA_ORD,
> > + CTL_RDMA_READ_THROTTLE,
> > + CTL_RDMA_STAT_RECV,
> > + CTL_RDMA_STAT_READ,
> > + CTL_RDMA_STAT_WRITE,
> > + CTL_RDMA_STAT_SQ_STARVE,
> > + CTL_RDMA_STAT_RQ_STARVE,
> > + CTL_RDMA_STAT_RQ_POLL,
> > + CTL_RDMA_STAT_RQ_PROD,
> > + CTL_RDMA_STAT_SQ_POLL,
> > + CTL_RDMA_STAT_SQ_PROD
> > };
> >
> > #endif /* _LINUX_SUNRPC_DEBUG_H_ */
> >
> >
> > -------------------------------------------------------------------------
> > This SF.net email is sponsored by DB2 Express
> > Download DB2 Express C - the FREE version of DB2 express and take
> > control of your XML. No limits. Just data. Click to get it now.
> > http://sourceforge.net/powerbar/db2/
> > _______________________________________________
> > NFS maillist - [email protected]
> > https://lists.sourceforge.net/lists/listinfo/nfs
>


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-05-21 03:19:50

by Greg Banks

[permalink] [raw]
Subject: Re: [RFC, PATCH 9/15] knfsd: add RDMA debug class to rpc_debug bitfield

On Fri, May 18, 2007 at 12:45:45PM -0500, Tom Tucker wrote:
>
> Add a bit to the rpc_debug bit field for RDMA transport
> debug messages and sysctl definitions for configuration
> and statistics files in /proc/sys/sunrpc/svc_rdma.
>
> Signed-off-by: Tom Tucker <[email protected]>
> ---
>
> include/linux/sunrpc/debug.h | 15 +++++++++++++++
> 1 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> index fc3a5dd..9bf7381 100644
> --- a/include/linux/sunrpc/debug.h
> +++ b/include/linux/sunrpc/debug.h
> @@ -24,6 +24,7 @@ #define RPCDBG_SVCSOCK 0x0100
> #define RPCDBG_SVCDSP 0x0200
> #define RPCDBG_MISC 0x0400
> #define RPCDBG_CACHE 0x0800
> +#define RPCDBG_RDMA 0x1000
> #define RPCDBG_ALL 0x7fff

Fine. Did you update the rpc_debug program in nfs-utils, which also
knows about these debug bits?

> #ifdef __KERNEL__
> @@ -93,6 +94,20 @@ enum {
> CTL_RDMA_MAXINLINEWRITE,
> CTL_RDMA_WRITEPADDING,
> CTL_RDMA_MEMREG,
> + CTL_SVCRDMA,
> + CTL_RDMA_MAX_REQUESTS,
> + CTL_RDMA_MAX_REQ_SIZE,
> + CTL_RDMA_ORD,
> + CTL_RDMA_READ_THROTTLE,
> + CTL_RDMA_STAT_RECV,
> + CTL_RDMA_STAT_READ,
> + CTL_RDMA_STAT_WRITE,
> + CTL_RDMA_STAT_SQ_STARVE,
> + CTL_RDMA_STAT_RQ_STARVE,
> + CTL_RDMA_STAT_RQ_POLL,
> + CTL_RDMA_STAT_RQ_PROD,
> + CTL_RDMA_STAT_SQ_POLL,
> + CTL_RDMA_STAT_SQ_PROD
> };
>
> #endif /* _LINUX_SUNRPC_DEBUG_H_ */

This is fine, but one tiny nit: you have the makings of two patches
here. This hunk belongs with the patch that uses the new defines
and registers the sysctls.

Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
Apparently, I'm Bedevere. Which MPHG character are you?
I don't speak for SGI.

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-05-21 03:26:06

by Tom Tucker

[permalink] [raw]
Subject: Re: [RFC, PATCH 9/15] knfsd: add RDMA debug class to rpc_debug bitfield


BTW, Chuck Lever also pointed out that we should probably have a generic
RPCDBG_XPRT debug flag instead of RPCDBG_RDMA. I think he's right. I think
this should be part of the transport patchset that you're working on. Do you
agree?

Tom

On 5/20/07 10:19 PM, "Greg Banks" <[email protected]> wrote:

> On Fri, May 18, 2007 at 12:45:45PM -0500, Tom Tucker wrote:
>>
>> Add a bit to the rpc_debug bit field for RDMA transport
>> debug messages and sysctl definitions for configuration
>> and statistics files in /proc/sys/sunrpc/svc_rdma.
>>
>> Signed-off-by: Tom Tucker <[email protected]>
>> ---
>>
>> include/linux/sunrpc/debug.h | 15 +++++++++++++++
>> 1 files changed, 15 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
>> index fc3a5dd..9bf7381 100644
>> --- a/include/linux/sunrpc/debug.h
>> +++ b/include/linux/sunrpc/debug.h
>> @@ -24,6 +24,7 @@ #define RPCDBG_SVCSOCK 0x0100
>> #define RPCDBG_SVCDSP 0x0200
>> #define RPCDBG_MISC 0x0400
>> #define RPCDBG_CACHE 0x0800
>> +#define RPCDBG_RDMA 0x1000
>> #define RPCDBG_ALL 0x7fff
>
> Fine. Did you update the rpc_debug program in nfs-utils, which also
> knows about these debug bits?
>
>> #ifdef __KERNEL__
>> @@ -93,6 +94,20 @@ enum {
>> CTL_RDMA_MAXINLINEWRITE,
>> CTL_RDMA_WRITEPADDING,
>> CTL_RDMA_MEMREG,
>> + CTL_SVCRDMA,
>> + CTL_RDMA_MAX_REQUESTS,
>> + CTL_RDMA_MAX_REQ_SIZE,
>> + CTL_RDMA_ORD,
>> + CTL_RDMA_READ_THROTTLE,
>> + CTL_RDMA_STAT_RECV,
>> + CTL_RDMA_STAT_READ,
>> + CTL_RDMA_STAT_WRITE,
>> + CTL_RDMA_STAT_SQ_STARVE,
>> + CTL_RDMA_STAT_RQ_STARVE,
>> + CTL_RDMA_STAT_RQ_POLL,
>> + CTL_RDMA_STAT_RQ_PROD,
>> + CTL_RDMA_STAT_SQ_POLL,
>> + CTL_RDMA_STAT_SQ_PROD
>> };
>>
>> #endif /* _LINUX_SUNRPC_DEBUG_H_ */
>
> This is fine, but one tiny nit: you have the makings of two patches
> here. This hunk belongs with the patch that uses the new defines
> and registers the sysctls.
>
> Greg.



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2007-05-21 04:16:23

by Greg Banks

[permalink] [raw]
Subject: Re: [RFC, PATCH 9/15] knfsd: add RDMA debug class to rpc_debug bitfield

On Sun, May 20, 2007 at 10:31:15PM -0500, Tom Tucker wrote:
>
> BTW, Chuck Lever also pointed out that we should probably have a generic
> RPCDBG_XPRT debug flag instead of RPCDBG_RDMA.

So he did. Serves me right for not reading all my email before
replying.

> I think he's right. I think
> this should be part of the transport patchset that you're working on. Do you
> agree?

I'm a little confused as to precisely what is suggested.

Currently we have an existing RPCDBG_SVCSOCK which enables dprintks
in both the generic transport code and in the sockets-specific code
too; this is hard to separate because all the code is intermingled in
the one file svcsock.c.

If the suggestion is to have separate bits for the sockets-specific
and the generic code, then the two painful alternatives seem to be

a) splitting the sockets and generic code into separate .c files, or

b) changing half the dprintk()s in svcsock.c to dfprintks().

I wasn't planning to do either of those anytime soon; either would
be a significant change.

I think the ideal case would be a total of 3 debug bits: one for the
generic transport code, one for the sockets transport-specific code and
one for the RDMA transport-specific code. Getting there isn't easy.

However given that the new svc_rdma* code is large and in separate
files, giving it it's own RPCDBG_RDMA bit, as you've already done,
is easy and useful and a step in the right direction.

In any case I'm happy to do whatever is necessary in the transport
patchset, but I'd appreciate Neil's guidance on which approach he'd
prefer before I go in with the scalpel. Neil?

Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
Apparently, I'm Bedevere. Which MPHG character are you?
I don't speak for SGI.

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs