diff mbox series

[v2,03/13] sunrpc: pass in the sv_stats struct through svc_create*

Message ID ff6afd3ab9a70bf5ab90872497068719f2c1ec03.1706212208.git.josef@toxicpanda.com (mailing list archive)
State New
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
Since only one service actually reports the rpc stats there's not much
of a reason to have a pointer to it in the svc_program struct.  Adjust
the svc_create* functions to take the sv_stats as an argument and pass
the struct through there as desired instead of getting it from the
svc_program->pg_stats.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/lockd/svc.c             |  2 +-
 fs/nfs/callback.c          |  2 +-
 fs/nfsd/nfssvc.c           |  3 ++-
 include/linux/sunrpc/svc.h |  8 ++++----
 net/sunrpc/svc.c           | 17 ++++++++++-------
 5 files changed, 18 insertions(+), 14 deletions(-)

Comments

Chuck Lever Jan. 25, 2024, 8:56 p.m. UTC | #1
On Thu, Jan 25, 2024 at 02:53:13PM -0500, Josef Bacik wrote:
> Since only one service actually reports the rpc stats there's not much
> of a reason to have a pointer to it in the svc_program struct.  Adjust
> the svc_create* functions to take the sv_stats as an argument and pass
> the struct through there as desired instead of getting it from the
> svc_program->pg_stats.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/lockd/svc.c             |  2 +-
>  fs/nfs/callback.c          |  2 +-
>  fs/nfsd/nfssvc.c           |  3 ++-
>  include/linux/sunrpc/svc.h |  8 ++++----
>  net/sunrpc/svc.c           | 17 ++++++++++-------
>  5 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index ab8042a5b895..8fbbfc9aad69 100644
> --- a/fs/lockd/svc.c
> +++ b/fs/lockd/svc.c
> @@ -337,7 +337,7 @@ static int lockd_get(void)
>  		nlm_timeout = LOCKD_DFLT_TIMEO;
>  	nlmsvc_timeout = nlm_timeout * HZ;
>  
> -	serv = svc_create(&nlmsvc_program, LOCKD_BUFSIZE, lockd);
> +	serv = svc_create(&nlmsvc_program, NULL, LOCKD_BUFSIZE, lockd);
>  	if (!serv) {
>  		printk(KERN_WARNING "lockd_up: create service failed\n");
>  		return -ENOMEM;
> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> index 8adfcd4c8c1a..4d56b4e73525 100644
> --- a/fs/nfs/callback.c
> +++ b/fs/nfs/callback.c
> @@ -202,7 +202,7 @@ static struct svc_serv *nfs_callback_create_svc(int minorversion)
>  	if (minorversion)
>  		return ERR_PTR(-ENOTSUPP);
>  #endif
> -	serv = svc_create(&nfs4_callback_program, NFS4_CALLBACK_BUFSIZE,
> +	serv = svc_create(&nfs4_callback_program, NULL, NFS4_CALLBACK_BUFSIZE,
>  			  threadfn);
>  	if (!serv) {
>  		printk(KERN_ERR "nfs_callback_create_svc: create service failed\n");
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index a0b117107e86..d640f893021a 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -661,7 +661,8 @@ int nfsd_create_serv(struct net *net)
>  	if (nfsd_max_blksize == 0)
>  		nfsd_max_blksize = nfsd_get_default_max_blksize();
>  	nfsd_reset_versions(nn);
> -	serv = svc_create_pooled(&nfsd_program, nfsd_max_blksize, nfsd);
> +	serv = svc_create_pooled(&nfsd_program, &nfsd_svcstats,
> +				 nfsd_max_blksize, nfsd);
>  	if (serv == NULL)
>  		return -ENOMEM;
>  
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index 67cf1c9efd80..2a1447fa5ef2 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -402,8 +402,8 @@ struct svc_procedure {
>  int svc_rpcb_setup(struct svc_serv *serv, struct net *net);
>  void svc_rpcb_cleanup(struct svc_serv *serv, struct net *net);
>  int svc_bind(struct svc_serv *serv, struct net *net);
> -struct svc_serv *svc_create(struct svc_program *, unsigned int,
> -			    int (*threadfn)(void *data));
> +struct svc_serv *svc_create(struct svc_program *, struct svc_stat *,
> +			    unsigned int, int (*threadfn)(void *data));
>  struct svc_rqst *svc_rqst_alloc(struct svc_serv *serv,
>  					struct svc_pool *pool, int node);
>  bool		   svc_rqst_replace_page(struct svc_rqst *rqstp,
> @@ -411,8 +411,8 @@ bool		   svc_rqst_replace_page(struct svc_rqst *rqstp,
>  void		   svc_rqst_release_pages(struct svc_rqst *rqstp);
>  void		   svc_rqst_free(struct svc_rqst *);
>  void		   svc_exit_thread(struct svc_rqst *);
> -struct svc_serv *  svc_create_pooled(struct svc_program *, unsigned int,
> -				     int (*threadfn)(void *data));
> +struct svc_serv *  svc_create_pooled(struct svc_program *, struct svc_stat *,
> +				     unsigned int, int (*threadfn)(void *data));
>  int		   svc_set_num_threads(struct svc_serv *, struct svc_pool *, int);
>  int		   svc_pool_stats_open(struct svc_info *si, struct file *file);
>  void		   svc_process(struct svc_rqst *rqstp);
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index d2e6f3d59218..f76ef8a3dd43 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -451,8 +451,8 @@ __svc_init_bc(struct svc_serv *serv)
>   * Create an RPC service
>   */
>  static struct svc_serv *
> -__svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> -	     int (*threadfn)(void *data))
> +__svc_create(struct svc_program *prog, struct svc_stat *stats,
> +	     unsigned int bufsize, int npools, int (*threadfn)(void *data))
>  {
>  	struct svc_serv	*serv;
>  	unsigned int vers;
> @@ -463,7 +463,7 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
>  		return NULL;
>  	serv->sv_name      = prog->pg_name;
>  	serv->sv_program   = prog;
> -	serv->sv_stats     = prog->pg_stats;
> +	serv->sv_stats     = stats;
>  	if (bufsize > RPCSVC_MAXPAYLOAD)
>  		bufsize = RPCSVC_MAXPAYLOAD;
>  	serv->sv_max_payload = bufsize? bufsize : 4096;
> @@ -521,34 +521,37 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
>  /**
>   * svc_create - Create an RPC service
>   * @prog: the RPC program the new service will handle
> + * @stats: the stats struct if desired
>   * @bufsize: maximum message size for @prog
>   * @threadfn: a function to service RPC requests for @prog
>   *
>   * Returns an instantiated struct svc_serv object or NULL.
>   */

Here's the only minor quibble I have so far:

svc_create's callers don't use stats, so maybe you don't need
to add an @stats argument for this API.

Fwiw, I haven't gotten all the way to the end of the series yet.


> -struct svc_serv *svc_create(struct svc_program *prog, unsigned int bufsize,
> -			    int (*threadfn)(void *data))
> +struct svc_serv *svc_create(struct svc_program *prog, struct svc_stat *stats,
> +			    unsigned int bufsize, int (*threadfn)(void *data))
>  {
> -	return __svc_create(prog, bufsize, 1, threadfn);
> +	return __svc_create(prog, stats, bufsize, 1, threadfn);
>  }
>  EXPORT_SYMBOL_GPL(svc_create);
>  
>  /**
>   * svc_create_pooled - Create an RPC service with pooled threads
>   * @prog: the RPC program the new service will handle
> + * @stats: the stats struct if desired
>   * @bufsize: maximum message size for @prog
>   * @threadfn: a function to service RPC requests for @prog
>   *
>   * Returns an instantiated struct svc_serv object or NULL.
>   */
>  struct svc_serv *svc_create_pooled(struct svc_program *prog,
> +				   struct svc_stat *stats,
>  				   unsigned int bufsize,
>  				   int (*threadfn)(void *data))
>  {
>  	struct svc_serv *serv;
>  	unsigned int npools = svc_pool_map_get();
>  
> -	serv = __svc_create(prog, bufsize, npools, threadfn);
> +	serv = __svc_create(prog, stats, bufsize, npools, threadfn);
>  	if (!serv)
>  		goto out_err;
>  	return serv;
> -- 
> 2.43.0
> 
>
Josef Bacik Jan. 25, 2024, 9:56 p.m. UTC | #2
On Thu, Jan 25, 2024 at 03:56:00PM -0500, Chuck Lever wrote:
> On Thu, Jan 25, 2024 at 02:53:13PM -0500, Josef Bacik wrote:
> > Since only one service actually reports the rpc stats there's not much
> > of a reason to have a pointer to it in the svc_program struct.  Adjust
> > the svc_create* functions to take the sv_stats as an argument and pass
> > the struct through there as desired instead of getting it from the
> > svc_program->pg_stats.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > ---
> >  fs/lockd/svc.c             |  2 +-
> >  fs/nfs/callback.c          |  2 +-
> >  fs/nfsd/nfssvc.c           |  3 ++-
> >  include/linux/sunrpc/svc.h |  8 ++++----
> >  net/sunrpc/svc.c           | 17 ++++++++++-------
> >  5 files changed, 18 insertions(+), 14 deletions(-)
> > 
> > diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> > index ab8042a5b895..8fbbfc9aad69 100644
> > --- a/fs/lockd/svc.c
> > +++ b/fs/lockd/svc.c
> > @@ -337,7 +337,7 @@ static int lockd_get(void)
> >  		nlm_timeout = LOCKD_DFLT_TIMEO;
> >  	nlmsvc_timeout = nlm_timeout * HZ;
> >  
> > -	serv = svc_create(&nlmsvc_program, LOCKD_BUFSIZE, lockd);
> > +	serv = svc_create(&nlmsvc_program, NULL, LOCKD_BUFSIZE, lockd);
> >  	if (!serv) {
> >  		printk(KERN_WARNING "lockd_up: create service failed\n");
> >  		return -ENOMEM;
> > diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> > index 8adfcd4c8c1a..4d56b4e73525 100644
> > --- a/fs/nfs/callback.c
> > +++ b/fs/nfs/callback.c
> > @@ -202,7 +202,7 @@ static struct svc_serv *nfs_callback_create_svc(int minorversion)
> >  	if (minorversion)
> >  		return ERR_PTR(-ENOTSUPP);
> >  #endif
> > -	serv = svc_create(&nfs4_callback_program, NFS4_CALLBACK_BUFSIZE,
> > +	serv = svc_create(&nfs4_callback_program, NULL, NFS4_CALLBACK_BUFSIZE,
> >  			  threadfn);
> >  	if (!serv) {
> >  		printk(KERN_ERR "nfs_callback_create_svc: create service failed\n");
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index a0b117107e86..d640f893021a 100644
> > --- a/fs/nfsd/nfssvc.c
> > +++ b/fs/nfsd/nfssvc.c
> > @@ -661,7 +661,8 @@ int nfsd_create_serv(struct net *net)
> >  	if (nfsd_max_blksize == 0)
> >  		nfsd_max_blksize = nfsd_get_default_max_blksize();
> >  	nfsd_reset_versions(nn);
> > -	serv = svc_create_pooled(&nfsd_program, nfsd_max_blksize, nfsd);
> > +	serv = svc_create_pooled(&nfsd_program, &nfsd_svcstats,
> > +				 nfsd_max_blksize, nfsd);
> >  	if (serv == NULL)
> >  		return -ENOMEM;
> >  
> > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> > index 67cf1c9efd80..2a1447fa5ef2 100644
> > --- a/include/linux/sunrpc/svc.h
> > +++ b/include/linux/sunrpc/svc.h
> > @@ -402,8 +402,8 @@ struct svc_procedure {
> >  int svc_rpcb_setup(struct svc_serv *serv, struct net *net);
> >  void svc_rpcb_cleanup(struct svc_serv *serv, struct net *net);
> >  int svc_bind(struct svc_serv *serv, struct net *net);
> > -struct svc_serv *svc_create(struct svc_program *, unsigned int,
> > -			    int (*threadfn)(void *data));
> > +struct svc_serv *svc_create(struct svc_program *, struct svc_stat *,
> > +			    unsigned int, int (*threadfn)(void *data));
> >  struct svc_rqst *svc_rqst_alloc(struct svc_serv *serv,
> >  					struct svc_pool *pool, int node);
> >  bool		   svc_rqst_replace_page(struct svc_rqst *rqstp,
> > @@ -411,8 +411,8 @@ bool		   svc_rqst_replace_page(struct svc_rqst *rqstp,
> >  void		   svc_rqst_release_pages(struct svc_rqst *rqstp);
> >  void		   svc_rqst_free(struct svc_rqst *);
> >  void		   svc_exit_thread(struct svc_rqst *);
> > -struct svc_serv *  svc_create_pooled(struct svc_program *, unsigned int,
> > -				     int (*threadfn)(void *data));
> > +struct svc_serv *  svc_create_pooled(struct svc_program *, struct svc_stat *,
> > +				     unsigned int, int (*threadfn)(void *data));
> >  int		   svc_set_num_threads(struct svc_serv *, struct svc_pool *, int);
> >  int		   svc_pool_stats_open(struct svc_info *si, struct file *file);
> >  void		   svc_process(struct svc_rqst *rqstp);
> > diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> > index d2e6f3d59218..f76ef8a3dd43 100644
> > --- a/net/sunrpc/svc.c
> > +++ b/net/sunrpc/svc.c
> > @@ -451,8 +451,8 @@ __svc_init_bc(struct svc_serv *serv)
> >   * Create an RPC service
> >   */
> >  static struct svc_serv *
> > -__svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> > -	     int (*threadfn)(void *data))
> > +__svc_create(struct svc_program *prog, struct svc_stat *stats,
> > +	     unsigned int bufsize, int npools, int (*threadfn)(void *data))
> >  {
> >  	struct svc_serv	*serv;
> >  	unsigned int vers;
> > @@ -463,7 +463,7 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> >  		return NULL;
> >  	serv->sv_name      = prog->pg_name;
> >  	serv->sv_program   = prog;
> > -	serv->sv_stats     = prog->pg_stats;
> > +	serv->sv_stats     = stats;
> >  	if (bufsize > RPCSVC_MAXPAYLOAD)
> >  		bufsize = RPCSVC_MAXPAYLOAD;
> >  	serv->sv_max_payload = bufsize? bufsize : 4096;
> > @@ -521,34 +521,37 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> >  /**
> >   * svc_create - Create an RPC service
> >   * @prog: the RPC program the new service will handle
> > + * @stats: the stats struct if desired
> >   * @bufsize: maximum message size for @prog
> >   * @threadfn: a function to service RPC requests for @prog
> >   *
> >   * Returns an instantiated struct svc_serv object or NULL.
> >   */
> 
> Here's the only minor quibble I have so far:
> 
> svc_create's callers don't use stats, so maybe you don't need
> to add an @stats argument for this API.
> 
> Fwiw, I haven't gotten all the way to the end of the series yet.

Yup you're right, I can drop this bit.  Thanks,

Josef
diff mbox series

Patch

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index ab8042a5b895..8fbbfc9aad69 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -337,7 +337,7 @@  static int lockd_get(void)
 		nlm_timeout = LOCKD_DFLT_TIMEO;
 	nlmsvc_timeout = nlm_timeout * HZ;
 
-	serv = svc_create(&nlmsvc_program, LOCKD_BUFSIZE, lockd);
+	serv = svc_create(&nlmsvc_program, NULL, LOCKD_BUFSIZE, lockd);
 	if (!serv) {
 		printk(KERN_WARNING "lockd_up: create service failed\n");
 		return -ENOMEM;
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 8adfcd4c8c1a..4d56b4e73525 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -202,7 +202,7 @@  static struct svc_serv *nfs_callback_create_svc(int minorversion)
 	if (minorversion)
 		return ERR_PTR(-ENOTSUPP);
 #endif
-	serv = svc_create(&nfs4_callback_program, NFS4_CALLBACK_BUFSIZE,
+	serv = svc_create(&nfs4_callback_program, NULL, NFS4_CALLBACK_BUFSIZE,
 			  threadfn);
 	if (!serv) {
 		printk(KERN_ERR "nfs_callback_create_svc: create service failed\n");
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index a0b117107e86..d640f893021a 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -661,7 +661,8 @@  int nfsd_create_serv(struct net *net)
 	if (nfsd_max_blksize == 0)
 		nfsd_max_blksize = nfsd_get_default_max_blksize();
 	nfsd_reset_versions(nn);
-	serv = svc_create_pooled(&nfsd_program, nfsd_max_blksize, nfsd);
+	serv = svc_create_pooled(&nfsd_program, &nfsd_svcstats,
+				 nfsd_max_blksize, nfsd);
 	if (serv == NULL)
 		return -ENOMEM;
 
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 67cf1c9efd80..2a1447fa5ef2 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -402,8 +402,8 @@  struct svc_procedure {
 int svc_rpcb_setup(struct svc_serv *serv, struct net *net);
 void svc_rpcb_cleanup(struct svc_serv *serv, struct net *net);
 int svc_bind(struct svc_serv *serv, struct net *net);
-struct svc_serv *svc_create(struct svc_program *, unsigned int,
-			    int (*threadfn)(void *data));
+struct svc_serv *svc_create(struct svc_program *, struct svc_stat *,
+			    unsigned int, int (*threadfn)(void *data));
 struct svc_rqst *svc_rqst_alloc(struct svc_serv *serv,
 					struct svc_pool *pool, int node);
 bool		   svc_rqst_replace_page(struct svc_rqst *rqstp,
@@ -411,8 +411,8 @@  bool		   svc_rqst_replace_page(struct svc_rqst *rqstp,
 void		   svc_rqst_release_pages(struct svc_rqst *rqstp);
 void		   svc_rqst_free(struct svc_rqst *);
 void		   svc_exit_thread(struct svc_rqst *);
-struct svc_serv *  svc_create_pooled(struct svc_program *, unsigned int,
-				     int (*threadfn)(void *data));
+struct svc_serv *  svc_create_pooled(struct svc_program *, struct svc_stat *,
+				     unsigned int, int (*threadfn)(void *data));
 int		   svc_set_num_threads(struct svc_serv *, struct svc_pool *, int);
 int		   svc_pool_stats_open(struct svc_info *si, struct file *file);
 void		   svc_process(struct svc_rqst *rqstp);
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index d2e6f3d59218..f76ef8a3dd43 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -451,8 +451,8 @@  __svc_init_bc(struct svc_serv *serv)
  * Create an RPC service
  */
 static struct svc_serv *
-__svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
-	     int (*threadfn)(void *data))
+__svc_create(struct svc_program *prog, struct svc_stat *stats,
+	     unsigned int bufsize, int npools, int (*threadfn)(void *data))
 {
 	struct svc_serv	*serv;
 	unsigned int vers;
@@ -463,7 +463,7 @@  __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
 		return NULL;
 	serv->sv_name      = prog->pg_name;
 	serv->sv_program   = prog;
-	serv->sv_stats     = prog->pg_stats;
+	serv->sv_stats     = stats;
 	if (bufsize > RPCSVC_MAXPAYLOAD)
 		bufsize = RPCSVC_MAXPAYLOAD;
 	serv->sv_max_payload = bufsize? bufsize : 4096;
@@ -521,34 +521,37 @@  __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
 /**
  * svc_create - Create an RPC service
  * @prog: the RPC program the new service will handle
+ * @stats: the stats struct if desired
  * @bufsize: maximum message size for @prog
  * @threadfn: a function to service RPC requests for @prog
  *
  * Returns an instantiated struct svc_serv object or NULL.
  */
-struct svc_serv *svc_create(struct svc_program *prog, unsigned int bufsize,
-			    int (*threadfn)(void *data))
+struct svc_serv *svc_create(struct svc_program *prog, struct svc_stat *stats,
+			    unsigned int bufsize, int (*threadfn)(void *data))
 {
-	return __svc_create(prog, bufsize, 1, threadfn);
+	return __svc_create(prog, stats, bufsize, 1, threadfn);
 }
 EXPORT_SYMBOL_GPL(svc_create);
 
 /**
  * svc_create_pooled - Create an RPC service with pooled threads
  * @prog: the RPC program the new service will handle
+ * @stats: the stats struct if desired
  * @bufsize: maximum message size for @prog
  * @threadfn: a function to service RPC requests for @prog
  *
  * Returns an instantiated struct svc_serv object or NULL.
  */
 struct svc_serv *svc_create_pooled(struct svc_program *prog,
+				   struct svc_stat *stats,
 				   unsigned int bufsize,
 				   int (*threadfn)(void *data))
 {
 	struct svc_serv *serv;
 	unsigned int npools = svc_pool_map_get();
 
-	serv = __svc_create(prog, bufsize, npools, threadfn);
+	serv = __svc_create(prog, stats, bufsize, npools, threadfn);
 	if (!serv)
 		goto out_err;
 	return serv;