Return-Path: linux-nfs-owner@vger.kernel.org Received: from mail-wg0-f49.google.com ([74.125.82.49]:56488 "EHLO mail-wg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751067AbaKXTU1 (ORCPT ); Mon, 24 Nov 2014 14:20:27 -0500 Received: by mail-wg0-f49.google.com with SMTP id x12so13148134wgg.36 for ; Mon, 24 Nov 2014 11:20:26 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20141028143047.5c1fca24@tlielax.poochiereds.net> References: <1414520695-1701-1-git-send-email-jlayton@primarydata.com> <20141028143047.5c1fca24@tlielax.poochiereds.net> Date: Mon, 24 Nov 2014 14:20:25 -0500 Message-ID: Subject: Re: [PATCH] sunrpc: add debugfs file for displaying client rpc_task queue From: Trond Myklebust To: Jeff Layton Cc: Linux NFS Mailing List Content-Type: text/plain; charset=UTF-8 Sender: linux-nfs-owner@vger.kernel.org List-ID: On Tue, Oct 28, 2014 at 2:30 PM, Jeff Layton wrote: > On Tue, 28 Oct 2014 14:24:55 -0400 > Jeff Layton wrote: > >> It's possible to get a dump of the RPC task queue by writing a value to >> /proc/sys/sunrpc/rpc_debug. If you write any value to that file, you get >> a dump of the RPC client task list into the log buffer. This is a rather >> inconvenient interface however, and makes it hard to get immediate info >> about the task queue. >> >> Add a new file under debugfs that shows similar info. There are some >> small differences -- we avoid printing raw kernel addresses in favor of >> symbolic names and the XID is also displayed. >> >> Signed-off-by: Jeff Layton >> --- >> include/linux/sunrpc/clnt.h | 1 + >> include/linux/sunrpc/debug.h | 13 ++++ >> net/sunrpc/Kconfig | 1 + >> net/sunrpc/Makefile | 1 + >> net/sunrpc/clnt.c | 3 +- >> net/sunrpc/debugfs.c | 153 +++++++++++++++++++++++++++++++++++++++++++ >> net/sunrpc/sunrpc_syms.c | 8 +++ >> 7 files changed, 179 insertions(+), 1 deletion(-) >> create mode 100644 net/sunrpc/debugfs.c >> >> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h >> index 70736b98c721..265d40bd3109 100644 >> --- a/include/linux/sunrpc/clnt.h >> +++ b/include/linux/sunrpc/clnt.h >> @@ -176,5 +176,6 @@ size_t rpc_peeraddr(struct rpc_clnt *, struct sockaddr *, size_t); >> const char *rpc_peeraddr2str(struct rpc_clnt *, enum rpc_display_format_t); >> int rpc_localaddr(struct rpc_clnt *, struct sockaddr *, size_t); >> >> +const char *rpc_proc_name(const struct rpc_task *task); >> #endif /* __KERNEL__ */ >> #endif /* _LINUX_SUNRPC_CLNT_H */ >> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h >> index 9385bd74c860..7c1ef9530087 100644 >> --- a/include/linux/sunrpc/debug.h >> +++ b/include/linux/sunrpc/debug.h >> @@ -68,6 +68,19 @@ extern unsigned int nlm_debug; >> #ifdef RPC_DEBUG >> void rpc_register_sysctl(void); >> void rpc_unregister_sysctl(void); >> +int sunrpc_debugfs_init(void); >> +void sunrpc_debugfs_exit(void); >> +#else >> +static inline int >> +sunrpc_debugfs_init(void) >> +{ >> + return 0; >> +} >> +static inline void >> +sunrpc_debugfs_exit(void) >> +{ >> + return; >> +} >> #endif >> >> #endif /* _LINUX_SUNRPC_DEBUG_H_ */ >> diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig >> index 0754d0f466d2..fb78117b896c 100644 >> --- a/net/sunrpc/Kconfig >> +++ b/net/sunrpc/Kconfig >> @@ -35,6 +35,7 @@ config RPCSEC_GSS_KRB5 >> config SUNRPC_DEBUG >> bool "RPC: Enable dprintk debugging" >> depends on SUNRPC && SYSCTL >> + select DEBUG_FS >> help >> This option enables a sysctl-based debugging interface >> that is be used by the 'rpcdebug' utility to turn on or off >> diff --git a/net/sunrpc/Makefile b/net/sunrpc/Makefile >> index e5a7a1cac8f3..15e6f6c23c5d 100644 >> --- a/net/sunrpc/Makefile >> +++ b/net/sunrpc/Makefile >> @@ -14,6 +14,7 @@ sunrpc-y := clnt.o xprt.o socklib.o xprtsock.o sched.o \ >> addr.o rpcb_clnt.o timer.o xdr.o \ >> sunrpc_syms.o cache.o rpc_pipe.o \ >> svc_xprt.o >> +sunrpc-$(CONFIG_SUNRPC_DEBUG) += debugfs.o >> sunrpc-$(CONFIG_SUNRPC_BACKCHANNEL) += backchannel_rqst.o bc_svc.o >> sunrpc-$(CONFIG_PROC_FS) += stats.o >> sunrpc-$(CONFIG_SYSCTL) += sysctl.o >> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c >> index 9acd6ce88db7..5b2e2d3d37c1 100644 >> --- a/net/sunrpc/clnt.c >> +++ b/net/sunrpc/clnt.c >> @@ -1397,7 +1397,8 @@ rpc_restart_call(struct rpc_task *task) >> EXPORT_SYMBOL_GPL(rpc_restart_call); >> >> #ifdef RPC_DEBUG >> -static const char *rpc_proc_name(const struct rpc_task *task) >> +const char >> +*rpc_proc_name(const struct rpc_task *task) >> { >> const struct rpc_procinfo *proc = task->tk_msg.rpc_proc; >> >> diff --git a/net/sunrpc/debugfs.c b/net/sunrpc/debugfs.c >> new file mode 100644 >> index 000000000000..48769338172e >> --- /dev/null >> +++ b/net/sunrpc/debugfs.c >> @@ -0,0 +1,153 @@ >> +/** >> + * debugfs interface for sunrpc >> + * >> + * (c) 2014 Jeff Layton >> + */ >> + >> +#include >> +#include >> +#include >> +#include "netns.h" >> + >> +static struct dentry *topdir; >> + >> +static int >> +tasks_show(struct seq_file *f, void *v) >> +{ >> + u32 xid = 0; >> + struct rpc_task *task = v; >> + struct rpc_clnt *clnt = task->tk_client; >> + const char *rpc_waitq = "none"; >> + >> + if (RPC_IS_QUEUED(task)) >> + rpc_waitq = rpc_qname(task->tk_waitqueue); >> + >> + if (task->tk_rqstp) >> + xid = be32_to_cpu(task->tk_rqstp->rq_xid); >> + >> + seq_printf(f, "%5u %04x %6d 0x%x 0x%x %8ld %ps %sv%u %s a:%ps q:%s\n", >> + task->tk_pid, task->tk_flags, task->tk_status, >> + clnt->cl_clid, xid, task->tk_timeout, task->tk_ops, >> + clnt->cl_program->name, clnt->cl_vers, rpc_proc_name(task), >> + task->tk_action, rpc_waitq); >> + return 0; >> +} >> + >> +static void * >> +tasks_start(struct seq_file *f, loff_t *ppos) >> + __acquires(&sn->rpc_client_lock) >> + __acquires(&clnt->cl_lock) >> +{ >> + loff_t *p = f->private, pos = *ppos; >> + struct rpc_clnt *clnt; >> + struct rpc_task *task; >> + struct sunrpc_net *sn = net_generic(current->nsproxy->net_ns, >> + sunrpc_net_id); >> + >> + *p = pos + 1; >> + spin_lock(&sn->rpc_client_lock); >> + list_for_each_entry(clnt, &sn->all_clients, cl_clients) { >> + spin_lock(&clnt->cl_lock); >> + list_for_each_entry(task, &clnt->cl_tasks, tk_task) >> + if (pos-- == 0) >> + return task; >> + spin_unlock(&clnt->cl_lock); >> + } >> + return NULL; >> +} Instead of having a single pseudofile that iterates through all rpc_clients and all rpc_tasks, should we perhaps have a hierarchy of subdirectories? I'm thinking we might want a directory per rpc_client, each containing a pseudo-file that iterates through all rpc_tasks for that rpc_client only. The reason for doing so would be so that we can add pseudofiles to display more per-rpc_client debugging info, such as cl_nodename, cl_program->name, cl_prog, cl_vers. Maybe also with a symlink to another subdirectory that displays per-xprt debugging information? i.e. a hierarchy of the form debugfs/ rpc_clients/ 1/ cl_tasks cl_xprt->../../rpc_xprt/1 ..... 2/ cl_tasks cl_xprt->../../rpc_xprt/1 ..... rpc_xprt/ 1/ servername ..... -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@primarydata.com