diff mbox series

[v2,05/13] sunrpc: add a struct rpc_stats arg to rpc_create_args

Message ID 4cc2a7aca55ff56ac3ced32aafa861f57f59db02.1706212208.git.josef@toxicpanda.com (mailing list archive)
State New, archived
Headers show
Series Make nfs and nfsd stats visible in network ns | expand

Commit Message

Josef Bacik Jan. 25, 2024, 7:53 p.m. UTC
We want to be able to have our rpc stats handled in a per network
namespace manner, so add an option to rpc_create_args to specify a
different rpc_stats struct instead of using the one on the rpc_program.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/nfs/client.c             | 2 +-
 include/linux/sunrpc/clnt.h | 1 +
 net/sunrpc/clnt.c           | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

Comments

Chuck Lever Jan. 25, 2024, 8:53 p.m. UTC | #1
On Thu, Jan 25, 2024 at 02:53:15PM -0500, Josef Bacik wrote:
> We want to be able to have our rpc stats handled in a per network
> namespace manner, so add an option to rpc_create_args to specify a
> different rpc_stats struct instead of using the one on the rpc_program.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/nfs/client.c             | 2 +-
>  include/linux/sunrpc/clnt.h | 1 +
>  net/sunrpc/clnt.c           | 2 +-
>  3 files changed, 3 insertions(+), 2 deletions(-)

I know it isn't obvious to an outside observer, but the
maintainership of the NFS server and client are separate.

NFS client patches go To: Trond and Anna, Cc: linux-nfs

NFS server patches go To: Jeff and Chuck, Cc: linux-nfs

and you can Cc: server patches on the reviewers listed in
MAINTAINERS too if you like.


> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 44eca51b2808..590be14f182f 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -73,7 +73,7 @@ const struct rpc_program nfs_program = {
>  	.number			= NFS_PROGRAM,
>  	.nrvers			= ARRAY_SIZE(nfs_version),
>  	.version		= nfs_version,
> -	.stats			= &nfs_rpcstat,
> +	.stats                  = &nfs_rpcstat,
>  	.pipe_dir_name		= NFS_PIPE_DIRNAME,
>  };
>  
> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> index 5e9d1469c6fa..5321585c778f 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -139,6 +139,7 @@ struct rpc_create_args {
>  	const char		*servername;
>  	const char		*nodename;
>  	const struct rpc_program *program;
> +	struct rpc_stat		*stats;
>  	u32			prognumber;	/* overrides program->number */
>  	u32			version;
>  	rpc_authflavor_t	authflavor;
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index cda0935a68c9..bc8c209fc0c7 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -405,7 +405,7 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args,
>  	clnt->cl_maxproc  = version->nrprocs;
>  	clnt->cl_prog     = args->prognumber ? : program->number;
>  	clnt->cl_vers     = version->number;
> -	clnt->cl_stats    = program->stats;
> +	clnt->cl_stats    = args->stats ? : program->stats;
>  	clnt->cl_metrics  = rpc_alloc_iostats(clnt);
>  	rpc_init_pipe_dir_head(&clnt->cl_pipedir_objects);
>  	err = -ENOMEM;
> -- 
> 2.43.0
> 
>
Josef Bacik Jan. 25, 2024, 9:54 p.m. UTC | #2
On Thu, Jan 25, 2024 at 03:53:24PM -0500, Chuck Lever wrote:
> On Thu, Jan 25, 2024 at 02:53:15PM -0500, Josef Bacik wrote:
> > We want to be able to have our rpc stats handled in a per network
> > namespace manner, so add an option to rpc_create_args to specify a
> > different rpc_stats struct instead of using the one on the rpc_program.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > ---
> >  fs/nfs/client.c             | 2 +-
> >  include/linux/sunrpc/clnt.h | 1 +
> >  net/sunrpc/clnt.c           | 2 +-
> >  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> I know it isn't obvious to an outside observer, but the
> maintainership of the NFS server and client are separate.
> 
> NFS client patches go To: Trond and Anna, Cc: linux-nfs
> 
> NFS server patches go To: Jeff and Chuck, Cc: linux-nfs
> 
> and you can Cc: server patches on the reviewers listed in
> MAINTAINERS too if you like.

Sounds good, should I split the series up completely?  I think the nfsd and nfs
sunrpc related changes are unique to either stack so I can split it out into two
different series if that would help.  Thanks,

Josef
Jeff Layton Jan. 25, 2024, 10:30 p.m. UTC | #3
On Thu, 2024-01-25 at 16:54 -0500, Josef Bacik wrote:
> On Thu, Jan 25, 2024 at 03:53:24PM -0500, Chuck Lever wrote:
> > On Thu, Jan 25, 2024 at 02:53:15PM -0500, Josef Bacik wrote:
> > > We want to be able to have our rpc stats handled in a per network
> > > namespace manner, so add an option to rpc_create_args to specify a
> > > different rpc_stats struct instead of using the one on the rpc_program.
> > > 
> > > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > > ---
> > >  fs/nfs/client.c             | 2 +-
> > >  include/linux/sunrpc/clnt.h | 1 +
> > >  net/sunrpc/clnt.c           | 2 +-
> > >  3 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > I know it isn't obvious to an outside observer, but the
> > maintainership of the NFS server and client are separate.
> > 
> > NFS client patches go To: Trond and Anna, Cc: linux-nfs
> > 
> > NFS server patches go To: Jeff and Chuck, Cc: linux-nfs
> > 
> > and you can Cc: server patches on the reviewers listed in
> > MAINTAINERS too if you like.
> 
> Sounds good, should I split the series up completely?  I think the nfsd and nfs
> sunrpc related changes are unique to either stack so I can split it out into two
> different series if that would help.  Thanks,
> 

Yes, that'd probably be best. The client-side changes will probably go
in via Trond or Anna, and Chuck will (likely) take the server-side
changes.

Cheers,
Chuck Lever Jan. 26, 2024, 1:49 p.m. UTC | #4
> On Jan 25, 2024, at 4:54 PM, Josef Bacik <josef@toxicpanda.com> wrote:
> 
> On Thu, Jan 25, 2024 at 03:53:24PM -0500, Chuck Lever wrote:
>> On Thu, Jan 25, 2024 at 02:53:15PM -0500, Josef Bacik wrote:
>>> We want to be able to have our rpc stats handled in a per network
>>> namespace manner, so add an option to rpc_create_args to specify a
>>> different rpc_stats struct instead of using the one on the rpc_program.
>>> 
>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>> ---
>>> fs/nfs/client.c             | 2 +-
>>> include/linux/sunrpc/clnt.h | 1 +
>>> net/sunrpc/clnt.c           | 2 +-
>>> 3 files changed, 3 insertions(+), 2 deletions(-)
>> 
>> I know it isn't obvious to an outside observer, but the
>> maintainership of the NFS server and client are separate.
>> 
>> NFS client patches go To: Trond and Anna, Cc: linux-nfs
>> 
>> NFS server patches go To: Jeff and Chuck, Cc: linux-nfs
>> 
>> and you can Cc: server patches on the reviewers listed in
>> MAINTAINERS too if you like.
> 
> Sounds good, should I split the series up completely?  I think the nfsd and nfs
> sunrpc related changes are unique to either stack

That's what it looks like to me too.


> so I can split it out into two different series if that would help.

Let's do that for v3.

--
Chuck Lever
diff mbox series

Patch

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 44eca51b2808..590be14f182f 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -73,7 +73,7 @@  const struct rpc_program nfs_program = {
 	.number			= NFS_PROGRAM,
 	.nrvers			= ARRAY_SIZE(nfs_version),
 	.version		= nfs_version,
-	.stats			= &nfs_rpcstat,
+	.stats                  = &nfs_rpcstat,
 	.pipe_dir_name		= NFS_PIPE_DIRNAME,
 };
 
diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 5e9d1469c6fa..5321585c778f 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -139,6 +139,7 @@  struct rpc_create_args {
 	const char		*servername;
 	const char		*nodename;
 	const struct rpc_program *program;
+	struct rpc_stat		*stats;
 	u32			prognumber;	/* overrides program->number */
 	u32			version;
 	rpc_authflavor_t	authflavor;
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index cda0935a68c9..bc8c209fc0c7 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -405,7 +405,7 @@  static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args,
 	clnt->cl_maxproc  = version->nrprocs;
 	clnt->cl_prog     = args->prognumber ? : program->number;
 	clnt->cl_vers     = version->number;
-	clnt->cl_stats    = program->stats;
+	clnt->cl_stats    = args->stats ? : program->stats;
 	clnt->cl_metrics  = rpc_alloc_iostats(clnt);
 	rpc_init_pipe_dir_head(&clnt->cl_pipedir_objects);
 	err = -ENOMEM;