linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] NFSv4.1 mark qualified async operations as MOVEABLE tasks
@ 2022-04-13 13:22 Olga Kornievskaia
  2022-05-16 16:34 ` Olga Kornievskaia
  0 siblings, 1 reply; 4+ messages in thread
From: Olga Kornievskaia @ 2022-04-13 13:22 UTC (permalink / raw)
  To: trond.myklebust, anna.schumaker; +Cc: linux-nfs

From: Olga Kornievskaia <kolga@netapp.com>

Mark async operations such as RENAME, REMOVE, COMMIT MOVEABLE
for the nfsv4.1+ sessions.

Fixes: 85e39feead948 ("NFSv4.1 identify and mark RPC tasks that can move between transports")
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4proc.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 16106f805ffa..f593bad996af 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4780,7 +4780,11 @@ static void nfs4_proc_unlink_setup(struct rpc_message *msg,
 
 static void nfs4_proc_unlink_rpc_prepare(struct rpc_task *task, struct nfs_unlinkdata *data)
 {
-	nfs4_setup_sequence(NFS_SB(data->dentry->d_sb)->nfs_client,
+	struct nfs_client *clp = NFS_SB(data->dentry->d_sb)->nfs_client;
+
+	if (clp->cl_minorversion)
+		task->tk_flags |= RPC_TASK_MOVEABLE;
+	nfs4_setup_sequence(clp,
 			&data->args.seq_args,
 			&data->res.seq_res,
 			task);
@@ -4823,7 +4827,11 @@ static void nfs4_proc_rename_setup(struct rpc_message *msg,
 
 static void nfs4_proc_rename_rpc_prepare(struct rpc_task *task, struct nfs_renamedata *data)
 {
-	nfs4_setup_sequence(NFS_SERVER(data->old_dir)->nfs_client,
+	struct nfs_client *clp = NFS_SERVER(data->old_dir)->nfs_client;
+
+	if (clp->cl_minorversion)
+		task->tk_flags |= RPC_TASK_MOVEABLE;
+	nfs4_setup_sequence(clp,
 			&data->args.seq_args,
 			&data->res.seq_res,
 			task);
@@ -5457,7 +5465,11 @@ static void nfs4_proc_read_setup(struct nfs_pgio_header *hdr,
 static int nfs4_proc_pgio_rpc_prepare(struct rpc_task *task,
 				      struct nfs_pgio_header *hdr)
 {
-	if (nfs4_setup_sequence(NFS_SERVER(hdr->inode)->nfs_client,
+	struct nfs_client *clp = NFS_SERVER(hdr->inode)->nfs_client;
+
+	if (clp->cl_minorversion)
+		task->tk_flags |= RPC_TASK_MOVEABLE;
+	if (nfs4_setup_sequence(clp,
 			&hdr->args.seq_args,
 			&hdr->res.seq_res,
 			task))
@@ -5595,7 +5607,11 @@ static void nfs4_proc_write_setup(struct nfs_pgio_header *hdr,
 
 static void nfs4_proc_commit_rpc_prepare(struct rpc_task *task, struct nfs_commit_data *data)
 {
-	nfs4_setup_sequence(NFS_SERVER(data->inode)->nfs_client,
+	struct nfs_client *clp = NFS_SERVER(data->inode)->nfs_client;
+
+	if (clp->cl_minorversion)
+		task->tk_flags |= RPC_TASK_MOVEABLE;
+	nfs4_setup_sequence(clp,
 			&data->args.seq_args,
 			&data->res.seq_res,
 			task);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] NFSv4.1 mark qualified async operations as MOVEABLE tasks
  2022-04-13 13:22 [PATCH 1/1] NFSv4.1 mark qualified async operations as MOVEABLE tasks Olga Kornievskaia
@ 2022-05-16 16:34 ` Olga Kornievskaia
  2022-05-16 18:16   ` Trond Myklebust
  0 siblings, 1 reply; 4+ messages in thread
From: Olga Kornievskaia @ 2022-05-16 16:34 UTC (permalink / raw)
  To: Trond Myklebust, Anna Schumaker; +Cc: linux-nfs

Hi Trond and Anna,

Any update on taking this patch? Thank you.

On Wed, Apr 13, 2022 at 9:22 AM Olga Kornievskaia
<olga.kornievskaia@gmail.com> wrote:
>
> From: Olga Kornievskaia <kolga@netapp.com>
>
> Mark async operations such as RENAME, REMOVE, COMMIT MOVEABLE
> for the nfsv4.1+ sessions.
>
> Fixes: 85e39feead948 ("NFSv4.1 identify and mark RPC tasks that can move between transports")
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/nfs4proc.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 16106f805ffa..f593bad996af 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4780,7 +4780,11 @@ static void nfs4_proc_unlink_setup(struct rpc_message *msg,
>
>  static void nfs4_proc_unlink_rpc_prepare(struct rpc_task *task, struct nfs_unlinkdata *data)
>  {
> -       nfs4_setup_sequence(NFS_SB(data->dentry->d_sb)->nfs_client,
> +       struct nfs_client *clp = NFS_SB(data->dentry->d_sb)->nfs_client;
> +
> +       if (clp->cl_minorversion)
> +               task->tk_flags |= RPC_TASK_MOVEABLE;
> +       nfs4_setup_sequence(clp,
>                         &data->args.seq_args,
>                         &data->res.seq_res,
>                         task);
> @@ -4823,7 +4827,11 @@ static void nfs4_proc_rename_setup(struct rpc_message *msg,
>
>  static void nfs4_proc_rename_rpc_prepare(struct rpc_task *task, struct nfs_renamedata *data)
>  {
> -       nfs4_setup_sequence(NFS_SERVER(data->old_dir)->nfs_client,
> +       struct nfs_client *clp = NFS_SERVER(data->old_dir)->nfs_client;
> +
> +       if (clp->cl_minorversion)
> +               task->tk_flags |= RPC_TASK_MOVEABLE;
> +       nfs4_setup_sequence(clp,
>                         &data->args.seq_args,
>                         &data->res.seq_res,
>                         task);
> @@ -5457,7 +5465,11 @@ static void nfs4_proc_read_setup(struct nfs_pgio_header *hdr,
>  static int nfs4_proc_pgio_rpc_prepare(struct rpc_task *task,
>                                       struct nfs_pgio_header *hdr)
>  {
> -       if (nfs4_setup_sequence(NFS_SERVER(hdr->inode)->nfs_client,
> +       struct nfs_client *clp = NFS_SERVER(hdr->inode)->nfs_client;
> +
> +       if (clp->cl_minorversion)
> +               task->tk_flags |= RPC_TASK_MOVEABLE;
> +       if (nfs4_setup_sequence(clp,
>                         &hdr->args.seq_args,
>                         &hdr->res.seq_res,
>                         task))
> @@ -5595,7 +5607,11 @@ static void nfs4_proc_write_setup(struct nfs_pgio_header *hdr,
>
>  static void nfs4_proc_commit_rpc_prepare(struct rpc_task *task, struct nfs_commit_data *data)
>  {
> -       nfs4_setup_sequence(NFS_SERVER(data->inode)->nfs_client,
> +       struct nfs_client *clp = NFS_SERVER(data->inode)->nfs_client;
> +
> +       if (clp->cl_minorversion)
> +               task->tk_flags |= RPC_TASK_MOVEABLE;
> +       nfs4_setup_sequence(clp,
>                         &data->args.seq_args,
>                         &data->res.seq_res,
>                         task);
> --
> 2.27.0
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] NFSv4.1 mark qualified async operations as MOVEABLE tasks
  2022-05-16 16:34 ` Olga Kornievskaia
