[v7,0/4] copy_file_range in cephfs kernel client

Message

Luis Henriques Oct. 15, 2018, 3:45 p.m. UTC

Changes since v6:

- Mostly adding back the dst_fadvise_flags that I had removed in v6.
  And adjusted the flags in ceph_copy_file_range.

Changes since v5:

[ Most of these changes were done in libceph patch (0002), but there are
  still a few changes to the FS code as well. ]

- Fixed mess regarding 'copy-from' Op flags.  Pulled in flags
  CEPH_OSD_COPY_FROM_FLAG_* and started using them when calling
  ceph_osdc_copy_from from ceph_copy_file_range.  Also changed
  CEPH_OSD_OP_FLAG_FADVISE_WILLNEED to CEPH_OSD_OP_FLAG_FADVISE_DONTNEED
  since it seems to make more sense for the copy_file_range use cases.

- Dropped dst_snapid parameter and renamed dst_fadvise_flags to
  copy_from_flags in ceph_osdc_copy_from

- New helper osd_req_op_copy_from_init to initialise the req_op.  Also
  dropped ceph_release_page_vector; instead do the release in
  osd_req_op_data_release, which will now include the copy_from Op.

- Use GFP_KERNEL for memory allocation

- Use ceph_osdc_wait_request instead of wait_request_timeout

- Drop calls to ceph_oid_destroy in ceph_copy_file_range as CephFS
  object names use the inline_name version, so no memory allocation is
  done

- MDS notification about size change (and Fw dirty) needs to be done
  after remote copy.  Refactored code to do that in copy_file_range

Changes since v4:

- Complete rewrite of ceph_copy_file_range function.  Now the copy loop
  includes only the remote object copies, while do_splice_direct is
  invoked (at most) twice -- before and after object copy loop.

- Check sizes after every put/get caps cycle

- Added new checks to ensure the files being copied have the same
  layouts

Changes since v3:

- release/get caps before doing the do_splice_direct calls

- if an error occurs after data has been copied already, return the
  amount of bytes already copied instead of the error

- fix oid init/destroy (was broken since a pre-v1 version of the patch)

- always mark Fw as dirty, without FILE_BUFFER

- call file_update_time on destination file

- added an extra mount option (nocopyfrom) which allows an admin to force
  a fallback to VFS copy_file_range

- added a some more debug messages

Changes since v2:

- Files size checks are now done after we have all the required caps

Here's the main changes since v1, after Zheng's review:

1. ceph_osdc_copy_from() now receives source and destination snapids
   instead of ceph_vino structs

2. Also get FILE_RD capabilities in ceph_copy_file_range() for source
   file as other clients may have dirty data in their cache.

3. Fallback to VFS copy_file_range default implementation if we're
  copying beyond source file EOF

Note that 2. required an extra patch modifying ceph_try_get_caps() so
that it could perform a non-blocking attempt at getting CEPH_CAP_FILE_RD
capabilities.

And here's the original (v1) RFC cover letter just for reference:

This series is my initial attempt at getting a copy_file_range syscall
implementation in the kernel cephfs client using the 'copy-from' RADOS
operation.

The idea of getting this implemented was from Greg -- or, at least, he
created a feature in the tracker [1].  I just decided to give it a try
as the feature wasn't assigned to anyone ;-)

I have this patchset sitting on my laptop for a while already, waiting
for me to revisit it, review some of its TODOs... but I finally decided
to send it out as-is instead, to get some early feedback.

The first patch implements the copy-from operation in the libceph
module.  Unfortunately, the documentation for this operation is
nonexistent and I had to do a lot of digging to figure out the details
(and I probably I missed something!).  For example, initially I was
hoping that this operation could be used to copy more than one object at
the time.  Doing an OSD request per object copy is not ideal, but
unfortunately it seems to be the only way.  Anyway, my expectations are
that this new operation will be useful for other features in the future.

The 2nd patch is where the copy_file_range is implemented and could
probably be optimised, but I didn't bother with that for now.  The
important bit is that we still may need to do some manual copies if the
offsets aren't object aligned or if the length is smaller than the
object size.  I'm using do_splice_direct() for the manual copies as it
was the easiest way to get a PoC running, but maybe there are better
ways.

