diff mbox series

[1/1] NFSv4.1 mark qualified async operations as MOVEABLE tasks

Message ID 20220413132207.52825-1-olga.kornievskaia@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/1] NFSv4.1 mark qualified async operations as MOVEABLE tasks | expand

Commit Message

Olga Kornievskaia April 13, 2022, 1:22 p.m. UTC 
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(-)

Comments

Olga Kornievskaia May 16, 2022, 4:34 p.m. UTC | #1 
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
>
Trond Myklebust May 16, 2022, 6:16 p.m. UTC | #2 
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
> >
diff mbox series

Patch

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);