2024-06-12 03:08:16

by Mike Snitzer

Subject: [RFC PATCH v2 09/15] NFS: Enable localio for non-pNFS I/O

From: Trond Myklebust <[email protected]>

Try a local open of the file we're writing to, and if it succeeds, then
do local I/O.

Signed-off-by: Trond Myklebust <[email protected]>
Signed-off-by: Mike Snitzer <[email protected]>
---
fs/nfs/pagelist.c | 19 ++++++++++---------
fs/nfs/write.c | 7 ++++++-
2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index b08420b8e664..3ee78da5ebc4 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -1063,6 +1063,7 @@ EXPORT_SYMBOL_GPL(nfs_generic_pgio);
static int nfs_generic_pg_pgios(struct nfs_pageio_descriptor *desc)
{
struct nfs_pgio_header *hdr;
+ struct file *filp;
int ret;
unsigned short task_flags = 0;

@@ -1074,18 +1075,18 @@ static int nfs_generic_pg_pgios(struct nfs_pageio_descriptor *desc)
nfs_pgheader_init(desc, hdr, nfs_pgio_header_free);
ret = nfs_generic_pgio(desc, hdr);
if (ret == 0) {
+ struct nfs_client *clp = NFS_SERVER(hdr->inode)->nfs_client;
+
+ filp = nfs_local_file_open(clp, hdr->cred, hdr->args.fh,
+ hdr->args.context);
+
if (NFS_SERVER(hdr->inode)->nfs_client->cl_minorversion)
task_flags = RPC_TASK_MOVEABLE;
- ret = nfs_initiate_pgio(desc,
- NFS_SERVER(hdr->inode)->nfs_client,
- NFS_CLIENT(hdr->inode),
- hdr,
- hdr->cred,
- NFS_PROTO(hdr->inode),
- desc->pg_rpc_callops,
- desc->pg_ioflags,
+ ret = nfs_initiate_pgio(desc, clp, NFS_CLIENT(hdr->inode),
+ hdr, hdr->cred, NFS_PROTO(hdr->inode),
+ desc->pg_rpc_callops, desc->pg_ioflags,
RPC_TASK_CRED_NOREF | task_flags,
- NULL);
+ filp);
}
return ret;
}
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index b29b0fd5431f..b2c06b8b88cd 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1802,6 +1802,8 @@ nfs_commit_list(struct inode *inode, struct list_head *head, int how,
struct nfs_commit_info *cinfo)
{
struct nfs_commit_data *data;
+ struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
+ struct file *filp;
unsigned short task_flags = 0;

/* another commit raced with us */
@@ -1818,9 +1820,12 @@ nfs_commit_list(struct inode *inode, struct list_head *head, int how,
nfs_init_commit(data, head, NULL, cinfo);
if (NFS_SERVER(inode)->nfs_client->cl_minorversion)
task_flags = RPC_TASK_MOVEABLE;
+
+ filp = nfs_local_file_open(clp, data->cred, data->args.fh,
+ data->context);
return nfs_initiate_commit(NFS_CLIENT(inode), data, NFS_PROTO(inode),
data->mds_ops, how,
- RPC_TASK_CRED_NOREF | task_flags, NULL);
+ RPC_TASK_CRED_NOREF | task_flags, filp);
}

/*
--
2.44.0

2024-06-12 05:12:42

by NeilBrown

Subject: Re: [RFC PATCH v2 09/15] NFS: Enable localio for non-pNFS I/O

On Wed, 12 Jun 2024, Mike Snitzer wrote:
> From: Trond Myklebust <[email protected]>
>
> Try a local open of the file we're writing to, and if it succeeds, then
> do local I/O.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> Signed-off-by: Mike Snitzer <[email protected]>
> ---
> fs/nfs/pagelist.c | 19 ++++++++++---------
> fs/nfs/write.c | 7 ++++++-
> 2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
> index b08420b8e664..3ee78da5ebc4 100644
> --- a/fs/nfs/pagelist.c
> +++ b/fs/nfs/pagelist.c
> @@ -1063,6 +1063,7 @@ EXPORT_SYMBOL_GPL(nfs_generic_pgio);
> static int nfs_generic_pg_pgios(struct nfs_pageio_descriptor *desc)
> {
> struct nfs_pgio_header *hdr;
> + struct file *filp;
> int ret;
> unsigned short task_flags = 0;
>
> @@ -1074,18 +1075,18 @@ static int nfs_generic_pg_pgios(struct nfs_pageio_descriptor *desc)
> nfs_pgheader_init(desc, hdr, nfs_pgio_header_free);
> ret = nfs_generic_pgio(desc, hdr);
> if (ret == 0) {
> + struct nfs_client *clp = NFS_SERVER(hdr->inode)->nfs_client;
> +
> + filp = nfs_local_file_open(clp, hdr->cred, hdr->args.fh,
> + hdr->args.context);
> +
> if (NFS_SERVER(hdr->inode)->nfs_client->cl_minorversion)
> task_flags = RPC_TASK_MOVEABLE;
> - ret = nfs_initiate_pgio(desc,
> - NFS_SERVER(hdr->inode)->nfs_client,
> - NFS_CLIENT(hdr->inode),
> - hdr,
> - hdr->cred,
> - NFS_PROTO(hdr->inode),
> - desc->pg_rpc_callops,
> - desc->pg_ioflags,
> + ret = nfs_initiate_pgio(desc, clp, NFS_CLIENT(hdr->inode),
> + hdr, hdr->cred, NFS_PROTO(hdr->inode),
> + desc->pg_rpc_callops, desc->pg_ioflags,
> RPC_TASK_CRED_NOREF | task_flags,
> - NULL);
> + filp);

Is this rearrangement really an improvement? I guess it is personal
taste question, but in this case it makes the patch a little harder to
read.

At first glance it looks like filp doesn't get closed, but I guess it is
getting stored in 'desc' and gets closed when 'desc' is freed.

NeilBrown