LWN: Comments on "How useful should copy_file_range() be?"

How useful should copy_file_range() be?

flussence — Sat, 10 Jul 2021 23:16:44 +0000

>Is the screen a file?
>Is the keyboard a file?

You're being facetious, but it's occasionally very useful to be able to do things like check which port a monitor is plugged in on or framedump the console on a server using only ssh.

How useful should copy_file_range() be?

Cyberax — Sat, 10 Jul 2021 21:48:42 +0000

> Or at least, perhaps, allow open, for a hierarchical namespace, but not read, and reveal all information via ioctl.
Windows NT tried that. It failed miserably.

How useful should copy_file_range() be?

mpr22 — Sat, 10 Jul 2021 13:54:25 +0000

Those things are not files, but allowing userspace to treat them as file-like for the simplest common use case (sequential I/O) is one of the things that contributed to Unix eating most of the rest of the server operating system industry's breakfast, lunch, dinner, and face.

How useful should copy_file_range() be?

jaykrell — Sat, 10 Jul 2021 12:57:39 +0000

The bug is that /proc exists at all.
This data should be retrieved through strongly typed special purpose function calls.
Or at least, perhaps, allow open, for a hierarchical namespace, but not read, and reveal all information via ioctl.

Not everything is a file!

In fact, most things are not a file.
Is the screen a file?
Is the keyboard a file?
Are sockets files? They have read/write, but how about seek and mmap?

How useful should copy_file_range() be?

zuzzurro — Thu, 25 Feb 2021 11:12:21 +0000

Given the amount of confusion that is visible in the thread where kernel programmers are trying to find the best way to fix this I find it sad that people have tried to just throw the issue off to the userland.

How useful should copy_file_range() be?

jsmith45 — Tue, 23 Feb 2021 19:02:38 +0000

Very true. The problem is that there is almost certainly a bunch of programs out there that will break if the file types of /proc pseudofiles changes to be anything but a normal file.

How useful should copy_file_range() be?

markh — Sat, 20 Feb 2021 17:01:45 +0000

The bug is that the kernel is reporting it as a regular file (in st_mode), but then does not satisfy the requirements for regular files. If the kernel does not want to satisfy those requirements, all that is needed is to report it as a type of file that does not have the requirements that it cannot satisfy. It is not reasonable to expect userspace programs to somehow guess that a file reported as a regular file cannot be relied upon to behave as such.

How useful should copy_file_range() be?

smurf — Sat, 20 Feb 2021 14:45:12 +0000

Those fake kernel files don't have POSIX semantics, period. Their reported size doesn't correspond to the number of readable bytes, if they're writeable you can't just write '1' and then '23' when you intend to write '123', and so on.

So the bug is on the user. If you treat these things like ordinary files and expect all the posicky corner cases to work "correctly", you're SOL. These files will never have POSIX semantics. No, you can't use libc to emulate it. Deal.

Yes there should be a way to ask the kernel whether a file conforms to 100% posix. Well, we don't have that. Deal.

One possible workaround is to check the file size. If it's smaller than pagesize*4 or so then it's probably cheaper to copy its data the old-fashioned way anyway.

How useful should copy_file_range() be?

jengelh — Sat, 20 Feb 2021 12:16:37 +0000

Looking at the POSIX description for the lseek(2) POSIX-C function, it is required to operate in terms of bytes.

I am not getting that with the /proc/self/sched file (uses `seq_lseek`, which operates in terms of records). Is sys_llseek *meant* to be POSIX-compatible?
If yes, it's a kernel bug.
If no, then it's a libc bug because it failed to provide/emulate POSIX semantics on top of an (unposixy) kernel interface.
So, which is it, where should I file a bug?

How useful should copy_file_range() be?

the8472 — Fri, 19 Feb 2021 20:29:02 +0000

You're looking at the linux implementation for io::copy which can copy arbitrary readers to writers but specializes to various syscalls when types wrapping file descriptors of unknown type are passed, hence the complexity.

There also is fs::copy which shares some code but has fewer cases to cover and does fewer checks before invoking copy_file_range since it's only meant to copy entire regular files.

The relevant parts are here and here

. Note that it doesn't use stat information to decide what to do in the fs::copy case, it just tries copy_file_range and then falls back in various cases, including 0 byte reads.

How useful should copy_file_range() be?

fw — Fri, 19 Feb 2021 18:55:50 +0000

I see, there is a different code path that reaches the copy_file_range system call.

On the other hand, it is still deeply nested within the library, and it is not immediately obvious whether the callers of the universal copy routines expect consistent file offsets on errors. For a function that is directly modeled on the system call (which seems to be the case for Go), predictable file offset behavior seems quite important.

How useful should copy_file_range() be?

NYKevin — Fri, 19 Feb 2021 17:33:20 +0000

If they are unwilling to flag the filesystem as a whole as FS_GENERATED_CONTENT, I can't imagine they will be eager to flag *each individual file* with something else.

