On Mon, Dec 4, 2023 at 7:16 PM Jan Kara <jack@xxxxxxx> wrote:
>
> On Mon 04-12-23 16:29:02, Amir Goldstein wrote:
> > On Mon, Dec 4, 2023 at 4:07 PM Christoph Hellwig <hch@xxxxxx> wrote:
> > > On Mon, Dec 04, 2023 at 03:29:43PM +0200, Amir Goldstein wrote:
> > > Well, splice_file_range makes sense if it is a separate helper. But when
> > > is the default implementation for ->copy_file_range and matches the
> > > signature, naming it that way is not only sensible but required to keep
> > > sanity.
> > >
> >
> > It is NOT a default implementation of ->copy_file_range(), but
> > a fallback helper.
> > Specifically, it is never expected to have a filesystem that does
> > .copy_file_range = generic_copy_file_range,
> > so getting rid of generic_copy_file_range() would be good.
> >
> > Note also that generic_copy_file_range() gets a flags argument
> > that is COPY_FILE_* flags (currently only for the vfs level)
> > and this flags argument is NOT the splice flags argument, so
> > I intentionally removed the flags argument from splice_file_range()
> > to reduce confusion.
> >
> > I like the idea of moving MAX_RW_COUNT into splice_file_range()
> > and replacing generic_copy_file_range() calls with splice_file_range().
> >
> > I do not feel strongly against splice_copy_file_range() name, but
> > I would like to get feedback from other reviewers that approved the
> > name splice_file_range() before changing it.
>
> For me the name is not a big deal either way.
I would rather add this inline wrapper than uniting the two helpers:
static inline long splice_copy_file_range(struct file *in, loff_t pos_in,
struct file *out, loff_t pos_out,
size_t len)
{
return splice_file_range(in, &pos_in, out, &pos_out,
min_t(size_t, len, MAX_RW_COUNT));
}
It is keeping the same signature as copy_file_range() minus flags,
to be used for ->copy_file_range() fallback and keeps the current
splice_file_range() helpers for special cases as ceph that want to
update the in/out offset.
Thanks,
Amir.
