[Toybox] copy_file_range and (1<<30)

[enh]

On Thu, Jan 16, 2025 at 7:37 AM Rob Landley <rob at landley.net> wrote:
>
> On 1/15/25 10:02, enh wrote:
> >> Is there a big performance difference between queueing up 1 gigabyte at
> >> a time and 2 gigabytes at a time?
> >
> > that's not really my concern here, so much as "was this an actual bug,
> > and which range of kernel versions did it affect?", so that i can pass
> > the advice on to other projects using copy_file_range() [and add it to
> > the man page], or even just add the clamping in libc itself so no-one
> > has to think about this.
>
> Looking for the motivation for the commit, there's nothing in my blog
> and the relevant poke to look at this area on the list was a patch from
> you that says "found by inspection":
>
> http://lists.landley.net/pipermail/toybox-landley.net/2021-March/020378.html
>
> So it looks like I just looked up the relevant limit in the kernel and
> rounded it down to the next power of 2.
>
> >> The constraint is there because older kernels would error out on too big
> >> a value, rather than clamping, and I didn't want to query the kernel
> >> version and have two codepaths.
> >
> > that's what i was trying to find, and i still haven't. `git log -p
> > read_write.c` makes it look like the first commit that started calling
> > do_splice_direct() had the clamping.
> ...
> > but in particular, i don't think the kernel commit you quote --
> > f16acc9d9b376 -- _adds_ a sanity check; it just refactors it into a
> > new function?
>
> Sounds plausible.
>
> If there's honestly some performance increase due to something like
> hugepage mapping under the covers where the small value copies data and
> the big value connects existing mappings together or something, I'm
> happy to change it. I dunno where you're getting the performance
> difference you mentioned from, seems like it _should_ be in the noise?

like i said: this is nothing to do with performance ... i'm looking at
this because i'm seeing more and more _other_ code that's using
copy_file_range() without this hack, and i'm trying to find evidence
that this hack was ever needed (so we can add it in other code and/or
just hide this in libc), or remove unnecessary confusion from toybox.

as far as i can tell from the kernel git history, this was never
broken. (and so far, of all the callers i've found, toybox is the only
one with this hack.)

weirdly, the closest i've found is that emacs has a similar hack for
2.6 kernels --- but that's for read() and write(), not
copy_file_range()!
https://bugzilla.redhat.com/show_bug.cgi?format=multiple&id=612839 was
the bug (marked as something they weren't going to fix) but emacs
added https://mail.gnu.org/r/emacs-devel/2011-04/msg00488.html anyway.

(insert "if emacs jumped off a bridge..." joke here :-) )

> Rob