How useful should copy_file_range() be?

Deewiant — Fri, 19 Feb 2021 07:32:42 +0000

The current code in Rust does seem to handle arbitrary fds too. It's all here, but there's a lot of code to read through (which supports your point that it's difficult): https://github.com/rust-lang/rust/blob/7647d03c33339bd85a...

Looks like it starts by checking the underlying file type (with a stat() if necessary) and only tries copy_file_range on regular files of nonzero size (lines 122 and 168; and 284 and 469 for the logic leading to the stat() itself), while still falling back to other methods if copy_file_range only copied zero bytes (lines 175 and 563). Overall there seems to be a lot of logic around keeping track of what was actually written vs. what was reported by the syscalls.

How useful should copy_file_range() be?

fw — Fri, 19 Feb 2021 07:14:21 +0000

The Go implementation works on file descriptors, not files. Full userspace emulation of copy_file_range is very hard: append-only output files, non-seekable input files that cannot restore the correct input read position after an output failure, other error conditions that are not recoverable because system calls fail during rollback. If it's difficult for the kernel, it's probably hard for userspace, too.

If you can just close the descriptors and report an error (because the function opened them locally), these issues do not apply.

How useful should copy_file_range() be?

drinkcat — Fri, 19 Feb 2021 00:17:26 +0000

Yeah... except for sysfs files that report a size of 4096 bytes, copy_file_range would appear to work on these.

How useful should copy_file_range() be?

JoeBuck — Thu, 18 Feb 2021 23:49:51 +0000

Perhaps I'm missing something, but I thought that for the generated files, the call always transfers 0 bytes, and the Rust patch immediately falls back when it sees this. So how is seeking an issue?

How useful should copy_file_range() be?

drinkcat — Thu, 18 Feb 2021 22:37:17 +0000

That workaround should work in most cases. But another tricky thing with copy_file_range is that in case of partial writes, it's supposed to be able to seek in the input file (which is not usually possible on generated files).

How useful should copy_file_range() be?

JoeBuck — Thu, 18 Feb 2021 19:21:26 +0000

Looks like others can just port the Rust fix.

How useful should copy_file_range() be?

smurf — Thu, 18 Feb 2021 18:38:47 +0000

Some of these /proc files are quasi-seekable IIRC, in that they return an offset which indicates your read position but which does not correspond to the character count from beginning-of-file to wherever you were when you called lseek(fd,SEEK_CUR).

How useful should copy_file_range() be?

Deewiant — Thu, 18 Feb 2021 18:24:37 +0000

Rust's standard library has been using copy_file_range for years. Though apparently a fix for these kinds of issues landed only six months ago: https://github.com/rust-lang/rust/commit/4ddedd521418d67e...

How useful should copy_file_range() be?

dezgeg — Thu, 18 Feb 2021 17:31:10 +0000

There were objections to the FS_GENERATED_CONTENT flag due to "madness and constant auditing"... but how many such virtual filesystem types that need the flag actually exist, besides the 4 touched in the patchset? Is that really a number that one cannot count with fingers?

How useful should copy_file_range() be?

matthias — Thu, 18 Feb 2021 16:43:17 +0000

As I understand the man page, the offsets are adjusted by the number of bytes copied. So if you read n bytes and only write m<n bytes, then both offsets are adjusted by m.

How useful should copy_file_range() be?

zuzzurro — Thu, 18 Feb 2021 16:41:36 +0000

What if these "non seekable" files were simply flagged as named pipes?

How useful should copy_file_range() be?

zuzzurro — Thu, 18 Feb 2021 16:36:49 +0000

Isn't the problem caused by the fact that we have in the systems files that pretend to be files but don't really behave like ones? If that's the case, in the good old days people would create a new file type and flag them as such (p for named pipes, m for multiplexed files, s for sockets). So that useland would not assume that the normal file behaviour was available.
I guess it's too late to do the same anymore...

How useful should copy_file_range() be?

dullfire — Thu, 18 Feb 2021 15:48:18 +0000

Never mind, failed to re-read the man page before commenting. Seems the kernel already does this.

So either the author was mistaken and it's not actually an issue, or the kernel doesn't update the read-size loff_t when there is a write failure, in which case the flag would just change this behavior

How useful should copy_file_range() be?

dullfire — Thu, 18 Feb 2021 15:44:56 +0000

I'm not sure adding the ability to report if the sort copy was due to the read or the write is really worth while.

However if that was really desired: I imagine a fairly simple way to do that is: add a flag, something like CFR_UPDATE_OFF_SIZE. Setting it would make the kernel update the loff_t's (pointed to by off_in and off_out) with the bytes read/written correspondingly. Userspace can easily tell which side failed then. If the two sides are equal, it was a read size failure, if the read side is greater, it was a write failure. Note that it is a logic error is the write size were to be greater than the read size.