* 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
* Re: [DISCUSSION] proposed mctl() API
From: Johannes Weiner @ 2025-06-05 12:31 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, Tejun Heo
In-Reply-To: <20250604121923.GB1431@cmpxchg.org>
CCing Tejun - with the right mutt alias this time.
On Wed, Jun 04, 2025 at 08:19:28AM -0400, Johannes Weiner wrote:
> 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
* [PATCH RFT v17 0/8] fork: Support shadow stacks in clone3()
From: Mark Brown @ 2025-06-09 12:54 UTC (permalink / raw)
To: Rick P. Edgecombe, Deepak Gupta, Szabolcs Nagy, H.J. Lu,
Florian Weimer, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Peter Zijlstra, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, Christian Brauner, Shuah Khan
Cc: linux-kernel, Catalin Marinas, Will Deacon, jannh, bsegall,
Yury Khrustalev, Wilco Dijkstra, linux-kselftest, linux-api,
Mark Brown, Kees Cook, Kees Cook, Shuah Khan
The kernel has recently added support for shadow stacks, currently
x86 only using their CET feature but both arm64 and RISC-V have
equivalent features (GCS and Zicfiss respectively), I am actively
working on GCS[1]. With shadow stacks the hardware maintains an
additional stack containing only the return addresses for branch
instructions which is not generally writeable by userspace and ensures
that any returns are to the recorded addresses. This provides some
protection against ROP attacks and making it easier to collect call
stacks. These shadow stacks are allocated in the address space of the
userspace process.
Our API for shadow stacks does not currently offer userspace any
flexiblity for managing the allocation of shadow stacks for newly
created threads, instead the kernel allocates a new shadow stack with
the same size as the normal stack whenever a thread is created with the
feature enabled. The stacks allocated in this way are freed by the
kernel when the thread exits or shadow stacks are disabled for the
thread. This lack of flexibility and control isn't ideal, in the vast
majority of cases the shadow stack will be over allocated and the
implicit allocation and deallocation is not consistent with other
interfaces. As far as I can tell the interface is done in this manner
mainly because the shadow stack patches were in development since before
clone3() was implemented.
Since clone3() is readily extensible let's add support for specifying a
shadow stack when creating a new thread or process, keeping the current
implicit allocation behaviour if one is not specified either with
clone3() or through the use of clone(). The user must provide a shadow
stack pointer, this must point to memory mapped for use as a shadow
stackby map_shadow_stack() with an architecture specified shadow stack
token at the top of the stack.
Yuri Khrustalev has raised questions from the libc side regarding
discoverability of extended clone3() structure sizes[2], this seems like
a general issue with clone3(). There was a suggestion to add a hwcap on
arm64 which isn't ideal but is doable there, though architecture
specific mechanisms would also be needed for x86 (and RISC-V if it's
support gets merged before this does).
Please note that the x86 portions of this code are build tested only, I
don't appear to have a system that can run CET available to me.
[1] https://lore.kernel.org/linux-arm-kernel/20241001-arm64-gcs-v13-0-222b78d87eee@kernel.org/T/#mc58f97f27461749ccf400ebabf6f9f937116a86b
[2] https://lore.kernel.org/r/aCs65ccRQtJBnZ_5@arm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Changes in v17:
- Rebase onto v6.16-rc1.
- Link to v16: https://lore.kernel.org/r/20250416-clone3-shadow-stack-v16-0-2ffc9ca3917b@kernel.org
Changes in v16:
- Rebase onto v6.15-rc2.
- Roll in fixes from x86 testing from Rick Edgecombe.
- Rework so that the argument is shadow_stack_token.
- Link to v15: https://lore.kernel.org/r/20250408-clone3-shadow-stack-v15-0-3fa245c6e3be@kernel.org
Changes in v15:
- Rebase onto v6.15-rc1.
- Link to v14: https://lore.kernel.org/r/20250206-clone3-shadow-stack-v14-0-805b53af73b9@kernel.org
Changes in v14:
- Rebase onto v6.14-rc1.
- Link to v13: https://lore.kernel.org/r/20241203-clone3-shadow-stack-v13-0-93b89a81a5ed@kernel.org
Changes in v13:
- Rebase onto v6.13-rc1.
- Link to v12: https://lore.kernel.org/r/20241031-clone3-shadow-stack-v12-0-7183eb8bee17@kernel.org
Changes in v12:
- Add the regular prctl() to the userspace API document since arm64
support is queued in -next.
- Link to v11: https://lore.kernel.org/r/20241005-clone3-shadow-stack-v11-0-2a6a2bd6d651@kernel.org
Changes in v11:
- Rebase onto arm64 for-next/gcs, which is based on v6.12-rc1, and
integrate arm64 support.
- Rework the interface to specify a shadow stack pointer rather than a
base and size like we do for the regular stack.
- Link to v10: https://lore.kernel.org/r/20240821-clone3-shadow-stack-v10-0-06e8797b9445@kernel.org
Changes in v10:
- Integrate fixes & improvements for the x86 implementation from Rick
Edgecombe.
- Require that the shadow stack be VM_WRITE.
- Require that the shadow stack base and size be sizeof(void *) aligned.
- Clean up trailing newline.
- Link to v9: https://lore.kernel.org/r/20240819-clone3-shadow-stack-v9-0-962d74f99464@kernel.org
Changes in v9:
- Pull token validation earlier and report problems with an error return
to parent rather than signal delivery to the child.
- Verify that the top of the supplied shadow stack is VM_SHADOW_STACK.
- Rework token validation to only do the page mapping once.
- Drop no longer needed support for testing for signals in selftest.
- Fix typo in comments.
- Link to v8: https://lore.kernel.org/r/20240808-clone3-shadow-stack-v8-0-0acf37caf14c@kernel.org
Changes in v8:
- Fix token verification with user specified shadow stack.
- Don't track user managed shadow stacks for child processes.
- Link to v7: https://lore.kernel.org/r/20240731-clone3-shadow-stack-v7-0-a9532eebfb1d@kernel.org
Changes in v7:
- Rebase onto v6.11-rc1.
- Typo fixes.
- Link to v6: https://lore.kernel.org/r/20240623-clone3-shadow-stack-v6-0-9ee7783b1fb9@kernel.org
Changes in v6:
- Rebase onto v6.10-rc3.
- Ensure we don't try to free the parent shadow stack in error paths of
x86 arch code.
- Spelling fixes in userspace API document.
- Additional cleanups and improvements to the clone3() tests to support
the shadow stack tests.
- Link to v5: https://lore.kernel.org/r/20240203-clone3-shadow-stack-v5-0-322c69598e4b@kernel.org
Changes in v5:
- Rebase onto v6.8-rc2.
- Rework ABI to have the user allocate the shadow stack memory with
map_shadow_stack() and a token.
- Force inlining of the x86 shadow stack enablement.
- Move shadow stack enablement out into a shared header for reuse by
other tests.
- Link to v4: https://lore.kernel.org/r/20231128-clone3-shadow-stack-v4-0-8b28ffe4f676@kernel.org
Changes in v4:
- Formatting changes.
- Use a define for minimum shadow stack size and move some basic
validation to fork.c.
- Link to v3: https://lore.kernel.org/r/20231120-clone3-shadow-stack-v3-0-a7b8ed3e2acc@kernel.org
Changes in v3:
- Rebase onto v6.7-rc2.
- Remove stale shadow_stack in internal kargs.
- If a shadow stack is specified unconditionally use it regardless of
CLONE_ parameters.
- Force enable shadow stacks in the selftest.
- Update changelogs for RISC-V feature rename.
- Link to v2: https://lore.kernel.org/r/20231114-clone3-shadow-stack-v2-0-b613f8681155@kernel.org
Changes in v2:
- Rebase onto v6.7-rc1.
- Remove ability to provide preallocated shadow stack, just specify the
desired size.
- Link to v1: https://lore.kernel.org/r/20231023-clone3-shadow-stack-v1-0-d867d0b5d4d0@kernel.org
---
Mark Brown (8):
arm64/gcs: Return a success value from gcs_alloc_thread_stack()
Documentation: userspace-api: Add shadow stack API documentation
selftests: Provide helper header for shadow stack testing
fork: Add shadow stack support to clone3()
selftests/clone3: Remove redundant flushes of output streams
selftests/clone3: Factor more of main loop into test_clone3()
selftests/clone3: Allow tests to flag if -E2BIG is a valid error code
selftests/clone3: Test shadow stack support
Documentation/userspace-api/index.rst | 1 +
Documentation/userspace-api/shadow_stack.rst | 44 +++++
arch/arm64/include/asm/gcs.h | 8 +-
arch/arm64/kernel/process.c | 8 +-
arch/arm64/mm/gcs.c | 61 +++++-
arch/x86/include/asm/shstk.h | 11 +-
arch/x86/kernel/process.c | 2 +-
arch/x86/kernel/shstk.c | 57 +++++-
include/asm-generic/cacheflush.h | 11 ++
include/linux/sched/task.h | 17 ++
include/uapi/linux/sched.h | 9 +-
kernel/fork.c | 96 +++++++--
tools/testing/selftests/clone3/clone3.c | 226 ++++++++++++++++++----
tools/testing/selftests/clone3/clone3_selftests.h | 65 ++++++-
tools/testing/selftests/ksft_shstk.h | 98 ++++++++++
15 files changed, 633 insertions(+), 81 deletions(-)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20231019-clone3-shadow-stack-15d40d2bf536
Best regards,
--
Mark Brown <broonie@kernel.org>
^ 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