@ 2022-05-16 18:16   ` Trond Myklebust
  0 siblings, 0 replies; 4+ messages in thread
From: Trond Myklebust @ 2022-05-16 18:16 UTC (permalink / raw)
  To: anna.schumaker, olga.kornievskaia; +Cc: linux-nfs

On Mon, 2022-05-16 at 12:34 -0400, Olga Kornievskaia wrote:
> Hi Trond and Anna,
> 
> Any update on taking this patch? Thank you.

Can we please rewrite this to not use clp->cl_minorversion? The problem
with assigning functionality to the value of the minor version field is
that you have no guarantees that future minor version updates will
support the same functionality.

I'd therefore much prefer to see a capability assigned to the 'RPC task
is movable' functionality, and that any tests take their cue from the
value of that flag (just set that flag in the 'init_caps' field in
nfs_v4_1_minor_ops and nfs_v4_1_minor_ops). Please also change the
existing tests in the code to use the new flag.

The second suggestion is that we move these tests themselves to the
functions that set up the RPC call. IOW: nfs_initiate_pgio(),
nfs_initiate_commit(), nfs_do_call_unlink(), nfs_async_rename(), etc.
They don't need to be in the rpc_call_prepare() callback, since there
is no condition that might change the outcome of the test once the RPC
call has been set up.

> 
> On Wed, Apr 13, 2022 at 9:22 AM Olga Kornievskaia
> <olga.kornievskaia@gmail.com> wrote:
> > 
> > From: Olga Kornievskaia <kolga@netapp.com>
> > 
> > Mark async operations such as RENAME, REMOVE, COMMIT MOVEABLE
> > for the nfsv4.1+ sessions.
> > 
> > Fixes: 85e39feead948 ("NFSv4.1 identify and mark RPC tasks that can
> > move between transports")
> > Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> > ---
> >  fs/nfs/nfs4proc.c | 24 ++++++++++++++++++++----
> >  1 file changed, 20 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 16106f805ffa..f593bad996af 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -4780,7 +4780,11 @@ static void nfs4_proc_unlink_setup(struct
> > rpc_message *msg,
> > 
> >  static void nfs4_proc_unlink_rpc_prepare(struct rpc_task *task,
> > struct nfs_unlinkdata *data)
> >  {
> > -       nfs4_setup_sequence(NFS_SB(data->dentry->d_sb)->nfs_client,
> > +       struct nfs_client *clp = NFS_SB(data->dentry->d_sb)-
> > >nfs_client;
> > +
> > +       if (clp->cl_minorversion)
> > +               task->tk_flags |= RPC_TASK_MOVEABLE;
> > +       nfs4_setup_sequence(clp,
> >                         &data->args.seq_args,
> >                         &data->res.seq_res,
> >                         task);
> > @@ -4823,7 +4827,11 @@ static void nfs4_proc_rename_setup(struct
> > rpc_message *msg,
> > 
> >  static void nfs4_proc_rename_rpc_prepare(struct rpc_task *task,
> > struct nfs_renamedata *data)
> >  {
> > -       nfs4_setup_sequence(NFS_SERVER(data->old_dir)->nfs_client,
> > +       struct nfs_client *clp = NFS_SERVER(data->old_dir)-
> > >nfs_client;
> > +
> > +       if (clp->cl_minorversion)
> > +               task->tk_flags |= RPC_TASK_MOVEABLE;
> > +       nfs4_setup_sequence(clp,
> >                         &data->args.seq_args,
> >                         &data->res.seq_res,
> >                         task);
> > @@ -5457,7 +5465,11 @@ static void nfs4_proc_read_setup(struct
> > nfs_pgio_header *hdr,
> >  static int nfs4_proc_pgio_rpc_prepare(struct rpc_task *task,
> >                                       struct nfs_pgio_header *hdr)
> >  {
> > -       if (nfs4_setup_sequence(NFS_SERVER(hdr->inode)->nfs_client,
> > +       struct nfs_client *clp = NFS_SERVER(hdr->inode)-
> > >nfs_client;
> > +
> > +       if (clp->cl_minorversion)
> > +               task->tk_flags |= RPC_TASK_MOVEABLE;
> > +       if (nfs4_setup_sequence(clp,
> >                         &hdr->args.seq_args,
> >                         &hdr->res.seq_res,
> >                         task))
> > @@ -5595,7 +5607,11 @@ static void nfs4_proc_write_setup(struct
> > nfs_pgio_header *hdr,
> > 
> >  static void nfs4_proc_commit_rpc_prepare(struct rpc_task *task,
> > struct nfs_commit_data *data)
> >  {
> > -       nfs4_setup_sequence(NFS_SERVER(data->inode)->nfs_client,
> > +       struct nfs_client *clp = NFS_SERVER(data->inode)-
> > >nfs_client;
> > +
> > +       if (clp->cl_minorversion)
> > +               task->tk_flags |= RPC_TASK_MOVEABLE;
> > +       nfs4_setup_sequence(clp,
> >                         &data->args.seq_args,
> >                         &data->res.seq_res,
> >                         task);
> > --
> > 2.27.0
> > 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/1] NFSv4.1 mark qualified async operations as MOVEABLE tasks
@ 2022-05-25 16:12 Olga Kornievskaia
  0 siblings, 0 replies; 4+ messages in thread
From: Olga Kornievskaia @ 2022-05-25 16:12 UTC (permalink / raw)
  To: trond.myklebust, anna.schumaker; +Cc: linux-nfs

From: Olga Kornievskaia <kolga@netapp.com>

Mark async operations such as RENAME, REMOVE, COMMIT MOVEABLE
for the nfsv4.1+ sessions.

v2
Change how tasks are checked for whether or not they can
be moved based on server's capability. All v1 and v2 version's
tasks are marked as moveable

Fixes: 85e39feead948 ("NFSv4.1 identify and mark RPC tasks that can move between transports")
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4proc.c         | 26 ++++++++++++++------------
 fs/nfs/pagelist.c         |  3 +++
 fs/nfs/unlink.c           |  8 ++++++++
 fs/nfs/write.c            |  4 ++++
 include/linux/nfs_fs_sb.h |  1 +
 5 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 0dfdbb406f96..895808beba25 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1162,7 +1162,7 @@ static int nfs4_call_sync_sequence(struct rpc_clnt *clnt,
 {
 	unsigned short task_flags = 0;
 
-	if (server->nfs_client->cl_minorversion)
+	if (server->caps & NFS_CAP_MOVEABLE)
 		task_flags = RPC_TASK_MOVEABLE;
 	return nfs4_do_call_sync(clnt, server, msg, args, res, task_flags);
 }
@@ -2568,7 +2568,7 @@ static int nfs4_run_open_task(struct nfs4_opendata *data,
 	};
 	int status;
 
-	if (server->nfs_client->cl_minorversion)
+	if (nfs_server_capable(dir, NFS_CAP_MOVEABLE))
 		task_setup_data.flags |= RPC_TASK_MOVEABLE;
 
 	kref_get(&data->kref);
@@ -3737,7 +3737,7 @@ int nfs4_do_close(struct nfs4_state *state, gfp_t gfp_mask, int wait)
 	};
 	int status = -ENOMEM;
 
-	if (server->nfs_client->cl_minorversion)
+	if (nfs_server_capable(state->inode, NFS_CAP_MOVEABLE))
 		task_setup_data.flags |= RPC_TASK_MOVEABLE;
 
 	nfs4_state_protect(server->nfs_client, NFS_SP4_MACH_CRED_CLEANUP,
@@ -4408,7 +4408,7 @@ static int _nfs4_proc_lookup(struct rpc_clnt *clnt, struct inode *dir,
 	};
 	unsigned short task_flags = 0;
 
-	if (server->nfs_client->cl_minorversion)
+	if (nfs_server_capable(dir, NFS_CAP_MOVEABLE))
 		task_flags = RPC_TASK_MOVEABLE;
 
 	/* Is this is an attribute revalidation, subject to softreval? */
@@ -6640,10 +6640,13 @@ static int _nfs4_proc_delegreturn(struct inode *inode, const struct cred *cred,
 		.rpc_client = server->client,
 		.rpc_message = &msg,
 		.callback_ops = &nfs4_delegreturn_ops,
-		.flags = RPC_TASK_ASYNC | RPC_TASK_TIMEOUT | RPC_TASK_MOVEABLE,
+		.flags = RPC_TASK_ASYNC | RPC_TASK_TIMEOUT,
 	};
 	int status = 0;
 
+	if (nfs_server_capable(inode, NFS_CAP_MOVEABLE))
+		task_setup_data.flags |= RPC_TASK_MOVEABLE;
+
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (data == NULL)
 		return -ENOMEM;
@@ -6957,10 +6960,8 @@ static struct rpc_task *nfs4_do_unlck(struct file_lock *fl,
 		.workqueue = nfsiod_workqueue,
 		.flags = RPC_TASK_ASYNC,
 	};
-	struct nfs_client *client =
-		NFS_SERVER(lsp->ls_state->inode)->nfs_client;
 
-	if (client->cl_minorversion)
+	if (nfs_server_capable(lsp->ls_state->inode, NFS_CAP_MOVEABLE))
 		task_setup_data.flags |= RPC_TASK_MOVEABLE;
 
 	nfs4_state_protect(NFS_SERVER(lsp->ls_state->inode)->nfs_client,
@@ -7231,9 +7232,8 @@ static int _nfs4_do_setlk(struct nfs4_state *state, int cmd, struct file_lock *f
 		.flags = RPC_TASK_ASYNC | RPC_TASK_CRED_NOREF,
 	};
 	int ret;
-	struct nfs_client *client = NFS_SERVER(state->inode)->nfs_client;
 
-	if (client->cl_minorversion)
+	if (nfs_server_capable(state->inode, NFS_CAP_MOVEABLE))
 		task_setup_data.flags |= RPC_TASK_MOVEABLE;
 
 	data = nfs4_alloc_lockdata(fl, nfs_file_open_context(fl->fl_file),
@@ -10468,7 +10468,8 @@ static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
 		| NFS_CAP_POSIX_LOCK
 		| NFS_CAP_STATEID_NFSV41
 		| NFS_CAP_ATOMIC_OPEN_V1
-		| NFS_CAP_LGOPEN,
+		| NFS_CAP_LGOPEN
+		| NFS_CAP_MOVEABLE,
 	.init_client = nfs41_init_client,
 	.shutdown_client = nfs41_shutdown_client,
 	.match_stateid = nfs41_match_stateid,
@@ -10503,7 +10504,8 @@ static const struct nfs4_minor_version_ops nfs_v4_2_minor_ops = {
 		| NFS_CAP_LAYOUTSTATS
 		| NFS_CAP_CLONE
 		| NFS_CAP_LAYOUTERROR
-		| NFS_CAP_READ_PLUS,
+		| NFS_CAP_READ_PLUS
+		| NFS_CAP_MOVEABLE,
 	.init_client = nfs41_init_client,
 	.shutdown_client = nfs41_shutdown_client,
 	.match_stateid = nfs41_match_stateid,
diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
index 9157dd19b8b4..317cedfa52bf 100644
--- a/fs/nfs/pagelist.c
+++ b/fs/nfs/pagelist.c
@@ -767,6 +767,9 @@ int nfs_initiate_pgio(struct rpc_clnt *clnt, struct nfs_pgio_header *hdr,
 		.flags = RPC_TASK_ASYNC | flags,
 	};
 
+	if (nfs_server_capable(hdr->inode, NFS_CAP_MOVEABLE))
+		task_setup_data.flags |= RPC_TASK_MOVEABLE;
+
 	hdr->rw_ops->rw_initiate(hdr, &msg, rpc_ops, &task_setup_data, how);
 
 	dprintk("NFS: initiated pgio call "
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 6f325e10056c..9697cd5d2561 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -102,6 +102,10 @@ static void nfs_do_call_unlink(struct inode *inode, struct nfs_unlinkdata *data)
 	};
 	struct rpc_task *task;
 	struct inode *dir = d_inode(data->dentry->d_parent);
+
+	if (nfs_server_capable(inode, NFS_CAP_MOVEABLE))
+		task_setup_data.flags |= RPC_TASK_MOVEABLE;
+
 	nfs_sb_active(dir->i_sb);
 	data->args.fh = NFS_FH(dir);
 	nfs_fattr_init(data->res.dir_attr);
@@ -344,6 +348,10 @@ nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
 		.flags = RPC_TASK_ASYNC | RPC_TASK_CRED_NOREF,
 	};
 
+	if (nfs_server_capable(old_dir, NFS_CAP_MOVEABLE) &&
+	    nfs_server_capable(new_dir, NFS_CAP_MOVEABLE))
+		task_setup_data.flags |= RPC_TASK_MOVEABLE;
+
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (data == NULL)
 		return ERR_PTR(-ENOMEM);
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 2f41659e232e..1c706465d090 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1709,6 +1709,10 @@ int nfs_initiate_commit(struct rpc_clnt *clnt, struct nfs_commit_data *data,
 		.flags = RPC_TASK_ASYNC | flags,
 		.priority = priority,
 	};
+
+	if (nfs_server_capable(data->inode, NFS_CAP_MOVEABLE))
+		task_setup_data.flags |= RPC_TASK_MOVEABLE;
+
 	/* Set up the initial task struct.  */
 	nfs_ops->commit_setup(data, &msg, &task_setup_data.rpc_client);
 	trace_nfs_initiate_commit(data);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 157d2bd6b241..ea2f7e6b1b0b 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -287,4 +287,5 @@ struct nfs_server {
 #define NFS_CAP_XATTR		(1U << 28)
 #define NFS_CAP_READ_PLUS	(1U << 29)
 #define NFS_CAP_FS_LOCATIONS	(1U << 30)
+#define NFS_CAP_MOVEABLE	(1U << 31)
 #endif
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-05-25 16:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-13 13:22 [PATCH 1/1] NFSv4.1 mark qualified async operations as MOVEABLE tasks Olga Kornievskaia
2022-05-16 16:34 ` Olga Kornievskaia
2022-05-16 18:16   ` Trond Myklebust
2022-05-25 16:12 Olga Kornievskaia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).