| Message ID | 426191138f5801148a6f7df470ccb59d219452d6.1691481752.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFSD: reduce code duplication between pool_stats_operations and nfsd_rpc_status_operations | expand |
On Tue, 2023-08-08 at 10:05 +0200, Lorenzo Bianconi wrote: > Introduce nfsd_stats_open utility routine in order to reduce code > duplication between pool_stats_operations and > nfsd_rpc_status_operations. > Rename nfsd_pool_stats_release in nfsd_stats_release. > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > fs/nfsd/nfsctl.c | 4 ++-- > fs/nfsd/nfsd.h | 2 +- > fs/nfsd/nfssvc.c | 35 ++++++++++++++++++++--------------- > 3 files changed, 23 insertions(+), 18 deletions(-) > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > index 6bf168b6a410..83eb5c6d894e 100644 > --- a/fs/nfsd/nfsctl.c > +++ b/fs/nfsd/nfsctl.c > @@ -179,7 +179,7 @@ static const struct file_operations pool_stats_operations = { > .open = nfsd_pool_stats_open, > .read = seq_read, > .llseek = seq_lseek, > - .release = nfsd_pool_stats_release, > + .release = nfsd_stats_release, > }; > > DEFINE_SHOW_ATTRIBUTE(nfsd_reply_cache_stats); > @@ -200,7 +200,7 @@ static const struct file_operations nfsd_rpc_status_operations = { > .open = nfsd_rpc_status_open, > .read = seq_read, > .llseek = seq_lseek, > - .release = nfsd_pool_stats_release, > + .release = nfsd_stats_release, > }; > > /* > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h > index 38c390fb2cf9..3e8a47b93fd4 100644 > --- a/fs/nfsd/nfsd.h > +++ b/fs/nfsd/nfsd.h > @@ -93,7 +93,7 @@ int nfsd_nrpools(struct net *); > int nfsd_get_nrthreads(int n, int *, struct net *); > int nfsd_set_nrthreads(int n, int *, struct net *); > int nfsd_pool_stats_open(struct inode *, struct file *); > -int nfsd_pool_stats_release(struct inode *, struct file *); > +int nfsd_stats_release(struct inode *, struct file *); > int nfsd_rpc_status_open(struct inode *inode, struct file *file); > void nfsd_shutdown_threads(struct net *net); > > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c > index 51a6f7a8d3f7..33ad91dd3a2d 100644 > --- a/fs/nfsd/nfssvc.c > +++ b/fs/nfsd/nfssvc.c > @@ -1079,23 +1079,34 @@ bool nfssvc_encode_voidres(struct svc_rqst *rqstp, struct xdr_stream *xdr) > return true; > } > > -int nfsd_pool_stats_open(struct inode *inode, struct file *file) > +static int nfsd_stats_open(struct inode *inode) > { > - int ret; > struct nfsd_net *nn = net_generic(inode->i_sb->s_fs_info, nfsd_net_id); > > mutex_lock(&nfsd_mutex); > - if (nn->nfsd_serv == NULL) { > + if (!nn->nfsd_serv) { > mutex_unlock(&nfsd_mutex); > return -ENODEV; > } > + > svc_get(nn->nfsd_serv); > - ret = svc_pool_stats_open(nn->nfsd_serv, file); Note that svc_pool_stats_open used to be called under the nfsd_mutex and it won't be after this change. I think that's ok though since you hold a reference to the serv. > mutex_unlock(&nfsd_mutex); > - return ret; > + > + return 0; > } > > -int nfsd_pool_stats_release(struct inode *inode, struct file *file) > +int nfsd_pool_stats_open(struct inode *inode, struct file *file) > +{ > + struct nfsd_net *nn = net_generic(inode->i_sb->s_fs_info, nfsd_net_id); > + int ret = nfsd_stats_open(inode); > + > + if (ret) > + return ret; > + > + return svc_pool_stats_open(nn->nfsd_serv, file); > +} > + > +int nfsd_stats_release(struct inode *inode, struct file *file) > { > int ret = seq_release(inode, file); > struct net *net = inode->i_sb->s_fs_info; > @@ -1217,16 +1228,10 @@ static int nfsd_rpc_status_show(struct seq_file *m, void *v) > */ > int nfsd_rpc_status_open(struct inode *inode, struct file *file) > { > - struct nfsd_net *nn = net_generic(inode->i_sb->s_fs_info, nfsd_net_id); > + int ret = nfsd_stats_open(inode); > > - mutex_lock(&nfsd_mutex); > - if (!nn->nfsd_serv) { > - mutex_unlock(&nfsd_mutex); > - return -ENODEV; > - } > - > - svc_get(nn->nfsd_serv); > - mutex_unlock(&nfsd_mutex); > + if (ret) > + return ret; > > return single_open(file, nfsd_rpc_status_show, inode->i_private); > } Meh. I'm not sure this change is really worth it, but it seems to be correct. Reviewed-by: Jeff Layton <jlayton@kernel.org>
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 6bf168b6a410..83eb5c6d894e 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -179,7 +179,7 @@ static const struct file_operations pool_stats_operations = { .open = nfsd_pool_stats_open, .read = seq_read, .llseek = seq_lseek, - .release = nfsd_pool_stats_release, + .release = nfsd_stats_release, }; DEFINE_SHOW_ATTRIBUTE(nfsd_reply_cache_stats); @@ -200,7 +200,7 @@ static const struct file_operations nfsd_rpc_status_operations = { .open = nfsd_rpc_status_open, .read = seq_read, .llseek = seq_lseek, - .release = nfsd_pool_stats_release, + .release = nfsd_stats_release, }; /* diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h index 38c390fb2cf9..3e8a47b93fd4 100644 --- a/fs/nfsd/nfsd.h +++ b/fs/nfsd/nfsd.h @@ -93,7 +93,7 @@ int nfsd_nrpools(struct net *); int nfsd_get_nrthreads(int n, int *, struct net *); int nfsd_set_nrthreads(int n, int *, struct net *); int nfsd_pool_stats_open(struct inode *, struct file *); -int nfsd_pool_stats_release(struct inode *, struct file *); +int nfsd_stats_release(struct inode *, struct file *); int nfsd_rpc_status_open(struct inode *inode, struct file *file); void nfsd_shutdown_threads(struct net *net); diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index 51a6f7a8d3f7..33ad91dd3a2d 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -1079,23 +1079,34 @@ bool nfssvc_encode_voidres(struct svc_rqst *rqstp, struct xdr_stream *xdr) return true; } -int nfsd_pool_stats_open(struct inode *inode, struct file *file) +static int nfsd_stats_open(struct inode *inode) { - int ret; struct nfsd_net *nn = net_generic(inode->i_sb->s_fs_info, nfsd_net_id); mutex_lock(&nfsd_mutex); - if (nn->nfsd_serv == NULL) { + if (!nn->nfsd_serv) { mutex_unlock(&nfsd_mutex); return -ENODEV; } + svc_get(nn->nfsd_serv); - ret = svc_pool_stats_open(nn->nfsd_serv, file); mutex_unlock(&nfsd_mutex); - return ret; + + return 0; } -int nfsd_pool_stats_release(struct inode *inode, struct file *file) +int nfsd_pool_stats_open(struct inode *inode, struct file *file) +{ + struct nfsd_net *nn = net_generic(inode->i_sb->s_fs_info, nfsd_net_id); + int ret = nfsd_stats_open(inode); + + if (ret) + return ret; + + return svc_pool_stats_open(nn->nfsd_serv, file); +} + +int nfsd_stats_release(struct inode *inode, struct file *file) { int ret = seq_release(inode, file); struct net *net = inode->i_sb->s_fs_info; @@ -1217,16 +1228,10 @@ static int nfsd_rpc_status_show(struct seq_file *m, void *v) */ int nfsd_rpc_status_open(struct inode *inode, struct file *file) { - struct nfsd_net *nn = net_generic(inode->i_sb->s_fs_info, nfsd_net_id); + int ret = nfsd_stats_open(inode); - mutex_lock(&nfsd_mutex); - if (!nn->nfsd_serv) { - mutex_unlock(&nfsd_mutex); - return -ENODEV; - } - - svc_get(nn->nfsd_serv); - mutex_unlock(&nfsd_mutex); + if (ret) + return ret; return single_open(file, nfsd_rpc_status_show, inode->i_private); }
Introduce nfsd_stats_open utility routine in order to reduce code duplication between pool_stats_operations and nfsd_rpc_status_operations. Rename nfsd_pool_stats_release in nfsd_stats_release. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- fs/nfsd/nfsctl.c | 4 ++-- fs/nfsd/nfsd.h | 2 +- fs/nfsd/nfssvc.c | 35 ++++++++++++++++++++--------------- 3 files changed, 23 insertions(+), 18 deletions(-)