Re: [PATCH 01/18] vfs: introduce new file range exchange ioctl

On Thu, Apr 01, 2021 at 06:32:02AM +0300, Amir Goldstein wrote:

On Thu, Apr 1, 2021 at 4:13 AM Darrick J. Wong [off-list ref] wrote:

quoted

From: Darrick J. Wong <djwong@kernel.org>

Introduce a new ioctl to handle swapping ranges of bytes between files.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 Documentation/filesystems/vfs.rst |   16 ++
 fs/ioctl.c                        |   42 +++++
 fs/remap_range.c                  |  283 +++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_fs.h            |    1
 include/linux/fs.h                |   14 ++
 include/uapi/linux/fiexchange.h   |  101 +++++++++++++
 6 files changed, 456 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/fiexchange.h

diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst
index 2049bbf5e388..9f16b260bc7e 100644
--- a/Documentation/filesystems/vfs.rst
+++ b/Documentation/filesystems/vfs.rst

@@ -1006,6 +1006,8 @@ This describes how the VFS can manipulate an open file.  As of kernel
                loff_t (*remap_file_range)(struct file *file_in, loff_t pos_in,
                                           struct file *file_out, loff_t pos_out,
                                           loff_t len, unsigned int remap_flags);
+                int (*xchg_file_range)(struct file *file1, struct file *file2,
+                                       struct file_xchg_range *fxr);

An obvious question: why is the xchgn_file_range op not using the
unified remap_file_range() method with REMAP_XCHG_ flags?
Surely replacing the remap_flags arg with struct file_remap_range.

I went to look for reasons and I didn't find them.
Can you share your reasons for that?

Code simplicity.  The file2 freshness parameters don't apply to clone or
dedupe, and the current set of remap flags don't apply to exchange.  I'd
have to hunt down all the ->remap_range implementations and modify them
to error out on REMAP_FILE_EXCHANGE.  Multiplexing flags in this manner
would also require additional remap_flags interpretation code to
safeguard against callers who mix up which flags go with what piece of
functionality.

IOWS: it's not hard to do, but not something I want to do for an RFC
because the goal here is to gauge interest in having a userspace
interface at all.  Until I get to that point, tangling up the code
diverts my time towards rebasing and dealing with merge conflicts, at
the cost of time I can spend concentrating on making the algorithms
right.

--D

Thanks,
Amir.