As explained in https://lore.kernel.org/all/20250419183545.1982187-1-shakeel.butt@linux.dev/,
writing to memory.max or memory.high triggers synchronous memory reclaim
if the limit is lowered. This can end up taking nonnegligible amounts
of time, completely blocking pid1 from doing any other work while the
reclaim is ongoing.
To address this problem, the kernel going to add O_NONBLOCK semantics
to memory.max and memory.high. If the file is opened with O_NONBLOCK,
the synchronous memory reclaim is skipped and only triggered later
without blocking the process writing the file. Let's make sure we make
use of this by opening cgroupv2 attribute files with O_NONBLOCK.
We opt to do this for all cgroupv2 attribute files, to make sure that
if the same problem happens elsewhere in the future and is fixed in the
same way, we immediately take advantage of that fix without having to
make changes in systemd as well. We probably never want to block when
writing cgroupv2 attributes and any cases where we do want to block should
indicate so explicitly instead of blocking by default.
- Drop effectively unused "terminator" param, imply whitespace
- Make ret param optional
- Return ENODATA if the requested key is not found, rather than
ENOENT
- Turn ENOENT -> ENOSYS if /proc/ is not mounted
- Don't skip whitespaces before ':', nothing needs this handling
anyways
- Remove the special treatment for all "0"s. We don't actually
use this for capabilities given pidref_get_capability() exists
- Switch away from read_full_virtual_file() - files using "field"
scheme under /proc/ seem all to be "seq_file"s (refer to
da65941c3e for details on file types)
Kernel API file systems typically use either "raw" or "seq_file" to
implement their various interface files. The former are really simple
(to point I'd call them broken), in that they have no understanding of
file offsets, and return their contents again and again on every read(),
and thus EOF is indicated by a short read, not by a zero read. The
latter otoh works like a typical file: you read until you get a
zero-sized read back.
We have read_virtual_file() to read the "raw" files, and can use regular
read_full_file() to read the "seq_file" ones.
Apparently all files in the top-level /proc/ directory use 'seq_file'.
but we accidentally used read_virtual_file() for them. Fix that.
Also clarify in a comment what the rules are.
Fixes: #36131
- Rename to script_get_shebang_interpreter and return
-EMEDIUMTYPE if the executable is not a script.
We nowadays utilize the scheme of making ret param
of getters optional, and use them directly as checkers.
- Don't unnecessarily read the whole line, but check
only the shebang first.
This brings two benefits: we will label the created file only if it is
actually created, and we can correctly delete any file we create again
on failure.
Given that we have the LabelOps abstraction these days, we can teach
write_string_file() to use it, which means we can get rid of
fileio-label.[ch] as a separate concept.
(The only reason that fileio-label.[ch] exists independently of
fileio.[ch] was that the former linekd to libselinux potentially, and
thus had to be in src/shared/ while the other always was in src/basic/.
But the LabelOps vtable provides us with a nice work-around)
let's rename the "_ts" flavour of these calls "_full" instead, exposing
the full functionality. And then keep two more minimal versions around:
one "_at" (which has the ts parameter suppressed, but keeps the dir_fd
one). And one without suffix (which supresses both).
Do the same for the label versions of these calls.
The return value of read_stripped_line() is changed. Before we'd return the
number of characters read, but that number was not meaningful after we called
strstrip(). So just return 0 if nothing was read (EOF), and 1 if something was
read (not EOF). All the callers were only checking for <0 or ==0.
Let's modernize and clean up search_and_fopen a bit: let's add support
for regular open() (instead of fopen()), as well as access() (if caller
just wants to check if a file exists without opening it.
This unifies much of the code involved, which previously was duplicated
in search_and_fopen() and search_and_fopen_nulstr()
When READ_FULL_FILE_UNBASE64 (or READ_FULL_FILE_UNHEX) is specified,
setting size argument by caller is difficult, as it is hard to estimate
the encoded length.
This makes when size is specified with decoding option, let's read file
more, and check decoded size later with the specified size.
path_simplify_full()/path_simplify() are changed to allow a NULL path, for
which a NULL is returned. Generally, callers have already asserted before that
the argument is nonnull. This way path_simplify_full()/path_simplify() and
path_simplify_alloc() behave consistently.
In sd-device.c, logging in device_set_syspath() is intentionally dropped: other
branches don't log.
In mount-tool.c, logging in parse_argv() is changed to log the user-specified
value, not the simplified string. In an error message, we should show the
actual argument we got, not some transformed version.
Chasing symlinks is a core function that's used in a lot of places
so it deservers a less verbose names so let's rename it to chase()
and chaseat().
We also slightly change the pattern used for the chaseat() helpers
so we get chase_and_openat() and similar.
This is a combination of fdopen() and fd_reopen(). i.e. it first reopens
the fd, and then converts that into a FILE*.
We do this at various places already manually. let's move this into a
helper call of its own.
These combine strndup() + strspn()/strcspn() into one.
There are a bunch of strndupa() calls that could use similar treatment
(or should be converted to strdup[c]spn(), but this commit doesn't
bother with that.
-1 was used everywhere, but -EBADF or -EBADFD started being used in various
places. Let's make things consistent in the new style.
Note that there are two candidates:
EBADF 9 Bad file descriptor
EBADFD 77 File descriptor in bad state
Since we're initializating the fd, we're just assigning a value that means
"no fd yet", so it's just a bad file descriptor, and the first errno fits
better. If instead we had a valid file descriptor that became invalid because
of some operation or state change, the other errno would fit better.
In some places, initialization is dropped if unnecessary.
This way we can connect correctly to any AF_UNIX socket in the file
system, and even save some code. Yay!
This also adds some test code for this, that ensures read_file_full()
works correctly for AF_UNIX sockets that violate the 108 char limit.
Supporting sockets like this kinda matters I think, for the simple
reason that apps want to build socket paths via XDG_RUNTIME_DIR and
suchlike, and we should be able to connect to them, even via
non-normalized paths.
let's not make up new errors in these checks that validate if connect()
work at all. After all, we don't really know if the ENXIO we saw earlier
actually is really caused by the inode being an AF_UNIX socket, we just
have the suspicion...
Daan De Meyer
Mike Yuan
Lennart Poettering
Yu Watanabe
gerblesh
Zbigniew Jędrzejewski-Szmek
Dimitri Papadopoulos