lkml.org 
[lkml]   [2021]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v2] vfs: prevent copy_file_range to copy across devices
    Date
    On Mon, 2021-02-15 at 18:34 +0200, Amir Goldstein wrote:
    > On Mon, Feb 15, 2021 at 5:42 PM Luis Henriques <lhenriques@suse.de>
    > wrote:
    > >
    > > Nicolas Boichat reported an issue when trying to use the
    > > copy_file_range
    > > syscall on a tracefs file.  It failed silently because the file
    > > content is
    > > generated on-the-fly (reporting a size of zero) and copy_file_range
    > > needs
    > > to know in advance how much data is present.
    > >
    > > This commit restores the cross-fs restrictions that existed prior
    > > to
    > > 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices")
    > > and
    > > removes generic_copy_file_range() calls from ceph, cifs, fuse, and
    > > nfs.
    > >
    > > Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across
    > > devices")
    > > Link:
    > > https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drinkcat@chromium.org/
    > > Cc: Nicolas Boichat <drinkcat@chromium.org>
    > > Signed-off-by: Luis Henriques <lhenriques@suse.de>
    >
    > Code looks ok.
    > You may add:
    >
    > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
    >
    > I agree with Trond that the first paragraph of the commit message
    > could
    > be improved.
    > The purpose of this change is to fix the change of behavior that
    > caused the regression.
    >
    > Before v5.3, behavior was -EXDEV and userspace could fallback to
    > read.
    > After v5.3, behavior is zero size copy.
    >
    > It does not matter so much what makes sense for CFR to do in this
    > case (generic cross-fs copy).  What matters is that nobody asked for
    > this change and that it caused problems.
    >

    No. I'm saying that this patch should be NACKed unless there is a real
    explanation for why we give crap about this tracefs corner case and why
    it can't be fixed.

    There are plenty of reasons why copy offload across filesystems makes
    sense, and particularly when you're doing NAS. Clone just doesn't cut
    it when it comes to disaster recovery (whereas backup to a different
    storage unit does). If the client has to do the copy, then you're
    effectively doubling the load on the server, and you're adding
    potentially unnecessary network traffic (or at the very least you are
    doubling that traffic).

    --
    Trond Myklebust
    Linux NFS client maintainer, Hammerspace
    trond.myklebust@hammerspace.com


    \
     
     \ /
      Last update: 2021-02-15 18:38    [W:4.359 / U:0.112 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site