| Message ID | 20210521190938.24820-3-dai.ngo@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFSD: delay unmount source's export after inter-server copy completed. | expand |
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 <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
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
On Mon, Jul 12, 2021 at 1:41 PM Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote: > > 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. So I don't see anything wrong with this patch, and I'm assuming it just got missed due to Bruce initially "claiming" it before realizing it's for the wrong subsystem. Anyway, I just put it into my tree for the next merge. Anna > > > > > --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
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);
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 <dai.ngo@oracle.com> --- fs/nfs/nfs4file.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)