* 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
* [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: [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
* Re: [DISCUSSION] proposed mctl() API
From: Usama Arif @ 2025-05-29 17:21 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton, Shakeel Butt, Liam R . Howlett,
David Hildenbrand, Vlastimil Babka, Jann Horn, Arnd Bergmann,
Christian Brauner, SeongJae Park, 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 29/05/2025 15:43, Lorenzo Stoakes wrote:
> ## 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.
>
Hi Lorenzo,
Thanks for writing the proposal, this is awesome!
Whatever the community agrees with, whether its this or prctl, happy to
move forward with either as both should accomplish the usecase proposed.
I will just add some points over here in defence of prctl, this is just for
discussion, and if the community disagrees, completely happy to move forward
with new syscall as well.
When it comes to having mm code in kernel/sys.c, we can just do something
like below that can actually clean it up?
diff --git a/kernel/sys.c b/kernel/sys.c
index 3a2df1bd9f64..bfadc339e2c5 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2467,6 +2467,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
error = 0;
switch (option) {
+ case PR_SET_MM:
+ case PR_GET_THP_DISABLE:
+ case PR_SET_THP_DISABLE:
+ case PR_NEW_MM_THING:
+ error = some_function_in_mm_folder(); // in mm/mctl.c ?
+ break;
case PR_SET_PDEATHSIG:
if (!valid_signal(arg2)) {
error = -EINVAL;
when it comes to prctl becoming a catch-all thing, with above clean up,
we can be a lot more careful to what gets added to the mm side of prctl.
The advantage of this is it avoids having another syscall.
My personal view (which can be wrong :)) is that a new syscall should be
for something major,
and I feel that PR_DEFAULT_MADV_HUGEPAGE and PR_DEFAULT_MADV_NOHUGEPAGE
might be small enough to fit in prctl? but I completely understand
your point of view as well!
> ## 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.
>
Sounds good to me. Just adding this here, the solution will be used in systemd
in exec_invoke, similar to how KSM is done with prctl in [1], so for the THP
solution, we would need MCTL_SET_DEFAULT_EXEC as it would need to be inherited
across fork+exec.
[1] https://github.com/systemd/systemd/blob/2e72d3efafa88c1cb4d9b28dd4ade7c6ab7be29a/src/core/exec-invoke.c#L5046
> 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?
>
My knowledge is security is limited, so please bare with me, but I actually
didn't understand the security issue and the need for CAP_SYS_ADMIN for
doing VM_(NO)HUGEPAGE.
A process can already madvise its own VMAs, and this is just doing that
for the entire process. And VM_INIT_DEF_MASK is already set to VM_NOHUGEPAGE
so it will be inherited by the parent. Just adding VM_HUGEPAGE shouldnt be
a issue? Inheriting MMF_VM_HUGEPAGE will mean that khugepaged would enter
for that process as well, which again doesnt seem like a security issue
to me.
> ## 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
Again thanks for the proposal! Happy to move forward with this or prctl.
Just adding my 2 cents in this email.
Thanks
Usama
^ permalink raw reply related
* Re: [DISCUSSION] proposed mctl() API
From: Shakeel Butt @ 2025-05-29 17:54 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Lorenzo Stoakes, Andrew Morton, 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: <aDh9LtSLCiTLjg2X@casper.infradead.org>
Hi Willy,
On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
> 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,
Can you please expand on people putting more effort? Is it about
workloads or maybe malloc implementations (tcmalloc, jemalloc) being
more intelligent in managing their allocations/frees to keep more used
memory in hugepage aligned regions? And conveying to kernel which
regions they prefer hugepage backed and which they do not? Or something
else?
> instead of coming up with these baroque reasons to blame
> the sysadmin for not having tweaked some magic knob.
To me this is not about blaming sysadmin but more about sysadmin wanting
more fine grained control on THP allocation policies for different
workloads running in a multi-tenant environment.
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Matthew Wilcox @ 2025-05-29 18:13 UTC (permalink / raw)
To: Shakeel Butt
Cc: Lorenzo Stoakes, Andrew Morton, 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: <c7fqqny5yv7smhxnxe5o4rc2wepmc6uei76teymfhxoana34nk@sfqnc2qclf23>
On Thu, May 29, 2025 at 10:54:34AM -0700, Shakeel Butt wrote:
> On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
> > People should put more effort into allocating THPs automatically and
> > monitoring where they're helping performance and where they're hurting
> > performance,
>
> Can you please expand on people putting more effort? Is it about
> workloads or maybe malloc implementations (tcmalloc, jemalloc) being
> more intelligent in managing their allocations/frees to keep more used
> memory in hugepage aligned regions? And conveying to kernel which
> regions they prefer hugepage backed and which they do not? Or something
> else?
We need infrastructure inside the kernel to monitor whether a task is
making effective use of the THPs that it has, and if it's not then move
those THPs over to where they will be better used.
I don't necessarily object to userspace giving hints like "I think I'm
going to use all of this 20MB region quite heavily", but the kernel should
treat those hints with the appropriate skepticism, otherwise it's just
a turbo button that nobody would ever _not_ press.
> > instead of coming up with these baroque reasons to blame
> > the sysadmin for not having tweaked some magic knob.
>
> To me this is not about blaming sysadmin but more about sysadmin wanting
> more fine grained control on THP allocation policies for different
> workloads running in a multi-tenant environment.
That's the same thing. Linux should be auto-tuning, not relying on some
omniscient sysadmin to fix it up.
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Usama Arif @ 2025-05-29 18:32 UTC (permalink / raw)
To: Matthew Wilcox, Shakeel Butt
Cc: Lorenzo Stoakes, Andrew Morton, Liam R . Howlett,
David Hildenbrand, Vlastimil Babka, Jann Horn, Arnd Bergmann,
Christian Brauner, SeongJae Park, Mike Rapoport, Johannes Weiner,
Barry Song, linux-mm, linux-arch, linux-kernel, linux-api,
Pedro Falcato
In-Reply-To: <aDij5brAsGJVHCFK@casper.infradead.org>
On 29/05/2025 19:13, Matthew Wilcox wrote:
> On Thu, May 29, 2025 at 10:54:34AM -0700, Shakeel Butt wrote:
>> On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
>>> People should put more effort into allocating THPs automatically and
>>> monitoring where they're helping performance and where they're hurting
>>> performance,
>>
>> Can you please expand on people putting more effort? Is it about
>> workloads or maybe malloc implementations (tcmalloc, jemalloc) being
>> more intelligent in managing their allocations/frees to keep more used
>> memory in hugepage aligned regions? And conveying to kernel which
>> regions they prefer hugepage backed and which they do not? Or something
>> else?
>
> We need infrastructure inside the kernel to monitor whether a task is
> making effective use of the THPs that it has, and if it's not then move
> those THPs over to where they will be better used.
>
I think this is the really difficult part.
If we have 2 workloads on the same server, For e.g. one is database where THPs
just dont do well, but the other one is AI where THPs do really well. How
will the kernel monitor that the database workload is performing worse
and the AI one isnt?
I added THP shrinker to hopefully try and do this automatically, and it does
really help. But unfortunately it is not a complete solution.
There are severely memory bound workloads where even a tiny increase
in memory will lead to an OOM. And if you colocate the container thats running
that workload with one in which we will benefit with THPs, we unfortunately
can't just rely on the system doing the right thing.
It would be awesome if THPs are truly transparent and don't require
any input, but unfortunately I don't think that there is a solution
for this with just kernel monitoring.
This is just a big hint from the user. If the global system policy is madvise
and the workload owner has done their own benchmarks and see benefits
with always, they set DEFAULT_MADV_HUGEPAGE for the process to optin as "always".
If the global system policy is always and the workload owner has done their own
benchmarks and see worse results with always, they set DEFAULT_MADV_NOHUGEPAGE for
the process to optin as "madvise".
> I don't necessarily object to userspace giving hints like "I think I'm
> going to use all of this 20MB region quite heavily", but the kernel should
> treat those hints with the appropriate skepticism, otherwise it's just
> a turbo button that nobody would ever _not_ press.
>
>>> instead of coming up with these baroque reasons to blame
>>> the sysadmin for not having tweaked some magic knob.
>>
>> To me this is not about blaming sysadmin but more about sysadmin wanting
>> more fine grained control on THP allocation policies for different
>> workloads running in a multi-tenant environment.
>
> That's the same thing. Linux should be auto-tuning, not relying on some
> omniscient sysadmin to fix it up.
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Andy Lutomirski @ 2025-05-29 18:50 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 7:44 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> 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.
It's hard to comment without a more complete proposal (*what* behavior
is persisted?), but off the top of my head, this isn't so great.
First, the name means nothing to me. What's "default exec"? Even
aside from that, why are fork and exec the same flag?
Beyond this, persisting anything across exec is a giant can of worms.
We have PR_SET_NO_NEW_PRIVS to make it less hazardous, but, in
general, it's risky and potentially quite confusing to do anything
that affects exec'd processes.
Oh, and this whole scheme is also potentially nasty for a different
reason: it's not thread safe. If one thread wants to spawn a process,
it should not interfere with another thread doing something else. So
making an mm flag that persists across close can interfere a bit, and
persisting it across clone + exec is even gnarlier.
For any of these, there should be matching query features -- CRIU,
debugging, etc should not be an afterthought.
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Johannes Weiner @ 2025-05-29 21:14 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Lorenzo Stoakes, Andrew Morton, Shakeel Butt, Liam R . Howlett,
David Hildenbrand, Vlastimil Babka, Jann Horn, Arnd Bergmann,
Christian Brauner, SeongJae Park, Usama Arif, Mike Rapoport,
Barry Song, linux-mm, linux-arch, linux-kernel, linux-api,
Pedro Falcato
In-Reply-To: <aDh9LtSLCiTLjg2X@casper.infradead.org>
On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
> 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.
I actually explained why I think this is a terrible idea. But okay, I
tried the patch anyway.
This is 'git log' on a hot kernel repo after a large IO stream:
VANILLA BARRY
Real time 49.93 ( +0.00%) 60.36 ( +20.48%)
User time 32.10 ( +0.00%) 32.09 ( -0.04%)
System time 14.41 ( +0.00%) 14.64 ( +1.50%)
pgmajfault 9227.00 ( +0.00%) 18390.00 ( +99.30%)
workingset_refault_file 184.00 ( +0.00%) 236899.00 (+127954.05%)
Clearly we can't generally ignore page cache hits just because the
mmaps() are intermittent.
The whole point is to cache across processes and their various
apertures into a common, long-lived filesystem space.
Barry knows something about the relationship between certain processes
and certain files that he could exploit with MADV_COLD-on-exit
semantics. But that's not something the kernel can safely assume. Not
without defeating the page cache for an entire class of file accesses.
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Liam R. Howlett @ 2025-05-29 21:24 UTC (permalink / raw)
To: Johannes Weiner
Cc: Matthew Wilcox, Lorenzo Stoakes, Andrew Morton, Shakeel Butt,
David Hildenbrand, Vlastimil Babka, Jann Horn, Arnd Bergmann,
Christian Brauner, SeongJae Park, Usama Arif, Mike Rapoport,
Barry Song, linux-mm, linux-arch, linux-kernel, linux-api,
Pedro Falcato
In-Reply-To: <20250529211423.GA1271329@cmpxchg.org>
* Johannes Weiner <hannes@cmpxchg.org> [250529 17:14]:
> On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
> > 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.
>
> I actually explained why I think this is a terrible idea. But okay, I
> tried the patch anyway.
>
> This is 'git log' on a hot kernel repo after a large IO stream:
Can you clarify this benchmark please?
Is this running 'git log', then stream IO, then running 'git log' again?
>
> VANILLA BARRY
> Real time 49.93 ( +0.00%) 60.36 ( +20.48%)
> User time 32.10 ( +0.00%) 32.09 ( -0.04%)
> System time 14.41 ( +0.00%) 14.64 ( +1.50%)
> pgmajfault 9227.00 ( +0.00%) 18390.00 ( +99.30%)
> workingset_refault_file 184.00 ( +0.00%) 236899.00 (+127954.05%)
>
> Clearly we can't generally ignore page cache hits just because the
> mmaps() are intermittent.
>
> The whole point is to cache across processes and their various
> apertures into a common, long-lived filesystem space.
>
> Barry knows something about the relationship between certain processes
> and certain files that he could exploit with MADV_COLD-on-exit
> semantics. But that's not something the kernel can safely assume. Not
> without defeating the page cache for an entire class of file accesses.
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Andrew Morton @ 2025-05-29 21:31 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: 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, 29 May 2025 15:43:26 +0100 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
>
> madvise()
>
> ...
>
> process_madvise()
>
> ...
>
> prctl()
>
> ...
>
Yeah. I think there has always been an attitude "ooh, new syscalls are
scary and I probably need permission from someone so let's graft it
onto something which is already there and hope nobody notices".
New syscalls are super easy and are cheap - it's just a table entry!
If it makes sense as a standalone thing, do that.
> The proposed interface is simply:
>
> int mctl(int pidfd, int action, unsigned int flags);
Well, why `flags'? One could even add a syscall per operation.
Debatable.
> 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.
Permissions needs careful consideration on a case-by-case basis.
Clearly in many cases, user A should be able to manipulate user A's
mm's.
And if different modes of mctl() end up with different permission
structures, one wonders why everything was clumped under the same
syscall! mctl() becomes "prctl() for memory".
Anyway, fun discussion. I'm with willy ;)
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Johannes Weiner @ 2025-05-29 23:14 UTC (permalink / raw)
To: Liam R. Howlett
Cc: Matthew Wilcox, Lorenzo Stoakes, Andrew Morton, Shakeel Butt,
David Hildenbrand, Vlastimil Babka, Jann Horn, Arnd Bergmann,
Christian Brauner, SeongJae Park, Usama Arif, Mike Rapoport,
Barry Song, linux-mm, linux-arch, linux-kernel, linux-api,
Pedro Falcato
In-Reply-To: <klrw3rjymes6phs5erz7eqkjji5oe3bg4j4jbqpjv3qzuw4vra@k6ei5pssfany>
On Thu, May 29, 2025 at 05:24:23PM -0400, Liam R. Howlett wrote:
> * Johannes Weiner <hannes@cmpxchg.org> [250529 17:14]:
> > On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
> > > 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.
> >
> > I actually explained why I think this is a terrible idea. But okay, I
> > tried the patch anyway.
> >
> > This is 'git log' on a hot kernel repo after a large IO stream:
>
> Can you clarify this benchmark please?
>
> Is this running 'git log', then stream IO, then running 'git log' again?
Yes, but it's running git log twice first. On the vanilla kernel this
is the number of references when we usually activate.
You can substitute any sequence of commands that would interact with
the git objects repeatedly before a pause where programmer thinks.
You can probably get similar mmapIO patterns with sqlite, lmdb, etc.
Periodically running executables and scripts are another case. They
tend to be less latency-sensitive I suppose, but would still
unnecessarily eat into the available IO bandwidth.
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Barry Song @ 2025-05-30 7:52 UTC (permalink / raw)
To: Johannes Weiner
Cc: Matthew Wilcox, Lorenzo Stoakes, Andrew Morton, Shakeel Butt,
Liam R . Howlett, David Hildenbrand, Vlastimil Babka, Jann Horn,
Arnd Bergmann, Christian Brauner, SeongJae Park, Usama Arif,
Mike Rapoport, linux-mm, linux-arch, linux-kernel, linux-api,
Pedro Falcato
In-Reply-To: <20250529211423.GA1271329@cmpxchg.org>
On Fri, May 30, 2025 at 9:14 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
> > 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.
>
> I actually explained why I think this is a terrible idea. But okay, I
> tried the patch anyway.
>
> This is 'git log' on a hot kernel repo after a large IO stream:
>
> VANILLA BARRY
> Real time 49.93 ( +0.00%) 60.36 ( +20.48%)
> User time 32.10 ( +0.00%) 32.09 ( -0.04%)
> System time 14.41 ( +0.00%) 14.64 ( +1.50%)
> pgmajfault 9227.00 ( +0.00%) 18390.00 ( +99.30%)
> workingset_refault_file 184.00 ( +0.00%) 236899.00 (+127954.05%)
>
> Clearly we can't generally ignore page cache hits just because the
> mmaps() are intermittent.
Hi Johannes,
Thanks!
Are you on v1, which lacks folio demotion[1], or v2, which includes it [2]?
[1] https://lore.kernel.org/linux-mm/20250412085852.48524-1-21cnbao@gmail.com/
[2] https://lore.kernel.org/linux-mm/20250514070820.51793-1-21cnbao@gmail.com/
>
> The whole point is to cache across processes and their various
> apertures into a common, long-lived filesystem space.
>
> Barry knows something about the relationship between certain processes
> and certain files that he could exploit with MADV_COLD-on-exit
> semantics. But that's not something the kernel can safely assume. Not
> without defeating the page cache for an entire class of file accesses.
Best Regards
Barry
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Vlastimil Babka @ 2025-05-30 10:31 UTC (permalink / raw)
To: Johannes Weiner, Matthew Wilcox
Cc: Lorenzo Stoakes, Andrew Morton, Shakeel Butt, Liam R . Howlett,
David Hildenbrand, Jann Horn, Arnd Bergmann, Christian Brauner,
SeongJae Park, Usama Arif, Mike Rapoport, Barry Song, linux-mm,
linux-arch, linux-kernel, linux-api, Pedro Falcato
In-Reply-To: <20250529211423.GA1271329@cmpxchg.org>
On 5/29/25 23:14, Johannes Weiner wrote:
> On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
>> 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.
>
> I actually explained why I think this is a terrible idea. But okay, I
> tried the patch anyway.
>
> This is 'git log' on a hot kernel repo after a large IO stream:
>
> VANILLA BARRY
> Real time 49.93 ( +0.00%) 60.36 ( +20.48%)
> User time 32.10 ( +0.00%) 32.09 ( -0.04%)
> System time 14.41 ( +0.00%) 14.64 ( +1.50%)
> pgmajfault 9227.00 ( +0.00%) 18390.00 ( +99.30%)
> workingset_refault_file 184.00 ( +0.00%) 236899.00 (+127954.05%)
>
> Clearly we can't generally ignore page cache hits just because the
> mmaps() are intermittent.
>
> The whole point is to cache across processes and their various
> apertures into a common, long-lived filesystem space.
>
> Barry knows something about the relationship between certain processes
> and certain files that he could exploit with MADV_COLD-on-exit
> semantics. But that's not something the kernel can safely assume. Not
> without defeating the page cache for an entire class of file accesses.
I've just read the previous threads about Barry's proposal and if doing this
always isn't feasible, I'm wondering if memcg would be a better interface to
opt-in for this kind of behavior than both prctl or mctl. I think at least
conceptually it fits what memcg is doing? The question is if the
implementation would be feasible, and if android puts apps in separate memcgs...
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Lorenzo Stoakes @ 2025-05-30 13:10 UTC (permalink / raw)
To: Usama Arif
Cc: Andrew Morton, Shakeel Butt, Liam R . Howlett, David Hildenbrand,
Vlastimil Babka, Jann Horn, Arnd Bergmann, Christian Brauner,
SeongJae Park, Mike Rapoport, Johannes Weiner, Barry Song,
linux-mm, linux-arch, linux-kernel, linux-api, Pedro Falcato
In-Reply-To: <e166592f-aeb3-4573-bb73-270a2eb90be3@gmail.com>
On Thu, May 29, 2025 at 06:21:55PM +0100, Usama Arif wrote:
>
>
> On 29/05/2025 15:43, Lorenzo Stoakes wrote:
> > ## 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.
> >
>
>
> Hi Lorenzo,
>
> Thanks for writing the proposal, this is awesome!
Thanks :)
>
> Whatever the community agrees with, whether its this or prctl, happy to
> move forward with either as both should accomplish the usecase proposed.
Awesome!
>
> I will just add some points over here in defence of prctl, this is just for
> discussion, and if the community disagrees, completely happy to move forward
> with new syscall as well.
Please do - it's vital we have all sides of the argument!
>
> When it comes to having mm code in kernel/sys.c, we can just do something
> like below that can actually clean it up?
>
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 3a2df1bd9f64..bfadc339e2c5 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -2467,6 +2467,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
>
> error = 0;
> switch (option) {
> + case PR_SET_MM:
> + case PR_GET_THP_DISABLE:
> + case PR_SET_THP_DISABLE:
> + case PR_NEW_MM_THING:
> + error = some_function_in_mm_folder(); // in mm/mctl.c ?
> + break;
> case PR_SET_PDEATHSIG:
> if (!valid_signal(arg2)) {
> error = -EINVAL;
>
> when it comes to prctl becoming a catch-all thing, with above clean up,
> we can be a lot more careful to what gets added to the mm side of prctl.
Sure, and I think we were to decide prctl() was the way to go this would be the
way it should look, or at least specifically for this case.
This addresses the bit rot issue but not how hidden the functionality is, what
prctl() generally represents de-facto (that is - a catch-all for obscure stuff),
depending on the interface of the function it might be harder/impossible to
userland-test and of course we have the beautiful vector-interface that means we
can't restrict behaviour in the API in the way we'd ideally like.
I mean overall there is no beautiful solution here.
As Matthew says, the reason we're struggling here is probably more so because
this is just showing up a flaw in how THP is these days - we really ideally need
something automagic where the kernel figures things out for us.
But in the meantime we have this frankly kinda terrible interface that's rather
off/on and yet want to establish policy _ourselves_ when the kernel should be
able to figure it out.
But with that said, we must trade that off with the reality that there is a need
for this kind of fine-grained control now, in this less than ideal world we
currently inhabit :)
>
> The advantage of this is it avoids having another syscall.
> My personal view (which can be wrong :)) is that a new syscall should be
> for something major,
> and I feel that PR_DEFAULT_MADV_HUGEPAGE and PR_DEFAULT_MADV_NOHUGEPAGE
> might be small enough to fit in prctl? but I completely understand
> your point of view as well!
See Andrew's point on the syscall thing, they're cheap ;)
I mean the idea of course is that we can put all future stuff like that here
too.
We could even port some existing prctl() stuff across, in theory. But having 2
ways of doing the same thing would probably suck.
>
> > ## 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.
> >
>
> Sounds good to me. Just adding this here, the solution will be used in systemd
> in exec_invoke, similar to how KSM is done with prctl in [1], so for the THP
> solution, we would need MCTL_SET_DEFAULT_EXEC as it would need to be inherited
> across fork+exec.
Right yeah, this was the intent.
>
> [1] https://github.com/systemd/systemd/blob/2e72d3efafa88c1cb4d9b28dd4ade7c6ab7be29a/src/core/exec-invoke.c#L5046
>
> > 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?
> >
>
> My knowledge is security is limited, so please bare with me, but I actually
> didn't understand the security issue and the need for CAP_SYS_ADMIN for
> doing VM_(NO)HUGEPAGE.
>
> A process can already madvise its own VMAs, and this is just doing that
> for the entire process. And VM_INIT_DEF_MASK is already set to VM_NOHUGEPAGE
> so it will be inherited by the parent. Just adding VM_HUGEPAGE shouldnt be
> a issue? Inheriting MMF_VM_HUGEPAGE will mean that khugepaged would enter
> for that process as well, which again doesnt seem like a security issue
> to me.
W.R.T. the current process, the Issue is one Jann raised, in relation to
propagation of behaviour to privileged (e.g. setuid) processes.
W.R.T. remote processes, obviously we want to make sure we are permitted to do
so.
>
> > ## 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
>
> Again thanks for the proposal! Happy to move forward with this or prctl.
> Just adding my 2 cents in this email.
Thank you! And your input is most appreciated - it's important we get all sides
of the story here!
>
> Thanks
> Usama
>
Cheers, Lorenzo
^ permalink raw reply
* Re: Extending clone_args for clone3()
From: Yury Khrustalev @ 2025-06-02 14:10 UTC (permalink / raw)
To: linux-kernel
Cc: Christian Brauner, Arnd Bergmann, Mark Brown, Mark Rutland,
linux-api
In-Reply-To: <aCs65ccRQtJBnZ_5@arm.com>
Hi everyone,
A gentle ping :)
On Mon, May 19, 2025 at 03:06:29PM +0100, Yury Khrustalev wrote:
> Hi,
>
> I'm working on an RFC patch for Glibc to make use of the newly added
> shadow_stack_token field in struct clone_args in [1] on arm64 targets.
>
> I encountered the following problem. Glibc might be built with newer
> version of struct clone_args than the currently running kernel. In
> this case, we may attempt to use a non-zero value in the new field
> in args (and pass size bigger than expected by the kernel) and the
> kernel will reject the syscall with E2BIG error.
>
> This seems to be due to a fail-early approach. The unexpected non-
> zero values beyond what's supported by the kernel may indicate that
> userspace expects something to happen (and may even have allocated
> some resources). So it's better to indicate a problem rather than
> silently ignore this and have userspace encounter an error later.
>
> However, it creates difficulty with using extended "versions" of
> the clone3 syscall. AFAIK, there is no way to ask kernel about
> the supported size of struct clone_args except for making syscalls
> with decreasing value of size until we stop getting E2BIG.
>
> This seems fragile and may call for writing cumbersome code. In essence,
> we will have to have clone30(), clone31(), clone32()... wrappers which
> probably defeats the point of why clone3 was added:
>
>
> if (clone32_supported && clone32(...) == -1 && errno == E2BIG)
> {
> clone32_supported = false;
> /* ... */
> }
> else if (clone31_supported && clone31(...) == -1 && errno == E2BIG)
> {
> clone12_supported = false;
> /* ... */
> }
> ...
>
> Is there a neat way to work around this? What was the idea for extending
> clone_args in practice?
>
> I suppose we can't rely on kernel version because support for extended
> clone_args can be backported. In any case, we'd have to do a syscall
> for this (it would probably be great to have kernel version in auxv).
>
> I appreciate any advice here.
>
> Thanks,
> Yury
>
>
> [1]: https://lore.kernel.org/all/20250416-clone3-shadow-stack-v16-0-2ffc9ca3917b@kernel.org/
>
Kind regards,
Yury
^ permalink raw reply
* [PATCH 5.4 193/204] pidfd: check pid has attached task in fdinfo
From: Greg Kroah-Hartman @ 2025-06-02 13:48 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, Jann Horn, Christian Kellner,
linux-api, Christian Brauner, Oleg Nesterov, Christian Brauner
In-Reply-To: <20250602134255.449974357@linuxfoundation.org>
5.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Christian Brauner <christian.brauner@ubuntu.com>
commit 3d6d8da48d0b214d65ea0227d47228abc75d7c88 upstream.
Currently, when a task is dead we still print the pid it used to use in
the fdinfo files of its pidfds. This doesn't make much sense since the
pid may have already been reused. So verify that the task is still alive
by introducing the pid_has_task() helper which will be used by other
callers in follow-up patches.
If the task is not alive anymore, we will print -1. This allows us to
differentiate between a task not being present in a given pid namespace
- in which case we already print 0 - and a task having been reaped.
Note that this uses PIDTYPE_PID for the check. Technically, we could've
checked PIDTYPE_TGID since pidfds currently only refer to thread-group
leaders but if they won't anymore in the future then this check becomes
problematic without it being immediately obvious to non-experts imho. If
a thread is created via clone(CLONE_THREAD) than struct pid has a single
non-empty list pid->tasks[PIDTYPE_PID] and this pid can't be used as a
PIDTYPE_TGID meaning pid->tasks[PIDTYPE_TGID] will return NULL even
though the thread-group leader might still be very much alive. So
checking PIDTYPE_PID is fine and is easier to maintain should we ever
allow pidfds to refer to threads.
Cc: Jann Horn <jannh@google.com>
Cc: Christian Kellner <christian@kellner.me>
Cc: linux-api@vger.kernel.org
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Link: https://lore.kernel.org/r/20191017101832.5985-1-christian.brauner@ubuntu.com
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
include/linux/pid.h | 4 ++++
kernel/fork.c | 10 ++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -85,6 +85,10 @@ static inline struct pid *get_pid(struct
extern void put_pid(struct pid *pid);
extern struct task_struct *pid_task(struct pid *pid, enum pid_type);
+static inline bool pid_has_task(struct pid *pid, enum pid_type type)
+{
+ return !hlist_empty(&pid->tasks[type]);
+}
extern struct task_struct *get_pid_task(struct pid *pid, enum pid_type);
extern struct pid *get_task_pid(struct task_struct *task, enum pid_type type);
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1703,10 +1703,16 @@ static int pidfd_release(struct inode *i
#ifdef CONFIG_PROC_FS
static void pidfd_show_fdinfo(struct seq_file *m, struct file *f)
{
- struct pid_namespace *ns = proc_pid_ns(file_inode(m->file));
struct pid *pid = f->private_data;
+ struct pid_namespace *ns;
+ pid_t nr = -1;
- seq_put_decimal_ull(m, "Pid:\t", pid_nr_ns(pid, ns));
+ if (likely(pid_has_task(pid, PIDTYPE_PID))) {
+ ns = proc_pid_ns(file_inode(m->file));
+ nr = pid_nr_ns(pid, ns);
+ }
+
+ seq_put_decimal_ll(m, "Pid:\t", nr);
seq_putc(m, '\n');
}
#endif
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Matthew Wilcox @ 2025-06-02 18:01 UTC (permalink / raw)
To: Johannes Weiner
Cc: Lorenzo Stoakes, Andrew Morton, Shakeel Butt, Liam R . Howlett,
David Hildenbrand, Vlastimil Babka, Jann Horn, Arnd Bergmann,
Christian Brauner, SeongJae Park, Usama Arif, Mike Rapoport,
Barry Song, linux-mm, linux-arch, linux-kernel, linux-api,
Pedro Falcato
In-Reply-To: <20250529211423.GA1271329@cmpxchg.org>
On Thu, May 29, 2025 at 05:14:23PM -0400, Johannes Weiner wrote:
> On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
> > 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.
>
> I actually explained why I think this is a terrible idea. But okay, I
> tried the patch anyway.
Sorry, I must've missed that one ;-(
> This is 'git log' on a hot kernel repo after a large IO stream:
>
> VANILLA BARRY
> Real time 49.93 ( +0.00%) 60.36 ( +20.48%)
> User time 32.10 ( +0.00%) 32.09 ( -0.04%)
> System time 14.41 ( +0.00%) 14.64 ( +1.50%)
> pgmajfault 9227.00 ( +0.00%) 18390.00 ( +99.30%)
> workingset_refault_file 184.00 ( +0.00%) 236899.00 (+127954.05%)
>
> Clearly we can't generally ignore page cache hits just because the
> mmaps() are intermittent.
>
> The whole point is to cache across processes and their various
> apertures into a common, long-lived filesystem space.
>
> Barry knows something about the relationship between certain processes
> and certain files that he could exploit with MADV_COLD-on-exit
> semantics. But that's not something the kernel can safely assume. Not
> without defeating the page cache for an entire class of file accesses.
So what about distinguishing between exited-normally processes (ie git
log) vs killed-by-oom processes (ie Barry's usecase)? Update the
referenced bit in the first case and not the second?
^ permalink raw reply
* Re: Extending clone_args for clone3()
From: Arnd Bergmann @ 2025-06-04 8:29 UTC (permalink / raw)
To: Yury Khrustalev, linux-kernel
Cc: Christian Brauner, Mark Brown, Mark Rutland, linux-api
In-Reply-To: <aCs65ccRQtJBnZ_5@arm.com>
On Mon, May 19, 2025, at 16:06, Yury Khrustalev wrote:
>
> This seems fragile and may call for writing cumbersome code. In essence,
> we will have to have clone30(), clone31(), clone32()... wrappers which
> probably defeats the point of why clone3 was added:
>
>
> if (clone32_supported && clone32(...) == -1 && errno == E2BIG)
> {
> clone32_supported = false;
> /* ... */
> }
> else if (clone31_supported && clone31(...) == -1 && errno == E2BIG)
> {
> clone12_supported = false;
> /* ... */
> }
> ...
>
> Is there a neat way to work around this? What was the idea for extending
> clone_args in practice?
>
> I suppose we can't rely on kernel version because support for extended
> clone_args can be backported. In any case, we'd have to do a syscall
> for this (it would probably be great to have kernel version in auxv).
I don't think there is a generic way to handle extended syscalls
from libc, it really depends on the specific feature it's trying
to use that requires the additional fields to be nonzero: some features
may have a reasonable fallback implementation in libc, other features
still require an error to be passed back to the caller.
As I understand the shadow stack feature, we want this to be enabled
whenever the kernel and hardware supports it, completely transparent
to an application, right?
I think ideally we'd check for HWCAP_GCS on arm64 or the equivalent
feature on other architectures and expect clone3 to support the
longer argument whenever that is set, but it looks like that would
break on current kernels that already support HWCAP_GCS but not
the clone3 argument.
Adding one more hwcap flag would be ugly, but that seems to be
the easiest way. That way, glibc can just test for the new hwcap
flag only use the extra clone3 word if all prerequisites (hardware
support, kernel gcs support, clone3 argument support) are there.
Arnd
^ permalink raw reply
* Re: Extending clone_args for clone3()
From: Mark Brown @ 2025-06-04 11:05 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Yury Khrustalev, linux-kernel, Christian Brauner, Mark Rutland,
linux-api
In-Reply-To: <71c9bd77-85fd-4f00-a36a-8621640ebbb5@app.fastmail.com>
[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]
On Wed, Jun 04, 2025 at 10:29:48AM +0200, Arnd Bergmann wrote:
> As I understand the shadow stack feature, we want this to be enabled
> whenever the kernel and hardware supports it, completely transparent
> to an application, right?
Slightly more involved, but roughly. The application and all libraries
linked into it should be built targeting shadow stacks (most binaries
will already be compatible but it's possible they wouldn't be so we
can't just asssume that) then if everything is compatible shadow stacks
will be enabled transparently by libc if the system supports them.
> I think ideally we'd check for HWCAP_GCS on arm64 or the equivalent
> feature on other architectures and expect clone3 to support the
> longer argument whenever that is set, but it looks like that would
> break on current kernels that already support HWCAP_GCS but not
> the clone3 argument.
> Adding one more hwcap flag would be ugly, but that seems to be
> the easiest way. That way, glibc can just test for the new hwcap
> flag only use the extra clone3 word if all prerequisites (hardware
> support, kernel gcs support, clone3 argument support) are there.
We'd also have to add something similar for x86 since that's had the
support even longer, and the RISC-V series looks like it's getting near
to being merged too so we'll likely have the same problem there given
that the clone3() series is not progressing super fast.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Johannes Weiner @ 2025-06-04 12:00 UTC (permalink / raw)
To: Barry Song
Cc: Matthew Wilcox, Lorenzo Stoakes, Andrew Morton, Shakeel Butt,
Liam R . Howlett, David Hildenbrand, Vlastimil Babka, Jann Horn,
Arnd Bergmann, Christian Brauner, SeongJae Park, Usama Arif,
Mike Rapoport, linux-mm, linux-arch, linux-kernel, linux-api,
Pedro Falcato
In-Reply-To: <CAGsJ_4yKDqUu8yZjHSmWBz3CpQhU6DM0=EhibfTwHbTo+QWvZA@mail.gmail.com>
On Fri, May 30, 2025 at 07:52:28PM +1200, Barry Song wrote:
> On Fri, May 30, 2025 at 9:14 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
> >
> > On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
> > > 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.
> >
> > I actually explained why I think this is a terrible idea. But okay, I
> > tried the patch anyway.
> >
> > This is 'git log' on a hot kernel repo after a large IO stream:
> >
> > VANILLA BARRY
> > Real time 49.93 ( +0.00%) 60.36 ( +20.48%)
> > User time 32.10 ( +0.00%) 32.09 ( -0.04%)
> > System time 14.41 ( +0.00%) 14.64 ( +1.50%)
> > pgmajfault 9227.00 ( +0.00%) 18390.00 ( +99.30%)
> > workingset_refault_file 184.00 ( +0.00%) 236899.00 (+127954.05%)
> >
> > Clearly we can't generally ignore page cache hits just because the
> > mmaps() are intermittent.
>
> Hi Johannes,
> Thanks!
>
> Are you on v1, which lacks folio demotion[1], or v2, which includes it [2]?
>
> [1] https://lore.kernel.org/linux-mm/20250412085852.48524-1-21cnbao@gmail.com/
> [2] https://lore.kernel.org/linux-mm/20250514070820.51793-1-21cnbao@gmail.com/
The subthread is about whether the reference dismissal / demotion
should be unconditional (v1) or opt-in (v2).
I'm arguing for v2.
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: David Hildenbrand @ 2025-06-04 12:05 UTC (permalink / raw)
To: Johannes Weiner, Barry Song
Cc: Matthew Wilcox, Lorenzo Stoakes, Andrew Morton, Shakeel Butt,
Liam R . Howlett, Vlastimil Babka, Jann Horn, Arnd Bergmann,
Christian Brauner, SeongJae Park, Usama Arif, Mike Rapoport,
linux-mm, linux-arch, linux-kernel, linux-api, Pedro Falcato
In-Reply-To: <20250604120013.GA1431@cmpxchg.org>
On 04.06.25 14:00, Johannes Weiner wrote:
> On Fri, May 30, 2025 at 07:52:28PM +1200, Barry Song wrote:
>> On Fri, May 30, 2025 at 9:14 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
>>>
>>> On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
>>>> 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.
>>>
>>> I actually explained why I think this is a terrible idea. But okay, I
>>> tried the patch anyway.
>>>
>>> This is 'git log' on a hot kernel repo after a large IO stream:
>>>
>>> VANILLA BARRY
>>> Real time 49.93 ( +0.00%) 60.36 ( +20.48%)
>>> User time 32.10 ( +0.00%) 32.09 ( -0.04%)
>>> System time 14.41 ( +0.00%) 14.64 ( +1.50%)
>>> pgmajfault 9227.00 ( +0.00%) 18390.00 ( +99.30%)
>>> workingset_refault_file 184.00 ( +0.00%) 236899.00 (+127954.05%)
>>>
>>> Clearly we can't generally ignore page cache hits just because the
>>> mmaps() are intermittent.
>>
>> Hi Johannes,
>> Thanks!
>>
>> Are you on v1, which lacks folio demotion[1], or v2, which includes it [2]?
>>
>> [1] https://lore.kernel.org/linux-mm/20250412085852.48524-1-21cnbao@gmail.com/
>> [2] https://lore.kernel.org/linux-mm/20250514070820.51793-1-21cnbao@gmail.com/
>
> The subthread is about whether the reference dismissal / demotion
> should be unconditional (v1) or opt-in (v2).
>
> I'm arguing for v2.
+1
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Johannes Weiner @ 2025-06-04 12:19 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Matthew Wilcox, Lorenzo Stoakes, Andrew Morton, Shakeel Butt,
Liam R . Howlett, David Hildenbrand, Jann Horn, Arnd Bergmann,
Christian Brauner, SeongJae Park, Usama Arif, Mike Rapoport,
Barry Song, linux-mm, linux-arch, linux-kernel, linux-api,
Pedro Falcato, tj
In-Reply-To: <0aeb6d8b-2abb-43a7-b47d-448f37f8a3bf@suse.cz>
On Fri, May 30, 2025 at 12:31:35PM +0200, Vlastimil Babka wrote:
> On 5/29/25 23:14, Johannes Weiner wrote:
> > On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
> >> 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.
> >
> > I actually explained why I think this is a terrible idea. But okay, I
> > tried the patch anyway.
> >
> > This is 'git log' on a hot kernel repo after a large IO stream:
> >
> > VANILLA BARRY
> > Real time 49.93 ( +0.00%) 60.36 ( +20.48%)
> > User time 32.10 ( +0.00%) 32.09 ( -0.04%)
> > System time 14.41 ( +0.00%) 14.64 ( +1.50%)
> > pgmajfault 9227.00 ( +0.00%) 18390.00 ( +99.30%)
> > workingset_refault_file 184.00 ( +0.00%) 236899.00 (+127954.05%)
> >
> > Clearly we can't generally ignore page cache hits just because the
> > mmaps() are intermittent.
> >
> > The whole point is to cache across processes and their various
> > apertures into a common, long-lived filesystem space.
> >
> > Barry knows something about the relationship between certain processes
> > and certain files that he could exploit with MADV_COLD-on-exit
> > semantics. But that's not something the kernel can safely assume. Not
> > without defeating the page cache for an entire class of file accesses.
>
> I've just read the previous threads about Barry's proposal and if doing this
> always isn't feasible, I'm wondering if memcg would be a better interface to
> opt-in for this kind of behavior than both prctl or mctl. I think at least
> conceptually it fits what memcg is doing? The question is if the
> implementation would be feasible, and if android puts apps in separate memcgs...
CCing Tejun.
Cgroups has been trying to resist flag settings like these. The cgroup
tree is a nested hierarchical structure designed for dividing up
system resources. But flag properties don't have natural inheritance
rules. What does it mean if the parent group says one thing and the
child says another? Which one has precedence?
Hence the proposal to make it a per-process property that propagates
through fork() and exec(). This also enables the container usecase (by
setting the flag in the container launching process), without there
being any confusion what the *effective* setting for any given process
in the system is.
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Lorenzo Stoakes @ 2025-06-04 12:28 UTC (permalink / raw)
To: Matthew Wilcox
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: <aDh9LtSLCiTLjg2X@casper.infradead.org>
On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
> 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.
>
So from my perspective - I very much agree with the concept of doing nothing
here, ideally :)
But I feel we need to look at this problem from both the short term and the
long term - in the long run I absolutely agree with what you say.
In the short term this proposal is broadly 'what is the least worst means
of establishing policy like this?'.
I think there is broad agreement that prctl() is sub-optimal, so an
mm-specific API makes sense.
So it comes down to a question of - how badly do we need to be able to do
this _now_?
^ permalink raw reply
* Re: [DISCUSSION] proposed mctl() API
From: Johannes Weiner @ 2025-06-04 13:21 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Lorenzo Stoakes, Andrew Morton, Shakeel Butt, Liam R . Howlett,
David Hildenbrand, Vlastimil Babka, Jann Horn, Arnd Bergmann,
Christian Brauner, SeongJae Park, Usama Arif, Mike Rapoport,
Barry Song, linux-mm, linux-arch, linux-kernel, linux-api,
Pedro Falcato
In-Reply-To: <aD3nFMS9kWHp7a4F@casper.infradead.org>
On Mon, Jun 02, 2025 at 07:01:56PM +0100, Matthew Wilcox wrote:
> On Thu, May 29, 2025 at 05:14:23PM -0400, Johannes Weiner wrote:
> > On Thu, May 29, 2025 at 04:28:46PM +0100, Matthew Wilcox wrote:
> > > 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.
> >
> > I actually explained why I think this is a terrible idea. But okay, I
> > tried the patch anyway.
>
> Sorry, I must've missed that one ;-(
Apologies for my tone. The discussion is spread out over too many
threads...
> > This is 'git log' on a hot kernel repo after a large IO stream:
> >
> > VANILLA BARRY
> > Real time 49.93 ( +0.00%) 60.36 ( +20.48%)
> > User time 32.10 ( +0.00%) 32.09 ( -0.04%)
> > System time 14.41 ( +0.00%) 14.64 ( +1.50%)
> > pgmajfault 9227.00 ( +0.00%) 18390.00 ( +99.30%)
> > workingset_refault_file 184.00 ( +0.00%) 236899.00 (+127954.05%)
> >
> > Clearly we can't generally ignore page cache hits just because the
> > mmaps() are intermittent.
> >
> > The whole point is to cache across processes and their various
> > apertures into a common, long-lived filesystem space.
> >
> > Barry knows something about the relationship between certain processes
> > and certain files that he could exploit with MADV_COLD-on-exit
> > semantics. But that's not something the kernel can safely assume. Not
> > without defeating the page cache for an entire class of file accesses.
>
> So what about distinguishing between exited-normally processes (ie git
> log) vs killed-by-oom processes (ie Barry's usecase)? Update the
> referenced bit in the first case and not the second?
In cloud environments, it's common to restart a workload immediately
after an OOM kill.
The hosts tend to handle a fairly dynamic mix of batch jobs and
semi-predictable user request load, all while also trying to target
decent average host utilization. Adapting to external load peaks is
laggy (spawning new workers, rebalancing).
In such setups, OOM conditions are generally assumed to be highly
transient. And quick restarts are important to avoid cascading
failures in the worker pool during load peaks.
So I don't think OOM is a good universal signal that the workload is
gone and the memory is cold.
^ 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