Darrick J. Wong
2018-Oct-05 17:50 UTC
[Ocfs2-devel] [PATCH 09/15] vfs: pass operation flags to {clone, dedupe}_file_range implementations
On Fri, Oct 05, 2018 at 10:07:43AM +0300, Amir Goldstein wrote:> On Fri, Oct 5, 2018 at 3:46 AM Darrick J. Wong <darrick.wong at oracle.com> wrote: > > > > From: Darrick J. Wong <darrick.wong at oracle.com> > > > > Pass operational flags to the per-filesystem clone and dedupe > > implementations. This enables the vfs to signal when it can deal with > > short clone and short dedupe operations. > > > > Signed-off-by: Darrick J. Wong <darrick.wong at oracle.com> > > --- > [...] > > --- a/include/linux/fs.h > > +++ b/include/linux/fs.h > > @@ -1761,7 +1761,7 @@ struct file_operations { > > loff_t, size_t, unsigned int); > > s64 (*clone_file_range)(struct file *file_in, loff_t pos_in, > > struct file *file_out, loff_t pos_out, > > - u64 count); > > + u64 count, unsigned int flags); > > s64 (*dedupe_file_range)(struct file *file_in, loff_t pos_in, > > struct file *file_out, loff_t pos_out, > > u64 count); > > @@ -1827,9 +1827,15 @@ extern ssize_t vfs_readv(struct file *, const struct iovec __user *, > > unsigned long, loff_t *, rwf_t); > > extern ssize_t vfs_copy_file_range(struct file *, loff_t , struct file *, > > loff_t, size_t, unsigned int); > > +/* Caller can handle a shortened operation. */ > > +#define CLONERANGE_SHORT (1 << 0) > > +/* End operation at the source file's EOF. */ > > +#define CLONERANGE_EOF (1 << 1) > > +/* Operation is actually dedupe, not clone. */ > > +#define CLONERANGE_DEDUPE (1 << 2) > > That's cool. But you know what's going to be the next step, right? > Merging the 3 file operation interfaces into a single one. > copy_file_range() already has the flags arg for future extensions > and as you wrote somewhere, clone is really an optimized copy. > ovl_copyfile() already does that internally. > > So the only take away for this patch series, please use constant > names COPYRANGE_* and also explicitly define: > > /* Operation is actually clone, not copy. */ > #define COPYRANGE_CLONE (1 << 2) > /* Operation is actually dedupe, not copy. */ > #define COPYRANGE_DEDUPE (1 << 3)Yeah, I was too tired to try to throw that one on top of the flaming garbage pile. But I guess since I have a bunch more work to do to the previous patch I might as well do that... --D> > Thanks, > Amir.
Christoph Hellwig
2018-Oct-06 10:44 UTC
[Ocfs2-devel] [PATCH 09/15] vfs: pass operation flags to {clone, dedupe}_file_range implementations
On Fri, Oct 05, 2018 at 10:50:08AM -0700, Darrick J. Wong wrote:> > That's cool. But you know what's going to be the next step, right? > > Merging the 3 file operation interfaces into a single one. > > copy_file_range() already has the flags arg for future extensions > > and as you wrote somewhere, clone is really an optimized copy. > > ovl_copyfile() already does that internally. > > > > So the only take away for this patch series, please use constant > > names COPYRANGE_* and also explicitly define: > > > > /* Operation is actually clone, not copy. */ > > #define COPYRANGE_CLONE (1 << 2) > > /* Operation is actually dedupe, not copy. */ > > #define COPYRANGE_DEDUPE (1 << 3) > > Yeah, I was too tired to try to throw that one on top of the flaming > garbage pile. But I guess since I have a bunch more work to do to the > previous patch I might as well do that...I'm not totally sold on just merging everything, but I very much despise what is done in this patch, as it creates a completely confusing interface.