I've done some functional testing on this PoC.  And it also passes the
generic xfstest suite, in particular the copy_file_range specific tests
(430-434).  But I haven't done any benchmarks to measure any performance
changes in using this syscall.

Any feedback is welcome, specially regarding the TODOs on the code.

[1] https://tracker.ceph.com/issues/21944

Luis Henriques (4):
  ceph: add non-blocking parameter to ceph_try_get_caps()
  libceph: support the RADOS copy-from operation
  ceph: support copy_file_range file operation
  ceph: new mount option to disable usage of RADOS 'copy-from' op

 Documentation/filesystems/ceph.txt |   5 +
 fs/ceph/addr.c                     |   2 +-
 fs/ceph/caps.c                     |   7 +-
 fs/ceph/file.c                     | 299 ++++++++++++++++++++++++++++-
 fs/ceph/super.c                    |  13 ++
 fs/ceph/super.h                    |   3 +-
 include/linux/ceph/osd_client.h    |  16 ++
 include/linux/ceph/rados.h         |  28 +++
 net/ceph/osd_client.c              |  90 +++++++++
 9 files changed, 457 insertions(+), 6 deletions(-)


Luis Henriques (4):
  ceph: add non-blocking parameter to ceph_try_get_caps()
  libceph: support the RADOS copy-from operation
  ceph: support copy_file_range file operation
  ceph: new mount option to disable usage of RADOS 'copy-from' op

 Documentation/filesystems/ceph.txt |   5 +
 fs/ceph/addr.c                     |   2 +-
 fs/ceph/caps.c                     |   7 +-
 fs/ceph/file.c                     | 297 ++++++++++++++++++++++++++++-
 fs/ceph/super.c                    |  13 ++
 fs/ceph/super.h                    |   3 +-
 include/linux/ceph/osd_client.h    |  17 ++
 include/linux/ceph/rados.h         |  28 +++
 net/ceph/osd_client.c              |  88 +++++++++
 9 files changed, 454 insertions(+), 6 deletions(-)

Comments

Luis Henriques Oct. 27, 2018, 5:47 p.m. UTC | #1

Hi,

Since it's currently being discussed upstream the removal of the cross
device check from copy_file_range (see [1] for details), it may be worth
having the check in the patch below added into this patchset (But we can
also wait for those VFS changes to be merged first, of course.)

The patch below should do the trick, but in the future we could also
think about something more elaborate.  For example, it shouldn't be too
difficult to add the possibility of doing remote copies between 2
different filesystems within the same cluster.  (Well, at least doing
*full* file copies, but maybe also partial copies.)

[1] https://marc.info/?l=linux-fsdevel&m=154058466308234&w=2

Cheers,

Yan, Zheng Oct. 29, 2018, 1 a.m. UTC | #2

> On Oct 28, 2018, at 01:47, Luis Henriques <[e-mail]> wrote:
> 
> Hi,
> 
> Since it's currently being discussed upstream the removal of the cross
> device check from copy_file_range (see [1] for details), it may be worth
> having the check in the patch below added into this patchset (But we can
> also wait for those VFS changes to be merged first, of course.)
> 
> The patch below should do the trick, but in the future we could also
> think about something more elaborate.  For example, it shouldn't be too
> difficult to add the possibility of doing remote copies between 2
> different filesystems within the same cluster.  (Well, at least doing
> *full* file copies, but maybe also partial copies.)
> 
> [1] https://marc.info/?l=linux-fsdevel&m=154058466308234&w=2
> 
> Cheers,
> -- 
> Luis
> 
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index b4607baa8969..813d49094ad5 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -1904,6 +1904,9 @@ static ssize_t ceph_copy_file_range(struct file *src_file, loff_t src_off,
> 	int src_got = 0, dst_got = 0, err, dirty;
> 	bool do_final_copy = false;
> 
> +	if (src_inode->i_sb != dst_inode->i_sb)
> +		return -EXDEV;
> +
> 	if (src_inode == dst_inode)
> 		return -EINVAL;
> 	if (ceph_snap(dst_inode) != CEPH_NOSNAP)

Applied, thanks

Yan, Zheng