pg_upgrade --copy-file-range fails with EINVAL on Linux 4.19

Hi,

pg_upgrade --copy-file-range aborts on Linux 4.19 while copying relation files.
The probe does one copy_file_range() and succeeds; the copy loop
always makes a second call with NULL + SSIZE_MAX, and 4.19 returns
EINVAL because pos + SSIZE_MAX overflows.
The new cluster already exists at that point, so the upgrade has to be
run again.
I would like a choice between tightening the probe (1) and fixing the
copy loop (2).

On openEuler 20.03 (kernel 4.19.90), we hit this during "make check"
in the "--copy-file-range" subtest of
'src/bin/pg_upgrade/t/006_transfer_modes.pl':

error while copying relation "....": could not copy file range from
"..." to "...": Invalid argument

That is the pg_fatal() from "copyFileByRange()":

error while copying relation "%s.%s": could not copy file range from
"%s" to "%s": %m

The status line was "Copying user relation files with
copy_file_range". errno is EINVAL.

'file.c' has "check_copy_file_range()" (lines 236-270) as a probe.
'check.c' calls it first for "TRANSFER_MODE_COPY_FILE_RANGE"; it
issues a single copy_file_range() on the old cluster's 'PG_VERSION'
file.
If that call succeeds, pg_upgrade continues.
If copy_file_range() fails, it reports:

could not copy file range between old and new data directories: %m

If PostgreSQL was built without HAVE_COPY_FILE_RANGE, the same function reports:

copy_file_range not supported on this platform

