2021-05-21 20:27:43

by Dai Ngo

[permalink] [raw]
Subject: [PATCH v7 0/2] NFSD: delay unmount source's export after inter-server copy completed.

Hi Olga, Bruce,

Currently the source's export is mounted and unmounted on every
inter-server copy operation. This causes unnecessary overhead
for each copy.

This patch series is an enhancement to allow the export to remain
mounted for a configurable period (default to 15 minutes). If the
export is not being used for the configured time it will be unmounted
by a delayed task. If it's used again then its expiration time is
extended for another period.

Since mount and unmount are no longer done on every copy request,
the restriction of copy size (14*rsize), in __nfs4_copy_file_range,
is removed.

-Dai

v2: fix compiler warning of missing prototype.
v3: remove the used of semaphore.
eliminated all RPC calls for subsequence mount by allowing
all exports from one server to share one vfsmount.
make inter-server threshold a module configuration parameter.
v4: convert nsui_refcnt to use refcount_t.
add note about 20secs wait in nfsd4_interssc_connect.
removed (14*rsize) restriction from __nfs4_copy_file_range.
v5: make use of the laundomat thread to service delayed unmount list.
destroy delayed unmount list when nfsd is shutdown.
make delayed unmount list per nfsd_net to support container.
v6: move content of nfsd4_ssc_umount in to nfsd_net.
move code that manages the delayed unmount list
from nfsd/nfs4proc.c to nfsd/nfs4state.c
cleanup left over from previous versions.
rebase to 5.13-rc2
v7: simplify nfsd4_interssc_connect by moving changes into
functions.




2021-05-21 20:27:53

by Dai Ngo

[permalink] [raw]
Subject: [PATCH v7 2/2] NFSv4.2: remove restriction of copy size for inter-server copy.

Currently inter-server copy is allowed only if the copy size is larger
than (rsize*14) which is the over-head of the mount operation of the
source export. 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.

Signed-off-by: Dai Ngo <[email protected]>
---
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

2021-07-07 00:11:30

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH v7 2/2] NFSv4.2: remove restriction of copy size for inter-server copy.

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....

--b.

>
> Signed-off-by: Dai Ngo <[email protected]>
> ---
> 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

2021-07-12 17:41:19

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH v7 2/2] NFSv4.2: remove restriction of copy size for inter-server copy.

On Tue, Jul 6, 2021 at 8:10 PM J. Bruce Fields <[email protected]> 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 <[email protected]>
> > ---
> > 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