Re: [PATCH v2 1/3] fs: fork splice_file_range() from do_splice_direct()

[Christoph Hellwig <hch@lst.de>]

On Mon, Dec 04, 2023 at 03:29:43PM +0200, Amir Goldstein wrote:
> > > Shouldn't ceph be switched to use generic_copy_file_range?
> > > That does the capping of the size which we want, and doesn't update
> > > the file offsets, which would require recalculation in the ceph code.
> > >
> 
> IDK. I did not want to change the logic of the ceph code.
> I am not sure that we must impose MAX_RW_COUNT limit on ceph,
> although, i_layout.object_size may already be limited? Jeff?

We better don't go beyond it, as it is called from the copy_file_range
implementation which is expected to never return more than MAX_RW_COUNT.
So either it is a noop change, or it fixes a bug.

> 
> > > But this could avoid another exported API as splice_file_range could
> > > simply be folded into generic_copy_file_range which should reduce
> > > confusion.  And splice really is a mess for so many different layers
> > > of the onion being exposed.  I've been wanting to reduce some of that
> > > for a while but haven't found a really nice way yet.
> >
> > (and generic_copy_file_range really should be renamed to
> > splice_copy_file_range and moved to splice.c)
> 
> That depends if we are keeping two helpers.
> One with a cap of MAX_RW_COUNT and one without.
> If we are going to keep two helpers, I'd rather keep things as they are.
> If one helper, then I personally prefer splice_file_range() over
> splice_copy_file_range() and other reviewers (Jan) liked this
> name as well.

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.

> 
> Thanks,
> Amir.
---end quoted text---