In our case the probe succeeded, so relation files were still copied
with copy_file_range.
't/006_transfer_modes.pl' treats "copy_file_range not supported on
this platform" and "could not .* data directories" as an expected
failure, so TAP still passes.
The "copyFileByRange()" line quoted at the start of this mail ("could
not copy file range from ... to ...") is not in that regex, which is
why TAP went red.
The check-phase ".*between old and new data directories" line is not in the log.

"copyFileByRange()" in 'src/bin/pg_upgrade/file.c' (lines 164-171) always does:

do
{
nbytes = copy_file_range(src_fd, NULL, dest_fd, NULL, SSIZE_MAX, 0);
if (nbytes < 0)
pg_fatal("error while copying relation \"%s.%s\": could
not copy file range from \"%s\" to \"%s\": %m",
schemaName, relName, src, dst);
}
while (nbytes > 0);

On Linux 4.19 ('fs/read_write.c'), when copy_file_range() is passed
NULL for the off_in and off_out arguments, the kernel uses the current
f_pos of each fd, and writes the new position back after a successful
copy:

https://github.com/torvalds/linux/blob/v4.19/fs/read_write.c
SYSCALL_DEFINE6(copy_file_range) (around line 1627)

SYSCALL_DEFINE6(copy_file_range, int, fd_in, loff_t __user *, off_in,
int, fd_out, loff_t __user *, off_out,
size_t, len, unsigned int, flags)
{
...
if (off_in)
copy_from_user(&pos_in, off_in, sizeof(loff_t));
else
pos_in = f_in.file->f_pos; /* PG passes NULL */

/* same for off_out -> pos_out */

ret = vfs_copy_file_range(f_in.file, pos_in, f_out.file, pos_out,
len, flags);
if (ret > 0) {
pos_in += ret;
pos_out += ret;
if (!off_in)
f_in.file->f_pos = pos_in;
if (!off_out)
f_out.file->f_pos = pos_out;
}
return ret;
}

vfs_copy_file_range() calls rw_verify_area() before any copy (around line 1549):

ret = rw_verify_area(READ, file_in, &pos_in, len);
if (unlikely(ret))
return ret;
ret = rw_verify_area(WRITE, file_out, &pos_out, len);

The second call dies in rw_verify_area() (around line 365):

int rw_verify_area(int read_write, struct file *file, const loff_t *ppos,
size_t count)
{
int retval = -EINVAL;
...
pos = *ppos;
if (pos < 0) {
...
} else if ((loff_t)(pos + count) < 0) {
if (!unsigned_offsets(file))
return retval; /* round 2: pos>0, count=SSIZE_MAX */
}
...
}

Round 1 has pos=0 and len=SSIZE_MAX, so the range check passes, some
bytes are copied, and f_pos is updated.
Round 2 passes the same six userspace arguments, but pos is already >
0 and len is still SSIZE_MAX.
(loff_t)(pos + count) < 0, so it returns -EINVAL and nothing after
that check runs.
The kernel is rejecting pos + SSIZE_MAX as a signed 64-bit overflow.

"check_copy_file_range()" passed because it only calls once; the
arguments match "copyFileByRange()", and 'PG_VERSION' is a few bytes,
so round 1 succeeds.
"copyFileByRange()" loops with do { ... } while (nbytes > 0).

I did not need to build PostgreSQL to match this.
On Debian 10 (4.19.0-21-amd64, ext4) I used the same calling pattern
as "copyFileByRange()": one pair of file descriptors,
copy_file_range(..., NULL, ..., NULL, SSIZE_MAX, 0) on every
iteration, looping while the return value is greater than 0.
Only the source file size changed:

1KB round 1 returned 1024, round 2 errno 22
8KB round 1 returned 8192, round 2 errno 22 (empty index page)
1GB round 1 returned 1GiB, round 2 errno 22 (default segment size)

A single call succeeds on all three, which is what
"check_copy_file_range()" does today.

"copyFileByRange()" is written like read(): call again while the
return is > 0, and wait for the kernel to return 0.
On 4.19 a non-empty source makes round 1 return > 0, so round 2 always
runs, always returns EINVAL, and we pg_fatal.
An empty file would return 0 on round 1 and skip round 2; relation
files are not empty.
8KB versus 1GB does not change that.
GNU cp hit the same kernel overflow (https://debbugs.gnu.org/63850),
but that report is a file larger than about 2GiB: round 1 copies at
most MAX_RW_COUNT, round 2 is EINVAL, and the destination is left
short.
pg_upgrade's usual case is the extra call after round 1 has already
finished a small file.

Linux 5.3 reworked copy_file_range (the man page says applications
should target 5.3).
generic_copy_file_checks() (commit 96e6e8f4a68d) runs before
rw_verify_area() and shortens len to the source file length (i_size):

/* Shorten the copy to EOF */
size_in = i_size_read(inode_in);
if (pos_in >= size_in)
count = 0;
else
count = min(count, size_in - (uint64_t)pos_in);

size_in is the source file size.
If pos is already at EOF, count becomes 0 and vfs_copy_file_range() returns 0.
Otherwise count is at most the remaining bytes, so pos + count no
longer overflows with SSIZE_MAX.
The same do-while then gets 0 on round 2 and finishes.
4.19 LTS is EOL; distro kernels still on 4.19, such as Debian 10 and
openEuler 20.03, have not picked this up.

Based on the above, I have three suggestions.
I would like opinions on (1) versus (2).
(3) would silently fall back to "copyFile()".
That goes against how this option was proposed: the user opts in with
--copy-file-range, and we error out if it fails (for example EXDEV
across filesystems on older Linux), rather than falling back
internally.

/messages/by-id/CA+hUKGKe7Hb0-UNih8VD5UNZy5-ojxFb3Pr3xSBBL8qj2M2=dQ@mail.gmail.com
(1) is a small probe change: --copy-file-range still cannot be used on
4.19, but the failure happens before relation files are copied.
(2) is a larger change to the copy loop, and would make the mode work on 4.19.
(3) is the silent EINVAL/EXDEV fallback; I do not want that.

1. The change I would make first: only "check_copy_file_range()".
After the existing successful copy_file_range(), call it once more on
the same fds with the same arguments (NULL + SSIZE_MAX).
Copying a non-empty relation file already does that extra call.
On 4.19 round 2 is EINVAL, "check_copy_file_range()" fails, and the
user is asked to pick --copy / --link / --clone before relation files
are copied.
On 5.3+ round 2 returns 0, so the probe looks as it does today.
On 4.19, TAP should then treat this as the expected probe failure (the
"between old and new data directories" regex) and pass.

2. Do not treat "kernel returns 0 at EOF" as the loop condition.
In "copyFileByRange()", take the remaining length before each call,
pass that as len, subtract after a successful copy, and stop when
remaining is 0.
Do not ask for SSIZE_MAX again at EOF.
Non-empty files can then be copied on 4.19.

3. Fall back to "copyFile()" when copy_file_range() returns EINVAL or EXDEV.
That would let --copy-file-range keep going on 4.19 by switching to a
normal copy after the syscall fails.
I do not want that.
It goes against the original proposal (same thread as above): opt in,
and read the error if it fails.
Commit d93627bcbe implemented that.

This is in 17 / 18 / 19 / master.
If we change it, I think a backpatch to the stable branches is warranted.
't/006_transfer_modes.pl' should still pass on 5.3+.
I can write a patch for (1) or (2) once we pick.

Thanks,

Johnny