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@xxxxxxxxxxxxxx>
> > ---
> > 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
