On Mon, 2024-08-19 at 14:17 -0400, Mike Snitzer wrote:
> Change nfsd_open_local_fh() to return an nfsd_file, instead of file,
> and move associated reference counting to the nfs client's
> nfs_local_open_fh().
>
> This is the first step in allowing proper use of nfsd_file for localio
> (such that the benefits of nfsd's filecache can be realized).
>
> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> ---
> fs/nfs/localio.c | 7 +++++--
> fs/nfs_common/nfslocalio.c | 2 +-
> fs/nfsd/localio.c | 11 ++++-------
> fs/nfsd/vfs.h | 2 +-
> include/linux/nfslocalio.h | 2 +-
> 5 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
> index 38303427e0b3..7d63d7e34643 100644
> --- a/fs/nfs/localio.c
> +++ b/fs/nfs/localio.c
> @@ -237,13 +237,14 @@ nfs_local_open_fh(struct nfs_client *clp, const struct cred *cred,
> struct nfs_fh *fh, const fmode_t mode)
> {
> struct file *filp;
> + struct nfsd_file *nf;
> int status;
>
> if (mode & ~(FMODE_READ | FMODE_WRITE))
> return ERR_PTR(-EINVAL);
>
> status = nfs_to.nfsd_open_local_fh(clp->cl_nfssvc_net, clp->cl_nfssvc_dom,
> - clp->cl_rpcclient, cred, fh, mode, &filp);
> + clp->cl_rpcclient, cred, fh, mode, &nf);
> if (status < 0) {
> trace_nfs_local_open_fh(fh, mode, status);
> switch (status) {
> @@ -255,8 +256,10 @@ nfs_local_open_fh(struct nfs_client *clp, const struct cred *cred,
> case -ETIMEDOUT:
> status = -EAGAIN;
> }
> - filp = ERR_PTR(status);
> + return ERR_PTR(status);
> }
> + filp = get_file(nfs_to.nfsd_file_file(nf));
> + nfs_to.nfsd_file_put(nf);
> return filp;
> }
> EXPORT_SYMBOL_GPL(nfs_local_open_fh);
> diff --git a/fs/nfs_common/nfslocalio.c b/fs/nfs_common/nfslocalio.c
> index 087649911b52..f59167e596d3 100644
> --- a/fs/nfs_common/nfslocalio.c
> +++ b/fs/nfs_common/nfslocalio.c
> @@ -98,7 +98,7 @@ static void put_##NFSD_SYMBOL(void) \
> /* The nfs localio code needs to call into nfsd to map filehandle -> struct nfsd_file */
> extern int nfsd_open_local_fh(struct net *, struct auth_domain *, struct rpc_clnt *,
> const struct cred *, const struct nfs_fh *,
> - const fmode_t, struct file **);
> + const fmode_t, struct nfsd_file **);
> DEFINE_NFS_TO_NFSD_SYMBOL(nfsd_open_local_fh);
>
> /* The nfs localio code needs to call into nfsd to acquire the nfsd_file */
> diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
> index 008b935a3a6c..2ceab49f3cb6 100644
> --- a/fs/nfsd/localio.c
> +++ b/fs/nfsd/localio.c
> @@ -32,13 +32,13 @@
> * @cred: cred that the client established
> * @nfs_fh: filehandle to lookup
> * @fmode: fmode_t to use for open
> - * @pfilp: returned file pointer that maps to @nfs_fh
> + * @pnf: returned nfsd_file pointer that maps to @nfs_fh
> *
> * This function maps a local fh to a path on a local filesystem.
> * This is useful when the nfs client has the local server mounted - it can
> * avoid all the NFS overhead with reads, writes and commits.
> *
> - * On successful return, caller is responsible for calling path_put. Also
> + * On successful return, caller is responsible for calling nfsd_file_put. Also
> * note that this is called from nfs.ko via find_symbol() to avoid an explicit
> * dependency on knfsd. So, there is no forward declaration in a header file
> * for it that is shared with the client.
> @@ -49,7 +49,7 @@ int nfsd_open_local_fh(struct net *cl_nfssvc_net,
> const struct cred *cred,
> const struct nfs_fh *nfs_fh,
> const fmode_t fmode,
> - struct file **pfilp)
> + struct nfsd_file **pnf)
I think I'd prefer to see this change made earlier in the series, when
nfsd_open_local_fh is first introduced (patch #10), and then just adapt
the later patches to deal with the change.
> {
> int mayflags = NFSD_MAY_LOCALIO;
> int status = 0;
> @@ -57,7 +57,6 @@ int nfsd_open_local_fh(struct net *cl_nfssvc_net,
> const struct cred *save_cred;
> struct svc_cred rq_cred;
> struct svc_fh fh;
> - struct nfsd_file *nf;
> __be32 beres;
>
> if (nfs_fh->size > NFS4_FHSIZE)
> @@ -91,13 +90,11 @@ int nfsd_open_local_fh(struct net *cl_nfssvc_net,
> rpcauth_map_clnt_to_svc_cred_local(rpc_clnt, cred, &rq_cred);
>
> beres = nfsd_file_acquire_local(cl_nfssvc_net, &rq_cred, rpc_clnt->cl_vers,
> - cl_nfssvc_dom, &fh, mayflags, &nf);
> + cl_nfssvc_dom, &fh, mayflags, pnf);
> if (beres) {
> status = nfs_stat_to_errno(be32_to_cpu(beres));
> goto out_fh_put;
> }
> - *pfilp = get_file(nf->nf_file);
> - nfsd_file_put(nf);
I do like eliminating the unnecessary refcount shuffle here. Much
simpler.
> out_fh_put:
> fh_put(&fh);
> if (rq_cred.cr_group_info)
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index 9720951c49a0..ec8a8aae540b 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -166,7 +166,7 @@ int nfsd_open_local_fh(struct net *net,
> const struct cred *cred,
> const struct nfs_fh *nfs_fh,
> const fmode_t fmode,
> - struct file **pfilp);
> + struct nfsd_file **pnf);
>
> static inline int fh_want_write(struct svc_fh *fh)
> {
> diff --git a/include/linux/nfslocalio.h b/include/linux/nfslocalio.h
> index 6302d36f9112..7e09ff621d93 100644
> --- a/include/linux/nfslocalio.h
> +++ b/include/linux/nfslocalio.h
> @@ -35,7 +35,7 @@ struct nfsd_file;
> typedef int (*nfs_to_nfsd_open_local_fh_t)(struct net *, struct auth_domain *,
> struct rpc_clnt *, const struct cred *,
> const struct nfs_fh *, const fmode_t,
> - struct file **);
> + struct nfsd_file **);
> typedef struct nfsd_file * (*nfs_to_nfsd_file_get_t)(struct nfsd_file *);
> typedef void (*nfs_to_nfsd_file_put_t)(struct nfsd_file *);
> typedef struct file * (*nfs_to_nfsd_file_file_t)(struct nfsd_file *);
--
Jeff Layton <jlayton@xxxxxxxxxx>
