* Re: [DISCUSSION] proposed mctl() API
From: Matthew Wilcox @ 2025-05-29 15:28 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Shakeel Butt, Liam R . Howlett, David Hildenbrand,
Vlastimil Babka, Jann Horn, Arnd Bergmann, Christian Brauner,
SeongJae Park, Usama Arif, Mike Rapoport, Johannes Weiner,
Barry Song, linux-mm, linux-arch, linux-kernel, linux-api,
Pedro Falcato
In-Reply-To: <85778a76-7dc8-4ea8-8827-acb45f74ee05@lucifer.local>
On Thu, May 29, 2025 at 03:43:26PM +0100, Lorenzo Stoakes wrote:
> After discussions in various threads (Usama's series adding a new prctl()
> in [0], and a proposal to adapt process_madvise() to do the same -
> conception in [1] and RFC in [2]), it seems fairly clear that it would make
> sense to explore a dedicated API to explicitly allow for actions which
> affect the virtual address space as a whole.
>
> Also, Barry is implementing a feature (currently under RFC) which could
> additionally make use of this API (see [3]).
I think the reason that you're having trouble coming up with a good
place to put these ideas is because they are all bad ideas. Do none of
them. Problem solved.
People should put more effort into allocating THPs automatically and
monitoring where they're helping performance and where they're hurting
performance, instead of coming up with these baroque reasons to blame
the sysadmin for not having tweaked some magic knob.
Barry's problem is that we're all nervous about possibly regressing
performance on some unknown workloads. Just try Barry's proposal, see
if anyone actually compains or if we're just afraid of our own shadows.
^ permalink raw reply
* [DISCUSSION] proposed mctl() API
From: Lorenzo Stoakes @ 2025-05-29 14:43 UTC (permalink / raw)
To: Andrew Morton, Shakeel Butt, Liam R . Howlett, David Hildenbrand,
Vlastimil Babka, Jann Horn, Arnd Bergmann, Christian Brauner,
SeongJae Park, Usama Arif, Mike Rapoport, Johannes Weiner,
Barry Song, linux-mm, linux-arch, linux-kernel, linux-api,
Pedro Falcato
## INTRODUCTION
After discussions in various threads (Usama's series adding a new prctl()
in [0], and a proposal to adapt process_madvise() to do the same -
conception in [1] and RFC in [2]), it seems fairly clear that it would make
sense to explore a dedicated API to explicitly allow for actions which
affect the virtual address space as a whole.
Also, Barry is implementing a feature (currently under RFC) which could
additionally make use of this API (see [3]).
[0]: https://lore.kernel.org/all/20250515133519.2779639-1-usamaarif642@gmail.com/
[1]: https://lore.kernel.org/linux-mm/c390dd7e-0770-4d29-bb0e-f410ff6678e3@lucifer.local/
[2]: https://lore.kernel.org/all/cover.1747686021.git.lorenzo.stoakes@oracle.com/
[3]: https://lore.kernel.org/all/20250514070820.51793-1-21cnbao@gmail.com/
While madvise() and process_madvise() are useful for altering the
attributes of VMAs within a virtual address space, it isn't the right fit
for something that affects the whole address space.
Additionally, a requirement of Usama's proposal (see [0]) is that we have
the ability to propagate the change in behaviour across fork/exec. This
further suggests the need for a dedicated interface, as this really sits
outside the ordinary behaviour of [process_]madvise().
prctl() is too broad and encourages mm code to migrate to kernel/sys.c
where it is at risk of bit-rotting. It can make it harder/impossible to
isolate mm logic for testing and logic there might be missed in changes
moving forward.
It also, like so many kernel interfaces, has 'grown roots out of its pot'
so to speak - while it started off as an ostensible 'process' manipulation
interface, prctl() operations manipulate a myriad of task, virtual
address space and even specific VMA attributes.
At this stage it really is a 'catch-all' for things we simply couldn't fit
elsewhere.
Therefore, as suggested by the rather excellent Liam Howlett, I propose an
mm-specific interface that _explicitly_ manipulates attributes of the
virtual address space as a whole.
I think something that mimics the simplicity of [process_]madvise() makes
sense - have a limited set of actions that can be taken, and treat them as
a simple action - a user requests you do XXX to the virtual address space
(that is, across the mm_struct), and you do it.
## INTERFACE
The proposed interface is simply:
int mctl(int pidfd, int action, unsigned int flags);
Since PIDFD_SELF is now available, it is easy to invoke this for the
current process, while also adding the flexibility of being able to apply
actions to other processes also.
The function will return 0 on success, -1 on failure, with errno set to the
error that arose, standard stuff.
The behaviour will be tailored to each action taken.
To begin with, I propose a single flag:
- MCTL_SET_DEFAULT_EXEC - Persists this behaviour across fork/exec.
This again will be tailored - only certain actions will be allowed to set
this flag, and we will of course assert appropriate capabilities, etc. upon
its use.
All actions would, impact every VMA (if adjustments to VMAs are required).
## SECURITY
Of course, security will be of utmost concern (Jann's input is important
here :)
We can vary security requirements depending on the action taken.
For an initial version I suggest we simply limit operations which:
- Operate on a remote process
- Use the MCTL_SET_DEFAULT_EXEC flag
To those tasks which possess the CAP_SYS_ADMIN capability.
This may be too restrictive - be good to get some feedback on this.
I know Jann raised concerns around privileged execution and perhaps it'd be
useful to see whether this would make more sense for the SET_DEFAULT_EXEC
case or not.
Usama - would requiring CAP_SYS_ADMIN be egregious to your use case?
## IMPLEMENTATION
I think that sensibly we'd need to add some new files here, mm/mctl.c,
include/linux/mctl.h (the latter of providing the MCTL_xxx actions and
flags).
We could find ways to share code between mm files where appropriate to
avoid too much duplication.
I suggest that the best way forward, if we were minded to examine how this
would look in practice, would be for me to implement an RFC that adds the
interface, and a simple MCTL_SET_NOHUGEPAGE, MCTL_CLEAR_NOHUGEPAGE
implementation as a proof of concept.
If we wanted to then go ahead with a non-RFC version, this could then form
a foundation upon which Usama and Barry could implement their features,
with Usama then able to add MCTL_[SET/CLEAR]_HUGEPAGE and Barry
MCTL_[SET/CLEAR]_FADE_ON_DEATH.
Obviously I don't mean to presume to suggest how we might proceed here -
only suggesting this might be a good way of moving forward and getting
things done as quickly as possible while allowing you guys to move forward
with your features.
Let me know if this makes sense, alternatively I could try to find a
relatively benign action to implement as part of the base work, or we could
simply collaborate to do it all in one series with multiple authors?
## RFC
The above is all only in effect 'putting ideas out there' so this is
entirely an RFC in spirit and intent - let me know if this makes sense in
whole or part :)
Thanks!
Lorenzo
^ permalink raw reply
* Re: [RFC v2 00/16] Live Update Orchestrator
From: Mike Rapoport @ 2025-05-26 6:32 UTC (permalink / raw)
To: Pasha Tatashin
Cc: pratyush, jasonmiu, graf, changyuanl, dmatlack, rientjes, corbet,
rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm,
tj, yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
david, joel.granados, rostedt, anna.schumaker, song, zhangguopeng,
linux, linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, ptyadav,
linux-api
In-Reply-To: <20250515182322.117840-1-pasha.tatashin@soleen.com>
(cc'ing linux-api)
On Thu, May 15, 2025 at 06:23:04PM +0000, Pasha Tatashin wrote:
> This v2 series introduces the LUO, a kernel subsystem designed to
> facilitate live kernel updates with minimal downtime,
> particularly in cloud delplyoments aiming to update without fully
> disrupting running virtual machines.
>
> This series builds upon KHO framework [1] by adding programmatic
> control over KHO's lifecycle and leveraging KHO for persisting LUO's
> own metadata across the kexec boundary. The git branch for this series
> can be found at:
> https://github.com/googleprodkernel/linux-liveupdate/tree/luo/rfc-v2
>
> Changelog from v1:
> - Control Interface: Shifted from sysfs-based control
> (/sys/kernel/liveupdate/{prepare,finish}) to an ioctl interface
> (/dev/liveupdate). Sysfs is now primarily for monitoring the state.
> - Event/State Renaming: LIVEUPDATE_REBOOT event/phase is now
> LIVEUPDATE_FREEZE.
> - FD Preservation: A new component for preserving file descriptors.
> Subsystem Registration: A formal mechanism for kernel subsystems
> to participate.
> - Device Layer: removed device list handling from this series, it is
> going to be added separately.
> - Selftests: Kernel-side selftest hooks and userspace selftests are
> now included.
> KHO Enhancements:
> - KHO debugfs became optional, and kernel APIs for finalize/abort
> were added (driven by LUO's needs).
> - KHO unpreserve functions were also added.
>
> What is Live Update?
> Live Update is a specialized reboot process where selected kernel
> resources (memory, file descriptors, and eventually devices) are kept
> operational or their state preserved across a kernel transition (e.g.,
> via kexec). For certain resources, DMA and interrupt activity might
> continue with minimal interruption during the kernel reboot.
>
> LUO v2 Overview:
> LUO v2 provides a framework for coordinating live updates. It features:
> State Machine: Manages the live update process through states:
> NORMAL, PREPARED, FROZEN, UPDATED.
>
> KHO Integration:
>
> LUO programmatically drives KHO's finalization and abort sequences.
> KHO's debugfs interface is now optional configured via
> CONFIG_KEXEC_HANDOVER_DEBUG.
>
> LUO preserves its own metadata via KHO's kho_add_subtree and
> kho_preserve_phys() mechanisms.
>
> Subsystem Participation: A callback API liveupdate_register_subsystem()
> allows kernel subsystems (e.g., KVM, IOMMU, VFIO, PCI) to register
> handlers for LUO events (PREPARE, FREEZE, FINISH, CANCEL) and persist a
> u64 payload via the LUO FDT.
>
> File Descriptor Preservation: Infrastructure
> liveupdate_register_filesystem, luo_register_file, luo_retrieve_file to
> allow specific types of file descriptors (e.g., memfd, vfio) to be
> preserved and restored.
>
> Handlers for specific file types can be registered to manage their
> preservation and restoration, storing a u64 payload in the LUO FDT.
>
> Example WIP for memfd preservation can be found here [2].
>
> User-space Interface:
>
> ioctl (/dev/liveupdate): The primary control interface for
> triggering LUO state transitions (prepare, freeze, finish, cancel)
> and managing the preservation/restoration of file descriptors.
> Access requires CAP_SYS_ADMIN.
>
> sysfs (/sys/kernel/liveupdate/state): A read-only interface for
> monitoring the current LUO state. This allows userspace services to
> track progress and coordinate actions.
>
> Selftests: Includes kernel-side hooks and userspace selftests to
> verify core LUO functionality, particularly subsystem registration and
> basic state transitions.
>
> LUO State Machine and Events:
>
> NORMAL: Default operational state.
> PREPARED: Initial preparation complete after LIVEUPDATE_PREPARE
> event. Subsystems have saved initial state.
> FROZEN: Final "blackout window" state after LIVEUPDATE_FREEZE
> event, just before kexec. Workloads must be suspended.
> UPDATED: Next kernel has booted via live update. Awaiting restoration
> and LIVEUPDATE_FINISH.
>
> Events:
> LIVEUPDATE_PREPARE: Prepare for reboot, serialize state.
> LIVEUPDATE_FREEZE: Final opportunity to save state before kexec.
> LIVEUPDATE_FINISH: Post-reboot cleanup in the next kernel.
> LIVEUPDATE_CANCEL: Abort prepare or freeze, revert changes.
>
> [1] https://lore.kernel.org/all/20250509074635.3187114-1-changyuanl@google.com
> https://github.com/googleprodkernel/linux-liveupdate/tree/luo/kho-v8
> [2] https://github.com/googleprodkernel/linux-liveupdate/tree/luo/memfd-v0.1
>
> RFC v1: https://lore.kernel.org/all/20250320024011.2995837-1-pasha.tatashin@soleen.com
>
> Changyuan Lyu (1):
> kho: add kho_unpreserve_folio/phys
>
> Pasha Tatashin (15):
> kho: make debugfs interface optional
> kho: allow to drive kho from within kernel
> luo: luo_core: Live Update Orchestrator
> luo: luo_core: integrate with KHO
> luo: luo_subsystems: add subsystem registration
> luo: luo_subsystems: implement subsystem callbacks
> luo: luo_files: add infrastructure for FDs
> luo: luo_files: implement file systems callbacks
> luo: luo_ioctl: add ioctl interface
> luo: luo_sysfs: add sysfs state monitoring
> reboot: call liveupdate_reboot() before kexec
> luo: add selftests for subsystems un/registration
> selftests/liveupdate: add subsystem/state tests
> docs: add luo documentation
> MAINTAINERS: add liveupdate entry
>
> .../ABI/testing/sysfs-kernel-liveupdate | 51 ++
> Documentation/admin-guide/index.rst | 1 +
> Documentation/admin-guide/liveupdate.rst | 62 ++
> .../userspace-api/ioctl/ioctl-number.rst | 1 +
> MAINTAINERS | 14 +-
> drivers/misc/Kconfig | 1 +
> drivers/misc/Makefile | 1 +
> drivers/misc/liveupdate/Kconfig | 60 ++
> drivers/misc/liveupdate/Makefile | 7 +
> drivers/misc/liveupdate/luo_core.c | 547 +++++++++++++++
> drivers/misc/liveupdate/luo_files.c | 664 ++++++++++++++++++
> drivers/misc/liveupdate/luo_internal.h | 59 ++
> drivers/misc/liveupdate/luo_ioctl.c | 203 ++++++
> drivers/misc/liveupdate/luo_selftests.c | 283 ++++++++
> drivers/misc/liveupdate/luo_selftests.h | 23 +
> drivers/misc/liveupdate/luo_subsystems.c | 413 +++++++++++
> drivers/misc/liveupdate/luo_sysfs.c | 92 +++
> include/linux/kexec_handover.h | 27 +
> include/linux/liveupdate.h | 214 ++++++
> include/uapi/linux/liveupdate.h | 324 +++++++++
> kernel/Kconfig.kexec | 10 +
> kernel/Makefile | 1 +
> kernel/kexec_handover.c | 343 +++------
> kernel/kexec_handover_debug.c | 237 +++++++
> kernel/kexec_handover_internal.h | 74 ++
> kernel/reboot.c | 4 +
> tools/testing/selftests/Makefile | 1 +
> tools/testing/selftests/liveupdate/.gitignore | 1 +
> tools/testing/selftests/liveupdate/Makefile | 7 +
> tools/testing/selftests/liveupdate/config | 6 +
> .../testing/selftests/liveupdate/liveupdate.c | 440 ++++++++++++
> 31 files changed, 3933 insertions(+), 238 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-kernel-liveupdate
> create mode 100644 Documentation/admin-guide/liveupdate.rst
> create mode 100644 drivers/misc/liveupdate/Kconfig
> create mode 100644 drivers/misc/liveupdate/Makefile
> create mode 100644 drivers/misc/liveupdate/luo_core.c
> create mode 100644 drivers/misc/liveupdate/luo_files.c
> create mode 100644 drivers/misc/liveupdate/luo_internal.h
> create mode 100644 drivers/misc/liveupdate/luo_ioctl.c
> create mode 100644 drivers/misc/liveupdate/luo_selftests.c
> create mode 100644 drivers/misc/liveupdate/luo_selftests.h
> create mode 100644 drivers/misc/liveupdate/luo_subsystems.c
> create mode 100644 drivers/misc/liveupdate/luo_sysfs.c
> create mode 100644 include/linux/liveupdate.h
> create mode 100644 include/uapi/linux/liveupdate.h
> create mode 100644 kernel/kexec_handover_debug.c
> create mode 100644 kernel/kexec_handover_internal.h
> create mode 100644 tools/testing/selftests/liveupdate/.gitignore
> create mode 100644 tools/testing/selftests/liveupdate/Makefile
> create mode 100644 tools/testing/selftests/liveupdate/config
> create mode 100644 tools/testing/selftests/liveupdate/liveupdate.c
>
> --
> 2.49.0.1101.gccaa498523-goog
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Florian Weimer @ 2025-05-24 19:25 UTC (permalink / raw)
To: Zack Weinberg
Cc: Alejandro Colomar, Rich Felker, Vincent Lefevre, Jan Kara,
Alexander Viro, Christian Brauner, linux-fsdevel, linux-api,
GNU libc development
In-Reply-To: <8c47e10a-be82-4d5b-a45e-2526f6e95123@app.fastmail.com>
* Zack Weinberg:
> BUGS
> Prior to POSIX.1-2024, there was no official guarantee that
> close() would always close the file descriptor, even on error.
> Linux has always closed the file descriptor, even on error,
> but other implementations might not have.
>
> The only such implementation we have heard of is HP-UX; at least
> some versions of HP-UX’s man page for close() said it should be
> retried if it returned -1 with errno set to EINTR. (If you know
> exactly which versions of HP-UX are affected, or of any other
> Unix where close() doesn’t always close the file descriptor,
> please contact us about it.)
The AIX documentation also says this:
| The success of the close subroutine is undetermined if the following
| is true:
|
| EINTR The state of the FileDescriptor is undetermined. Retry the
| close routine to ensure that the FileDescriptor is closed.
<https://www.ibm.com/docs/en/aix/7.2.0?topic=c-close-subroutine>
So it's not just HP-UX.
For z/OS, it looks like some other errors leave the descriptor open:
| EAGAIN
|
| The call did not complete because the specified socket descriptor
| is currently being used by another thread in the same process.
|
| For example, in a multithreaded environment, close() fails and
| returns EAGAIN when the following sequence of events occurs (1)
| thread is blocked in a read() or select() call on a given file or
| socket descriptor and (2) another thread issues a simultaneous
| close() call for the same descriptor.
| […]
| EBADF
| fildes is not a valid open file descriptor, or the socket
| parameter is not a valid socket descriptor.
<https://www.ibm.com/docs/en/zos/2.1.0?topic=functions-close-close-file>
^ permalink raw reply
* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Rich Felker @ 2025-05-24 2:24 UTC (permalink / raw)
To: Zack Weinberg
Cc: Alejandro Colomar, Vincent Lefevre, Jan Kara, Alexander Viro,
Christian Brauner, linux-fsdevel, linux-api, GNU libc development
In-Reply-To: <8c47e10a-be82-4d5b-a45e-2526f6e95123@app.fastmail.com>
On Fri, May 23, 2025 at 02:10:57PM -0400, Zack Weinberg wrote:
> Taking everything said in this thread into account, I have attempted to
> wordsmith new language for the close(2) manpage. Please let me know
> what you think, and please help me with the bits marked in square
> brackets. I can make this into a proper patch for the manpages
> when everyone is happy with it.
>
> zw
>
> ---
>
> DESCRIPTION
> ... existing text ...
>
> close() always succeeds. That is, after it returns, _fd_ has
> always been disconnected from the open file it formerly referred
> to, and its number can be recycled to refer to some other file.
> Furthermore, if _fd_ was the last reference to the underlying
> open file description, the resources associated with the open file
> description will always have been scheduled to be released.
>
> However, close may report _delayed errors_ from a previous I/O
> operation. Therefore, its return value should not be ignored.
>
> RETURN VALUE
> close() returns zero if there are no delayed errors to report,
> or -1 if there _might_ be delayed errors.
>
> When close() returns -1, check _errno_ to see what the situation
> actually is. Most, but not all, _errno_ codes indicate a delayed
> I/O error that should be reported to the user. See ERRORS and
> NOTES for more detail.
>
> [QUERY: Is it ever possible to get delayed errors on close() from
> a file that was opened with O_RDONLY? What about a file that was
> opened with O_RDWR but never actually written to? If people only
> have to worry about delayed errors if the file was actually
> written to, we should say so at this point.
>
> It would also be good to mention whether it is possible to get a
> delayed error on close() even if a previous call to fsync() or
> fdatasync() succeeded and there haven’t been any more writes to
> that file *description* (not necessarily via the fd being closed)
> since.]
>
> ERRORS
> EBADF _fd_ wasn’t open in the first place, or is outside the
> valid numeric range for file descriptors.
>
> EINPROGRESS
> EINTR
> There are no delayed errors to report, but the kernel is
> still doing some clean-up work in the background. This
> situation should be treated the same as if close() had
> returned zero. Do not retry the close(), and do not report
> an error to the user.
Since this behavior for EINTR is non-conforming (and even prior to the
POSIX 2024 update, it was contrary to the general semantics for EINTR,
that no non-ignoreable side-effects have taken place), it should be
noted that it's Linux/glibc-specific.
> EDQUOT
> EFBIG
> EIO
> ENOSPC
> These are the most common errno codes associated with
> delayed I/O errors. They should be treated as a hard
> failure to write to the file that was formerly associated
> with _fd_, the same as if an earlier write(2) had failed
> with one of these codes. The file has still been closed!
> Do not retry the close(). But do report an error to the user.
>
> Depending on the underlying file, close() may return other errno
> codes; these should generally also be treated as delayed I/O errors.
>
> NOTES
> Dealing with error returns from close()
>
> As discussed above, close() always closes the file. Except when
> errno is set to EBADF, EINPROGRESS, or EINTR, an error return from
> close() reports a _delayed I/O error_ from a previous write()
> operation.
>
> It is vital to report delayed I/O errors to the user; failing to
> check the return value of close() can cause _silent_ loss of data.
> The most common situations where this actually happens involve
> networked filesystems, where, in the name of throughput, write()
> often returns success before the server has actually confirmed a
> successful write.
>
> However, it is also vital to understand that _no matter what_
> close() returns, and _no matter what_ it sets errno to, when it
> returns, _the file descriptor passed to close() has been closed_,
> and its number is _immediately_ available for reuse by open(2),
> dup(2), etc. Therefore, one should never retry a close(), not
> even if it set errno to a value that normally indicates the
> operation needs to be retried (e.g. EINTR). Retrying a close()
> is a serious bug, particularly in a multithreaded program; if
> the file descriptor number has already been reused, _that file_
> will get closed out from under whatever other thread opened it.
>
> [Possibly something about fsync/fdatasync here?]
While I agree with all of this, I think the tone is way too
proscriptive. The man pages are to document the behaviors, not tell
people how to program. And again, it should be noted that the standard
behavior is that you *do* have to retry on EINTR, or arrange to ensure
it never happens (e.g. by not installing interrupting signal handlers,
or blocking signals across calls to close), and that treating EINTR as
"fd has been closed" is something you should only do on
known-nonconforming systems.
Aside: the reason EINTR *has to* be specified this way is that pthread
cancellation is aligned with EINTR. If EINTR were defined to have
closed the fd, then acting on cancellation during close would also
have closed the fd, but the cancellation handler would have no way to
distinguish this, leading to a situation where you're forced to either
leak fds or introduce a double-close vuln.
> BUGS
> Prior to POSIX.1-2024, there was no official guarantee that
> close() would always close the file descriptor, even on error.
> Linux has always closed the file descriptor, even on error,
> but other implementations might not have.
>
> The only such implementation we have heard of is HP-UX; at least
> some versions of HP-UX’s man page for close() said it should be
> retried if it returned -1 with errno set to EINTR. (If you know
> exactly which versions of HP-UX are affected, or of any other
> Unix where close() doesn’t always close the file descriptor,
> please contact us about it.)
>
> Portable code should nonetheless never retry a failed close(); the
> consequences of a file descriptor leak are far less dangerous than
> the consequences of closing a file out from under another thread.
This is explicitly the opposite of what's specified for portable code.
It sounds like you are intentionally omitting that POSIX says the
opposite of what you want it to, and treating the standard behavior as
a historical HP-UX quirk/bug. This is polemic, not the sort of
documentation that belongs in a man page.
An outline of what I'd like to see instead:
- Clear explanation of why double-close is a serious bug that must
always be avoided. (I think we all agree on this.)
- Statement that the historical Linux/glibc behavior and current POSIX
requirement differ, without language that tries to paint the POSIX
behavior as a HP-UX bug/quirk. Possibly citing real sources/history
of the issue (Austin Group tracker items 529, 614; maybe others).
- Consequence of just assuming the Linux behavior (fd leaks on
conforming systems).
- Consequences of assuming the POSIX behavior (double-close vulns on
GNU/Linux, maybe others).
- Survey of methods for avoiding the problem (ways to preclude EINTR,
possibly ways to infer behavior, etc).
Rich
^ permalink raw reply
* Re: [RFC v1] man/man2/close.2: CAVEATS: Document divergence from POSIX.1-2024
From: Zack Weinberg @ 2025-05-23 18:10 UTC (permalink / raw)
To: Alejandro Colomar, Rich Felker
Cc: Vincent Lefevre, Jan Kara, Alexander Viro, Christian Brauner,
linux-fsdevel, linux-api, GNU libc development
In-Reply-To: <5jm7pblkwkhh4frqjptrw4ll4nwncn22ep2v7sli6kz5wxg5ik@pbnj6wfv66af>
Taking everything said in this thread into account, I have attempted to
wordsmith new language for the close(2) manpage. Please let me know
what you think, and please help me with the bits marked in square
brackets. I can make this into a proper patch for the manpages
when everyone is happy with it.
zw
---
DESCRIPTION
... existing text ...
close() always succeeds. That is, after it returns, _fd_ has
always been disconnected from the open file it formerly referred
to, and its number can be recycled to refer to some other file.
Furthermore, if _fd_ was the last reference to the underlying
open file description, the resources associated with the open file
description will always have been scheduled to be released.
However, close may report _delayed errors_ from a previous I/O
operation. Therefore, its return value should not be ignored.
RETURN VALUE
close() returns zero if there are no delayed errors to report,
or -1 if there _might_ be delayed errors.
When close() returns -1, check _errno_ to see what the situation
actually is. Most, but not all, _errno_ codes indicate a delayed
I/O error that should be reported to the user. See ERRORS and
NOTES for more detail.
[QUERY: Is it ever possible to get delayed errors on close() from
a file that was opened with O_RDONLY? What about a file that was
opened with O_RDWR but never actually written to? If people only
have to worry about delayed errors if the file was actually
written to, we should say so at this point.
It would also be good to mention whether it is possible to get a
delayed error on close() even if a previous call to fsync() or
fdatasync() succeeded and there haven’t been any more writes to
that file *description* (not necessarily via the fd being closed)
since.]
ERRORS
EBADF _fd_ wasn’t open in the first place, or is outside the
valid numeric range for file descriptors.
EINPROGRESS
EINTR
There are no delayed errors to report, but the kernel is
still doing some clean-up work in the background. This
situation should be treated the same as if close() had
returned zero. Do not retry the close(), and do not report
an error to the user.
EDQUOT
EFBIG
EIO
ENOSPC
These are the most common errno codes associated with
delayed I/O errors. They should be treated as a hard
failure to write to the file that was formerly associated
with _fd_, the same as if an earlier write(2) had failed
with one of these codes. The file has still been closed!
Do not retry the close(). But do report an error to the user.
Depending on the underlying file, close() may return other errno
codes; these should generally also be treated as delayed I/O errors.
NOTES
Dealing with error returns from close()
As discussed above, close() always closes the file. Except when
errno is set to EBADF, EINPROGRESS, or EINTR, an error return from
close() reports a _delayed I/O error_ from a previous write()
operation.
It is vital to report delayed I/O errors to the user; failing to
check the return value of close() can cause _silent_ loss of data.
The most common situations where this actually happens involve
networked filesystems, where, in the name of throughput, write()
often returns success before the server has actually confirmed a
successful write.
However, it is also vital to understand that _no matter what_
close() returns, and _no matter what_ it sets errno to, when it
returns, _the file descriptor passed to close() has been closed_,
and its number is _immediately_ available for reuse by open(2),
dup(2), etc. Therefore, one should never retry a close(), not
even if it set errno to a value that normally indicates the
operation needs to be retried (e.g. EINTR). Retrying a close()
is a serious bug, particularly in a multithreaded program; if
the file descriptor number has already been reused, _that file_
will get closed out from under whatever other thread opened it.
[Possibly something about fsync/fdatasync here?]
BUGS
Prior to POSIX.1-2024, there was no official guarantee that
close() would always close the file descriptor, even on error.
Linux has always closed the file descriptor, even on error,
but other implementations might not have.
The only such implementation we have heard of is HP-UX; at least
some versions of HP-UX’s man page for close() said it should be
retried if it returned -1 with errno set to EINTR. (If you know
exactly which versions of HP-UX are affected, or of any other
Unix where close() doesn’t always close the file descriptor,
please contact us about it.)
Portable code should nonetheless never retry a failed close(); the
consequences of a file descriptor leak are far less dangerous than
the consequences of closing a file out from under another thread.
^ permalink raw reply
* Re: [PATCH v5 3/7] selinux: implement inode_file_[g|s]etattr hooks
From: Paul Moore @ 2025-05-22 22:26 UTC (permalink / raw)
To: Andrey Albershteyn, Richard Henderson, Matt Turner, Russell King,
Catalin Marinas, Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Günther Noack, Arnd Bergmann, Pali Rohár, James Morris,
Serge E. Hallyn, Stephen Smalley, Ondrej Mosnacek, Tyler Hicks,
Miklos Szeredi, Amir Goldstein
Cc: linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k,
linux-mips, linux-parisc, linuxppc-dev, linux-s390, linux-sh,
sparclinux, linux-fsdevel, linux-security-module, linux-api,
linux-arch, selinux, ecryptfs, linux-unionfs, linux-xfs,
Andrey Albershteyn
In-Reply-To: <20250513-xattrat-syscall-v5-3-22bb9c6c767f@kernel.org>
On May 13, 2025 Andrey Albershteyn <aalbersh@redhat.com> wrote:
>
> These hooks are called on inode extended attribute retrieval/change.
>
> Cc: selinux@vger.kernel.org
> Cc: Paul Moore <paul@paul-moore.com>
>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
> security/selinux/hooks.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v5 2/7] lsm: introduce new hooks for setting/getting inode fsxattr
From: Paul Moore @ 2025-05-22 22:26 UTC (permalink / raw)
To: Andrey Albershteyn, Richard Henderson, Matt Turner, Russell King,
Catalin Marinas, Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E.J. Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
David S. Miller, Andreas Larsson, Andy Lutomirski,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Chris Zankel, Max Filippov, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Günther Noack, Arnd Bergmann, Pali Rohár, James Morris,
Serge E. Hallyn, Stephen Smalley, Ondrej Mosnacek, Tyler Hicks,
Miklos Szeredi, Amir Goldstein
Cc: linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k,
linux-mips, linux-parisc, linuxppc-dev, linux-s390, linux-sh,
sparclinux, linux-fsdevel, linux-security-module, linux-api,
linux-arch, selinux, ecryptfs, linux-unionfs, linux-xfs,
Andrey Albershteyn
In-Reply-To: <20250513-xattrat-syscall-v5-2-22bb9c6c767f@kernel.org>
On May 13, 2025 Andrey Albershteyn <aalbersh@redhat.com> wrote:
>
> Introduce new hooks for setting and getting filesystem extended
> attributes on inode (FS_IOC_FSGETXATTR).
>
> Cc: selinux@vger.kernel.org
> Cc: Paul Moore <paul@paul-moore.com>
>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
> fs/file_attr.c | 19 ++++++++++++++++---
> include/linux/lsm_hook_defs.h | 2 ++
> include/linux/security.h | 16 ++++++++++++++++
> security/security.c | 30 ++++++++++++++++++++++++++++++
> 4 files changed, 64 insertions(+), 3 deletions(-)
The only thing that gives me a slight pause is that on a set operation
we are going to hit both the get and set LSM hooks, but since the code
does call into the getter on a set operation this is arguably the right
thing.
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH 0/5] add process_madvise() flags to modify behaviour
From: Mike Rapoport @ 2025-05-22 12:12 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Liam R . Howlett, David Hildenbrand,
Vlastimil Babka, Jann Horn, Arnd Bergmann, Christian Brauner,
linux-mm, linux-arch, linux-kernel, SeongJae Park, Usama Arif,
linux-api
In-Reply-To: <cover.1747686021.git.lorenzo.stoakes@oracle.com>
(cc'ing linux-api)
On Mon, May 19, 2025 at 09:52:37PM +0100, Lorenzo Stoakes wrote:
> REVIEWERS NOTES:
> ================
>
> This is a VERY EARLY version of the idea, it's relatively untested, and I'm
> 'putting it out there' for feedback. Any serious version of this will add a
> bunch of self-tests to assert correct behaviour and I will more carefully
> confirm everything's working.
>
> This is based on discussion arising from Usama's series [0], SJ's input on
> the thread around process_madvise() behaviour [1] (and a subsequent
> response by me [2]) and prior discussion about a new madvise() interface
> [3].
>
> [0]: https://lore.kernel.org/linux-mm/20250515133519.2779639-1-usamaarif642@gmail.com/
> [1]: https://lore.kernel.org/linux-mm/20250517162048.36347-1-sj@kernel.org/
> [2]: https://lore.kernel.org/linux-mm/e3ba284c-3cb1-42c1-a0ba-9c59374d0541@lucifer.local/
> [3]: https://lore.kernel.org/linux-mm/c390dd7e-0770-4d29-bb0e-f410ff6678e3@lucifer.local/
>
> ================
>
> Currently, we are rather restricted in how madvise() operations
> proceed. While effort has been put in to expanding what process_madvise()
> can do (that is - unrestricted application of advice to the local process
> alongside recent improvements on the efficiency of TLB operations over
> these batvches), we are still constrained by existing madvise() limitations
> and default behaviours.
>
> This series makes use of the currently unused flags field in
> process_madvise() to provide more flexiblity.
>
> It introduces four flags:
>
> 1. PMADV_SKIP_ERRORS
>
> Currently, when an error arises applying advice in any individual VMA
> (keeping in mind that a range specified to madvise() or as part of the
> iovec passed to process_madvise()), the operation stops where it is and
> returns an error.
>
> This might not be the desired behaviour of the user, who may wish instead
> for the operation to be 'best effort'. By setting this flag, that behaviour
> is obtained.
>
> Since process_madvise() would trivially, if skipping errors, simply return
> the input vector size, we instead return the number of entries in the
> vector which completed successfully without error.
>
> The PMADV_SKIP_ERRORS flag implicitly implies PMADV_NO_ERROR_ON_UNMAPPED.
>
> 2. PMADV_NO_ERROR_ON_UNMAPPED
>
> Currently madvise() has the peculiar behaviour of, if the range specified
> to it contains unmapped range(s), completing the full operation, but
> ultimately returning -ENOMEM.
>
> In the case of process_madvise(), this is fatal, as the operation will stop
> immediately upon this occurring.
>
> By setting PMADV_NO_ERROR_ON_UNMAPPED, the user can indicate that it wishes
> unmapped areas to simply be entirely ignored.
>
> 3. PMADV_SET_FORK_EXEC_DEFAULT
>
> It may be desirable for a user to specify that all VMAs mapped in a process
> address space default to having an madvise() behaviour established by
> default, in such a fashion as that this persists across fork/exec.
>
> Since this is a very powerful option that would make no sense for many
> advice modes, we explicitly only permit known-safe flags here (currently
> MADV_HUGEPAGE and MADV_NOHUGEPAGE only).
>
> 4. PMADV_ENTIRE_ADDRESS_SPACE
>
> It can be annoying, should a user wish to apply madvise() to all VMAs in an
> address space, to have to add a singular large entry to the input iovec.
>
> So provide sugar to permit this - PMADV_ENTIRE_ADDRESS_SPACE. If specified,
> we expect the user to pass NULL and -1 to the vec and vlen parameters
> respectively so they explicitly acknowledge that these will be ignored,
> e.g.:
>
> process_madvise(PIDFD_SELF, NULL, -1, MADV_HUGEPAGE,
> PMADV_ENTIRE_ADDRESS_SPACE | PMADV_SKIP_ERRORS);
>
> Usually a user ought to prefer setting PMADV_SKIP_ERRORS here as it may
> well be the case that incompatible VMAs will be encountered that ought to
> be skipped.
>
> If this is not set, the PMADV_NO_ERROR_ON_UNMAPPED (which was otherwise
> implicitly implied by PMADV_SKIP_ERRORS) ought to be set as of course, the
> entire address space spans at least some gaps.
>
> Lorenzo Stoakes (5):
> mm: madvise: refactor madvise_populate()
> mm/madvise: add PMADV_SKIP_ERRORS process_madvise() flag
> mm/madvise: add PMADV_NO_ERROR_ON_UNMAPPED process_madvise() flag
> mm/madvise: add PMADV_SET_FORK_EXEC_DEFAULT process_madvise() flag
> mm/madvise: add PMADV_ENTIRE_ADDRESS_SPACE process_madvise() flag
>
> include/uapi/asm-generic/mman-common.h | 6 +
> mm/madvise.c | 206 +++++++++++++++++++------
> 2 files changed, 168 insertions(+), 44 deletions(-)
>
> --
> 2.49.0
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v3 0/7] prctl: introduce PR_SET/GET_THP_POLICY
From: Mike Rapoport @ 2025-05-22 12:10 UTC (permalink / raw)
To: Usama Arif
Cc: Andrew Morton, david, linux-mm, hannes, shakeel.butt, riel, ziy,
laoar.shao, baolin.wang, lorenzo.stoakes, Liam.Howlett, npache,
ryan.roberts, vbabka, jannh, Arnd Bergmann, linux-kernel,
linux-doc, kernel-team, linux-api
In-Reply-To: <20250519223307.3601786-1-usamaarif642@gmail.com>
(cc'ing linux-api)
On Mon, May 19, 2025 at 11:29:52PM +0100, Usama Arif wrote:
> This series allows to change the THP policy of a process, according to the
> value set in arg2, all of which will be inherited during fork+exec:
> - PR_DEFAULT_MADV_HUGEPAGE: This will set VM_HUGEPAGE and clear VM_NOHUGEPAGE
> for the default VMA flags. It will also iterate through every VMA in the
> process and call hugepage_madvise on it, with MADV_HUGEPAGE policy.
> This effectively allows setting MADV_HUGEPAGE on the entire process.
> In an environment where different types of workloads are run on the
> same machine, this will allow workloads that benefit from always having
> hugepages to do so, without regressing those that don't.
> - PR_DEFAULT_MADV_NOHUGEPAGE: This will set VM_NOHUGEPAGE and clear VM_HUGEPAGE
> for the default VMA flags. It will also iterate through every VMA in the
> process and call hugepage_madvise on it, with MADV_NOHUGEPAGE policy.
> This effectively allows setting MADV_NOHUGEPAGE on the entire process.
> In an environment where different types of workloads are run on the
> same machine,this will allow workloads that benefit from having
> hugepages on an madvise basis only to do so, without regressing those
> that benefit from having hugepages always.
> - PR_THP_POLICY_SYSTEM: This will reset (clear) both VM_HUGEPAGE and
> VM_NOHUGEPAGE process for the default flags.
>
> In hyperscalers, we have a single THP policy for the entire fleet.
> We have different types of workloads (e.g. AI/compute/databases/etc)
> running on a single server.
> Some of these workloads will benefit from always getting THP at fault
> (or collapsed by khugepaged), some of them will benefit by only getting
> them at madvise.
>
> This series is useful for 2 usecases:
> 1) global system policy = madvise, while we want some workloads to get THPs
> at fault and by khugepaged :- some processes (e.g. AI workloads) benefits
> from getting THPs at fault (and collapsed by khugepaged). Other workloads
> like databases will incur regression (either a performance regression or
> they are completely memory bound and even a very slight increase in memory
> will cause them to OOM). So what these patches will do is allow setting
> prctl(PR_DEFAULT_MADV_HUGEPAGE) on the AI workloads, (This is how
> workloads are deployed in our (Meta's/Facebook) fleet at this moment).
>
> 2) global system policy = always, while we want some workloads to get THPs
> only on madvise basis :- Same reason as 1). What these patches
> will do is allow setting prctl(PR_DEFAULT_MADV_NOHUGEPAGE) on the database
> workloads. (We hope this is us (Meta) in the near future, if a majority of
> workloads show that they benefit from always, we flip the default host
> setting to "always" across the fleet and workloads that regress can opt-out
> and be "madvise". New services developed will then be tested with always by
> default. "always" is also the default defconfig option upstream, so I would
> imagine this is faced by others as well.)
>
> v2->v3: (Thanks Lorenzo for all the below feedback!)
> v2: https://lore.kernel.org/all/20250515133519.2779639-1-usamaarif642@gmail.com/
> - no more flags2.
> - no more MMF2_...
> - renamed policy to PR_DEFAULT_MADV_(NO)HUGEPAGE
> - mmap_write_lock_killable acquired in PR_GET_THP_POLICY
> - mmap_write lock fixed in PR_SET_THP_POLICY
> - mmap assert check in process_default_madv_hugepage
> - check if hugepage_global_enabled is enabled in the call and account for s390
> - set mm->def_flags VM_HUGEPAGE and VM_NOHUGEPAGE according to the policy in
> the way done by madvise(). I believe VM merge will not be broken in
> this way.
> - process_default_madv_hugepage function that does for_each_vma and calls
> hugepage_madvise.
>
> v1->v2:
> - change from modifying the THP decision making for the process, to modifying
> VMA flags only. This prevents further complicating the logic used to
> determine THP order (Thanks David!)
> - change from using a prctl per policy change to just using PR_SET_THP_POLICY
> and arg2 to set the policy. (Zi Yan)
> - Introduce PR_THP_POLICY_DEFAULT_NOHUGE and PR_THP_POLICY_DEFAULT_SYSTEM
> - Add selftests and documentation.
>
> Usama Arif (7):
> mm: khugepaged: extract vm flag setting outside of hugepage_madvise
> prctl: introduce PR_DEFAULT_MADV_HUGEPAGE for the process
> prctl: introduce PR_DEFAULT_MADV_NOHUGEPAGE for the process
> prctl: introduce PR_THP_POLICY_SYSTEM for the process
> selftests: prctl: introduce tests for PR_DEFAULT_MADV_NOHUGEPAGE
> selftests: prctl: introduce tests for PR_THP_POLICY_DEFAULT_HUGE
> docs: transhuge: document process level THP controls
>
> Documentation/admin-guide/mm/transhuge.rst | 42 +++
> include/linux/huge_mm.h | 2 +
> include/linux/mm.h | 2 +-
> include/linux/mm_types.h | 4 +-
> include/uapi/linux/prctl.h | 6 +
> kernel/sys.c | 53 ++++
> mm/huge_memory.c | 13 +
> mm/khugepaged.c | 26 +-
> tools/include/uapi/linux/prctl.h | 6 +
> .../trace/beauty/include/uapi/linux/prctl.h | 6 +
> tools/testing/selftests/prctl/Makefile | 2 +-
> tools/testing/selftests/prctl/thp_policy.c | 286 ++++++++++++++++++
> 12 files changed, 436 insertions(+), 12 deletions(-)
> create mode 100644 tools/testing/selftests/prctl/thp_policy.c
>
> --
> 2.47.1
>
>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v4 4/7] futex: Create set_robust_list2
From: kernel test robot @ 2025-05-22 11:39 UTC (permalink / raw)
To: André Almeida, Thomas Gleixner, Ingo Molnar, Peter Zijlstra,
Darren Hart, Davidlohr Bueso, Shuah Khan, Arnd Bergmann,
Sebastian Andrzej Siewior, Waiman Long
Cc: oe-kbuild-all, linux-kernel, linux-kselftest, linux-api,
André Almeida
In-Reply-To: <20250520-tonyk-robust_futex-v4-4-1123093e59de@igalia.com>
Hi André,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 3ee84e3dd88e39b55b534e17a7b9a181f1d46809]
url: https://github.com/intel-lab-lkp/linux/commits/Andr-Almeida/selftests-futex-Add-ASSERT_-macros/20250521-045231
base: 3ee84e3dd88e39b55b534e17a7b9a181f1d46809
patch link: https://lore.kernel.org/r/20250520-tonyk-robust_futex-v4-4-1123093e59de%40igalia.com
patch subject: [PATCH v4 4/7] futex: Create set_robust_list2
config: arm-randconfig-r122-20250522 (https://download.01.org/0day-ci/archive/20250522/202505221953.JKgFsA3U-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce: (https://download.01.org/0day-ci/archive/20250522/202505221953.JKgFsA3U-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505221953.JKgFsA3U-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
kernel/futex/core.c:581:38: sparse: sparse: cast removes address space '__user' of expression
kernel/futex/core.c:581:51: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected unsigned int [noderef] [usertype] __user *naddr @@ got void * @@
kernel/futex/core.c:581:51: sparse: expected unsigned int [noderef] [usertype] __user *naddr
kernel/futex/core.c:581:51: sparse: got void *
kernel/futex/core.c:597:38: sparse: sparse: cast removes address space '__user' of expression
kernel/futex/core.c:597:51: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected unsigned int [noderef] [usertype] __user *naddr @@ got void * @@
kernel/futex/core.c:597:51: sparse: expected unsigned int [noderef] [usertype] __user *naddr
kernel/futex/core.c:597:51: sparse: got void *
kernel/futex/core.c:1268:59: sparse: sparse: cast removes address space '__user' of expression
>> kernel/futex/core.c:1268:59: sparse: sparse: incorrect type in argument 3 (different address spaces) @@ expected unsigned int [noderef] [usertype] __user *head @@ got unsigned int [usertype] * @@
kernel/futex/core.c:1268:59: sparse: expected unsigned int [noderef] [usertype] __user *head
kernel/futex/core.c:1268:59: sparse: got unsigned int [usertype] *
kernel/futex/core.c:978:9: sparse: sparse: context imbalance in 'futex_q_lockptr_lock' - wrong count at exit
vim +1268 kernel/futex/core.c
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1247
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1248 /*
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1249 * Walk curr->robust_list (very carefully, it's a userspace list!)
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1250 * and mark any locks found there dead, and notify any waiters.
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1251 *
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1252 * We silently return on any sign of list-walking problem.
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1253 */
1c5e99b662506e kernel/futex/core.c André Almeida 2025-05-20 1254 static void exit_robust_list32(struct task_struct *curr,
1c5e99b662506e kernel/futex/core.c André Almeida 2025-05-20 1255 struct robust_list_head32 __user *head)
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1256 {
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1257 struct robust_list __user *entry, *next_entry, *pending;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1258 unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
3f649ab728cda8 kernel/futex.c Kees Cook 2020-06-03 1259 unsigned int next_pi;
b9412773325c3a kernel/futex/core.c André Almeida 2025-05-20 1260 u32 uentry, next_uentry, upending;
b9412773325c3a kernel/futex/core.c André Almeida 2025-05-20 1261 s32 futex_offset;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1262 int rc;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1263
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1264 /*
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1265 * Fetch the list head (which was registered earlier, via
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1266 * sys_set_robust_list()):
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1267 */
b9412773325c3a kernel/futex/core.c André Almeida 2025-05-20 @1268 if (fetch_robust_entry32((u32 *)&uentry, &entry, (u32 *)&head->list.next, &pi))
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1269 return;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1270 /*
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1271 * Fetch the relative futex offset:
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1272 */
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1273 if (get_user(futex_offset, &head->futex_offset))
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1274 return;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1275 /*
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1276 * Fetch any possibly pending lock-add first, and handle it
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1277 * if it exists:
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1278 */
b9412773325c3a kernel/futex/core.c André Almeida 2025-05-20 1279 if (fetch_robust_entry32(&upending, &pending,
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1280 &head->list_op_pending, &pip))
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1281 return;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1282
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1283 next_entry = NULL; /* avoid warning with gcc */
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1284 while (entry != (struct robust_list __user *) &head->list) {
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1285 /*
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1286 * Fetch the next entry in the list before calling
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1287 * handle_futex_death:
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1288 */
b9412773325c3a kernel/futex/core.c André Almeida 2025-05-20 1289 rc = fetch_robust_entry32(&next_uentry, &next_entry,
b9412773325c3a kernel/futex/core.c André Almeida 2025-05-20 1290 (u32 __user *)&entry->next, &next_pi);
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1291 /*
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1292 * A pending lock might already be on the list, so
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1293 * dont process it twice:
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1294 */
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1295 if (entry != pending) {
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1296 void __user *uaddr = futex_uaddr(entry, futex_offset);
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1297
ca16d5bee59807 kernel/futex.c Yang Tao 2019-11-06 1298 if (handle_futex_death(uaddr, curr, pi,
ca16d5bee59807 kernel/futex.c Yang Tao 2019-11-06 1299 HANDLE_DEATH_LIST))
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1300 return;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1301 }
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1302 if (rc)
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1303 return;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1304 uentry = next_uentry;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1305 entry = next_entry;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1306 pi = next_pi;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1307 /*
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1308 * Avoid excessively long or circular lists:
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1309 */
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1310 if (!--limit)
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1311 break;
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1312
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1313 cond_resched();
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1314 }
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1315 if (pending) {
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1316 void __user *uaddr = futex_uaddr(pending, futex_offset);
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1317
ca16d5bee59807 kernel/futex.c Yang Tao 2019-11-06 1318 handle_futex_death(uaddr, curr, pip, HANDLE_DEATH_PENDING);
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1319 }
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1320 }
04e7712f446058 kernel/futex.c Arnd Bergmann 2018-04-17 1321
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH net-next v2 1/6] tcp: support writing to a socket in listening state
From: kernel test robot @ 2025-05-22 7:36 UTC (permalink / raw)
To: Jeremy Harris, netdev
Cc: llvm, oe-kbuild-all, linux-api, edumazet, ncardwell,
Jeremy Harris
In-Reply-To: <d3f47c9b5b08237b6e76f7b0739d59089683c86e.1747826775.git.jgh@exim.org>
Hi Jeremy,
kernel test robot noticed the following build warnings:
[auto build test WARNING on f685204c57e87d2a88b159c7525426d70ee745c9]
url: https://github.com/intel-lab-lkp/linux/commits/Jeremy-Harris/tcp-support-writing-to-a-socket-in-listening-state/20250521-195234
base: f685204c57e87d2a88b159c7525426d70ee745c9
patch link: https://lore.kernel.org/r/d3f47c9b5b08237b6e76f7b0739d59089683c86e.1747826775.git.jgh%40exim.org
patch subject: [PATCH net-next v2 1/6] tcp: support writing to a socket in listening state
config: i386-buildonly-randconfig-001-20250522 (https://download.01.org/0day-ci/archive/20250522/202505221529.hEVx1YPV-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250522/202505221529.hEVx1YPV-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505221529.hEVx1YPV-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> net/ipv4/tcp.c:1065:23: warning: variable 'sockc' set but not used [-Wunused-but-set-variable]
1065 | struct sockcm_cookie sockc;
| ^
1 warning generated.
vim +/sockc +1065 net/ipv4/tcp.c
1059
1060 /* Cut-down version of tcp_sendmsg_locked(), for writing on a listen socket
1061 */
1062 static int tcp_sendmsg_preload(struct sock *sk, struct msghdr *msg)
1063 {
1064 struct sk_buff *skb;
> 1065 struct sockcm_cookie sockc;
1066 int flags, err, copied = 0;
1067 int size_goal;
1068 int process_backlog = 0;
1069 long timeo;
1070
1071 if (sk->sk_state != TCP_LISTEN)
1072 return -EINVAL;
1073
1074 flags = msg->msg_flags;
1075
1076 sockc = (struct sockcm_cookie){ .tsflags = READ_ONCE(sk->sk_tsflags) };
1077
1078 timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
1079
1080 /* Ok commence sending. */
1081 restart:
1082 /* Use a arbitrary "mss" value */
1083 size_goal = 1000;
1084
1085 err = -EPIPE;
1086 if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
1087 goto do_error;
1088
1089 while (msg_data_left(msg)) {
1090 ssize_t copy = 0;
1091
1092 skb = tcp_write_queue_tail(sk);
1093 if (skb)
1094 copy = size_goal - skb->len;
1095
1096 trace_tcp_sendmsg_locked(sk, msg, skb, size_goal);
1097
1098 if (copy <= 0 || !tcp_skb_can_collapse_to(skb)) {
1099 bool first_skb = !skb;
1100
1101 /* Limit to only one skb on the sk write queue */
1102
1103 if (!first_skb)
1104 goto out_nopush;
1105
1106 if (!sk_stream_memory_free(sk))
1107 goto wait_for_space;
1108
1109 if (unlikely(process_backlog >= 16)) {
1110 process_backlog = 0;
1111 if (sk_flush_backlog(sk))
1112 goto restart;
1113 }
1114
1115 skb = tcp_stream_alloc_skb(sk, sk->sk_allocation,
1116 first_skb);
1117 if (!skb)
1118 goto wait_for_space;
1119
1120 process_backlog++;
1121
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v4 5/7] futex: Wire up set_robust_list2 syscall
From: kernel test robot @ 2025-05-21 20:23 UTC (permalink / raw)
To: André Almeida, Thomas Gleixner, Ingo Molnar, Peter Zijlstra,
Darren Hart, Davidlohr Bueso, Shuah Khan, Arnd Bergmann,
Sebastian Andrzej Siewior, Waiman Long
Cc: oe-kbuild-all, linux-kernel, linux-kselftest, linux-api,
André Almeida
In-Reply-To: <20250520-tonyk-robust_futex-v4-5-1123093e59de@igalia.com>
Hi André,
kernel test robot noticed the following build errors:
[auto build test ERROR on 3ee84e3dd88e39b55b534e17a7b9a181f1d46809]
url: https://github.com/intel-lab-lkp/linux/commits/Andr-Almeida/selftests-futex-Add-ASSERT_-macros/20250521-045231
base: 3ee84e3dd88e39b55b534e17a7b9a181f1d46809
patch link: https://lore.kernel.org/r/20250520-tonyk-robust_futex-v4-5-1123093e59de%40igalia.com
patch subject: [PATCH v4 5/7] futex: Wire up set_robust_list2 syscall
config: arc-randconfig-001-20250521 (https://download.01.org/0day-ci/archive/20250522/202505220350.ZektmdF0-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250522/202505220350.ZektmdF0-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505220350.ZektmdF0-lkp@intel.com/
All errors (new ones prefixed by >>):
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:460:1: note: in expansion of macro '__SYSCALL'
460 | __SYSCALL(459, sys_lsm_get_self_attr)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: warning: initialized field overwritten [-Woverride-init]
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:461:1: note: in expansion of macro '__SYSCALL'
461 | __SYSCALL(460, sys_lsm_set_self_attr)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: note: (near initialization for 'sys_call_table[460]')
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:461:1: note: in expansion of macro '__SYSCALL'
461 | __SYSCALL(460, sys_lsm_set_self_attr)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: warning: initialized field overwritten [-Woverride-init]
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:462:1: note: in expansion of macro '__SYSCALL'
462 | __SYSCALL(461, sys_lsm_list_modules)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: note: (near initialization for 'sys_call_table[461]')
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:462:1: note: in expansion of macro '__SYSCALL'
462 | __SYSCALL(461, sys_lsm_list_modules)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: warning: initialized field overwritten [-Woverride-init]
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:463:1: note: in expansion of macro '__SYSCALL'
463 | __SYSCALL(462, sys_mseal)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: note: (near initialization for 'sys_call_table[462]')
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:463:1: note: in expansion of macro '__SYSCALL'
463 | __SYSCALL(462, sys_mseal)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: warning: initialized field overwritten [-Woverride-init]
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:464:1: note: in expansion of macro '__SYSCALL'
464 | __SYSCALL(463, sys_setxattrat)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: note: (near initialization for 'sys_call_table[463]')
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:464:1: note: in expansion of macro '__SYSCALL'
464 | __SYSCALL(463, sys_setxattrat)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: warning: initialized field overwritten [-Woverride-init]
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:465:1: note: in expansion of macro '__SYSCALL'
465 | __SYSCALL(464, sys_getxattrat)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: note: (near initialization for 'sys_call_table[464]')
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:465:1: note: in expansion of macro '__SYSCALL'
465 | __SYSCALL(464, sys_getxattrat)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: warning: initialized field overwritten [-Woverride-init]
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:466:1: note: in expansion of macro '__SYSCALL'
466 | __SYSCALL(465, sys_listxattrat)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: note: (near initialization for 'sys_call_table[465]')
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:466:1: note: in expansion of macro '__SYSCALL'
466 | __SYSCALL(465, sys_listxattrat)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: warning: initialized field overwritten [-Woverride-init]
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:467:1: note: in expansion of macro '__SYSCALL'
467 | __SYSCALL(466, sys_removexattrat)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: note: (near initialization for 'sys_call_table[466]')
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:467:1: note: in expansion of macro '__SYSCALL'
467 | __SYSCALL(466, sys_removexattrat)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: warning: initialized field overwritten [-Woverride-init]
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:468:1: note: in expansion of macro '__SYSCALL'
468 | __SYSCALL(467, sys_open_tree_attr)
| ^~~~~~~~~
arch/arc/kernel/sys.c:13:36: note: (near initialization for 'sys_call_table[467]')
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
./arch/arc/include/generated/asm/syscall_table_32.h:468:1: note: in expansion of macro '__SYSCALL'
468 | __SYSCALL(467, sys_open_tree_attr)
| ^~~~~~~~~
>> ./arch/arc/include/generated/asm/syscall_table_32.h:469:16: error: 'sys_set_robust_list2' undeclared here (not in a function); did you mean 'sys_set_robust_list'?
469 | __SYSCALL(468, sys_set_robust_list2)
| ^~~~~~~~~~~~~~~~~~~~
arch/arc/kernel/sys.c:13:37: note: in definition of macro '__SYSCALL'
13 | #define __SYSCALL(nr, call) [nr] = (call),
| ^~~~
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v4 4/7] futex: Create set_robust_list2
From: kernel test robot @ 2025-05-21 14:24 UTC (permalink / raw)
To: André Almeida, Thomas Gleixner, Ingo Molnar, Peter Zijlstra,
Darren Hart, Davidlohr Bueso, Shuah Khan, Arnd Bergmann,
Sebastian Andrzej Siewior, Waiman Long
Cc: oe-kbuild-all, linux-kernel, linux-kselftest, linux-api,
André Almeida
In-Reply-To: <20250520-tonyk-robust_futex-v4-4-1123093e59de@igalia.com>
Hi André,
kernel test robot noticed the following build errors:
[auto build test ERROR on 3ee84e3dd88e39b55b534e17a7b9a181f1d46809]
url: https://github.com/intel-lab-lkp/linux/commits/Andr-Almeida/selftests-futex-Add-ASSERT_-macros/20250521-045231
base: 3ee84e3dd88e39b55b534e17a7b9a181f1d46809
patch link: https://lore.kernel.org/r/20250520-tonyk-robust_futex-v4-4-1123093e59de%40igalia.com
patch subject: [PATCH v4 4/7] futex: Create set_robust_list2
config: i386-buildonly-randconfig-003-20250521 (https://download.01.org/0day-ci/archive/20250521/202505212200.ditHHp5E-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250521/202505212200.ditHHp5E-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505212200.ditHHp5E-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from <command-line>:
>> ./usr/include/linux/futex.h:177:26: error: field 'list' has incomplete type
177 | struct list_head list;
| ^~~~
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v4 3/7] futex: Use explicit sizes for compat_exit_robust_list
From: kernel test robot @ 2025-05-21 13:32 UTC (permalink / raw)
To: André Almeida, Thomas Gleixner, Ingo Molnar, Peter Zijlstra,
Darren Hart, Davidlohr Bueso, Shuah Khan, Arnd Bergmann,
Sebastian Andrzej Siewior, Waiman Long
Cc: llvm, oe-kbuild-all, linux-kernel, linux-kselftest, linux-api,
André Almeida
In-Reply-To: <20250520-tonyk-robust_futex-v4-3-1123093e59de@igalia.com>
Hi André,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 3ee84e3dd88e39b55b534e17a7b9a181f1d46809]
url: https://github.com/intel-lab-lkp/linux/commits/Andr-Almeida/selftests-futex-Add-ASSERT_-macros/20250521-045231
base: 3ee84e3dd88e39b55b534e17a7b9a181f1d46809
patch link: https://lore.kernel.org/r/20250520-tonyk-robust_futex-v4-3-1123093e59de%40igalia.com
patch subject: [PATCH v4 3/7] futex: Use explicit sizes for compat_exit_robust_list
config: arm-randconfig-002-20250521 (https://download.01.org/0day-ci/archive/20250521/202505212147.5qoTgcmU-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250521/202505212147.5qoTgcmU-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505212147.5qoTgcmU-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from io_uring/notif.c:9:
In file included from io_uring/notif.h:5:
In file included from include/net/sock.h:46:
In file included from include/linux/netdevice.h:44:
In file included from include/uapi/linux/neighbour.h:6:
In file included from include/linux/netlink.h:9:
In file included from include/net/scm.h:13:
In file included from include/net/compat.h:8:
>> include/linux/compat.h:665:35: warning: declaration of 'struct robust_list_head32' will not be visible outside of this function [-Wvisibility]
665 | compat_sys_set_robust_list(struct robust_list_head32 __user *head,
| ^
1 warning generated.
--
In file included from io_uring/uring_cmd.c:9:
In file included from include/net/sock.h:46:
In file included from include/linux/netdevice.h:44:
In file included from include/uapi/linux/neighbour.h:6:
In file included from include/linux/netlink.h:9:
In file included from include/net/scm.h:13:
In file included from include/net/compat.h:8:
>> include/linux/compat.h:665:35: warning: declaration of 'struct robust_list_head32' will not be visible outside of this function [-Wvisibility]
665 | compat_sys_set_robust_list(struct robust_list_head32 __user *head,
| ^
io_uring/uring_cmd.c:311:19: warning: unused function 'io_uring_cmd_getsockopt' [-Wunused-function]
311 | static inline int io_uring_cmd_getsockopt(struct socket *sock,
| ^~~~~~~~~~~~~~~~~~~~~~~
io_uring/uring_cmd.c:338:19: warning: unused function 'io_uring_cmd_setsockopt' [-Wunused-function]
338 | static inline int io_uring_cmd_setsockopt(struct socket *sock,
| ^~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.
vim +665 include/linux/compat.h
621
622 #ifdef __ARCH_WANT_COMPAT_SYS_PWRITEV64
623 asmlinkage long compat_sys_pwritev64(unsigned long fd,
624 const struct iovec __user *vec,
625 unsigned long vlen, loff_t pos);
626 #endif
627 asmlinkage long compat_sys_sendfile(int out_fd, int in_fd,
628 compat_off_t __user *offset, compat_size_t count);
629 asmlinkage long compat_sys_sendfile64(int out_fd, int in_fd,
630 compat_loff_t __user *offset, compat_size_t count);
631 asmlinkage long compat_sys_pselect6_time32(int n, compat_ulong_t __user *inp,
632 compat_ulong_t __user *outp,
633 compat_ulong_t __user *exp,
634 struct old_timespec32 __user *tsp,
635 void __user *sig);
636 asmlinkage long compat_sys_pselect6_time64(int n, compat_ulong_t __user *inp,
637 compat_ulong_t __user *outp,
638 compat_ulong_t __user *exp,
639 struct __kernel_timespec __user *tsp,
640 void __user *sig);
641 asmlinkage long compat_sys_ppoll_time32(struct pollfd __user *ufds,
642 unsigned int nfds,
643 struct old_timespec32 __user *tsp,
644 const compat_sigset_t __user *sigmask,
645 compat_size_t sigsetsize);
646 asmlinkage long compat_sys_ppoll_time64(struct pollfd __user *ufds,
647 unsigned int nfds,
648 struct __kernel_timespec __user *tsp,
649 const compat_sigset_t __user *sigmask,
650 compat_size_t sigsetsize);
651 asmlinkage long compat_sys_signalfd4(int ufd,
652 const compat_sigset_t __user *sigmask,
653 compat_size_t sigsetsize, int flags);
654 asmlinkage long compat_sys_newfstatat(unsigned int dfd,
655 const char __user *filename,
656 struct compat_stat __user *statbuf,
657 int flag);
658 asmlinkage long compat_sys_newfstat(unsigned int fd,
659 struct compat_stat __user *statbuf);
660 /* No generic prototype for sync_file_range and sync_file_range2 */
661 asmlinkage long compat_sys_waitid(int, compat_pid_t,
662 struct compat_siginfo __user *, int,
663 struct compat_rusage __user *);
664 asmlinkage long
> 665 compat_sys_set_robust_list(struct robust_list_head32 __user *head,
666 compat_size_t len);
667 asmlinkage long
668 compat_sys_get_robust_list(int pid, compat_uptr_t __user *head_ptr,
669 compat_size_t __user *len_ptr);
670 asmlinkage long compat_sys_getitimer(int which,
671 struct old_itimerval32 __user *it);
672 asmlinkage long compat_sys_setitimer(int which,
673 struct old_itimerval32 __user *in,
674 struct old_itimerval32 __user *out);
675 asmlinkage long compat_sys_kexec_load(compat_ulong_t entry,
676 compat_ulong_t nr_segments,
677 struct compat_kexec_segment __user *,
678 compat_ulong_t flags);
679 asmlinkage long compat_sys_timer_create(clockid_t which_clock,
680 struct compat_sigevent __user *timer_event_spec,
681 timer_t __user *created_timer_id);
682 asmlinkage long compat_sys_ptrace(compat_long_t request, compat_long_t pid,
683 compat_long_t addr, compat_long_t data);
684 asmlinkage long compat_sys_sched_setaffinity(compat_pid_t pid,
685 unsigned int len,
686 compat_ulong_t __user *user_mask_ptr);
687 asmlinkage long compat_sys_sched_getaffinity(compat_pid_t pid,
688 unsigned int len,
689 compat_ulong_t __user *user_mask_ptr);
690 asmlinkage long compat_sys_sigaltstack(const compat_stack_t __user *uss_ptr,
691 compat_stack_t __user *uoss_ptr);
692 asmlinkage long compat_sys_rt_sigsuspend(compat_sigset_t __user *unewset,
693 compat_size_t sigsetsize);
694 #ifndef CONFIG_ODD_RT_SIGACTION
695 asmlinkage long compat_sys_rt_sigaction(int,
696 const struct compat_sigaction __user *,
697 struct compat_sigaction __user *,
698 compat_size_t);
699 #endif
700 asmlinkage long compat_sys_rt_sigprocmask(int how, compat_sigset_t __user *set,
701 compat_sigset_t __user *oset,
702 compat_size_t sigsetsize);
703 asmlinkage long compat_sys_rt_sigpending(compat_sigset_t __user *uset,
704 compat_size_t sigsetsize);
705 asmlinkage long compat_sys_rt_sigtimedwait_time32(compat_sigset_t __user *uthese,
706 struct compat_siginfo __user *uinfo,
707 struct old_timespec32 __user *uts, compat_size_t sigsetsize);
708 asmlinkage long compat_sys_rt_sigtimedwait_time64(compat_sigset_t __user *uthese,
709 struct compat_siginfo __user *uinfo,
710 struct __kernel_timespec __user *uts, compat_size_t sigsetsize);
711 asmlinkage long compat_sys_rt_sigqueueinfo(compat_pid_t pid, int sig,
712 struct compat_siginfo __user *uinfo);
713 /* No generic prototype for rt_sigreturn */
714 asmlinkage long compat_sys_times(struct compat_tms __user *tbuf);
715 asmlinkage long compat_sys_getrlimit(unsigned int resource,
716 struct compat_rlimit __user *rlim);
717 asmlinkage long compat_sys_setrlimit(unsigned int resource,
718 struct compat_rlimit __user *rlim);
719 asmlinkage long compat_sys_getrusage(int who, struct compat_rusage __user *ru);
720 asmlinkage long compat_sys_gettimeofday(struct old_timeval32 __user *tv,
721 struct timezone __user *tz);
722 asmlinkage long compat_sys_settimeofday(struct old_timeval32 __user *tv,
723 struct timezone __user *tz);
724 asmlinkage long compat_sys_sysinfo(struct compat_sysinfo __user *info);
725 asmlinkage long compat_sys_mq_open(const char __user *u_name,
726 int oflag, compat_mode_t mode,
727 struct compat_mq_attr __user *u_attr);
728 asmlinkage long compat_sys_mq_notify(mqd_t mqdes,
729 const struct compat_sigevent __user *u_notification);
730 asmlinkage long compat_sys_mq_getsetattr(mqd_t mqdes,
731 const struct compat_mq_attr __user *u_mqstat,
732 struct compat_mq_attr __user *u_omqstat);
733 asmlinkage long compat_sys_msgctl(int first, int second, void __user *uptr);
734 asmlinkage long compat_sys_msgrcv(int msqid, compat_uptr_t msgp,
735 compat_ssize_t msgsz, compat_long_t msgtyp, int msgflg);
736 asmlinkage long compat_sys_msgsnd(int msqid, compat_uptr_t msgp,
737 compat_ssize_t msgsz, int msgflg);
738 asmlinkage long compat_sys_semctl(int semid, int semnum, int cmd, int arg);
739 asmlinkage long compat_sys_shmctl(int first, int second, void __user *uptr);
740 asmlinkage long compat_sys_shmat(int shmid, compat_uptr_t shmaddr, int shmflg);
741 asmlinkage long compat_sys_recvfrom(int fd, void __user *buf, compat_size_t len,
742 unsigned flags, struct sockaddr __user *addr,
743 int __user *addrlen);
744 asmlinkage long compat_sys_sendmsg(int fd, struct compat_msghdr __user *msg,
745 unsigned flags);
746 asmlinkage long compat_sys_recvmsg(int fd, struct compat_msghdr __user *msg,
747 unsigned int flags);
748 /* No generic prototype for readahead */
749 asmlinkage long compat_sys_keyctl(u32 option,
750 u32 arg2, u32 arg3, u32 arg4, u32 arg5);
751 asmlinkage long compat_sys_execve(const char __user *filename, const compat_uptr_t __user *argv,
752 const compat_uptr_t __user *envp);
753 /* No generic prototype for fadvise64_64 */
754 /* CONFIG_MMU only */
755 asmlinkage long compat_sys_rt_tgsigqueueinfo(compat_pid_t tgid,
756 compat_pid_t pid, int sig,
757 struct compat_siginfo __user *uinfo);
758 asmlinkage long compat_sys_recvmmsg_time64(int fd, struct compat_mmsghdr __user *mmsg,
759 unsigned vlen, unsigned int flags,
760 struct __kernel_timespec __user *timeout);
761 asmlinkage long compat_sys_recvmmsg_time32(int fd, struct compat_mmsghdr __user *mmsg,
762 unsigned vlen, unsigned int flags,
763 struct old_timespec32 __user *timeout);
764 asmlinkage long compat_sys_wait4(compat_pid_t pid,
765 compat_uint_t __user *stat_addr, int options,
766 struct compat_rusage __user *ru);
767 asmlinkage long compat_sys_fanotify_mark(int, unsigned int, __u32, __u32,
768 int, const char __user *);
769 asmlinkage long compat_sys_open_by_handle_at(int mountdirfd,
770 struct file_handle __user *handle,
771 int flags);
772 asmlinkage long compat_sys_sendmmsg(int fd, struct compat_mmsghdr __user *mmsg,
773 unsigned vlen, unsigned int flags);
774 asmlinkage long compat_sys_execveat(int dfd, const char __user *filename,
775 const compat_uptr_t __user *argv,
776 const compat_uptr_t __user *envp, int flags);
777 asmlinkage ssize_t compat_sys_preadv2(compat_ulong_t fd,
778 const struct iovec __user *vec,
779 compat_ulong_t vlen, u32 pos_low, u32 pos_high, rwf_t flags);
780 asmlinkage ssize_t compat_sys_pwritev2(compat_ulong_t fd,
781 const struct iovec __user *vec,
782 compat_ulong_t vlen, u32 pos_low, u32 pos_high, rwf_t flags);
783 #ifdef __ARCH_WANT_COMPAT_SYS_PREADV64V2
784 asmlinkage long compat_sys_preadv64v2(unsigned long fd,
785 const struct iovec __user *vec,
786 unsigned long vlen, loff_t pos, rwf_t flags);
787 #endif
788
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH net-next v2 2/6] tcp: copy write-data from listen socket to accept child socket
From: Jeremy Harris @ 2025-05-21 11:45 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747826775.git.jgh@exim.org>
Set the request_sock flag for fastopen earlier, making it available
to the af_ops SYN-handler function.
In that function copy data from the listen socket write queue into an
sk_buff, allocating if needed and adding to the write queue of the
newly-created child socket.
Set sequence number values depending on the fastopen status.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
net/ipv4/tcp_fastopen.c | 3 ++-
net/ipv4/tcp_ipv4.c | 4 +--
net/ipv4/tcp_minisocks.c | 58 ++++++++++++++++++++++++++++++++++++----
3 files changed, 57 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 9b83d639b5ac..03a86d0b87ba 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -245,6 +245,8 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
struct sock *child;
bool own_req;
+ tcp_rsk(req)->tfo_listener = true;
+
child = inet_csk(sk)->icsk_af_ops->syn_recv_sock(sk, skb, req, NULL,
NULL, &own_req);
if (!child)
@@ -261,7 +263,6 @@ static struct sock *tcp_fastopen_create_child(struct sock *sk,
tp = tcp_sk(child);
rcu_assign_pointer(tp->fastopen_rsk, req);
- tcp_rsk(req)->tfo_listener = true;
/* RFC1323: The window in SYN & SYN/ACK segments is never
* scaled. So correct it appropriately.
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 6a14f9e6fef6..e488effdbdb2 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1747,8 +1747,8 @@ EXPORT_IPV6_MOD(tcp_v4_conn_request);
/*
- * The three way handshake has completed - we got a valid synack -
- * now create the new socket.
+ * The three way handshake has completed - we got a valid synack
+ * (or a FASTOPEN syn) - now create the new socket.
*/
struct sock *tcp_v4_syn_recv_sock(const struct sock *sk, struct sk_buff *skb,
struct request_sock *req,
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 43d7852ce07e..d471531b4a78 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -529,7 +529,7 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
struct inet_connection_sock *newicsk;
const struct tcp_sock *oldtp;
struct tcp_sock *newtp;
- u32 seq;
+ u32 seq, a_seq, n_seq;
if (!newsk)
return NULL;
@@ -550,9 +550,55 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
newtp->segs_in = 1;
seq = treq->snt_isn + 1;
- newtp->snd_sml = newtp->snd_una = seq;
- WRITE_ONCE(newtp->snd_nxt, seq);
- newtp->snd_up = seq;
+ n_seq = seq;
+ a_seq = seq;
+ newtp->write_seq = seq;
+ newtp->snd_una = seq;
+
+ /* If there is write-data sitting on the listen socket, copy it to
+ * the accept socket. If FASTOPEN we will send it on the synack,
+ * otherwise it sits there until 3rd-ack arrives.
+ */
+
+ if (unlikely(!skb_queue_empty(&sk->sk_write_queue))) {
+ struct sk_buff *l_skb = tcp_send_head(sk),
+ *a_skb = tcp_write_queue_tail(newsk);
+ ssize_t copy = 0;
+
+ if (a_skb)
+ copy = l_skb->len - a_skb->len;
+
+ if (copy <= 0 || !tcp_skb_can_collapse_to(a_skb)) {
+ bool first_skb = tcp_rtx_and_write_queues_empty(newsk);
+
+ a_skb = tcp_stream_alloc_skb(newsk,
+ newsk->sk_allocation,
+ first_skb);
+ if (!a_skb) {
+ /* is this the correct free? */
+ bh_unlock_sock(newsk);
+ sk_free(newsk);
+ return NULL;
+ }
+
+ tcp_skb_entail(newsk, a_skb);
+ }
+ copy = min_t(int, l_skb->len, skb_tailroom(a_skb));
+ skb_put_data(a_skb, l_skb->data, copy);
+
+ TCP_SKB_CB(a_skb)->end_seq += copy;
+
+ a_seq += l_skb->len;
+
+ if (treq->tfo_listener)
+ seq = a_seq;
+
+ /* assumes only one skb on the listen write queue */
+ }
+
+ newtp->snd_sml = seq;
+ WRITE_ONCE(newtp->snd_nxt, a_seq);
+ newtp->snd_up = n_seq;
INIT_LIST_HEAD(&newtp->tsq_node);
INIT_LIST_HEAD(&newtp->tsorted_sent_queue);
@@ -567,7 +613,9 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
newtp->total_retrans = req->num_retrans;
tcp_init_xmit_timers(newsk);
- WRITE_ONCE(newtp->write_seq, newtp->pushed_seq = treq->snt_isn + 1);
+
+ newtp->pushed_seq = n_seq;
+ WRITE_ONCE(newtp->write_seq, a_seq);
if (sock_flag(newsk, SOCK_KEEPOPEN))
tcp_reset_keepalive_timer(newsk, keepalive_time_when(newtp));
--
2.49.0
^ permalink raw reply related
* [PATCH net-next v2 0/6] tcp: support preloading data on a listening socket
From: Jeremy Harris @ 2025-05-21 11:44 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
v2 changes:
- Split out the preload operation to a separate routine from
tcp_sendmsg_locked() and restrict from looping over the supplied
iovec
------
Support write to a listen TCP socket, for immediate
transmission on all later passive connection establishments
parented by the listen socket.
On a normal connection transmission of the data is triggered by the receipt
of the 3rd-ack. On a fastopen (with accepted cookie) connection the data
is sent in the synack packet.
The data preload is done using a sendmsg with a newly-defined flag
(MSG_PRELOAD); the amount of data limited to a single linear sk_buff.
Note that this definition is the last-but-two bit available if "int"
is 32 bits.
Intent: lower latency for server-first protocols using TCP.
Known cases of this use are SMTP and MySQL.
Measurements:
Packet capture (laptop, loopback, TFO requeste) for initial SYN to first
client data packet (5 samples):
- baseline TFO-C 1064 1470 1455 1547 1595 usec
- patched non-TFO 140 150 159 144 153 usec
- patched TFO-C 142 149 149 125 125 usec
Out of scope:
- Client-first protocols
- TLS-on-connect
Testing:
A) packetdrill scripts for
- normal non-TFO
- normal TFO
- synack lost
- 3rd-ack acks only the SYN
- 3rd-ack acks partial data
(NB: packetdrill can only check the data size, not actual content)
B) Application use, running the application testsuite
and manual check of specific cases via packet capture
C) Daily-driver laptop use (not expected to trigger the feature;
only regression-test)
D) KASAN/syzkaller
- enable_syscalls "socket$inet_tcp", "listen", "sendmsg", "accept",
"read", "write", "close", "syz_emit_ethernet", "syz_extract_tcp_res"
- the coverage seems rather limited; the sendmsg onto a listen socket
is there, but I am not convinced actual TCP connections are being
excercised. tcp_minisocks.c is entirely uncovered.
- A need for limiting iteration in the above sendmesg was found (RCU
timeouts), hence v2, but no hint of locking problems.
Eric: could you expand on your previous comment? If it referred to
the listening socket, tcp_sendmsg_locked() is called with the sk
locked.
Jeremy Harris (6):
tcp: support writing to a socket in listening state
tcp: copy write-data from listen socket to accept child socket
tcp: fastopen: add write-data to fastopen synack packet
tcp: transmit any pending data on receipt of 3rd-ack
tcp: fastopen: retransmit data when only the SYN of a synack-with-data
is acked
tcp: fastopen: extend retransmit-queue trimming to handle linear
sk_buff
include/linux/socket.h | 1 +
net/ipv4/tcp.c | 115 ++++++++++++++++++
net/ipv4/tcp_fastopen.c | 3 +-
net/ipv4/tcp_input.c | 15 ++-
net/ipv4/tcp_ipv4.c | 4 +-
net/ipv4/tcp_minisocks.c | 58 ++++++++-
net/ipv4/tcp_output.c | 50 +++++++-
.../perf/trace/beauty/include/linux/socket.h | 1 +
tools/perf/trace/beauty/msg_flags.c | 3 +
9 files changed, 237 insertions(+), 13 deletions(-)
base-commit: f685204c57e87d2a88b159c7525426d70ee745c9
--
2.49.0
^ permalink raw reply
* [PATCH net-next v2 5/6] tcp: fastopen: retransmit data when only the SYN of a synack-with-data is acked
From: Jeremy Harris @ 2025-05-21 11:45 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747826775.git.jgh@exim.org>
A corner-case for the 3rd-ack after a data-on-synack is for only
the SYN to be acked. Handle this by, in ack processing, when in
SYN_RECV state (the state is not yet updated to ESTABLISHED)
marking the retransmit-queue sk_buff as having been lost.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
net/ipv4/tcp_input.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 345a08baaf02..a53021edddd5 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4069,6 +4069,18 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
&rexmit);
}
+ /* On receiving a 3rd-ack, if we never sent a packet via
+ * the normal means (which counts them), yet there is data
+ * remaining for retransmit, it was data-on-synack not acked;
+ * mark the skb for retransmission.
+ */
+ if (sk->sk_state == TCP_SYN_RECV && tp->segs_out == 0) {
+ struct sk_buff *skb = tcp_rtx_queue_head(sk);
+
+ if (skb)
+ tcp_mark_skb_lost(sk, skb);
+ }
+
/* If needed, reset TLP/RTO timer when RACK doesn't set. */
if (flag & FLAG_SET_XMIT_TIMER)
tcp_set_xmit_timer(sk);
--
2.49.0
^ permalink raw reply related
* [PATCH net-next v2 3/6] tcp: fastopen: add write-data to fastopen synack packet
From: Jeremy Harris @ 2025-05-21 11:45 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747826775.git.jgh@exim.org>
While building the synack packet, for a fastopen socket
copy data from write queue to the packet.
Move the data from write queue to retransmit queue.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
net/ipv4/tcp_output.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 3ac8d2d17e1f..c50553c1c795 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3702,7 +3702,7 @@ int tcp_send_synack(struct sock *sk)
/**
* tcp_make_synack - Allocate one skb and build a SYNACK packet.
- * @sk: listener socket
+ * @sk: listener socket (or child socket for fastopen)
* @dst: dst entry attached to the SYNACK. It is consumed and caller
* should not use it again.
* @req: request_sock pointer
@@ -3719,6 +3719,7 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
struct inet_request_sock *ireq = inet_rsk(req);
const struct tcp_sock *tp = tcp_sk(sk);
struct tcp_out_options opts;
+ struct sock *fastopen_sk = (struct sock *)sk;
struct tcp_key key = {};
struct sk_buff *skb;
int tcp_header_size;
@@ -3748,7 +3749,7 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
* cpu might call us concurrently.
* sk->sk_wmem_alloc in an atomic, we can promote to rw.
*/
- skb_set_owner_w(skb, (struct sock *)sk);
+ skb_set_owner_w(skb, fastopen_sk);
break;
}
skb_dst_set(skb, dst);
@@ -3831,6 +3832,33 @@ struct sk_buff *tcp_make_synack(const struct sock *sk, struct dst_entry *dst,
th->window = htons(min(req->rsk_rcv_wnd, 65535U));
tcp_options_write(th, NULL, tcp_rsk(req), &opts, &key);
th->doff = (tcp_header_size >> 2);
+
+ /* If this is a FASTOPEN, and there is write-data on the accept socket,
+ * re-copy it to the synack segment. If not FASTOPEN. any data waits
+ * until 3rd-ack arrival.
+ */
+
+ if (synack_type == TCP_SYNACK_FASTOPEN &&
+ !skb_queue_empty(&sk->sk_write_queue)) {
+ struct sk_buff *a_skb = tcp_write_queue_tail(sk);
+ int copy = min_t(int, a_skb->len, skb_tailroom(skb));
+
+ skb_put_data(skb, a_skb->data, copy);
+ TCP_SKB_CB(skb)->end_seq += copy;
+
+ tcp_skb_pcount_set(a_skb, 1);
+ WRITE_ONCE(tcp_sk(fastopen_sk)->write_seq,
+ TCP_SKB_CB(a_skb)->end_seq);
+
+ skb_set_delivery_time(a_skb, now, SKB_CLOCK_MONOTONIC);
+
+ /* Move the data to the retransmit queue.
+ * Code elsewhere implies this is a full child socket and
+ * can be treated as writeable - permitting the cast.
+ */
+ tcp_event_new_data_sent(fastopen_sk, a_skb);
+ }
+
TCP_INC_STATS(sock_net(sk), TCP_MIB_OUTSEGS);
/* Okay, we have all we need - do the md5 hash if needed */
--
2.49.0
^ permalink raw reply related
* [PATCH net-next v2 4/6] tcp: transmit any pending data on receipt of 3rd-ack
From: Jeremy Harris @ 2025-05-21 11:45 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747826775.git.jgh@exim.org>
For the non-fastopen case of prelaod, when the 3rd-ack arrives there
will be data on the write queue. Transmit it immediately
by allowing the SYN_SENT state to run the xmit-recovery code.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
net/ipv4/tcp_input.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 8ec92dec321a..345a08baaf02 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3900,7 +3900,8 @@ static void tcp_xmit_recovery(struct sock *sk, int rexmit)
{
struct tcp_sock *tp = tcp_sk(sk);
- if (rexmit == REXMIT_NONE || sk->sk_state == TCP_SYN_SENT)
+ if ((rexmit == REXMIT_NONE && sk->sk_state != TCP_SYN_RECV) ||
+ sk->sk_state == TCP_SYN_SENT)
return;
if (unlikely(rexmit == REXMIT_NEW)) {
--
2.49.0
^ permalink raw reply related
* [PATCH net-next v2 6/6] tcp: fastopen: extend retransmit-queue trimming to handle linear sk_buff
From: Jeremy Harris @ 2025-05-21 11:45 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747826775.git.jgh@exim.org>
A corner-case for the 3rd-ack after a data-on-synack is for
some but not all of the data to be acked. Support this by
adding to the retransmit-queue trim routine to handle a
linear sk_buff.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
net/ipv4/tcp_output.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index c50553c1c795..bff5934ff04b 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1708,8 +1708,22 @@ static int __pskb_trim_head(struct sk_buff *skb, int len)
struct skb_shared_info *shinfo;
int i, k, eat;
- DEBUG_NET_WARN_ON_ONCE(skb_headlen(skb));
- eat = len;
+ eat = skb_headlen(skb);
+ if (unlikely(eat)) {
+ if (len < eat)
+ eat = len;
+ skb->head += eat;
+ skb->len -= eat;
+ if (skb->data_len)
+ skb->data_len -= eat;
+
+ eat = len - eat;
+ if (eat == 0)
+ return len;
+ } else {
+ eat = len;
+ }
+
k = 0;
shinfo = skb_shinfo(skb);
for (i = 0; i < shinfo->nr_frags; i++) {
--
2.49.0
^ permalink raw reply related
* [PATCH net-next v2 1/6] tcp: support writing to a socket in listening state
From: Jeremy Harris @ 2025-05-21 11:45 UTC (permalink / raw)
To: netdev; +Cc: linux-api, edumazet, ncardwell, Jeremy Harris
In-Reply-To: <cover.1747826775.git.jgh@exim.org>
In the tcp sendmsg handler, permit a write in LISTENING state if
a MSG_PRELOAD flag is used. Copy from iovec to a linear sk_buff
for placement on the socket write queue.
Signed-off-by: Jeremy Harris <jgh@exim.org>
---
include/linux/socket.h | 1 +
net/ipv4/tcp.c | 115 ++++++++++++++++++
.../perf/trace/beauty/include/linux/socket.h | 1 +
tools/perf/trace/beauty/msg_flags.c | 3 +
4 files changed, 120 insertions(+)
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 3b262487ec06..b41f4cd4dc97 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -330,6 +330,7 @@ struct ucred {
#define MSG_SOCK_DEVMEM 0x2000000 /* Receive devmem skbs as cmsg */
#define MSG_ZEROCOPY 0x4000000 /* Use user data in kernel path */
#define MSG_SPLICE_PAGES 0x8000000 /* Splice the pages from the iterator in sendmsg() */
+#define MSG_PRELOAD 0x10000000 /* Preload tx data while listening */
#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
#define MSG_CMSG_CLOEXEC 0x40000000 /* Set close_on_exec for file
descriptor received through
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b7b6ab41b496..9a5daf27c980 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1057,6 +1057,118 @@ int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, int *copied,
return err;
}
+/* Cut-down version of tcp_sendmsg_locked(), for writing on a listen socket
+ */
+static int tcp_sendmsg_preload(struct sock *sk, struct msghdr *msg)
+{
+ struct sk_buff *skb;
+ struct sockcm_cookie sockc;
+ int flags, err, copied = 0;
+ int size_goal;
+ int process_backlog = 0;
+ long timeo;
+
+ if (sk->sk_state != TCP_LISTEN)
+ return -EINVAL;
+
+ flags = msg->msg_flags;
+
+ sockc = (struct sockcm_cookie){ .tsflags = READ_ONCE(sk->sk_tsflags) };
+
+ timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
+
+ /* Ok commence sending. */
+restart:
+ /* Use a arbitrary "mss" value */
+ size_goal = 1000;
+
+ err = -EPIPE;
+ if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
+ goto do_error;
+
+ while (msg_data_left(msg)) {
+ ssize_t copy = 0;
+
+ skb = tcp_write_queue_tail(sk);
+ if (skb)
+ copy = size_goal - skb->len;
+
+ trace_tcp_sendmsg_locked(sk, msg, skb, size_goal);
+
+ if (copy <= 0 || !tcp_skb_can_collapse_to(skb)) {
+ bool first_skb = !skb;
+
+ /* Limit to only one skb on the sk write queue */
+
+ if (!first_skb)
+ goto out_nopush;
+
+ if (!sk_stream_memory_free(sk))
+ goto wait_for_space;
+
+ if (unlikely(process_backlog >= 16)) {
+ process_backlog = 0;
+ if (sk_flush_backlog(sk))
+ goto restart;
+ }
+
+ skb = tcp_stream_alloc_skb(sk, sk->sk_allocation,
+ first_skb);
+ if (!skb)
+ goto wait_for_space;
+
+ process_backlog++;
+
+#ifdef CONFIG_SKB_DECRYPTED
+ skb->decrypted = !!(flags & MSG_SENDPAGE_DECRYPTED);
+#endif
+ tcp_skb_entail(sk, skb);
+ copy = size_goal;
+ }
+
+ /* Try to append data to the end of skb. */
+ if (copy > msg_data_left(msg))
+ copy = msg_data_left(msg);
+
+ copy = min_t(int, copy, skb_tailroom(skb));
+ err = skb_add_data_nocache(sk, skb, &msg->msg_iter, copy);
+ if (err)
+ goto do_error;
+
+ TCP_SKB_CB(skb)->end_seq += copy;
+ tcp_skb_pcount_set(skb, 0);
+
+ copied += copy;
+ goto out_nopush;
+
+wait_for_space:
+ set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+ tcp_remove_empty_skb(sk);
+
+ err = sk_stream_wait_memory(sk, &timeo);
+ if (err != 0)
+ goto do_error;
+ }
+
+out_nopush:
+ return copied;
+
+do_error:
+ tcp_remove_empty_skb(sk);
+
+ if (copied)
+ goto out_nopush;
+
+ err = sk_stream_error(sk, flags, err);
+ /* make sure we wake any epoll edge trigger waiter */
+ if (unlikely(tcp_rtx_and_write_queues_empty(sk) && err == -EAGAIN)) {
+ sk->sk_write_space(sk);
+ tcp_chrono_stop(sk, TCP_CHRONO_SNDBUF_LIMITED);
+ }
+
+ return err;
+}
+
int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
{
struct net_devmem_dmabuf_binding *binding = NULL;
@@ -1132,6 +1244,9 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
goto out_err;
}
+ if (unlikely(flags & MSG_PRELOAD))
+ return tcp_sendmsg_preload(sk, msg);
+
timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
tcp_rate_check_app_limited(sk); /* is sending application-limited? */
diff --git a/tools/perf/trace/beauty/include/linux/socket.h b/tools/perf/trace/beauty/include/linux/socket.h
index c3322eb3d686..e9ea498169f3 100644
--- a/tools/perf/trace/beauty/include/linux/socket.h
+++ b/tools/perf/trace/beauty/include/linux/socket.h
@@ -330,6 +330,7 @@ struct ucred {
#define MSG_SOCK_DEVMEM 0x2000000 /* Receive devmem skbs as cmsg */
#define MSG_ZEROCOPY 0x4000000 /* Use user data in kernel path */
#define MSG_SPLICE_PAGES 0x8000000 /* Splice the pages from the iterator in sendmsg() */
+#define MSG_PRELOAD 0x10000000 /* Preload tx data while listening */
#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
#define MSG_CMSG_CLOEXEC 0x40000000 /* Set close_on_exec for file
descriptor received through
diff --git a/tools/perf/trace/beauty/msg_flags.c b/tools/perf/trace/beauty/msg_flags.c
index 2da581ff0c80..27e40da9b02d 100644
--- a/tools/perf/trace/beauty/msg_flags.c
+++ b/tools/perf/trace/beauty/msg_flags.c
@@ -20,6 +20,9 @@
#ifndef MSG_SPLICE_PAGES
#define MSG_SPLICE_PAGES 0x8000000
#endif
+#ifndef MSG_PRELOAD
+#define MSG_PRELOAD 0x10000000
+#endif
#ifndef MSG_FASTOPEN
#define MSG_FASTOPEN 0x20000000
#endif
--
2.49.0
^ permalink raw reply related
* Re: [PATCH v5 0/7] fs: introduce file_getattr and file_setattr syscalls
From: Amir Goldstein @ 2025-05-21 10:44 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: pali, Dave Chinner, Christian Brauner, Arnd Bergmann,
Richard Henderson, Matt Turner, Russell King, Catalin Marinas,
Will Deacon, Geert Uytterhoeven, Michal Simek,
Thomas Bogendoerfer, James E . J . Bottomley, Helge Deller,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Naveen N Rao, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Rich Felker, John Paul Adrian Glaubitz, David S . Miller,
Andreas Larsson, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Chris Zankel,
Max Filippov, Alexander Viro, Jan Kara, Mickaël Salaün,
Günther Noack, Paul Moore, James Morris, Serge E. Hallyn,
Stephen Smalley, Ondrej Mosnacek, Tyler Hicks, Miklos Szeredi,
linux-alpha, linux-kernel, linux-arm-kernel, linux-m68k,
linux-mips, linux-parisc, linuxppc-dev, linux-s390, linux-sh,
sparclinux, linux-fsdevel, linux-security-module, linux-api,
Linux-Arch, selinux, ecryptfs, linux-unionfs, linux-xfs,
Andrey Albershteyn
In-Reply-To: <mw2d36mqwzqoveguw5vlggrnw2wirsbhdxkox33z3fg7k6huz6@hj4ntgg3oj7p>
On Wed, May 21, 2025 at 12:06 PM Andrey Albershteyn <aalbersh@redhat.com> wrote:
>
> On 2025-05-21 11:36:31, Amir Goldstein wrote:
> > On Wed, May 21, 2025 at 10:48 AM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> > >
> > > On 2025-05-19 21:37:04, Dave Chinner wrote:
> > > > On Thu, May 15, 2025 at 12:33:31PM +0200, Amir Goldstein wrote:
> > > > > On Thu, May 15, 2025 at 11:02 AM Christian Brauner <brauner@kernel.org> wrote:
> > > > > >
> > > > > > On Tue, May 13, 2025 at 11:53:23AM +0200, Arnd Bergmann wrote:
> > > > > > > On Tue, May 13, 2025, at 11:17, Andrey Albershteyn wrote:
> > > > > > >
> > > > > > > >
> > > > > > > > long syscall(SYS_file_getattr, int dirfd, const char *pathname,
> > > > > > > > struct fsxattr *fsx, size_t size, unsigned int at_flags);
> > > > > > > > long syscall(SYS_file_setattr, int dirfd, const char *pathname,
> > > > > > > > struct fsxattr *fsx, size_t size, unsigned int at_flags);
> > > > > > >
> > > > > > > I don't think we can have both the "struct fsxattr" from the uapi
> > > > > > > headers, and a variable size as an additional argument. I would
> > > > > > > still prefer not having the extensible structure at all and just
> > > > > >
> > > > > > We're not going to add new interfaces that are fixed size unless for the
> > > > > > very basic cases. I don't care if we're doing that somewhere else in the
> > > > > > kernel but we're not doing that for vfs apis.
> > > > > >
> > > > > > > use fsxattr, but if you want to make it extensible in this way,
> > > > > > > it should use a different structure (name). Otherwise adding
> > > > > > > fields after fsx_pad[] would break the ioctl interface.
> > > > > >
> > > > > > Would that really be a problem? Just along the syscall simply add
> > > > > > something like:
> > > > > >
> > > > > > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > > > > > index c91fd2b46a77..d3943805c4be 100644
> > > > > > --- a/fs/ioctl.c
> > > > > > +++ b/fs/ioctl.c
> > > > > > @@ -868,12 +868,6 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
> > > > > > case FS_IOC_SETFLAGS:
> > > > > > return ioctl_setflags(filp, argp);
> > > > > >
> > > > > > - case FS_IOC_FSGETXATTR:
> > > > > > - return ioctl_fsgetxattr(filp, argp);
> > > > > > -
> > > > > > - case FS_IOC_FSSETXATTR:
> > > > > > - return ioctl_fssetxattr(filp, argp);
> > > > > > -
> > > > > > case FS_IOC_GETFSUUID:
> > > > > > return ioctl_getfsuuid(filp, argp);
> > > > > >
> > > > > > @@ -886,6 +880,20 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
> > > > > > break;
> > > > > > }
> > > > > >
> > > > > > + switch (_IOC_NR(cmd)) {
> > > > > > + case _IOC_NR(FS_IOC_FSGETXATTR):
> > > > > > + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FS_IOC_FSGETXATTR)))
> > > > > > + return SOMETHING_SOMETHING;
> > > > > > + /* Only handle original size. */
> > > > > > + return ioctl_fsgetxattr(filp, argp);
> > > > > > +
> > > > > > + case _IOC_NR(FFS_IOC_FSSETXATTR):
> > > > > > + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FFS_IOC_FSSETXATTR)))
> > > > > > + return SOMETHING_SOMETHING;
> > > > > > + /* Only handle original size. */
> > > > > > + return ioctl_fssetxattr(filp, argp);
> > > > > > + }
> > > > > > +
> > > > >
> > > > > I think what Arnd means is that we will not be able to change struct
> > > > > sfxattr in uapi
> > > > > going forward, because we are not going to deprecate the ioctls and
> > > >
> > > > There's no need to deprecate anything to rev an ioctl API. We have
> > > > had to solve this "changing struct size" problem previously in XFS
> > > > ioctls. See XFS_IOC_FSGEOMETRY and the older XFS_IOC_FSGEOMETRY_V4
> > > > and XFS_IOC_FSGEOMETRY_V1 versions of the API/ABI.
> > > >
> > > > If we need to increase the structure size, we can rename the existing
> > > > ioctl and struct to fix the version in the API, then use the
> > > > original name for the new ioctl and structure definition.
> > > >
> > > > The only thing we have to make sure of is that the old and new
> > > > structures have exactly the same overlapping structure. i.e.
> > > > extension must always be done by appending new varibles, they can't
> > > > be put in the middle of the structure.
> > > >
> > > > This way applications being rebuild will pick up the new definition
> > > > automatically when the system asserts that it is suppored, whilst
> > > > existing binaries will always still be supported by the kernel.
> > > >
> > > > If the application wants/needs to support all possible kernels, then
> > > > if XFS_IOC_FSGEOMETRY is not supported, call XFS_IOC_FSGEOMETRY_V4,
> > > > and if that fails (only on really old irix!) or you only need
> > > > something in that original subset, call XFS_IOC_FSGEOMETRY_V1 which
> > > > will always succeed....
> > > >
> > > > > Should we will need to depart from this struct definition and we might
> > > > > as well do it for the initial release of the syscall rather than later on, e.g.:
> > > > >
> > > > > --- a/include/uapi/linux/fs.h
> > > > > +++ b/include/uapi/linux/fs.h
> > > > > @@ -148,6 +148,17 @@ struct fsxattr {
> > > > > unsigned char fsx_pad[8];
> > > > > };
> > > > >
> > > > > +/*
> > > > > + * Variable size structure for file_[sg]et_attr().
> > > > > + */
> > > > > +struct fsx_fileattr {
> > > > > + __u32 fsx_xflags; /* xflags field value (get/set) */
> > > > > + __u32 fsx_extsize; /* extsize field value (get/set)*/
> > > > > + __u32 fsx_nextents; /* nextents field value (get) */
> > > > > + __u32 fsx_projid; /* project identifier (get/set) */
> > > > > + __u32 fsx_cowextsize; /* CoW extsize field value (get/set)*/
> > > > > +};
> > > > > +
> > > > > +#define FSXATTR_SIZE_VER0 20
> > > > > +#define FSXATTR_SIZE_LATEST FSXATTR_SIZE_VER0
> > > >
> > > > If all the structures overlap the same, all that is needed in the
> > > > code is to define the structure size that should be copied in and
> > > > parsed. i.e:
> > > >
> > > > case FSXATTR..._V1:
> > > > return ioctl_fsxattr...(args, sizeof(fsx_fileattr_v1));
> > > > case FSXATTR..._V2:
> > > > return ioctl_fsxattr...(args, sizeof(fsx_fileattr_v2));
> > > > case FSXATTR...:
> > > > return ioctl_fsxattr...(args, sizeof(fsx_fileattr));
> > > >
> > > > -Dave.
> > > > --
> > > > Dave Chinner
> > > > david@fromorbit.com
> > > >
> > >
> > > So, looks like there's at least two solutions to this concern.
> > > Considering also that we have a bit of space in fsxattr,
> > > 'fsx_pad[8]', I think it's fine to stick with the current fsxattr
> > > for now.
> >
> > Not sure which two solutions you are referring to.
>
> Suggested by Christian and Dave
>
IIUC, those are suggestions of how we could cope with changing
struct fsxattr in the future, but it is easier not to have to do that.
> >
> > I proposed fsx_fileattr as what I think is the path of least resistance.
> > There are opinions that we may be able to avoid defining
> > this struct, but I don't think there was any objection to adding it.
> >
> > So unless I am missing an objection that I did not understand
> > define it and get over this hurdle?
>
> I see, sure, I misinterpreted the communication :) no problems, I
> will create 'struct fsx_fileattr' then.
>
> Pali, ah sorry, I forgot that you will extend fsxattr right away
>
Much less problems could be caused if fsxattr remain frozen in
time along with the ioctls as we continue to extend the syscalls.
Thanks,
Amir.
P.S. your CC list is a bit much.
I wouldn't trust get_maintainer.pl output when it provides such a huge list
it has some emails that bounce - not nice.
When you are at v5 you should be able to have figured out who is
participating in the review and for the rest, the public lists
linux-fsdevel, linux-api and linux-xfs should be enough.
^ permalink raw reply
* Re: [PATCH v5 0/7] fs: introduce file_getattr and file_setattr syscalls
From: Andrey Albershteyn @ 2025-05-21 10:06 UTC (permalink / raw)
To: Amir Goldstein, pali
Cc: Dave Chinner, Christian Brauner, Arnd Bergmann, Richard Henderson,
Matt Turner, Russell King, Catalin Marinas, Will Deacon,
Geert Uytterhoeven, Michal Simek, Thomas Bogendoerfer,
James E . J . Bottomley, Helge Deller, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S . Miller, Andreas Larsson,
Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Chris Zankel, Max Filippov,
Alexander Viro, Jan Kara, Mickaël Salaün,
Günther Noack, Pali Rohár, Paul Moore, James Morris,
Serge E. Hallyn, Stephen Smalley, Ondrej Mosnacek, Tyler Hicks,
Miklos Szeredi, linux-alpha, linux-kernel, linux-arm-kernel,
linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-fsdevel, linux-security-module,
linux-api, Linux-Arch, selinux, ecryptfs, linux-unionfs,
linux-xfs, Andrey Albershteyn
In-Reply-To: <CAOQ4uxiM+BBNODHfxu=v3XN2ezA-0k54qC5R4qdELbZW+W-xkg@mail.gmail.com>
On 2025-05-21 11:36:31, Amir Goldstein wrote:
> On Wed, May 21, 2025 at 10:48 AM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> >
> > On 2025-05-19 21:37:04, Dave Chinner wrote:
> > > On Thu, May 15, 2025 at 12:33:31PM +0200, Amir Goldstein wrote:
> > > > On Thu, May 15, 2025 at 11:02 AM Christian Brauner <brauner@kernel.org> wrote:
> > > > >
> > > > > On Tue, May 13, 2025 at 11:53:23AM +0200, Arnd Bergmann wrote:
> > > > > > On Tue, May 13, 2025, at 11:17, Andrey Albershteyn wrote:
> > > > > >
> > > > > > >
> > > > > > > long syscall(SYS_file_getattr, int dirfd, const char *pathname,
> > > > > > > struct fsxattr *fsx, size_t size, unsigned int at_flags);
> > > > > > > long syscall(SYS_file_setattr, int dirfd, const char *pathname,
> > > > > > > struct fsxattr *fsx, size_t size, unsigned int at_flags);
> > > > > >
> > > > > > I don't think we can have both the "struct fsxattr" from the uapi
> > > > > > headers, and a variable size as an additional argument. I would
> > > > > > still prefer not having the extensible structure at all and just
> > > > >
> > > > > We're not going to add new interfaces that are fixed size unless for the
> > > > > very basic cases. I don't care if we're doing that somewhere else in the
> > > > > kernel but we're not doing that for vfs apis.
> > > > >
> > > > > > use fsxattr, but if you want to make it extensible in this way,
> > > > > > it should use a different structure (name). Otherwise adding
> > > > > > fields after fsx_pad[] would break the ioctl interface.
> > > > >
> > > > > Would that really be a problem? Just along the syscall simply add
> > > > > something like:
> > > > >
> > > > > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > > > > index c91fd2b46a77..d3943805c4be 100644
> > > > > --- a/fs/ioctl.c
> > > > > +++ b/fs/ioctl.c
> > > > > @@ -868,12 +868,6 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
> > > > > case FS_IOC_SETFLAGS:
> > > > > return ioctl_setflags(filp, argp);
> > > > >
> > > > > - case FS_IOC_FSGETXATTR:
> > > > > - return ioctl_fsgetxattr(filp, argp);
> > > > > -
> > > > > - case FS_IOC_FSSETXATTR:
> > > > > - return ioctl_fssetxattr(filp, argp);
> > > > > -
> > > > > case FS_IOC_GETFSUUID:
> > > > > return ioctl_getfsuuid(filp, argp);
> > > > >
> > > > > @@ -886,6 +880,20 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
> > > > > break;
> > > > > }
> > > > >
> > > > > + switch (_IOC_NR(cmd)) {
> > > > > + case _IOC_NR(FS_IOC_FSGETXATTR):
> > > > > + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FS_IOC_FSGETXATTR)))
> > > > > + return SOMETHING_SOMETHING;
> > > > > + /* Only handle original size. */
> > > > > + return ioctl_fsgetxattr(filp, argp);
> > > > > +
> > > > > + case _IOC_NR(FFS_IOC_FSSETXATTR):
> > > > > + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FFS_IOC_FSSETXATTR)))
> > > > > + return SOMETHING_SOMETHING;
> > > > > + /* Only handle original size. */
> > > > > + return ioctl_fssetxattr(filp, argp);
> > > > > + }
> > > > > +
> > > >
> > > > I think what Arnd means is that we will not be able to change struct
> > > > sfxattr in uapi
> > > > going forward, because we are not going to deprecate the ioctls and
> > >
> > > There's no need to deprecate anything to rev an ioctl API. We have
> > > had to solve this "changing struct size" problem previously in XFS
> > > ioctls. See XFS_IOC_FSGEOMETRY and the older XFS_IOC_FSGEOMETRY_V4
> > > and XFS_IOC_FSGEOMETRY_V1 versions of the API/ABI.
> > >
> > > If we need to increase the structure size, we can rename the existing
> > > ioctl and struct to fix the version in the API, then use the
> > > original name for the new ioctl and structure definition.
> > >
> > > The only thing we have to make sure of is that the old and new
> > > structures have exactly the same overlapping structure. i.e.
> > > extension must always be done by appending new varibles, they can't
> > > be put in the middle of the structure.
> > >
> > > This way applications being rebuild will pick up the new definition
> > > automatically when the system asserts that it is suppored, whilst
> > > existing binaries will always still be supported by the kernel.
> > >
> > > If the application wants/needs to support all possible kernels, then
> > > if XFS_IOC_FSGEOMETRY is not supported, call XFS_IOC_FSGEOMETRY_V4,
> > > and if that fails (only on really old irix!) or you only need
> > > something in that original subset, call XFS_IOC_FSGEOMETRY_V1 which
> > > will always succeed....
> > >
> > > > Should we will need to depart from this struct definition and we might
> > > > as well do it for the initial release of the syscall rather than later on, e.g.:
> > > >
> > > > --- a/include/uapi/linux/fs.h
> > > > +++ b/include/uapi/linux/fs.h
> > > > @@ -148,6 +148,17 @@ struct fsxattr {
> > > > unsigned char fsx_pad[8];
> > > > };
> > > >
> > > > +/*
> > > > + * Variable size structure for file_[sg]et_attr().
> > > > + */
> > > > +struct fsx_fileattr {
> > > > + __u32 fsx_xflags; /* xflags field value (get/set) */
> > > > + __u32 fsx_extsize; /* extsize field value (get/set)*/
> > > > + __u32 fsx_nextents; /* nextents field value (get) */
> > > > + __u32 fsx_projid; /* project identifier (get/set) */
> > > > + __u32 fsx_cowextsize; /* CoW extsize field value (get/set)*/
> > > > +};
> > > > +
> > > > +#define FSXATTR_SIZE_VER0 20
> > > > +#define FSXATTR_SIZE_LATEST FSXATTR_SIZE_VER0
> > >
> > > If all the structures overlap the same, all that is needed in the
> > > code is to define the structure size that should be copied in and
> > > parsed. i.e:
> > >
> > > case FSXATTR..._V1:
> > > return ioctl_fsxattr...(args, sizeof(fsx_fileattr_v1));
> > > case FSXATTR..._V2:
> > > return ioctl_fsxattr...(args, sizeof(fsx_fileattr_v2));
> > > case FSXATTR...:
> > > return ioctl_fsxattr...(args, sizeof(fsx_fileattr));
> > >
> > > -Dave.
> > > --
> > > Dave Chinner
> > > david@fromorbit.com
> > >
> >
> > So, looks like there's at least two solutions to this concern.
> > Considering also that we have a bit of space in fsxattr,
> > 'fsx_pad[8]', I think it's fine to stick with the current fsxattr
> > for now.
>
> Not sure which two solutions you are referring to.
Suggested by Christian and Dave
>
> I proposed fsx_fileattr as what I think is the path of least resistance.
> There are opinions that we may be able to avoid defining
> this struct, but I don't think there was any objection to adding it.
>
> So unless I am missing an objection that I did not understand
> define it and get over this hurdle?
I see, sure, I misinterpreted the communication :) no problems, I
will create 'struct fsx_fileattr' then.
Pali, ah sorry, I forgot that you will extend fsxattr right away
--
- Andrey
^ permalink raw reply
* Re: [PATCH v5 0/7] fs: introduce file_getattr and file_setattr syscalls
From: Amir Goldstein @ 2025-05-21 9:36 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: Dave Chinner, Christian Brauner, Arnd Bergmann, Richard Henderson,
Matt Turner, Russell King, Catalin Marinas, Will Deacon,
Geert Uytterhoeven, Michal Simek, Thomas Bogendoerfer,
James E . J . Bottomley, Helge Deller, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy, Naveen N Rao,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Yoshinori Sato, Rich Felker,
John Paul Adrian Glaubitz, David S . Miller, Andreas Larsson,
Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Chris Zankel, Max Filippov,
Alexander Viro, Jan Kara, Mickaël Salaün,
Günther Noack, Pali Rohár, Paul Moore, James Morris,
Serge E. Hallyn, Stephen Smalley, Ondrej Mosnacek, Tyler Hicks,
Miklos Szeredi, linux-alpha, linux-kernel, linux-arm-kernel,
linux-m68k, linux-mips, linux-parisc, linuxppc-dev, linux-s390,
linux-sh, sparclinux, linux-fsdevel, linux-security-module,
linux-api, Linux-Arch, selinux, ecryptfs, linux-unionfs,
linux-xfs, Andrey Albershteyn
In-Reply-To: <sfmrojifgnrpeilqxtixyqrdjj5uvvpbvirxmlju5yce7z72vi@ondnx7qbie4y>
On Wed, May 21, 2025 at 10:48 AM Andrey Albershteyn <aalbersh@redhat.com> wrote:
>
> On 2025-05-19 21:37:04, Dave Chinner wrote:
> > On Thu, May 15, 2025 at 12:33:31PM +0200, Amir Goldstein wrote:
> > > On Thu, May 15, 2025 at 11:02 AM Christian Brauner <brauner@kernel.org> wrote:
> > > >
> > > > On Tue, May 13, 2025 at 11:53:23AM +0200, Arnd Bergmann wrote:
> > > > > On Tue, May 13, 2025, at 11:17, Andrey Albershteyn wrote:
> > > > >
> > > > > >
> > > > > > long syscall(SYS_file_getattr, int dirfd, const char *pathname,
> > > > > > struct fsxattr *fsx, size_t size, unsigned int at_flags);
> > > > > > long syscall(SYS_file_setattr, int dirfd, const char *pathname,
> > > > > > struct fsxattr *fsx, size_t size, unsigned int at_flags);
> > > > >
> > > > > I don't think we can have both the "struct fsxattr" from the uapi
> > > > > headers, and a variable size as an additional argument. I would
> > > > > still prefer not having the extensible structure at all and just
> > > >
> > > > We're not going to add new interfaces that are fixed size unless for the
> > > > very basic cases. I don't care if we're doing that somewhere else in the
> > > > kernel but we're not doing that for vfs apis.
> > > >
> > > > > use fsxattr, but if you want to make it extensible in this way,
> > > > > it should use a different structure (name). Otherwise adding
> > > > > fields after fsx_pad[] would break the ioctl interface.
> > > >
> > > > Would that really be a problem? Just along the syscall simply add
> > > > something like:
> > > >
> > > > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > > > index c91fd2b46a77..d3943805c4be 100644
> > > > --- a/fs/ioctl.c
> > > > +++ b/fs/ioctl.c
> > > > @@ -868,12 +868,6 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
> > > > case FS_IOC_SETFLAGS:
> > > > return ioctl_setflags(filp, argp);
> > > >
> > > > - case FS_IOC_FSGETXATTR:
> > > > - return ioctl_fsgetxattr(filp, argp);
> > > > -
> > > > - case FS_IOC_FSSETXATTR:
> > > > - return ioctl_fssetxattr(filp, argp);
> > > > -
> > > > case FS_IOC_GETFSUUID:
> > > > return ioctl_getfsuuid(filp, argp);
> > > >
> > > > @@ -886,6 +880,20 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
> > > > break;
> > > > }
> > > >
> > > > + switch (_IOC_NR(cmd)) {
> > > > + case _IOC_NR(FS_IOC_FSGETXATTR):
> > > > + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FS_IOC_FSGETXATTR)))
> > > > + return SOMETHING_SOMETHING;
> > > > + /* Only handle original size. */
> > > > + return ioctl_fsgetxattr(filp, argp);
> > > > +
> > > > + case _IOC_NR(FFS_IOC_FSSETXATTR):
> > > > + if (WARN_ON_ONCE(_IOC_TYPE(cmd) != _IOC_TYPE(FFS_IOC_FSSETXATTR)))
> > > > + return SOMETHING_SOMETHING;
> > > > + /* Only handle original size. */
> > > > + return ioctl_fssetxattr(filp, argp);
> > > > + }
> > > > +
> > >
> > > I think what Arnd means is that we will not be able to change struct
> > > sfxattr in uapi
> > > going forward, because we are not going to deprecate the ioctls and
> >
> > There's no need to deprecate anything to rev an ioctl API. We have
> > had to solve this "changing struct size" problem previously in XFS
> > ioctls. See XFS_IOC_FSGEOMETRY and the older XFS_IOC_FSGEOMETRY_V4
> > and XFS_IOC_FSGEOMETRY_V1 versions of the API/ABI.
> >
> > If we need to increase the structure size, we can rename the existing
> > ioctl and struct to fix the version in the API, then use the
> > original name for the new ioctl and structure definition.
> >
> > The only thing we have to make sure of is that the old and new
> > structures have exactly the same overlapping structure. i.e.
> > extension must always be done by appending new varibles, they can't
> > be put in the middle of the structure.
> >
> > This way applications being rebuild will pick up the new definition
> > automatically when the system asserts that it is suppored, whilst
> > existing binaries will always still be supported by the kernel.
> >
> > If the application wants/needs to support all possible kernels, then
> > if XFS_IOC_FSGEOMETRY is not supported, call XFS_IOC_FSGEOMETRY_V4,
> > and if that fails (only on really old irix!) or you only need
> > something in that original subset, call XFS_IOC_FSGEOMETRY_V1 which
> > will always succeed....
> >
> > > Should we will need to depart from this struct definition and we might
> > > as well do it for the initial release of the syscall rather than later on, e.g.:
> > >
> > > --- a/include/uapi/linux/fs.h
> > > +++ b/include/uapi/linux/fs.h
> > > @@ -148,6 +148,17 @@ struct fsxattr {
> > > unsigned char fsx_pad[8];
> > > };
> > >
> > > +/*
> > > + * Variable size structure for file_[sg]et_attr().
> > > + */
> > > +struct fsx_fileattr {
> > > + __u32 fsx_xflags; /* xflags field value (get/set) */
> > > + __u32 fsx_extsize; /* extsize field value (get/set)*/
> > > + __u32 fsx_nextents; /* nextents field value (get) */
> > > + __u32 fsx_projid; /* project identifier (get/set) */
> > > + __u32 fsx_cowextsize; /* CoW extsize field value (get/set)*/
> > > +};
> > > +
> > > +#define FSXATTR_SIZE_VER0 20
> > > +#define FSXATTR_SIZE_LATEST FSXATTR_SIZE_VER0
> >
> > If all the structures overlap the same, all that is needed in the
> > code is to define the structure size that should be copied in and
> > parsed. i.e:
> >
> > case FSXATTR..._V1:
> > return ioctl_fsxattr...(args, sizeof(fsx_fileattr_v1));
> > case FSXATTR..._V2:
> > return ioctl_fsxattr...(args, sizeof(fsx_fileattr_v2));
> > case FSXATTR...:
> > return ioctl_fsxattr...(args, sizeof(fsx_fileattr));
> >
> > -Dave.
> > --
> > Dave Chinner
> > david@fromorbit.com
> >
>
> So, looks like there's at least two solutions to this concern.
> Considering also that we have a bit of space in fsxattr,
> 'fsx_pad[8]', I think it's fine to stick with the current fsxattr
> for now.
Not sure which two solutions you are referring to.
I proposed fsx_fileattr as what I think is the path of least resistance.
There are opinions that we may be able to avoid defining
this struct, but I don't think there was any objection to adding it.
So unless I am missing an objection that I did not understand
define it and get over this hurdle?
Thanks,
Amir.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox