Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210521190938.24820-1-dai.ngo@oracle.com> <20210521190938.24820-3-dai.ngo@oracle.com>
 <20210707001056.GA26847@fieldses.org>
In-Reply-To: <20210707001056.GA26847@fieldses.org>
From:   Olga Kornievskaia <olga.kornievskaia@gmail.com>
Date:   Mon, 12 Jul 2021 13:40:31 -0400
Message-ID: <CAN-5tyERihHa9QVsRKy6FQ=t2VC-8_XRcpAsaXrgdUFoyzeuxw@mail.gmail.com>
Subject: Re: [PATCH v7 2/2] NFSv4.2: remove restriction of copy size for
 inter-server copy.
To:     "J. Bruce Fields" <bfields@fieldses.org>
Cc:     Dai Ngo <dai.ngo@oracle.com>,
        linux-nfs <linux-nfs@vger.kernel.org>,
        Trond Myklebust <trondmy@hammerspace.com>,
        Chuck Lever <chuck.lever@oracle.com>,
        Anna Schumaker <Anna.Schumaker@netapp.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Jul 6, 2021 at 8:10 PM J. Bruce Fields <bfields@fieldses.org> wrote:
>
> Whoops, I overlooked that this is client side, so it needs to go through
> Trond or Anna, not me.
>
> Also note:
>
> On Fri, May 21, 2021 at 03:09:38PM -0400, Dai Ngo wrote:
> > This patch, relying on the delayed unmount feature, removes this
> > restriction since the mount and unmount overhead is now not applicable
> > for every inter-server copy.
>
> There's no guarantee that the same kernel version is running on client
> and server, or even that the server is a Linux server.
>
> If there's reason to expect that the lower overhead should be more
> typical of servers in general, then say that....

I'm in support of this patch not because I expect lower overhead is
more typical but because the initial logic was there to match the
limitations of the only existing implementation (ie Linux server) that
was doing a mount for every copy that's out there. Now that we are
adding support for delayed unmount to the linux server, it is not
important to restrict the size of the copy. Linux server is the ONLY
known implementation of the inter-SSC feature thus it's hard to say
what's typical. Given the improved linux server implementation shows
that it's possible to do short copies as well, I think it's fair to
remove that restriction on the client and expect that other
implementations would be as good or better.

Yes I think client side maintainers need to chime in if they have
objections to the patch.

>
> --b.
>
> >
> > Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> > ---
> >  fs/nfs/nfs4file.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
> > index 441a2fa073c8..b5821ed46994 100644
> > --- a/fs/nfs/nfs4file.c
> > +++ b/fs/nfs/nfs4file.c
> > @@ -158,13 +158,11 @@ static ssize_t __nfs4_copy_file_range(struct file *file_in, loff_t pos_in,
> >               sync = true;
> >  retry:
> >       if (!nfs42_files_from_same_server(file_in, file_out)) {
> > -             /* for inter copy, if copy size if smaller than 12 RPC
> > -              * payloads, fallback to traditional copy. There are
> > -              * 14 RPCs during an NFSv4.x mount between source/dest
> > -              * servers.
> > +             /*
> > +              * for inter copy, if copy size is too small
> > +              * then fallback to generic copy.
> >                */
> > -             if (sync ||
> > -                     count <= 14 * NFS_SERVER(file_inode(file_in))->rsize)
> > +             if (sync)
> >                       return -EOPNOTSUPP;
> >               cn_resp = kzalloc(sizeof(struct nfs42_copy_notify_res),
> >                               GFP_NOFS);
> > --
> > 2.9.5