From: Jeff Layton <jlayton@redhat.com>
To: Christoph Hellwig <hch@lst.de>,
Trond Myklebust <trond.myklebust@primarydata.com>,
Anna Schumaker <anna.schumaker@netapp.com>,
"J. Bruce Fields" <bfields@fieldses.org>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH 15/33] sunrpc: move p_count out of struct rpc_procinfo
Date: Fri, 12 May 2017 14:24:36 -0400 [thread overview]
Message-ID: <1494613476.4227.3.camel@redhat.com> (raw)
In-Reply-To: <20170512161701.22468-16-hch@lst.de>
On Fri, 2017-05-12 at 18:16 +0200, Christoph Hellwig wrote:
> p_count is the only writeable memeber of struct rpc_procinfo, which is
> a good candidate to be const-ified as it contains function pointers.
>
> This patch moves it into out out struct rpc_procinfo, and into a
> separate writable array that is pointed to by struct rpc_version and
> indexed by p_statidx.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/lockd/clnt4xdr.c | 2 ++
> fs/lockd/clntxdr.c | 4 ++++
> fs/lockd/mon.c | 4 +++-
> fs/nfs/mount_clnt.c | 5 ++++-
> fs/nfs/nfs2xdr.c | 4 +++-
> fs/nfs/nfs3xdr.c | 6 +++++-
> fs/nfs/nfs4xdr.c | 4 +++-
> fs/nfsd/nfs4callback.c | 4 +++-
> include/linux/sunrpc/clnt.h | 2 +-
> net/sunrpc/auth_gss/gss_rpc_upcall.c | 3 ++-
> net/sunrpc/clnt.c | 6 ++++--
> net/sunrpc/rpcb_clnt.c | 12 +++++++++---
> net/sunrpc/stats.c | 3 +--
> 13 files changed, 44 insertions(+), 15 deletions(-)
>
> diff --git a/fs/lockd/clnt4xdr.c b/fs/lockd/clnt4xdr.c
> index f0ab7a99dd23..7c255d1d7c64 100644
> --- a/fs/lockd/clnt4xdr.c
> +++ b/fs/lockd/clnt4xdr.c
> @@ -602,8 +602,10 @@ static struct rpc_procinfo nlm4_procedures[] = {
> PROC(GRANTED_RES, res, norep),
> };
>
> +static unsigned int nlm_version4_counts[ARRAY_SIZE(nlm4_procedures)];
> const struct rpc_version nlm_version4 = {
> .number = 4,
> .nrprocs = ARRAY_SIZE(nlm4_procedures),
> .procs = nlm4_procedures,
> + .counts = nlm_version4_counts,
> };
> diff --git a/fs/lockd/clntxdr.c b/fs/lockd/clntxdr.c
> index bd8a976785ae..39500c5743a5 100644
> --- a/fs/lockd/clntxdr.c
> +++ b/fs/lockd/clntxdr.c
> @@ -600,16 +600,20 @@ static struct rpc_procinfo nlm_procedures[] = {
> PROC(GRANTED_RES, res, norep),
> };
>
> +static unsigned int nlm_version1_counts[ARRAY_SIZE(nlm_procedures)];
> static const struct rpc_version nlm_version1 = {
> .number = 1,
> .nrprocs = ARRAY_SIZE(nlm_procedures),
> .procs = nlm_procedures,
> + .counts = nlm_version1_counts,
> };
>
> +static unsigned int nlm_version3_counts[ARRAY_SIZE(nlm_procedures)];
> static const struct rpc_version nlm_version3 = {
> .number = 3,
> .nrprocs = ARRAY_SIZE(nlm_procedures),
> .procs = nlm_procedures,
> + .counts = nlm_version3_counts,
> };
>
> static const struct rpc_version *nlm_versions[] = {
> diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
> index 62424e929a7f..fe4ec82764fe 100644
> --- a/fs/lockd/mon.c
> +++ b/fs/lockd/mon.c
> @@ -552,10 +552,12 @@ static struct rpc_procinfo nsm_procedures[] = {
> },
> };
>
> +static unsigned int nsm_version1_counts[ARRAY_SIZE(nsm_procedures)];
> static const struct rpc_version nsm_version1 = {
> .number = 1,
> .nrprocs = ARRAY_SIZE(nsm_procedures),
> - .procs = nsm_procedures
> + .procs = nsm_procedures,
> + .counts = nsm_version1_counts,
> };
>
> static const struct rpc_version *nsm_version[] = {
> diff --git a/fs/nfs/mount_clnt.c b/fs/nfs/mount_clnt.c
> index 806657d65074..d25914aa8bf9 100644
> --- a/fs/nfs/mount_clnt.c
> +++ b/fs/nfs/mount_clnt.c
> @@ -504,17 +504,20 @@ static struct rpc_procinfo mnt3_procedures[] = {
> },
> };
>
> -
> +static unsigned int mnt_counts[ARRAY_SIZE(mnt_procedures)];
> static const struct rpc_version mnt_version1 = {
> .number = 1,
> .nrprocs = ARRAY_SIZE(mnt_procedures),
> .procs = mnt_procedures,
> + .counts = mnt_counts,
> };
>
> +static unsigned int mnt3_counts[ARRAY_SIZE(mnt_procedures)];
> static const struct rpc_version mnt_version3 = {
> .number = 3,
> .nrprocs = ARRAY_SIZE(mnt3_procedures),
> .procs = mnt3_procedures,
> + .counts = mnt3_counts,
> };
>
> static const struct rpc_version *mnt_version[] = {
> diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
> index a299648ea321..16b4526299c1 100644
> --- a/fs/nfs/nfs2xdr.c
> +++ b/fs/nfs/nfs2xdr.c
> @@ -1170,8 +1170,10 @@ struct rpc_procinfo nfs_procedures[] = {
> PROC(STATFS, fhandle, statfsres, 0),
> };
>
> +static unsigned int nfs_version2_counts[ARRAY_SIZE(nfs_procedures)];
> const struct rpc_version nfs_version2 = {
> .number = 2,
> .nrprocs = ARRAY_SIZE(nfs_procedures),
> - .procs = nfs_procedures
> + .procs = nfs_procedures,
> + .counts = nfs_version2_counts,
> };
> diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
> index cc272eb8be3e..a017ec5c7a9d 100644
> --- a/fs/nfs/nfs3xdr.c
> +++ b/fs/nfs/nfs3xdr.c
> @@ -2578,10 +2578,12 @@ struct rpc_procinfo nfs3_procedures[] = {
> PROC(COMMIT, commit, commit, 5),
> };
>
> +static unsigned int nfs_version3_counts[ARRAY_SIZE(nfs3_procedures)];
> const struct rpc_version nfs_version3 = {
> .number = 3,
> .nrprocs = ARRAY_SIZE(nfs3_procedures),
> - .procs = nfs3_procedures
> + .procs = nfs3_procedures,
> + .counts = nfs_version3_counts,
> };
>
> #ifdef CONFIG_NFS_V3_ACL
> @@ -2606,10 +2608,12 @@ static struct rpc_procinfo nfs3_acl_procedures[] = {
> },
> };
>
> +static unsigned int nfs3_acl_counts[ARRAY_SIZE(nfs3_acl_procedures)];
> const struct rpc_version nfsacl_version3 = {
> .number = 3,
> .nrprocs = sizeof(nfs3_acl_procedures)/
> sizeof(nfs3_acl_procedures[0]),
> .procs = nfs3_acl_procedures,
> + .counts = nfs3_acl_counts,
> };
> #endif /* CONFIG_NFS_V3_ACL */
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 797f3ce75286..40cf5529e65f 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -7661,10 +7661,12 @@ struct rpc_procinfo nfs4_procedures[] = {
> #endif /* CONFIG_NFS_V4_2 */
> };
>
> +static unsigned int nfs_version4_counts[ARRAY_SIZE(nfs4_procedures)];
> const struct rpc_version nfs_version4 = {
> .number = 4,
> .nrprocs = ARRAY_SIZE(nfs4_procedures),
> - .procs = nfs4_procedures
> + .procs = nfs4_procedures,
> + .counts = nfs_version4_counts,
> };
>
> /*
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index a2bedbd05b2b..afa961fe073c 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -705,6 +705,7 @@ static struct rpc_procinfo nfs4_cb_procedures[] = {
> PROC(CB_NOTIFY_LOCK, COMPOUND, cb_notify_lock, cb_notify_lock),
> };
>
> +static unsigned int nfs4_cb_counts[ARRAY_SIZE(nfs4_cb_procedures)];
> static struct rpc_version nfs_cb_version4 = {
> /*
> * Note on the callback rpc program version number: despite language in rfc
> @@ -715,7 +716,8 @@ static struct rpc_version nfs_cb_version4 = {
> */
> .number = 1,
> .nrprocs = ARRAY_SIZE(nfs4_cb_procedures),
> - .procs = nfs4_cb_procedures
> + .procs = nfs4_cb_procedures,
> + .counts = nfs4_cb_counts,
> };
>
> static const struct rpc_version *nfs_cb_version[] = {
> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> index 6095ecba0dde..c75ba37151fe 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -88,6 +88,7 @@ struct rpc_version {
> u32 number; /* version number */
> unsigned int nrprocs; /* number of procs */
> struct rpc_procinfo * procs; /* procedure array */
> + unsigned int *counts; /* call counts */
> };
>
> /*
> @@ -99,7 +100,6 @@ struct rpc_procinfo {
> kxdrdproc_t p_decode; /* XDR decode function */
> unsigned int p_arglen; /* argument hdr length (u32) */
> unsigned int p_replen; /* reply hdr length (u32) */
> - unsigned int p_count; /* call count */
> unsigned int p_timer; /* Which RTT timer to use */
> u32 p_statidx; /* Which procedure to account */
> const char * p_name; /* name of procedure */
> diff --git a/net/sunrpc/auth_gss/gss_rpc_upcall.c b/net/sunrpc/auth_gss/gss_rpc_upcall.c
> index a80b8e607478..f8729b647605 100644
> --- a/net/sunrpc/auth_gss/gss_rpc_upcall.c
> +++ b/net/sunrpc/auth_gss/gss_rpc_upcall.c
> @@ -364,11 +364,12 @@ void gssp_free_upcall_data(struct gssp_upcall_data *data)
> /*
> * Initialization stuff
> */
> -
> +static unsigned int gssp_version1_counts[ARRAY_SIZE(gssp_procedures)];
> static const struct rpc_version gssp_version1 = {
> .number = GSSPROXY_VERS_1,
> .nrprocs = ARRAY_SIZE(gssp_procedures),
> .procs = gssp_procedures,
> + .counts = gssp_version1_counts,
> };
>
> static const struct rpc_version *gssp_version[] = {
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 964d5c4a1b60..f2d1f971247b 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -1517,14 +1517,16 @@ static void
> call_start(struct rpc_task *task)
> {
> struct rpc_clnt *clnt = task->tk_client;
> + int idx = task->tk_msg.rpc_proc->p_statidx;
>
> dprintk("RPC: %5u call_start %s%d proc %s (%s)\n", task->tk_pid,
> clnt->cl_program->name, clnt->cl_vers,
> rpc_proc_name(task),
> (RPC_IS_ASYNC(task) ? "async" : "sync"));
>
> - /* Increment call count */
> - task->tk_msg.rpc_proc->p_count++;
> + /* Increment call count (version might not be valid for ping) */
> + if (clnt->cl_program->version[clnt->cl_vers])
> + clnt->cl_program->version[clnt->cl_vers]->counts[idx]++;
> clnt->cl_stats->rpccnt++;
> task->tk_action = call_reserve;
> }
>
Little bit of a behavior change here -- as you say version might not be
valid so we might lose this count in some cases? I don't think it's a
big deal, just pointing it out in case someone notices later.
> diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
> index f67b9e2897b4..9d47b9d3bbee 100644
> --- a/net/sunrpc/rpcb_clnt.c
> +++ b/net/sunrpc/rpcb_clnt.c
> @@ -1117,22 +1117,28 @@ static const struct rpcb_info rpcb_next_version6[] = {
> },
> };
>
> +static unsigned int rpcb_version2_counts[ARRAY_SIZE(rpcb_procedures2)];
> static const struct rpc_version rpcb_version2 = {
> .number = RPCBVERS_2,
> .nrprocs = ARRAY_SIZE(rpcb_procedures2),
> - .procs = rpcb_procedures2
> + .procs = rpcb_procedures2,
> + .counts = rpcb_version2_counts,
> };
>
> +static unsigned int rpcb_version3_counts[ARRAY_SIZE(rpcb_procedures3)];
> static const struct rpc_version rpcb_version3 = {
> .number = RPCBVERS_3,
> .nrprocs = ARRAY_SIZE(rpcb_procedures3),
> - .procs = rpcb_procedures3
> + .procs = rpcb_procedures3,
> + .counts = rpcb_version3_counts,
> };
>
> +static unsigned int rpcb_version4_counts[ARRAY_SIZE(rpcb_procedures4)];
> static const struct rpc_version rpcb_version4 = {
> .number = RPCBVERS_4,
> .nrprocs = ARRAY_SIZE(rpcb_procedures4),
> - .procs = rpcb_procedures4
> + .procs = rpcb_procedures4,
> + .counts = rpcb_version4_counts,
> };
>
> static const struct rpc_version *rpcb_version[] = {
> diff --git a/net/sunrpc/stats.c b/net/sunrpc/stats.c
> index caeb01ad2b5a..91c84d18bf9a 100644
> --- a/net/sunrpc/stats.c
> +++ b/net/sunrpc/stats.c
> @@ -55,8 +55,7 @@ static int rpc_proc_show(struct seq_file *seq, void *v) {
> seq_printf(seq, "proc%u %u",
> vers->number, vers->nrprocs);
> for (j = 0; j < vers->nrprocs; j++)
> - seq_printf(seq, " %u",
> - vers->procs[j].p_count);
> + seq_printf(seq, " %u", vers->counts[j]);
> seq_putc(seq, '\n');
> }
> return 0;
Reviewed-by: Jeff Layton <jlayton@redhat.com>
next prev parent reply other threads:[~2017-05-12 18:24 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-12 16:16 remove function pointer casts and constify function tables Christoph Hellwig
2017-05-12 16:16 ` [PATCH 01/33] sunrpc: properly type argument to kxdreproc_t Christoph Hellwig
2017-05-12 16:16 ` [PATCH 02/33] sunrpc: fix encoder callback prototypes Christoph Hellwig
2017-05-12 16:16 ` [PATCH 03/33] lockd: " Christoph Hellwig
2017-05-12 16:16 ` [PATCH 04/33] nfs: " Christoph Hellwig
2017-05-12 16:16 ` [PATCH 05/33] nfsd: " Christoph Hellwig
2017-05-12 16:16 ` [PATCH 06/33] sunrpc/auth_gss: " Christoph Hellwig
2017-05-12 16:16 ` [PATCH 07/33] sunrpc: properly type argument to kxdrdproc_t Christoph Hellwig
2017-05-12 16:16 ` [PATCH 08/33] sunrpc: fix decoder callback prototypes Christoph Hellwig
2017-05-12 16:16 ` [PATCH 09/33] sunrpc/auth_gss: " Christoph Hellwig
2017-05-12 16:16 ` [PATCH 10/33] nfsd: " Christoph Hellwig
2017-05-12 16:16 ` [PATCH 11/33] lockd: " Christoph Hellwig
2017-05-12 16:16 ` [PATCH 12/33] nfs: " Christoph Hellwig
2017-05-12 16:16 ` [PATCH 13/33] nfs: don't cast callback decode/proc/encode routines Christoph Hellwig
2017-05-12 16:16 ` [PATCH 14/33] lockd: fix some weird indentation Christoph Hellwig
2017-05-12 18:01 ` Jeff Layton
2017-05-12 16:16 ` [PATCH 15/33] sunrpc: move p_count out of struct rpc_procinfo Christoph Hellwig
2017-05-12 18:24 ` Jeff Layton [this message]
2017-05-12 16:16 ` [PATCH 16/33] nfs: use ARRAY_SIZE() in the nfsacl_version3 declaration Christoph Hellwig
2017-05-12 18:25 ` Jeff Layton
2017-05-12 16:16 ` [PATCH 17/33] sunrpc: mark all struct rpc_procinfo instances as const Christoph Hellwig
2017-05-12 18:30 ` Jeff Layton
2017-05-12 16:16 ` [PATCH 18/33] nfsd4: const-ify nfs_cb_version4 Christoph Hellwig
2017-05-12 18:31 ` Jeff Layton
2017-05-12 16:16 ` [PATCH 19/33] nfsd: use named initializers in PROC() Christoph Hellwig
2017-05-12 18:32 ` Jeff Layton
2017-05-12 16:16 ` [PATCH 20/33] nfsd: remove the unused PROC() macro in nfs3proc.c Christoph Hellwig
2017-05-12 18:33 ` Jeff Layton
2017-05-12 16:16 ` [PATCH 21/33] sunrpc: properly type pc_func callbacks Christoph Hellwig
2017-05-12 18:34 ` Jeff Layton
2017-05-12 16:16 ` [PATCH 22/33] sunrpc: properly type pc_release callbacks Christoph Hellwig
2017-05-12 16:16 ` [PATCH 23/33] sunrpc: properly type pc_decode callbacks Christoph Hellwig
2017-05-12 16:16 ` [PATCH 24/33] sunrpc: properly type pc_encode callbacks Christoph Hellwig
2017-05-12 16:16 ` [PATCH 25/33] sunrpc: remove kxdrproc_t Christoph Hellwig
2017-05-12 16:16 ` [PATCH 26/33] nfsd4: properly type op_set_currentstateid callbacks Christoph Hellwig
2017-05-12 16:16 ` [PATCH 27/33] nfsd4: properly type op_get_currentstateid callbacks Christoph Hellwig
2017-05-12 16:16 ` [PATCH 28/33] nfsd4: remove nfsd4op_rsize Christoph Hellwig
2017-05-12 16:16 ` [PATCH 29/33] nfsd4: properly type op_func callbacks Christoph Hellwig
2017-05-12 16:16 ` [PATCH 30/33] sunrpc: move pc_count out of struct svc_procinfo Christoph Hellwig
2017-05-12 16:16 ` [PATCH 31/33] sunrpc: mark all struct svc_procinfo instances as const Christoph Hellwig
2017-05-12 16:17 ` [PATCH 32/33] sunrpc: mark all struct svc_version " Christoph Hellwig
2017-05-12 16:17 ` [PATCH 33/33] nfsd4: const-ify nfsd4_ops Christoph Hellwig
2017-05-12 18:42 ` Jeff Layton
2017-05-12 20:14 ` remove function pointer casts and constify function tables Trond Myklebust
2017-05-12 22:04 ` bfields
2017-05-13 7:25 ` hch
2017-05-13 16:10 ` Trond Myklebust
2017-05-15 15:21 ` bfields
2017-05-15 15:44 ` hch
2017-05-23 8:11 ` hch
2017-05-23 12:23 ` bfields
2017-05-26 15:08 ` bfields
2017-05-26 15:09 ` bfields
2017-05-26 19:31 ` bfields
2017-05-30 16:26 ` Michael S. Tsirkin
2017-05-30 16:26 ` Michael S. Tsirkin
2017-05-30 16:58 ` Michael S. Tsirkin
2017-05-31 20:57 ` bfields
2017-05-31 20:57 ` bfields
2017-05-30 16:58 ` Michael S. Tsirkin
2017-05-31 21:09 ` bfields
2017-05-31 21:09 ` bfields
2017-05-31 21:15 ` bfields
2017-05-31 21:15 ` bfields
2017-05-30 17:03 ` Michael S. Tsirkin
2017-05-30 17:03 ` Michael S. Tsirkin
2017-05-31 21:00 ` bfields
2017-05-31 21:00 ` bfields
2017-05-26 19:31 ` bfields
2017-05-26 15:09 ` bfields
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1494613476.4227.3.camel@redhat.com \
--to=jlayton@redhat.com \
--cc=anna.schumaker@netapp.com \
--cc=bfields@fieldses.org \
--cc=hch@lst.de \
--cc=linux-nfs@vger.kernel.org \
--cc=trond.myklebust@primarydata.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.