* [PATCH v1 1/3] fs: Don't copy beyond the end of the file
2017-03-02 16:02 [PATCH v1 0/3] VFS changes for NFSv4.2 "inter" server-to-server COPY op Olga Kornievskaia
@ 2017-03-02 16:02 ` Olga Kornievskaia
2017-03-02 16:58 ` Darrick J. Wong
2017-03-02 16:02 ` [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range Olga Kornievskaia
2017-03-02 16:02 ` [PATCH v1 3/3] VFS don't try clone if superblocks are different Olga Kornievskaia
2 siblings, 1 reply; 22+ messages in thread
From: Olga Kornievskaia @ 2017-03-02 16:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: hch, linux-nfs
From: Anna Schumaker <Anna.Schumaker@Netapp.com>
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
fs/read_write.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/read_write.c b/fs/read_write.c
index 5816d4c..1d9e305 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1526,6 +1526,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
if (unlikely(ret))
return ret;
+ if (pos_in >= i_size_read(inode_in))
+ return -EINVAL;
+
if (!(file_in->f_mode & FMODE_READ) ||
!(file_out->f_mode & FMODE_WRITE) ||
(file_out->f_flags & O_APPEND))
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/3] fs: Don't copy beyond the end of the file
2017-03-02 16:02 ` [PATCH v1 1/3] fs: Don't copy beyond the end of the file Olga Kornievskaia
@ 2017-03-02 16:58 ` Darrick J. Wong
2017-03-02 18:21 ` Olga Kornievskaia
0 siblings, 1 reply; 22+ messages in thread
From: Darrick J. Wong @ 2017-03-02 16:58 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: linux-fsdevel, hch, linux-nfs
On Thu, Mar 02, 2017 at 11:02:09AM -0500, Olga Kornievskaia wrote:
> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> ---
> fs/read_write.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 5816d4c..1d9e305 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -1526,6 +1526,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
> if (unlikely(ret))
> return ret;
>
> + if (pos_in >= i_size_read(inode_in))
> + return -EINVAL;
> +
This ought to go in vfs_clone_file_prep_inodes.
(He says, noticing that btrfs never got updated to use that validator...)
--D
> if (!(file_in->f_mode & FMODE_READ) ||
> !(file_out->f_mode & FMODE_WRITE) ||
> (file_out->f_flags & O_APPEND))
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/3] fs: Don't copy beyond the end of the file
2017-03-02 16:58 ` Darrick J. Wong
@ 2017-03-02 18:21 ` Olga Kornievskaia
2017-03-02 18:40 ` Darrick J. Wong
0 siblings, 1 reply; 22+ messages in thread
From: Olga Kornievskaia @ 2017-03-02 18:21 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-fsdevel, hch, linux-nfs
> On Mar 2, 2017, at 11:58 AM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Thu, Mar 02, 2017 at 11:02:09AM -0500, Olga Kornievskaia wrote:
>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>
>> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
>> ---
>> fs/read_write.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/read_write.c b/fs/read_write.c
>> index 5816d4c..1d9e305 100644
>> --- a/fs/read_write.c
>> +++ b/fs/read_write.c
>> @@ -1526,6 +1526,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
>> if (unlikely(ret))
>> return ret;
>>
>> + if (pos_in >= i_size_read(inode_in))
>> + return -EINVAL;
>> +
>
> This ought to go in vfs_clone_file_prep_inodes.
>
> (He says, noticing that btrfs never got updated to use that validator…)
I apologize I’m not fully understanding the suggestion here. How btrfs is related to the check that I’m suggesting for the copy_file_range(). I don’t see how it would fix the problem for the copy_file_range().
Is the suggestion that NFS’s clone implementation is suppose to call vfs_clone_file_prep_inodes() where the check would be added and thus because vfs_copy_file_range() first decides to call clone() instead of copy() then that check would be met?
>
> --D
>
>> if (!(file_in->f_mode & FMODE_READ) ||
>> !(file_out->f_mode & FMODE_WRITE) ||
>> (file_out->f_flags & O_APPEND))
>> --
>> 1.8.3.1
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/3] fs: Don't copy beyond the end of the file
2017-03-02 18:21 ` Olga Kornievskaia
@ 2017-03-02 18:40 ` Darrick J. Wong
2017-03-07 23:43 ` Christoph Hellwig
0 siblings, 1 reply; 22+ messages in thread
From: Darrick J. Wong @ 2017-03-02 18:40 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: linux-fsdevel, hch, linux-nfs
On Thu, Mar 02, 2017 at 01:21:49PM -0500, Olga Kornievskaia wrote:
>
> > On Mar 2, 2017, at 11:58 AM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> >
> > On Thu, Mar 02, 2017 at 11:02:09AM -0500, Olga Kornievskaia wrote:
> >> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> >>
> >> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> >> ---
> >> fs/read_write.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/fs/read_write.c b/fs/read_write.c
> >> index 5816d4c..1d9e305 100644
> >> --- a/fs/read_write.c
> >> +++ b/fs/read_write.c
> >> @@ -1526,6 +1526,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
> >> if (unlikely(ret))
> >> return ret;
> >>
> >> + if (pos_in >= i_size_read(inode_in))
> >> + return -EINVAL;
> >> +
> >
> > This ought to go in vfs_clone_file_prep_inodes.
> >
> > (He says, noticing that btrfs never got updated to use that validator…)
>
> I apologize I’m not fully understanding the suggestion here. How btrfs is
> related to the check that I’m suggesting for the copy_file_range(). I don’t
> see how it would fix the problem for the copy_file_range().
Never mind, I misread vfs_copy as vfs_clone and thought you were talking
about something else. :(
Sorry about the noise.
--D
> Is the suggestion that NFS’s clone implementation is suppose to call
> vfs_clone_file_prep_inodes() where the check would be added and thus
> because vfs_copy_file_range() first decides to call clone() instead of
> copy() then that check would be met?
>
> >
> > --D
> >
> >> if (!(file_in->f_mode & FMODE_READ) ||
> >> !(file_out->f_mode & FMODE_WRITE) ||
> >> (file_out->f_flags & O_APPEND))
> >> --
> >> 1.8.3.1
> >>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/3] fs: Don't copy beyond the end of the file
2017-03-02 18:40 ` Darrick J. Wong
@ 2017-03-07 23:43 ` Christoph Hellwig
2017-03-07 23:46 ` Olga Kornievskaia
0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2017-03-07 23:43 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Olga Kornievskaia, linux-fsdevel, hch, linux-nfs
On Thu, Mar 02, 2017 at 10:40:57AM -0800, Darrick J. Wong wrote:
> Never mind, I misread vfs_copy as vfs_clone and thought you were talking
> about something else. :(
>
> Sorry about the noise.
And either way we'll need test coverage for copy_file_range before we
do any more changes to it. I'm sick and tired of this doctoring around
without having any test coverage. I've asked for it again and again
and we've made very little progress despite having the code in the tree
for almost a year and a half.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/3] fs: Don't copy beyond the end of the file
2017-03-07 23:43 ` Christoph Hellwig
@ 2017-03-07 23:46 ` Olga Kornievskaia
2017-03-07 23:50 ` Christoph Hellwig
0 siblings, 1 reply; 22+ messages in thread
From: Olga Kornievskaia @ 2017-03-07 23:46 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Darrick J. Wong, linux-fsdevel, hch, linux-nfs
> On Mar 7, 2017, at 6:43 PM, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Mar 02, 2017 at 10:40:57AM -0800, Darrick J. Wong wrote:
>> Never mind, I misread vfs_copy as vfs_clone and thought you were talking
>> about something else. :(
>>
>> Sorry about the noise.
>
> And either way we'll need test coverage for copy_file_range before we
> do any more changes to it. I'm sick and tired of this doctoring around
> without having any test coverage. I've asked for it again and again
> and we've made very little progress despite having the code in the tree
> for almost a year and a half.
Anna has added test cases to the xfstest suite. Is there something that’s missing?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/3] fs: Don't copy beyond the end of the file
2017-03-07 23:46 ` Olga Kornievskaia
@ 2017-03-07 23:50 ` Christoph Hellwig
2017-03-08 15:39 ` Olga Kornievskaia
0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2017-03-07 23:50 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Christoph Hellwig, Darrick J. Wong, linux-fsdevel, hch, linux-nfs
On Tue, Mar 07, 2017 at 06:46:55PM -0500, Olga Kornievskaia wrote:
> Anna has added test cases to the xfstest suite. Is there something that’s missing?
No, we still don't have that. Anna did one attempt (or maybe two) but
didn't seem to have time to follow up the review comments. Maybe you
can give it a spin to make sure we at least have basic test coverage,
including the cases mentioned in the man page like this.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/3] fs: Don't copy beyond the end of the file
2017-03-07 23:50 ` Christoph Hellwig
@ 2017-03-08 15:39 ` Olga Kornievskaia
2017-03-08 15:57 ` Christoph Hellwig
0 siblings, 1 reply; 22+ messages in thread
From: Olga Kornievskaia @ 2017-03-08 15:39 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Darrick J. Wong, linux-fsdevel, Christoph Hellwig, linux-nfs
> On Mar 7, 2017, at 6:50 PM, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Mar 07, 2017 at 06:46:55PM -0500, Olga Kornievskaia wrote:
>> Anna has added test cases to the xfstest suite. Is there something that’s missing?
>
> No, we still don't have that. Anna did one attempt (or maybe two) but
> didn't seem to have time to follow up the review comments. Maybe you
> can give it a spin to make sure we at least have basic test coverage,
> including the cases mentioned in the man page like this.
I’m reading some mail archives and I see that Anna submitted patches for xfstests. The comment was to re-work it as a part of xfs_io command. She posted the patches for that. According to the announcement I see here: http://oss.sgi.com/archives/xfs/2016-08/msg00221.html it says it’s including the new copy_file_range command in xfs_io. I don’t run these tests so I don’t know but it look to me that the tests are in.
Can you please what I’m getting wrong?
Thank you.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 1/3] fs: Don't copy beyond the end of the file
2017-03-08 15:39 ` Olga Kornievskaia
@ 2017-03-08 15:57 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2017-03-08 15:57 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Christoph Hellwig, Darrick J. Wong, linux-fsdevel,
Christoph Hellwig, linux-nfs
On Wed, Mar 08, 2017 at 10:39:11AM -0500, Olga Kornievskaia wrote:
>
> I’m reading some mail archives and I see that Anna submitted patches for xfstests. The comment was to re-work it as a part of xfs_io command. She posted the patches for that. According to the announcement I see here: http://oss.sgi.com/archives/xfs/2016-08/msg00221.html it says it’s including the new copy_file_range command in xfs_io. I don’t run these tests so I don’t know but it look to me that the tests are in.
That's xfsprogs and the xfs_io command. Now that this step is done
someone (e.g. Anna or you) needs to send the patches to xfstests that
use this command.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range
2017-03-02 16:02 [PATCH v1 0/3] VFS changes for NFSv4.2 "inter" server-to-server COPY op Olga Kornievskaia
2017-03-02 16:02 ` [PATCH v1 1/3] fs: Don't copy beyond the end of the file Olga Kornievskaia
@ 2017-03-02 16:02 ` Olga Kornievskaia
2017-03-02 16:07 ` Christoph Hellwig
2018-08-31 16:13 ` Florian Weimer
2017-03-02 16:02 ` [PATCH v1 3/3] VFS don't try clone if superblocks are different Olga Kornievskaia
2 siblings, 2 replies; 22+ messages in thread
From: Olga Kornievskaia @ 2017-03-02 16:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: hch, linux-nfs
Allow nfs_copy_file_range to copy across devices.
NFSv4.2 inter server to server copy always copies across devices, and
NFSv4.2 intra server to server copy can copy across devices on the same
server.
If a file system's fileoperations copy_file_range operation prohibits
cross-device copies, fall back to do_splice_direct. This is needed for
nfsd_copy_file_range() which is called by the inter server to server
destination server acting as an NFS client, and reading the file from
the source server.
Signed-off-by: Andy Adamson <andros@netapp.com>
---
fs/read_write.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 1d9e305..75084cd 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1534,10 +1534,6 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
(file_out->f_flags & O_APPEND))
return -EBADF;
- /* this could be relaxed once a method supports cross-fs copies */
- if (inode_in->i_sb != inode_out->i_sb)
- return -EXDEV;
-
if (len == 0)
return 0;
@@ -1559,7 +1555,7 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
if (file_out->f_op->copy_file_range) {
ret = file_out->f_op->copy_file_range(file_in, pos_in, file_out,
pos_out, len, flags);
- if (ret != -EOPNOTSUPP)
+ if (ret != -EOPNOTSUPP && ret != -EXDEV)
goto done;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range
2017-03-02 16:02 ` [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range Olga Kornievskaia
@ 2017-03-02 16:07 ` Christoph Hellwig
2017-03-02 16:38 ` Olga Kornievskaia
2018-08-31 16:13 ` Florian Weimer
1 sibling, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2017-03-02 16:07 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: linux-fsdevel, hch, linux-nfs
On Thu, Mar 02, 2017 at 11:02:10AM -0500, Olga Kornievskaia wrote:
> Allow nfs_copy_file_range to copy across devices.
> NFSv4.2 inter server to server copy always copies across devices, and
> NFSv4.2 intra server to server copy can copy across devices on the same
> server.
>
> If a file system's fileoperations copy_file_range operation prohibits
> cross-device copies, fall back to do_splice_direct. This is needed for
> nfsd_copy_file_range() which is called by the inter server to server
> destination server acting as an NFS client, and reading the file from
> the source server.
NAK, we really should not do operations between different superblocks.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range
2017-03-02 16:07 ` Christoph Hellwig
@ 2017-03-02 16:38 ` Olga Kornievskaia
2017-03-07 20:35 ` Olga Kornievskaia
2017-03-15 18:09 ` J. Bruce Fields
0 siblings, 2 replies; 22+ messages in thread
From: Olga Kornievskaia @ 2017-03-02 16:38 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-fsdevel, Linux NFS Mailing List, ng-linux-team
> On Mar 2, 2017, at 11:07 AM, Christoph Hellwig <hch@lst.de> wrote:
>
> On Thu, Mar 02, 2017 at 11:02:10AM -0500, Olga Kornievskaia wrote:
>> Allow nfs_copy_file_range to copy across devices.
>> NFSv4.2 inter server to server copy always copies across devices, and
>> NFSv4.2 intra server to server copy can copy across devices on the same
>> server.
>>
>> If a file system's fileoperations copy_file_range operation prohibits
>> cross-device copies, fall back to do_splice_direct. This is needed for
>> nfsd_copy_file_range() which is called by the inter server to server
>> destination server acting as an NFS client, and reading the file from
>> the source server.
>
> NAK, we really should not do operations between different superblocks.
Can you provide some reasoning as to why? What would it break? The reasoning for including one is to allow for a file system to achieve better performance which seems like a feature that would be of great benefit.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range
2017-03-02 16:38 ` Olga Kornievskaia
@ 2017-03-07 20:35 ` Olga Kornievskaia
2017-03-15 18:09 ` J. Bruce Fields
1 sibling, 0 replies; 22+ messages in thread
From: Olga Kornievskaia @ 2017-03-07 20:35 UTC (permalink / raw)
To: Christoph Hellwig, viro
Cc: linux-fsdevel, Linux NFS Mailing List, ng-linux-team
> On Mar 2, 2017, at 11:38 AM, Olga Kornievskaia <kolga@netapp.com> wrote:
>
>
>> On Mar 2, 2017, at 11:07 AM, Christoph Hellwig <hch@lst.de> wrote:
>>
>> On Thu, Mar 02, 2017 at 11:02:10AM -0500, Olga Kornievskaia wrote:
>>> Allow nfs_copy_file_range to copy across devices.
>>> NFSv4.2 inter server to server copy always copies across devices, and
>>> NFSv4.2 intra server to server copy can copy across devices on the same
>>> server.
>>>
>>> If a file system's fileoperations copy_file_range operation prohibits
>>> cross-device copies, fall back to do_splice_direct. This is needed for
>>> nfsd_copy_file_range() which is called by the inter server to server
>>> destination server acting as an NFS client, and reading the file from
>>> the source server.
>>
>> NAK, we really should not do operations between different superblocks.
>
> Can you provide some reasoning as to why? What would it break? The reasoning for including one is to allow for a file system to achieve better performance which seems like a feature that would be of great benefit.
Christoph, could you please elaborate on your objection.
Al, could you weigh in with regards to if and what would it take to allow for copy_file_range() to allow for copies between different superblocks.
We would appreciate the feedback of how can we enable this performance feature to be useful to users.
Thank you.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range
2017-03-02 16:38 ` Olga Kornievskaia
2017-03-07 20:35 ` Olga Kornievskaia
@ 2017-03-15 18:09 ` J. Bruce Fields
2017-03-21 15:50 ` J. Bruce Fields
1 sibling, 1 reply; 22+ messages in thread
From: J. Bruce Fields @ 2017-03-15 18:09 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Christoph Hellwig, linux-fsdevel, Linux NFS Mailing List, ng-linux-team
On Thu, Mar 02, 2017 at 11:38:03AM -0500, Olga Kornievskaia wrote:
> > On Mar 2, 2017, at 11:07 AM, Christoph Hellwig <hch@lst.de> wrote:
> > NAK, we really should not do operations between different
> > superblocks.
>
> Can you provide some reasoning as to why? What would it break? The
> reasoning for including one is to allow for a file system to achieve
> better performance which seems like a feature that would be of great
> benefit. --
Yes, this has come up a few times. What's going on?:
- There was an explanation, and I missed it.
- The explanation is complicated and Christoph hasn't had time
to write it up.
- Christoph has a strong suspicion there are issues without
being sure exactly where they are.
- Something else?
I mean, the whole enterprise is dead in the water without this, as far
as I can tell, so I'd really like to understand the answer one way or
the other.
--b.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range
2017-03-15 18:09 ` J. Bruce Fields
@ 2017-03-21 15:50 ` J. Bruce Fields
[not found] ` <56CDE406-AE24-40E4-852C-1C47C5CCD37E@netapp.com>
0 siblings, 1 reply; 22+ messages in thread
From: J. Bruce Fields @ 2017-03-21 15:50 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Christoph Hellwig, linux-fsdevel, Linux NFS Mailing List, ng-linux-team
On Wed, Mar 15, 2017 at 02:09:13PM -0400, J. Bruce Fields wrote:
> On Thu, Mar 02, 2017 at 11:38:03AM -0500, Olga Kornievskaia wrote:
> > > On Mar 2, 2017, at 11:07 AM, Christoph Hellwig <hch@lst.de> wrote:
> > > NAK, we really should not do operations between different
> > > superblocks.
> >
> > Can you provide some reasoning as to why? What would it break? The
> > reasoning for including one is to allow for a file system to achieve
> > better performance which seems like a feature that would be of great
> > benefit. --
>
> Yes, this has come up a few times. What's going on?:
>
> - There was an explanation, and I missed it.
> - The explanation is complicated and Christoph hasn't had time
> to write it up.
> - Christoph has a strong suspicion there are issues without
> being sure exactly where they are.
> - Something else?
As far as I can tell from talking at LSF:
- file system operations crossing superblocks is unusual. No
specific known issue. Also not sure I understand why splice
isn't precedent.
- implementation (with server acting as a client, long-running
process, etc.) will be complicated and ugly. Sure, I guess
we'll see how complicated and weigh that against any
advantages. (Though I'm curious about case e.g. of btrfs
clone--can't it easily clone across filesystem boundaries in
some cases when they share storage?)
Anyway. I'll read the patches. Probably not this week.
--b.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range
2017-03-02 16:02 ` [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range Olga Kornievskaia
2017-03-02 16:07 ` Christoph Hellwig
@ 2018-08-31 16:13 ` Florian Weimer
2018-08-31 16:25 ` Olga Kornievskaia
1 sibling, 1 reply; 22+ messages in thread
From: Florian Weimer @ 2018-08-31 16:13 UTC (permalink / raw)
To: Olga Kornievskaia, linux-fsdevel; +Cc: hch, linux-nfs
On 03/02/2017 05:02 PM, Olga Kornievskaia wrote:
> Allow nfs_copy_file_range to copy across devices.
> NFSv4.2 inter server to server copy always copies across devices, and
> NFSv4.2 intra server to server copy can copy across devices on the same
> server.
>
> If a file system's fileoperations copy_file_range operation prohibits
> cross-device copies, fall back to do_splice_direct. This is needed for
> nfsd_copy_file_range() which is called by the inter server to server
> destination server acting as an NFS client, and reading the file from
> the source server.
>
> Signed-off-by: Andy Adamson<andros@netapp.com>
What has happened to the patch? We unwittingly used copy_file_range in
the glibc test suite, without realizing that it does not support
cross-device copies.
Thanks,
Florian
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range
2018-08-31 16:13 ` Florian Weimer
@ 2018-08-31 16:25 ` Olga Kornievskaia
2018-08-31 22:56 ` Steve French
0 siblings, 1 reply; 22+ messages in thread
From: Olga Kornievskaia @ 2018-08-31 16:25 UTC (permalink / raw)
To: fweimer; +Cc: Olga Kornievskaia, linux-fsdevel, Christoph Hellwig, linux-nfs
On Fri, Aug 31, 2018 at 12:14 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> On 03/02/2017 05:02 PM, Olga Kornievskaia wrote:
> > Allow nfs_copy_file_range to copy across devices.
> > NFSv4.2 inter server to server copy always copies across devices, and
> > NFSv4.2 intra server to server copy can copy across devices on the same
> > server.
> >
> > If a file system's fileoperations copy_file_range operation prohibits
> > cross-device copies, fall back to do_splice_direct. This is needed for
> > nfsd_copy_file_range() which is called by the inter server to server
> > destination server acting as an NFS client, and reading the file from
> > the source server.
> >
> > Signed-off-by: Andy Adamson<andros@netapp.com>
>
> What has happened to the patch? We unwittingly used copy_file_range in
> the glibc test suite, without realizing that it does not support
> cross-device copies.
I'm still planning to fight for the patch to be included. As per
maintainers request, I separated out the async copy patches out and
hopefully that will be going into 4.20. I'm working on the "inter"
copy offload functionality which does require this patch. I will start
submitting and pushing this patch along with the rest of the patches.
Are you interested in the copy_file_range() functionality that does
support cross-device copies? If so can you tell me how are you using
it? It would be also helpful to watch for the submission of the patch
and argue in its favor.
Thank you.
>
> Thanks,
> Florian
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range
2018-08-31 16:25 ` Olga Kornievskaia
@ 2018-08-31 22:56 ` Steve French
0 siblings, 0 replies; 22+ messages in thread
From: Steve French @ 2018-08-31 22:56 UTC (permalink / raw)
To: aglo; +Cc: fweimer, kolga, linux-fsdevel, Christoph Hellwig, linux-nfs
On Fri, Aug 31, 2018 at 11:41 AM Olga Kornievskaia <aglo@umich.edu> wrote:
>
> On Fri, Aug 31, 2018 at 12:14 PM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > On 03/02/2017 05:02 PM, Olga Kornievskaia wrote:
> > > Allow nfs_copy_file_range to copy across devices.
> > > NFSv4.2 inter server to server copy always copies across devices, and
> > > NFSv4.2 intra server to server copy can copy across devices on the same
> > > server.
> > >
> > > If a file system's fileoperations copy_file_range operation prohibits
> > > cross-device copies, fall back to do_splice_direct. This is needed for
> > > nfsd_copy_file_range() which is called by the inter server to server
> > > destination server acting as an NFS client, and reading the file from
> > > the source server.
> > >
> > > Signed-off-by: Andy Adamson<andros@netapp.com>
> >
> > What has happened to the patch? We unwittingly used copy_file_range in
> > the glibc test suite, without realizing that it does not support
> > cross-device copies.
>
> I'm still planning to fight for the patch to be included. As per
> maintainers request, I separated out the async copy patches out and
> hopefully that will be going into 4.20. I'm working on the "inter"
> copy offload functionality which does require this patch. I will start
> submitting and pushing this patch along with the rest of the patches.
>
> Are you interested in the copy_file_range() functionality that does
> support cross-device copies? If so can you tell me how are you using
> it? It would be also helpful to watch for the submission of the patch
> and argue in its favor.
SMB3 clients and server (Windows, Macs, Samba and probably most
every modern NAS out there) support the SMB3 "CopyChunk"
operation used by the Linux client.
I would expect that one of the most useful cases is cross device
For example you have two mounts to your server or server cluster
\\server\share1 is mounted to /mnt1
and
\\server\backup is mounted to /mnt2
and you want to do copy_file or copy_file_range of various
files from /mnt1 to /mnt2
As long as they are both the same file system type, why not
let the file system tell you whether it can do the copy.
Given that this is 10x to 100x faster than the alternative
and this is a common case (and there are many 100s of millions
of SMB3 server capable systems out there which already support
copychunk (and thus would support copy file) - why would we
want to restrict it.
It is much saner to let the file system tell the VFS if it can't
support the cross device copy.
---- and to make it even more obvious why this patch matters ---
Virtualization clients (and various Windows and NetApp server)
support another copy offload mechanism (not just CopyChunk)
ie via ODX which would support cross server not just cross share
copy. Enabling this would be a BIG incentive for finishing up
ODX copy support in Linux (cifs.ko using SMB3). This is
fairly widely supported by servers.
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v1 3/3] VFS don't try clone if superblocks are different
2017-03-02 16:02 [PATCH v1 0/3] VFS changes for NFSv4.2 "inter" server-to-server COPY op Olga Kornievskaia
2017-03-02 16:02 ` [PATCH v1 1/3] fs: Don't copy beyond the end of the file Olga Kornievskaia
2017-03-02 16:02 ` [PATCH v1 2/3] VFS permit cross device vfs_copy_file_range Olga Kornievskaia
@ 2017-03-02 16:02 ` Olga Kornievskaia
2 siblings, 0 replies; 22+ messages in thread
From: Olga Kornievskaia @ 2017-03-02 16:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: hch, linux-nfs
Only try clone_file_range if superblocks are the same.
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
fs/read_write.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 75084cd..00161bd 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1543,7 +1543,8 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
* Try cloning first, this is supported by more file systems, and
* more efficient if both clone and copy are supported (e.g. NFS).
*/
- if (file_in->f_op->clone_file_range) {
+ if (inode_in->i_sb == inode_out->i_sb &&
+ file_in->f_op->clone_file_range) {
ret = file_in->f_op->clone_file_range(file_in, pos_in,
file_out, pos_out, len);
if (ret == 0) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 22+ messages in thread