* Re: [PATCH v2 3/6] tracing: Use copy_from_user_nul() instead of copy_from_user()
From: Steven Rostedt @ 2026-01-13 18:03 UTC (permalink / raw)
To: Yury Norov
Cc: Fushuai Wang, tglx, peterz, mathieu.desnoyers, akpm, aliceryhl,
yury.norov, vmalik, kees, dave.hansen, luto, mingo, bp, hpa,
mhiramat, brauner, jack, cyphar, linux-kernel, x86,
linux-trace-kernel, wangfushuai
In-Reply-To: <aWZ7SXpUm8TwtUIT@yury>
On Tue, 13 Jan 2026 12:05:13 -0500
Yury Norov <ynorov@nvidia.com> wrote:
> On Mon, Jan 12, 2026 at 03:30:36PM +0800, Fushuai Wang wrote:
> > From: Fushuai Wang <wangfushuai@baidu.com>
> >
> > Use copy_from_user_nul() instead of copy_from_user() to simplify
> > the code.
> >
> > No functional change.
> >
> > Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
> > ---
> > kernel/trace/trace.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index baec63134ab6..b6ffd006fcf9 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -11266,10 +11266,9 @@ ssize_t trace_parse_run_command(struct file *file, const char __user *buffer,
> > if (size >= WRITE_BUFSIZE)
> > size = WRITE_BUFSIZE - 1;
> >
> > - if (copy_from_user(kbuf, buffer + done, size))
> > + if (copy_from_user_nul(kbuf, buffer + done, size))
> > return -EFAULT;
>
> This hides the original error. Can you switch it to:
>
> err = copy_xxx();
> if (err)
> return err;
No, the current way is fine. It's failing on reading user space. EFAULT
is good enough.
-- Steve
>
> I understand that in this case EFAULT is the only possible error, but
> the above pattern is really error-prone, and is reproduced again and
> again over the kernel.
>
> > - kbuf[size] = '\0';
> > buf = kbuf;
> > do {
> > tmp = strchr(buf, '\n');
> > --
> > 2.36.1
^ permalink raw reply
* Re: [PATCH v2 1/6] uaccess: Add copy_from_user_nul helper
From: Yury Norov @ 2026-01-13 18:00 UTC (permalink / raw)
To: Fushuai Wang
Cc: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
mhiramat, brauner, jack, cyphar, linux-kernel, x86,
linux-trace-kernel, wangfushuai
In-Reply-To: <20260112073039.1185-2-fushuai.wang@linux.dev>
On Mon, Jan 12, 2026 at 03:30:34PM +0800, Fushuai Wang wrote:
> From: Fushuai Wang <wangfushuai@baidu.com>
>
> Many places call copy_from_user() to copy a buffer from user space,
> and then manually add a NULL terminator to the destination buffer,
> e.g.:
6 is not many
>
> if (copy_from_user(dest, src, len))
> return -EFAULT;
> dest[len] = '\0';
>
> This is repetitive and error-prone. Add a copy_from_user_nul() helper to
> simplify such patterns. It copied n bytes from user space to kernel space,
> and NUL-terminates the destination buffer.
>
> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
I checked the cases you've found, and all them clearly abuse
copy_from_user(). For example, #2 in tlbflush_write_file():
if (copy_from_user(buf, user_buf, len))
return -EFAULT;
buf[len] = '\0';
if (kstrtoint(buf, 0, &ceiling))
return -EINVAL;
should be:
len = strncpy_from_user(buf, user_buf, len);
if (len < 0)
return len;
ret = kstrtoint(buf, 0, &ceiling);
if (ret)
return ret;
See, if you use the right API, you don't need this weird
copy_from_user_nul(). Also notice how nice the original version hides
possible ERANGE in kstrtoint().
Patches #3-5 in the series again copy strings with raw non-string API,
so should be converted to some flavor of strcpy().
#6 patches lib/kstrtox, which makes little sense because the whole
purpose of that library is to handle raw pieces of memory as valid
C strings. One would expect such patterns in library code, and I'd
prefer having them explicit.
I find copy_{from,to}_user_nul() useful for objects that must be
null-terminated, and may have \0 somewhere in the middle. Those are
not C strings. I suspect this isn't a popular format across the kernel.
On the other hand, adding the _nul() version of copy_from_user() would
make an API abuse like above simpler, which is a bad thing.
Can you drop copy_from_user_nul() and submit a series that switches
string manipulations to the dedicated string functions?
Thanks,
Yury
^ permalink raw reply
* Re: [PATCH v2 3/6] tracing: Use copy_from_user_nul() instead of copy_from_user()
From: Yury Norov @ 2026-01-13 17:05 UTC (permalink / raw)
To: Fushuai Wang
Cc: tglx, peterz, mathieu.desnoyers, akpm, aliceryhl, yury.norov,
vmalik, kees, dave.hansen, luto, mingo, bp, hpa, rostedt,
mhiramat, brauner, jack, cyphar, linux-kernel, x86,
linux-trace-kernel, wangfushuai
In-Reply-To: <20260112073039.1185-4-fushuai.wang@linux.dev>
On Mon, Jan 12, 2026 at 03:30:36PM +0800, Fushuai Wang wrote:
> From: Fushuai Wang <wangfushuai@baidu.com>
>
> Use copy_from_user_nul() instead of copy_from_user() to simplify
> the code.
>
> No functional change.
>
> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
> ---
> kernel/trace/trace.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index baec63134ab6..b6ffd006fcf9 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -11266,10 +11266,9 @@ ssize_t trace_parse_run_command(struct file *file, const char __user *buffer,
> if (size >= WRITE_BUFSIZE)
> size = WRITE_BUFSIZE - 1;
>
> - if (copy_from_user(kbuf, buffer + done, size))
> + if (copy_from_user_nul(kbuf, buffer + done, size))
> return -EFAULT;
This hides the original error. Can you switch it to:
err = copy_xxx();
if (err)
return err;
I understand that in this case EFAULT is the only possible error, but
the above pattern is really error-prone, and is reproduced again and
again over the kernel.
> - kbuf[size] = '\0';
> buf = kbuf;
> do {
> tmp = strchr(buf, '\n');
> --
> 2.36.1
^ permalink raw reply
* [PATCH v4 2/2] ftrace: Introduce and use ENTRIES_PER_PAGE_GROUP macro
From: Guenter Roeck @ 2026-01-13 15:22 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Guenter Roeck
In-Reply-To: <20260113152243.3557219-1-linux@roeck-us.net>
ENTRIES_PER_PAGE_GROUP() returns the number of dyn_ftrace entries in a page
group, identified by its order.
No functional change.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v4: New patch, optional. Squash, drop, or apply as separate patch.
kernel/trace/ftrace.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index aa758efc3731..df4ce244202e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1148,6 +1148,7 @@ struct ftrace_page {
};
#define ENTRY_SIZE sizeof(struct dyn_ftrace)
+#define ENTRIES_PER_PAGE_GROUP(order) ((PAGE_SIZE << (order)) / ENTRY_SIZE)
static struct ftrace_page *ftrace_pages_start;
static struct ftrace_page *ftrace_pages;
@@ -3862,7 +3863,7 @@ static int ftrace_allocate_records(struct ftrace_page *pg, int count,
*num_pages += 1 << order;
ftrace_number_of_groups++;
- cnt = (PAGE_SIZE << order) / ENTRY_SIZE;
+ cnt = ENTRIES_PER_PAGE_GROUP(order);
pg->order = order;
if (cnt > count)
@@ -7309,7 +7310,7 @@ static int ftrace_process_locs(struct module *mod,
long skip;
/* Count the number of entries unused and compare it to skipped. */
- pg_remaining = (PAGE_SIZE << pg->order) / ENTRY_SIZE - pg->index;
+ pg_remaining = ENTRIES_PER_PAGE_GROUP(pg->order) - pg->index;
if (!WARN(skipped < pg_remaining, "Extra allocated pages for ftrace")) {
@@ -7317,7 +7318,7 @@ static int ftrace_process_locs(struct module *mod,
for (pg = pg_unuse; pg && skip > 0; pg = pg->next) {
remaining += 1 << pg->order;
- skip -= (PAGE_SIZE << pg->order) / ENTRY_SIZE;
+ skip -= ENTRIES_PER_PAGE_GROUP(pg->order);
}
pages -= remaining;
--
2.45.2
^ permalink raw reply related
* [PATCH v4 1/2] ftrace: Do not over-allocate ftrace memory
From: Guenter Roeck @ 2026-01-13 15:22 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Guenter Roeck
The pg_remaining calculation in ftrace_process_locs() assumes that
ENTRIES_PER_PAGE multiplied by 2^order equals the actual capacity of the
allocated page group. However, ENTRIES_PER_PAGE is PAGE_SIZE / ENTRY_SIZE
(integer division). When PAGE_SIZE is not a multiple of ENTRY_SIZE (e.g.
4096 / 24 = 170 with remainder 16), high-order allocations (like 256 pages)
have significantly more capacity than 256 * 170. This leads to pg_remaining
being underestimated, which in turn makes skip (derived from skipped -
pg_remaining) larger than expected, causing the WARN(skip != remaining)
to trigger.
Extra allocated pages for ftrace: 2 with 654 skipped
WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:7295 ftrace_process_locs+0x5bf/0x5e0
A similar problem in ftrace_allocate_records() can result in allocating
too many pages. This can trigger the second warning in
ftrace_process_locs().
Extra allocated pages for ftrace
WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:7276 ftrace_process_locs+0x548/0x580
Use the actual capacity of a page group to determine the number of pages
to allocate. Have ftrace_allocate_pages() return the number of allocated
pages to avoid having to calculate it. Use the actual page group capacity
when validating the number of unused pages due to skipped entries.
Drop the definition of ENTRIES_PER_PAGE since it is no longer used.
Fixes: 4a3efc6baff93 ("ftrace: Update the mcount_loc check of skipped entries")
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v4: Renamed pages -> num_pages in ftrace_allocate_pages()
v3: Restore the code calculating the number of unused pages due to skipped
entries. Use the actual number of entries per page group when
calculating the number of entries in unused page groups.
v2: Have ftrace_allocate_pages() return the number of allocated pages,
and drop the page count calculation code as well as the associated
warnings from ftrace_process_locs().
kernel/trace/ftrace.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index ef2d5dca6f70..aa758efc3731 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1148,7 +1148,6 @@ struct ftrace_page {
};
#define ENTRY_SIZE sizeof(struct dyn_ftrace)
-#define ENTRIES_PER_PAGE (PAGE_SIZE / ENTRY_SIZE)
static struct ftrace_page *ftrace_pages_start;
static struct ftrace_page *ftrace_pages;
@@ -3834,7 +3833,8 @@ static int ftrace_update_code(struct module *mod, struct ftrace_page *new_pgs)
return 0;
}
-static int ftrace_allocate_records(struct ftrace_page *pg, int count)
+static int ftrace_allocate_records(struct ftrace_page *pg, int count,
+ unsigned long *num_pages)
{
int order;
int pages;
@@ -3844,7 +3844,7 @@ static int ftrace_allocate_records(struct ftrace_page *pg, int count)
return -EINVAL;
/* We want to fill as much as possible, with no empty pages */
- pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);
+ pages = DIV_ROUND_UP(count * ENTRY_SIZE, PAGE_SIZE);
order = fls(pages) - 1;
again:
@@ -3859,6 +3859,7 @@ static int ftrace_allocate_records(struct ftrace_page *pg, int count)
}
ftrace_number_of_pages += 1 << order;
+ *num_pages += 1 << order;
ftrace_number_of_groups++;
cnt = (PAGE_SIZE << order) / ENTRY_SIZE;
@@ -3887,12 +3888,14 @@ static void ftrace_free_pages(struct ftrace_page *pages)
}
static struct ftrace_page *
-ftrace_allocate_pages(unsigned long num_to_init)
+ftrace_allocate_pages(unsigned long num_to_init, unsigned long *num_pages)
{
struct ftrace_page *start_pg;
struct ftrace_page *pg;
int cnt;
+ *num_pages = 0;
+
if (!num_to_init)
return NULL;
@@ -3906,7 +3909,7 @@ ftrace_allocate_pages(unsigned long num_to_init)
* waste as little space as possible.
*/
for (;;) {
- cnt = ftrace_allocate_records(pg, num_to_init);
+ cnt = ftrace_allocate_records(pg, num_to_init, num_pages);
if (cnt < 0)
goto free_pages;
@@ -7192,8 +7195,6 @@ static int ftrace_process_locs(struct module *mod,
if (!count)
return 0;
- pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);
-
/*
* Sorting mcount in vmlinux at build time depend on
* CONFIG_BUILDTIME_MCOUNT_SORT, while mcount loc in
@@ -7206,7 +7207,7 @@ static int ftrace_process_locs(struct module *mod,
test_is_sorted(start, count);
}
- start_pg = ftrace_allocate_pages(count);
+ start_pg = ftrace_allocate_pages(count, &pages);
if (!start_pg)
return -ENOMEM;
@@ -7305,27 +7306,27 @@ static int ftrace_process_locs(struct module *mod,
/* We should have used all pages unless we skipped some */
if (pg_unuse) {
unsigned long pg_remaining, remaining = 0;
- unsigned long skip;
+ long skip;
/* Count the number of entries unused and compare it to skipped. */
- pg_remaining = (ENTRIES_PER_PAGE << pg->order) - pg->index;
+ pg_remaining = (PAGE_SIZE << pg->order) / ENTRY_SIZE - pg->index;
if (!WARN(skipped < pg_remaining, "Extra allocated pages for ftrace")) {
skip = skipped - pg_remaining;
- for (pg = pg_unuse; pg; pg = pg->next)
+ for (pg = pg_unuse; pg && skip > 0; pg = pg->next) {
remaining += 1 << pg->order;
+ skip -= (PAGE_SIZE << pg->order) / ENTRY_SIZE;
+ }
pages -= remaining;
- skip = DIV_ROUND_UP(skip, ENTRIES_PER_PAGE);
-
/*
* Check to see if the number of pages remaining would
* just fit the number of entries skipped.
*/
- WARN(skip != remaining, "Extra allocated pages for ftrace: %lu with %lu skipped",
+ WARN(pg || skip > 0, "Extra allocated pages for ftrace: %lu with %lu skipped",
remaining, skipped);
}
/* Need to synchronize with ftrace_location_range() */
--
2.45.2
^ permalink raw reply related
* Re: [PATCH v5] tracing: Guard __DECLARE_TRACE() use of __DO_TRACE_CALL() with SRCU-fast
From: Sebastian Andrzej Siewior @ 2026-01-13 14:23 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Steven Rostedt, Mathieu Desnoyers, LKML, Linux trace kernel, bpf,
Masami Hiramatsu, Paul E. McKenney, Thomas Gleixner
In-Reply-To: <CAADnVQKvY026HSFGOsavJppm3-Ajm-VsLzY-OeFUe+BaKMRnDg@mail.gmail.com>
On 2026-01-12 09:19:58 [-0800], Alexei Starovoitov wrote:
> > Now if you are saying that BPF will handle migrate_disable() on its own
> > and not require the tracepoint infrastructure to do it for it, then
> > this is perfect. And I can then simplify this code, and just use
> > srcu_fast for both RT and !RT.
>
> Agree. Just add migrate_disable to __bpf_trace_run,
> or, better yet, use rcu_read_lock_dont_migrate() in there.
Wonderful, thank you.
Is this "must remain on the same CPU and can be re-entrant" because BPF
core code such memory allocator/ data structures use per-CPU data
structures and must use the same through the whole invocation?
I did audit network related BPF code and their per-CPU usage usually had
a local_bh_disable() in the relevant spots.
Sebastian
^ permalink raw reply
* Re: [PATCH v13 2/3] mm: Fix OOM killer inaccuracy on large many-core systems
From: Michal Hocko @ 2026-01-13 14:11 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Andrew Morton, linux-kernel, Paul E. McKenney, Steven Rostedt,
Masami Hiramatsu, Dennis Zhou, Tejun Heo, Christoph Lameter,
Martin Liu, David Rientjes, christian.koenig, Shakeel Butt,
SeongJae Park, Johannes Weiner, Sweet Tea Dorminy,
Lorenzo Stoakes, Liam R . Howlett, Mike Rapoport,
Suren Baghdasaryan, Vlastimil Babka, Christian Brauner, Wei Yang,
David Hildenbrand, Miaohe Lin, Al Viro, linux-mm,
linux-trace-kernel, Yu Zhao, Roman Gushchin, Mateusz Guzik,
Matthew Wilcox, Baolin Wang, Aboorva Devarajan
In-Reply-To: <c99778c3-6ef0-48de-98ac-10913419ec90@efficios.com>
On Tue 13-01-26 08:51:45, Mathieu Desnoyers wrote:
> On 2026-01-13 04:24, Michal Hocko wrote:
[...]
> - Introduce new proc files, e.g.
>
> /proc/<pid>/rss/approximate
> /proc/<pid>/rss/precise
>
> Where the "approximate" file would export the following lines for each
> page type (MM_FILEPAGES, MM_ANONPAGES, MM_SWAPENTS, MM_SHMPAGES,
> allowing future additions):
>
> <page type> <approximate> <precise_sum_min> <precise_sum_max>
>
> And "precise" would export lines for each page type:
>
> <page type> <precise_sum>
>
> The key thing here is to have different files to query approximated
> vs precise values, so we don't have the overhead of the precise sum
> when all we need is an approximation.
>
> This would expose all the bits and pieces needed to allow userspace to
> implement something similar to the 2-pass algorithm I'm proposing for
> the OOM killer, but tweaked for other use-cases.
>
> This proposed ABI is purely hypothetical at this stage. Please let me
> know if you have something different in mind.
TBH, I am not convinced this is really needed. I would simply use the
new more-precise interface for /proc/<pid>/stat with numbers of
potential overhead payed by an increased precision. If we need to revert
to low precision then we can do that based on a specific report.
> When you mention "highlevel doc", which document do you have in mind ?
> Something related to lib/percpu_counter_tree.c or to the /proc ABI ?
Documentation/core-api/percpu_counter_tree.rst
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v5] tracing: Guard __DECLARE_TRACE() use of __DO_TRACE_CALL() with SRCU-fast
From: Sebastian Andrzej Siewior @ 2026-01-13 13:56 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Steven Rostedt, LKML, Linux trace kernel, bpf, Masami Hiramatsu,
Paul E. McKenney, Thomas Gleixner
In-Reply-To: <8252131c-4c54-43ca-9041-71481165e516@efficios.com>
On 2026-01-09 13:58:21 [-0500], Mathieu Desnoyers wrote:
> On 2026-01-09 12:21, Steven Rostedt wrote:
> > > Using SRCU-fast to protect tracepoint callback iteration makes sense
> > > for preempt-rt, but I'd recommend moving the migrate disable guard
> > > within the bpf callback code rather than slowing down other tracers
> > > which execute within a short amount of time. Other tracers can then
> > > choose to disable preemption rather than migration if that's a better
> > > fit for their needs.
> >
> > This is a discussion with the BPF folks.
>
> FWIW, the approach I'm proposing would be similar to what I've done for
> faultable syscall tracepoints.
I just started reading this thread but we could limit the
migrate_disable() to only the BPF callback part so it does not effect
the whole tracepoint if there not a BFP program attached to it.
Sebastian
^ permalink raw reply
* Re: [PATCH v13 2/3] mm: Fix OOM killer inaccuracy on large many-core systems
From: Mathieu Desnoyers @ 2026-01-13 13:51 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-kernel, Paul E. McKenney, Steven Rostedt,
Masami Hiramatsu, Dennis Zhou, Tejun Heo, Christoph Lameter,
Martin Liu, David Rientjes, christian.koenig, Shakeel Butt,
SeongJae Park, Johannes Weiner, Sweet Tea Dorminy,
Lorenzo Stoakes, Liam R . Howlett, Mike Rapoport,
Suren Baghdasaryan, Vlastimil Babka, Christian Brauner, Wei Yang,
David Hildenbrand, Miaohe Lin, Al Viro, linux-mm,
linux-trace-kernel, Yu Zhao, Roman Gushchin, Mateusz Guzik,
Matthew Wilcox, Baolin Wang, Aboorva Devarajan
In-Reply-To: <aWYPWNIv4lR2FpUZ@tiehlicka>
On 2026-01-13 04:24, Michal Hocko wrote:
[...]
>> Would you be OK with introducing changes in the following order ?
>>
>> 1) Fix the OOM killer inaccuracy by using counter sum (iteration on all
>> cpu counters) in task selection. This may slow down the oom killer,
>> but would at least fix its current inaccuracy issues. This could be
>> backported to stable kernels.
>>
>> 2) Introduce the hierarchical percpu counters on top, as a oom killer
>> task selection performance optimization (reduce latency of oom kill).
>>
>> This way, (2) becomes purely a performance optimization, so it's easy
>> to bissect and revert if it causes issues.
>
> Yes, this makes more sense.
>
>> I agree that bringing a fix along with a performance optimization within
>> a single commit makes it hard to backport to stable, and tricky to
>> revert if it causes problems.
>>
>> As for finding other users of the hpcc, I have ideas, but not so much
>> time available to try them out, as I'm pretty much doing this in my
>> spare time.
>
> I do understand this constrain and motivation to have OOM situation
> addressed with a priority. I am pretty sure that if you see issues in
> OOM path then other consumers of get_mm_counter would be affected as
> well. Namely /proc/<pid>/stat.
Indeed /proc/<pid>/stat (implemented in fs/proc/array.c:do_task_stat())
uses get_mm_rss() which currently exports the approximated value to
userspace.
> There might be others but I can imagine
> that some of them are more performance than precision sensitive.
Agreed.
> All that being said it seems that we need slow-and-precise and
> fast-approximate interfaces to have incremental path for other users as
> well. Looking at patch 1 it seems there are interfaces available for
> that. I think it would be great to call those out explicitly in the
> highlevel doc to give some guidance what to use when with what kind of
> expectations.
I figured I'd first focus on the oom killers internals before tackling
the userspace ABI aspect of the problem, but since you're bringing it
up, here is what I have in mind, more or less:
- Introduce new proc files, e.g.
/proc/<pid>/rss/approximate
/proc/<pid>/rss/precise
Where the "approximate" file would export the following lines for each
page type (MM_FILEPAGES, MM_ANONPAGES, MM_SWAPENTS, MM_SHMPAGES,
allowing future additions):
<page type> <approximate> <precise_sum_min> <precise_sum_max>
And "precise" would export lines for each page type:
<page type> <precise_sum>
The key thing here is to have different files to query approximated
vs precise values, so we don't have the overhead of the precise sum
when all we need is an approximation.
This would expose all the bits and pieces needed to allow userspace to
implement something similar to the 2-pass algorithm I'm proposing for
the OOM killer, but tweaked for other use-cases.
This proposed ABI is purely hypothetical at this stage. Please let me
know if you have something different in mind.
When you mention "highlevel doc", which document do you have in mind ?
Something related to lib/percpu_counter_tree.c or to the /proc ABI ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply
* Re: [PATCHv6 bpf-next 7/9] bpf: Add trampoline ip hash table
From: Jiri Olsa @ 2026-01-13 11:58 UTC (permalink / raw)
To: Alan Maguire
Cc: Jiri Olsa, Andrii Nakryiko, Steven Rostedt, Florent Revest,
Mark Rutland, bpf, linux-kernel, linux-trace-kernel,
linux-arm-kernel, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <98436a72-236a-43c4-b6ac-9d74b53b0223@oracle.com>
On Tue, Jan 13, 2026 at 11:02:33AM +0000, Alan Maguire wrote:
> On 12/01/2026 21:27, Jiri Olsa wrote:
> > On Fri, Jan 09, 2026 at 04:36:41PM -0800, Andrii Nakryiko wrote:
> >> On Tue, Dec 30, 2025 at 6:51 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >>>
> >>> Following changes need to lookup trampoline based on its ip address,
> >>> adding hash table for that.
> >>>
> >>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >>> ---
> >>> include/linux/bpf.h | 7 +++++--
> >>> kernel/bpf/trampoline.c | 30 +++++++++++++++++++-----------
> >>> 2 files changed, 24 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> >>> index 4e7d72dfbcd4..c85677aae865 100644
> >>> --- a/include/linux/bpf.h
> >>> +++ b/include/linux/bpf.h
> >>> @@ -1325,14 +1325,17 @@ struct bpf_tramp_image {
> >>> };
> >>>
> >>> struct bpf_trampoline {
> >>> - /* hlist for trampoline_table */
> >>> - struct hlist_node hlist;
> >>> + /* hlist for trampoline_key_table */
> >>> + struct hlist_node hlist_key;
> >>> + /* hlist for trampoline_ip_table */
> >>> + struct hlist_node hlist_ip;
> >>> struct ftrace_ops *fops;
> >>> /* serializes access to fields of this trampoline */
> >>> struct mutex mutex;
> >>> refcount_t refcnt;
> >>> u32 flags;
> >>> u64 key;
> >>> + unsigned long ip;
> >>> struct {
> >>> struct btf_func_model model;
> >>> void *addr;
> >>> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> >>> index 789ff4e1f40b..bdac9d673776 100644
> >>> --- a/kernel/bpf/trampoline.c
> >>> +++ b/kernel/bpf/trampoline.c
> >>> @@ -24,9 +24,10 @@ const struct bpf_prog_ops bpf_extension_prog_ops = {
> >>> #define TRAMPOLINE_HASH_BITS 10
> >>> #define TRAMPOLINE_TABLE_SIZE (1 << TRAMPOLINE_HASH_BITS)
> >>>
> >>> -static struct hlist_head trampoline_table[TRAMPOLINE_TABLE_SIZE];
> >>> +static struct hlist_head trampoline_key_table[TRAMPOLINE_TABLE_SIZE];
> >>> +static struct hlist_head trampoline_ip_table[TRAMPOLINE_TABLE_SIZE];
> >>>
> >>> -/* serializes access to trampoline_table */
> >>> +/* serializes access to trampoline tables */
> >>> static DEFINE_MUTEX(trampoline_mutex);
> >>>
> >>> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> >>> @@ -135,15 +136,15 @@ void bpf_image_ksym_del(struct bpf_ksym *ksym)
> >>> PAGE_SIZE, true, ksym->name);
> >>> }
> >>>
> >>> -static struct bpf_trampoline *bpf_trampoline_lookup(u64 key)
> >>> +static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
> >>> {
> >>> struct bpf_trampoline *tr;
> >>> struct hlist_head *head;
> >>> int i;
> >>>
> >>> mutex_lock(&trampoline_mutex);
> >>> - head = &trampoline_table[hash_64(key, TRAMPOLINE_HASH_BITS)];
> >>> - hlist_for_each_entry(tr, head, hlist) {
> >>> + head = &trampoline_key_table[hash_64(key, TRAMPOLINE_HASH_BITS)];
> >>> + hlist_for_each_entry(tr, head, hlist_key) {
> >>> if (tr->key == key) {
> >>> refcount_inc(&tr->refcnt);
> >>> goto out;
> >>> @@ -164,8 +165,12 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key)
> >>> #endif
> >>>
> >>> tr->key = key;
> >>> - INIT_HLIST_NODE(&tr->hlist);
> >>> - hlist_add_head(&tr->hlist, head);
> >>> + tr->ip = ftrace_location(ip);
> >>> + INIT_HLIST_NODE(&tr->hlist_key);
> >>> + INIT_HLIST_NODE(&tr->hlist_ip);
> >>> + hlist_add_head(&tr->hlist_key, head);
> >>> + head = &trampoline_ip_table[hash_64(tr->ip, TRAMPOLINE_HASH_BITS)];
> >>
> >> For key lookups we check that there is no existing trampoline for the
> >> given key. Can it happen that we have two trampolines at the same IP
> >> but using two different keys?
> >
> > so multiple keys (different static functions with same name) resolving to
> > the same ip happened in past and we should now be able to catch those in
> > pahole, right? CC-ing Alan ;-)
> >
>
> We could catch this I think, but today we don't. We have support to avoid
> encoding BTF where a function name has multiple instances (ambiguous address).
> Here you're concerned with mapping from ip to function name, where multiple
> names share the same ip, right?
so trampolines work only on top of BTF func record, so the 'key' represents
BTF_KIND_FUNC record.. and as such it can resolve to just single ip, because
pahole filters out functions with ambiguous instances IIUC
>
> A quick scan of System.map suggests there's a ~150 of these,
> excluding __pfx_ entries:
>
> $ awk 'NR > 1 && ($2 == "T" || $2 == "t") && $1 == prev_field { print;} { prev_field = $1}' System.map|egrep -v __pfx|wc -l
> 155
right, but these are just regular kernel symbols with aliases and other
shared stuff
jirka
^ permalink raw reply
* Re: [PATCH bpf-next 2/4] x86/fgraph,bpf: Switch kprobe_multi program stack unwind to hw_regs path
From: Jiri Olsa @ 2026-01-13 11:43 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Josh Poimboeuf, Mahe Tardy, Peter Zijlstra, bpf,
linux-trace-kernel, x86, Yonghong Song, Song Liu, Andrii Nakryiko
In-Reply-To: <20260112170757.4e41c0d8@gandalf.local.home>
On Mon, Jan 12, 2026 at 05:07:57PM -0500, Steven Rostedt wrote:
> On Mon, 12 Jan 2026 22:49:38 +0100
> Jiri Olsa <jolsa@kernel.org> wrote:
>
> > To recreate same stack setup for return probe as we have for entry
> > probe, we set the instruction pointer to the attached function address,
> > which gets us the same unwind setup and same stack trace.
> >
> > With the fix, entry probe:
> >
> > # bpftrace -e 'kprobe:__x64_sys_newuname* { print(kstack)}'
> > Attaching 1 probe...
> >
> > __x64_sys_newuname+9
> > do_syscall_64+134
> > entry_SYSCALL_64_after_hwframe+118
> >
> > return probe:
> >
> > # bpftrace -e 'kretprobe:__x64_sys_newuname* { print(kstack)}'
> > Attaching 1 probe...
> >
> > __x64_sys_newuname+4
> > do_syscall_64+134
> > entry_SYSCALL_64_after_hwframe+118
>
> But is this really correct?
>
> The stack trace of the return from __x86_sys_newuname is from offset "+4".
>
> The stack trace from entry is offset "+9". Isn't it confusing that the
> offset is likely not from the return portion of that function?
right, makes sense.. so standard kprobe actualy skips attached function
(__x86_sys_newuname) on return probe stacktrace.. perhaps we should do
the same for kprobe_multi
I managed to get that with the change below, but it's wrong wrt arch code,
note the ftrace_regs_set_stack_pointer(fregs, stack + 8) .. will try to
figure out better way when we agree on the solution
thanks,
jirka
---
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index c56e1e63b893..b0e8ce4934e7 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -71,6 +71,9 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)
#define ftrace_regs_set_instruction_pointer(fregs, _ip) \
do { arch_ftrace_regs(fregs)->regs.ip = (_ip); } while (0)
+#define ftrace_regs_set_stack_pointer(fregs, _sp) \
+ do { arch_ftrace_regs(fregs)->regs.sp = (_sp); } while (0)
+
static __always_inline unsigned long
ftrace_regs_get_return_address(struct ftrace_regs *fregs)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 6279e0a753cf..b1510c412dcb 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -717,7 +717,8 @@ int function_graph_enter_regs(unsigned long ret, unsigned long func,
/* Retrieve a function return address to the trace stack on thread info.*/
static struct ftrace_ret_stack *
ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
- unsigned long frame_pointer, int *offset)
+ unsigned long *stack, unsigned long frame_pointer,
+ int *offset)
{
struct ftrace_ret_stack *ret_stack;
@@ -762,6 +763,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
*offset += FGRAPH_FRAME_OFFSET;
*ret = ret_stack->ret;
+ *stack = (unsigned long) ret_stack->retp;
trace->func = ret_stack->func;
trace->overrun = atomic_read(¤t->trace_overrun);
trace->depth = current->curr_ret_depth;
@@ -810,12 +812,13 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
struct ftrace_ret_stack *ret_stack;
struct ftrace_graph_ret trace;
unsigned long bitmap;
+ unsigned long stack;
unsigned long ret;
int offset;
int bit;
int i;
- ret_stack = ftrace_pop_return_trace(&trace, &ret, frame_pointer, &offset);
+ ret_stack = ftrace_pop_return_trace(&trace, &ret, &stack, frame_pointer, &offset);
if (unlikely(!ret_stack)) {
ftrace_graph_stop();
@@ -824,8 +827,11 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
return (unsigned long)panic;
}
- if (fregs)
- ftrace_regs_set_instruction_pointer(fregs, trace.func);
+ if (fregs) {
+ ftrace_regs_set_instruction_pointer(fregs, ret);
+ ftrace_regs_set_stack_pointer(fregs, stack + 8);
+ }
+
bit = ftrace_test_recursion_trylock(trace.func, ret);
/*
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_ips.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_ips.c
index e1a9b55e07cb..852830536109 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_ips.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_ips.c
@@ -74,12 +74,20 @@ static void test_stacktrace_ips_kprobe_multi(bool retprobe)
load_kallsyms();
- check_stacktrace_ips(bpf_map__fd(skel->maps.stackmap), skel->bss->stack_key, 5,
- ksym_get_addr("bpf_testmod_stacktrace_test"),
- ksym_get_addr("bpf_testmod_stacktrace_test_3"),
- ksym_get_addr("bpf_testmod_stacktrace_test_2"),
- ksym_get_addr("bpf_testmod_stacktrace_test_1"),
- ksym_get_addr("bpf_testmod_test_read"));
+ if (retprobe) {
+ check_stacktrace_ips(bpf_map__fd(skel->maps.stackmap), skel->bss->stack_key, 4,
+ ksym_get_addr("bpf_testmod_stacktrace_test_3"),
+ ksym_get_addr("bpf_testmod_stacktrace_test_2"),
+ ksym_get_addr("bpf_testmod_stacktrace_test_1"),
+ ksym_get_addr("bpf_testmod_test_read"));
+ } else {
+ check_stacktrace_ips(bpf_map__fd(skel->maps.stackmap), skel->bss->stack_key, 5,
+ ksym_get_addr("bpf_testmod_stacktrace_test"),
+ ksym_get_addr("bpf_testmod_stacktrace_test_3"),
+ ksym_get_addr("bpf_testmod_stacktrace_test_2"),
+ ksym_get_addr("bpf_testmod_stacktrace_test_1"),
+ ksym_get_addr("bpf_testmod_test_read"));
+ }
cleanup:
stacktrace_ips__destroy(skel);
^ permalink raw reply related
* Re: [PATCHv6 bpf-next 7/9] bpf: Add trampoline ip hash table
From: Alan Maguire @ 2026-01-13 11:02 UTC (permalink / raw)
To: Jiri Olsa, Andrii Nakryiko
Cc: Steven Rostedt, Florent Revest, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <aWVnVzeqWJBXwBze@krava>
On 12/01/2026 21:27, Jiri Olsa wrote:
> On Fri, Jan 09, 2026 at 04:36:41PM -0800, Andrii Nakryiko wrote:
>> On Tue, Dec 30, 2025 at 6:51 AM Jiri Olsa <jolsa@kernel.org> wrote:
>>>
>>> Following changes need to lookup trampoline based on its ip address,
>>> adding hash table for that.
>>>
>>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>>> ---
>>> include/linux/bpf.h | 7 +++++--
>>> kernel/bpf/trampoline.c | 30 +++++++++++++++++++-----------
>>> 2 files changed, 24 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>>> index 4e7d72dfbcd4..c85677aae865 100644
>>> --- a/include/linux/bpf.h
>>> +++ b/include/linux/bpf.h
>>> @@ -1325,14 +1325,17 @@ struct bpf_tramp_image {
>>> };
>>>
>>> struct bpf_trampoline {
>>> - /* hlist for trampoline_table */
>>> - struct hlist_node hlist;
>>> + /* hlist for trampoline_key_table */
>>> + struct hlist_node hlist_key;
>>> + /* hlist for trampoline_ip_table */
>>> + struct hlist_node hlist_ip;
>>> struct ftrace_ops *fops;
>>> /* serializes access to fields of this trampoline */
>>> struct mutex mutex;
>>> refcount_t refcnt;
>>> u32 flags;
>>> u64 key;
>>> + unsigned long ip;
>>> struct {
>>> struct btf_func_model model;
>>> void *addr;
>>> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
>>> index 789ff4e1f40b..bdac9d673776 100644
>>> --- a/kernel/bpf/trampoline.c
>>> +++ b/kernel/bpf/trampoline.c
>>> @@ -24,9 +24,10 @@ const struct bpf_prog_ops bpf_extension_prog_ops = {
>>> #define TRAMPOLINE_HASH_BITS 10
>>> #define TRAMPOLINE_TABLE_SIZE (1 << TRAMPOLINE_HASH_BITS)
>>>
>>> -static struct hlist_head trampoline_table[TRAMPOLINE_TABLE_SIZE];
>>> +static struct hlist_head trampoline_key_table[TRAMPOLINE_TABLE_SIZE];
>>> +static struct hlist_head trampoline_ip_table[TRAMPOLINE_TABLE_SIZE];
>>>
>>> -/* serializes access to trampoline_table */
>>> +/* serializes access to trampoline tables */
>>> static DEFINE_MUTEX(trampoline_mutex);
>>>
>>> #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>>> @@ -135,15 +136,15 @@ void bpf_image_ksym_del(struct bpf_ksym *ksym)
>>> PAGE_SIZE, true, ksym->name);
>>> }
>>>
>>> -static struct bpf_trampoline *bpf_trampoline_lookup(u64 key)
>>> +static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
>>> {
>>> struct bpf_trampoline *tr;
>>> struct hlist_head *head;
>>> int i;
>>>
>>> mutex_lock(&trampoline_mutex);
>>> - head = &trampoline_table[hash_64(key, TRAMPOLINE_HASH_BITS)];
>>> - hlist_for_each_entry(tr, head, hlist) {
>>> + head = &trampoline_key_table[hash_64(key, TRAMPOLINE_HASH_BITS)];
>>> + hlist_for_each_entry(tr, head, hlist_key) {
>>> if (tr->key == key) {
>>> refcount_inc(&tr->refcnt);
>>> goto out;
>>> @@ -164,8 +165,12 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key)
>>> #endif
>>>
>>> tr->key = key;
>>> - INIT_HLIST_NODE(&tr->hlist);
>>> - hlist_add_head(&tr->hlist, head);
>>> + tr->ip = ftrace_location(ip);
>>> + INIT_HLIST_NODE(&tr->hlist_key);
>>> + INIT_HLIST_NODE(&tr->hlist_ip);
>>> + hlist_add_head(&tr->hlist_key, head);
>>> + head = &trampoline_ip_table[hash_64(tr->ip, TRAMPOLINE_HASH_BITS)];
>>
>> For key lookups we check that there is no existing trampoline for the
>> given key. Can it happen that we have two trampolines at the same IP
>> but using two different keys?
>
> so multiple keys (different static functions with same name) resolving to
> the same ip happened in past and we should now be able to catch those in
> pahole, right? CC-ing Alan ;-)
>
We could catch this I think, but today we don't. We have support to avoid
encoding BTF where a function name has multiple instances (ambiguous address).
Here you're concerned with mapping from ip to function name, where multiple
names share the same ip, right?
A quick scan of System.map suggests there's a ~150 of these,
excluding __pfx_ entries:
$ awk 'NR > 1 && ($2 == "T" || $2 == "t") && $1 == prev_field { print;} { prev_field = $1}' System.map|egrep -v __pfx|wc -l
155
Alan
^ permalink raw reply
* Re: [PATCH v13 2/3] mm: Fix OOM killer inaccuracy on large many-core systems
From: Michal Hocko @ 2026-01-13 9:24 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Andrew Morton, linux-kernel, Paul E. McKenney, Steven Rostedt,
Masami Hiramatsu, Dennis Zhou, Tejun Heo, Christoph Lameter,
Martin Liu, David Rientjes, christian.koenig, Shakeel Butt,
SeongJae Park, Johannes Weiner, Sweet Tea Dorminy,
Lorenzo Stoakes, Liam R . Howlett, Mike Rapoport,
Suren Baghdasaryan, Vlastimil Babka, Christian Brauner, Wei Yang,
David Hildenbrand, Miaohe Lin, Al Viro, linux-mm,
linux-trace-kernel, Yu Zhao, Roman Gushchin, Mateusz Guzik,
Matthew Wilcox, Baolin Wang, Aboorva Devarajan
In-Reply-To: <b779c646-64c7-49f8-8847-8819227e3f1f@efficios.com>
On Mon 12-01-26 19:47:54, Mathieu Desnoyers wrote:
> On 2026-01-12 14:48, Michal Hocko wrote:
> > On Mon 12-01-26 14:37:49, Mathieu Desnoyers wrote:
> > > On 2026-01-12 03:42, Michal Hocko wrote:
> > > > Hi,
> > > > sorry to jump in this late but the timing of previous versions didn't
> > > > really work well for me.
> > > >
> > > > On Sun 11-01-26 14:49:57, Mathieu Desnoyers wrote:
> > > > [...]
> > > > > Here is a (possibly incomplete) list of the prior approaches that were
> > > > > used or proposed, along with their downside:
> > > > >
> > > > > 1) Per-thread rss tracking: large error on many-thread processes.
> > > > >
> > > > > 2) Per-CPU counters: up to 12% slower for short-lived processes and 9%
> > > > > increased system time in make test workloads [1]. Moreover, the
> > > > > inaccuracy increases with O(n^2) with the number of CPUs.
> > > > >
> > > > > 3) Per-NUMA-node counters: requires atomics on fast-path (overhead),
> > > > > error is high with systems that have lots of NUMA nodes (32 times
> > > > > the number of NUMA nodes).
> > > > >
> > > > > The approach proposed here is to replace this by the hierarchical
> > > > > per-cpu counters, which bounds the inaccuracy based on the system
> > > > > topology with O(N*logN).
> > > >
> > > > The concept of hierarchical pcp counter is interesting and I am
> > > > definitely not opposed if there are more users that would benefit.
> > > >
> > > > From the OOM POV, IIUC the primary problem is that get_mm_counter
> > > > (percpu_counter_read_positive) is too imprecise on systems when the task
> > > > is moving around a large number of cpus. In the list of alternative
> > > > solutions I do not see percpu_counter_sum_positive to be mentioned.
> > > > oom_badness() is a really slow path and taking the slow path to
> > > > calculate a much more precise value seems acceptable. Have you
> > > > considered that option?
> > > I must admit I assumed that since there was already a mechanism in place
> > > to ensure it's not necessary to sum per-cpu counters when the oom killer
> > > is trying to select tasks, it must be because this
> > >
> > > O(nr_possible_cpus * nr_processes)
> > >
> > > operation must be too slow for the oom killer requirements.
> > >
> > > AFAIU, the oom killer is executed when the memory allocator fails to
> > > allocate memory, which can be within code paths which need to progress
> > > eventually. So even though it's a slow path compared to the allocator
> > > fast path, there must be at least _some_ expectations about it
> > > completing within a decent amount of time. What would that ballpark be ?
> >
> > I do not think we have ever promissed more than the oom killer will try
> > to unlock the system blocked on memory shortage.
> >
> > > To give an order of magnitude, I've tried modifying the upstream
> > > oom killer to use percpu_counter_sum_positive and compared it to
> > > the hierarchical approach:
> > >
> > > AMD EPYC 9654 96-Core (2 sockets)
> > > Within a KVM, configured with 256 logical cpus.
> > >
> > > nr_processes=40 nr_processes=10000
> > > Counter sum: 0.4 ms 81.0 ms
> > > HPCC with 2-pass: 0.3 ms 9.3 ms
> >
> > These are peanuts for the global oom situations. We have had situations
> > when soft lockup detector triggered because of the process tree
> > traversal so adding 100ms is not really critical.
> >
> > > So as we scale up the number of processes on large SMP systems,
> > > the latency caused by the oom killer task selection greatly
> > > increases with the counter sums compared with the hierarchical
> > > approach.
> >
> > Yes, I am not really questioning the hierarchical approach will perform
> > much better but I am thinking of a good enough solution and calculating
> > the number might be just that stop gap solution (that would be also
> > suitable for stable tree backports). I am not ruling out improving on
> > top of that by a more clever solution like your hierarchical counters
> > approach. Especially if there are more benefits from that elsewhere.
> >
>
> Would you be OK with introducing changes in the following order ?
>
> 1) Fix the OOM killer inaccuracy by using counter sum (iteration on all
> cpu counters) in task selection. This may slow down the oom killer,
> but would at least fix its current inaccuracy issues. This could be
> backported to stable kernels.
>
> 2) Introduce the hierarchical percpu counters on top, as a oom killer
> task selection performance optimization (reduce latency of oom kill).
>
> This way, (2) becomes purely a performance optimization, so it's easy
> to bissect and revert if it causes issues.
Yes, this makes more sense.
> I agree that bringing a fix along with a performance optimization within
> a single commit makes it hard to backport to stable, and tricky to
> revert if it causes problems.
>
> As for finding other users of the hpcc, I have ideas, but not so much
> time available to try them out, as I'm pretty much doing this in my
> spare time.
I do understand this constrain and motivation to have OOM situation
addressed with a priority. I am pretty sure that if you see issues in
OOM path then other consumers of get_mm_counter would be affected as
well. Namely /proc/<pid>/stat. There might be others but I can imagine
that some of them are more performance than precision sensitive.
All that being said it seems that we need slow-and-precise and
fast-approximate interfaces to have incremental path for other users as
well. Looking at patch 1 it seems there are interfaces available for
that. I think it would be great to call those out explicitly in the
highlevel doc to give some guidance what to use when with what kind of
expectations.
Thanks!
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [PATCH v3 2/2] tracing: Add autoremove feature to the backup instance
From: Masami Hiramatsu (Google) @ 2026-01-13 6:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <176828713313.2161268.62228177827727824.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Since the backup instance is readonly, after reading all data
via pipe, no data is left on the instance. Thus it can be
removed safely after closing all files.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/trace.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++-
kernel/trace/trace.h | 6 +++++
2 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2589dac77519..eb97ee9b26e4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -590,6 +590,55 @@ void trace_set_ring_buffer_expanded(struct trace_array *tr)
tr->ring_buffer_expanded = true;
}
+static int __remove_instance(struct trace_array *tr);
+
+static void trace_array_autoremove(struct work_struct *work)
+{
+ struct trace_array *tr = container_of(work, struct trace_array, autoremove_work);
+
+ guard(mutex)(&event_mutex);
+ guard(mutex)(&trace_types_lock);
+
+ /*
+ * This can be fail if someone gets @tr before starting this
+ * function, but in that case, this will be kicked again when
+ * putting it. So we don't care the result.
+ */
+ __remove_instance(tr);
+}
+
+static struct workqueue_struct *autoremove_wq;
+
+static void trace_array_init_autoremove(struct trace_array *tr)
+{
+ INIT_WORK(&tr->autoremove_work, trace_array_autoremove);
+}
+
+static void trace_array_kick_autoremove(struct trace_array *tr)
+{
+ if (!work_pending(&tr->autoremove_work) && autoremove_wq)
+ queue_work(autoremove_wq, &tr->autoremove_work);
+}
+
+static void trace_array_cancel_autoremove(struct trace_array *tr)
+{
+ if (work_pending(&tr->autoremove_work))
+ cancel_work(&tr->autoremove_work);
+}
+
+__init static int trace_array_init_autoremove_wq(void)
+{
+ autoremove_wq = alloc_workqueue("tr_autoremove_wq",
+ WQ_UNBOUND | WQ_HIGHPRI, 0);
+ if (!autoremove_wq) {
+ pr_err("Unable to allocate tr_autoremove_wq\n");
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+late_initcall_sync(trace_array_init_autoremove_wq);
+
LIST_HEAD(ftrace_trace_arrays);
int trace_array_get(struct trace_array *this_tr)
@@ -598,7 +647,7 @@ int trace_array_get(struct trace_array *this_tr)
guard(mutex)(&trace_types_lock);
list_for_each_entry(tr, &ftrace_trace_arrays, list) {
- if (tr == this_tr) {
+ if (tr == this_tr && !tr->free_on_close) {
tr->ref++;
return 0;
}
@@ -611,6 +660,12 @@ static void __trace_array_put(struct trace_array *this_tr)
{
WARN_ON(!this_tr->ref);
this_tr->ref--;
+ /*
+ * When free_on_close is set, prepare removing the array
+ * when the last reference is released.
+ */
+ if (this_tr->ref == 1 && this_tr->free_on_close)
+ trace_array_kick_autoremove(this_tr);
}
/**
@@ -6217,6 +6272,10 @@ static void update_last_data(struct trace_array *tr)
/* Only if the buffer has previous boot data clear and update it. */
tr->flags &= ~TRACE_ARRAY_FL_LAST_BOOT;
+ /* If this is a backup instance, mark it for autoremove. */
+ if (tr->flags & TRACE_ARRAY_FL_VMALLOC)
+ tr->free_on_close = true;
+
/* Reset the module list and reload them */
if (tr->scratch) {
struct trace_scratch *tscratch = tr->scratch;
@@ -10386,6 +10445,8 @@ trace_array_create_systems(const char *name, const char *systems,
if (ftrace_allocate_ftrace_ops(tr) < 0)
goto out_free_tr;
+ trace_array_init_autoremove(tr);
+
ftrace_init_trace_array(tr);
init_trace_flags_index(tr);
@@ -10534,6 +10595,7 @@ static int __remove_instance(struct trace_array *tr)
if (update_marker_trace(tr, 0))
synchronize_rcu();
+ trace_array_cancel_autoremove(tr);
tracing_set_nop(tr);
clear_ftrace_function_probes(tr);
event_trace_del_tracer(tr);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index a098011951cc..947f641a9cf0 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -447,6 +447,12 @@ struct trace_array {
* we do not waste memory on systems that are not using tracing.
*/
bool ring_buffer_expanded;
+ /*
+ * If the ring buffer is a read only backup instance, it will be
+ * removed after dumping all data via pipe, because no readable data.
+ */
+ bool free_on_close;
+ struct work_struct autoremove_work;
};
enum {
^ permalink raw reply related
* [PATCH v3 1/2] tracing: Make the backup instance readonly
From: Masami Hiramatsu (Google) @ 2026-01-13 6:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <176828713313.2161268.62228177827727824.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Since there is no reason to reuse the backup instance, make it
readonly. Note that only backup instances are readonly, because
other trace instances will be empty unless it is writable.
Only backup instances have copy entries from the original.
With this change, most of the trace control files are removed
from the backup instance, including eventfs enable/filter etc.
# find /sys/kernel/tracing/instances/backup/events/ | wc -l
4093
# find /sys/kernel/tracing/instances/boot_map/events/ | wc -l
9573
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v3:
- Resuse the beginning part of event_entries for readonly files.
- Remove readonly file_operations and checking readonly flag in
each write operation.
Changes in v2:
- Use readonly file_operations to prohibit writing instead of
checking flags in write() callbacks.
- Remove writable files from eventfs.
---
kernel/trace/trace.c | 100 ++++++++++++++++++++++++++++++-------------
kernel/trace/trace.h | 8 +++
kernel/trace/trace_boot.c | 5 +-
kernel/trace/trace_events.c | 68 ++++++++++++++++++-----------
4 files changed, 122 insertions(+), 59 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 38f7a7a55c23..2589dac77519 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4888,6 +4888,9 @@ static int tracing_open(struct inode *inode, struct file *file)
int cpu = tracing_get_cpu(inode);
struct array_buffer *trace_buf = &tr->array_buffer;
+ if (trace_array_is_readonly(tr))
+ return -EPERM;
+
#ifdef CONFIG_TRACER_MAX_TRACE
if (tr->current_trace->print_max)
trace_buf = &tr->max_buffer;
@@ -5030,6 +5033,11 @@ static ssize_t
tracing_write_stub(struct file *filp, const char __user *ubuf,
size_t count, loff_t *ppos)
{
+ struct trace_array *tr = file_inode(filp)->i_private;
+
+ if (trace_array_is_readonly(tr))
+ return -EPERM;
+
return count;
}
@@ -5130,6 +5138,9 @@ tracing_cpumask_write(struct file *filp, const char __user *ubuf,
cpumask_var_t tracing_cpumask_new;
int err;
+ if (trace_array_is_readonly(tr))
+ return -EPERM;
+
if (count == 0 || count > KMALLOC_MAX_SIZE)
return -EINVAL;
@@ -6413,6 +6424,9 @@ tracing_set_trace_write(struct file *filp, const char __user *ubuf,
size_t ret;
int err;
+ if (trace_array_is_readonly(tr))
+ return -EPERM;
+
ret = cnt;
if (cnt > MAX_TRACER_SIZE)
@@ -7047,6 +7061,9 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
unsigned long val;
int ret;
+ if (trace_array_is_readonly(tr))
+ return -EPERM;
+
ret = kstrtoul_from_user(ubuf, cnt, 10, &val);
if (ret)
return ret;
@@ -7800,6 +7817,9 @@ static ssize_t tracing_clock_write(struct file *filp, const char __user *ubuf,
const char *clockstr;
int ret;
+ if (trace_array_is_readonly(tr))
+ return -EPERM;
+
if (cnt >= sizeof(buf))
return -EINVAL;
@@ -9353,12 +9373,16 @@ static void
tracing_init_tracefs_percpu(struct trace_array *tr, long cpu)
{
struct dentry *d_percpu = tracing_dentry_percpu(tr, cpu);
+ umode_t writable_mode = TRACE_MODE_WRITE;
struct dentry *d_cpu;
char cpu_dir[30]; /* 30 characters should be more than enough */
if (!d_percpu)
return;
+ if (trace_array_is_readonly(tr))
+ writable_mode = TRACE_MODE_READ;
+
snprintf(cpu_dir, 30, "cpu%ld", cpu);
d_cpu = tracefs_create_dir(cpu_dir, d_percpu);
if (!d_cpu) {
@@ -9371,7 +9395,7 @@ tracing_init_tracefs_percpu(struct trace_array *tr, long cpu)
tr, cpu, &tracing_pipe_fops);
/* per cpu trace */
- trace_create_cpu_file("trace", TRACE_MODE_WRITE, d_cpu,
+ trace_create_cpu_file("trace", writable_mode, d_cpu,
tr, cpu, &tracing_fops);
trace_create_cpu_file("trace_pipe_raw", TRACE_MODE_READ, d_cpu,
@@ -9581,7 +9605,6 @@ struct dentry *trace_create_file(const char *name,
return ret;
}
-
static struct dentry *trace_options_init_dentry(struct trace_array *tr)
{
struct dentry *d_tracer;
@@ -9811,6 +9834,9 @@ rb_simple_write(struct file *filp, const char __user *ubuf,
unsigned long val;
int ret;
+ if (trace_array_is_readonly(tr))
+ return -EPERM;
+
ret = kstrtoul_from_user(ubuf, cnt, 10, &val);
if (ret)
return ret;
@@ -9917,6 +9943,9 @@ buffer_subbuf_size_write(struct file *filp, const char __user *ubuf,
int pages;
int ret;
+ if (trace_array_is_readonly(tr))
+ return -EPERM;
+
ret = kstrtoul_from_user(ubuf, cnt, 10, &val);
if (ret)
return ret;
@@ -10597,47 +10626,61 @@ static __init void create_trace_instances(struct dentry *d_tracer)
static void
init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer)
{
+ umode_t writable_mode = TRACE_MODE_WRITE;
+ bool readonly = trace_array_is_readonly(tr);
int cpu;
+ if (readonly)
+ writable_mode = TRACE_MODE_READ;
+
trace_create_file("available_tracers", TRACE_MODE_READ, d_tracer,
- tr, &show_traces_fops);
+ tr, &show_traces_fops);
- trace_create_file("current_tracer", TRACE_MODE_WRITE, d_tracer,
- tr, &set_tracer_fops);
+ trace_create_file("current_tracer", writable_mode, d_tracer,
+ tr, &set_tracer_fops);
- trace_create_file("tracing_cpumask", TRACE_MODE_WRITE, d_tracer,
+ trace_create_file("tracing_cpumask", writable_mode, d_tracer,
tr, &tracing_cpumask_fops);
+ /* Options are used for changing print-format even for readonly instance. */
trace_create_file("trace_options", TRACE_MODE_WRITE, d_tracer,
tr, &tracing_iter_fops);
- trace_create_file("trace", TRACE_MODE_WRITE, d_tracer,
+ trace_create_file("trace", writable_mode, d_tracer,
tr, &tracing_fops);
trace_create_file("trace_pipe", TRACE_MODE_READ, d_tracer,
tr, &tracing_pipe_fops);
- trace_create_file("buffer_size_kb", TRACE_MODE_WRITE, d_tracer,
+ trace_create_file("buffer_size_kb", writable_mode, d_tracer,
tr, &tracing_entries_fops);
trace_create_file("buffer_total_size_kb", TRACE_MODE_READ, d_tracer,
tr, &tracing_total_entries_fops);
- trace_create_file("free_buffer", 0200, d_tracer,
- tr, &tracing_free_buffer_fops);
+ if (!readonly) {
+ trace_create_file("free_buffer", 0200, d_tracer,
+ tr, &tracing_free_buffer_fops);
- trace_create_file("trace_marker", 0220, d_tracer,
- tr, &tracing_mark_fops);
+ trace_create_file("trace_marker", 0220, d_tracer,
+ tr, &tracing_mark_fops);
- tr->trace_marker_file = __find_event_file(tr, "ftrace", "print");
+ tr->trace_marker_file = __find_event_file(tr, "ftrace", "print");
- trace_create_file("trace_marker_raw", 0220, d_tracer,
- tr, &tracing_mark_raw_fops);
+ trace_create_file("trace_marker_raw", 0220, d_tracer,
+ tr, &tracing_mark_raw_fops);
- trace_create_file("trace_clock", TRACE_MODE_WRITE, d_tracer, tr,
+ trace_create_file("buffer_percent", TRACE_MODE_WRITE, d_tracer,
+ tr, &buffer_percent_fops);
+
+ trace_create_file("syscall_user_buf_size", TRACE_MODE_WRITE, d_tracer,
+ tr, &tracing_syscall_buf_fops);
+ }
+
+ trace_create_file("trace_clock", writable_mode, d_tracer, tr,
&trace_clock_fops);
- trace_create_file("tracing_on", TRACE_MODE_WRITE, d_tracer,
+ trace_create_file("tracing_on", writable_mode, d_tracer,
tr, &rb_simple_fops);
trace_create_file("timestamp_mode", TRACE_MODE_READ, d_tracer, tr,
@@ -10645,41 +10688,38 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer)
tr->buffer_percent = 50;
- trace_create_file("buffer_percent", TRACE_MODE_WRITE, d_tracer,
- tr, &buffer_percent_fops);
-
- trace_create_file("buffer_subbuf_size_kb", TRACE_MODE_WRITE, d_tracer,
+ trace_create_file("buffer_subbuf_size_kb", writable_mode, d_tracer,
tr, &buffer_subbuf_size_fops);
- trace_create_file("syscall_user_buf_size", TRACE_MODE_WRITE, d_tracer,
- tr, &tracing_syscall_buf_fops);
-
create_trace_options_dir(tr);
#ifdef CONFIG_TRACER_MAX_TRACE
- trace_create_maxlat_file(tr, d_tracer);
+ if (!readonly)
+ trace_create_maxlat_file(tr, d_tracer);
#endif
- if (ftrace_create_function_files(tr, d_tracer))
+ if (!readonly && ftrace_create_function_files(tr, d_tracer))
MEM_FAIL(1, "Could not allocate function filter files");
if (tr->range_addr_start) {
trace_create_file("last_boot_info", TRACE_MODE_READ, d_tracer,
tr, &last_boot_fops);
#ifdef CONFIG_TRACER_SNAPSHOT
- } else {
+ } else if (!readonly) {
trace_create_file("snapshot", TRACE_MODE_WRITE, d_tracer,
tr, &snapshot_fops);
#endif
}
- trace_create_file("error_log", TRACE_MODE_WRITE, d_tracer,
- tr, &tracing_err_log_fops);
+ if (!readonly)
+ trace_create_file("error_log", TRACE_MODE_WRITE, d_tracer,
+ tr, &tracing_err_log_fops);
for_each_tracing_cpu(cpu)
tracing_init_tracefs_percpu(tr, cpu);
- ftrace_init_tracefs(tr, d_tracer);
+ if (!readonly)
+ ftrace_init_tracefs(tr, d_tracer);
}
#ifdef CONFIG_TRACEFS_AUTOMOUNT_DEPRECATED
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b6d42fe06115..a098011951cc 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -33,6 +33,7 @@
#define TRACE_MODE_WRITE 0640
#define TRACE_MODE_READ 0440
+#define TRACE_MODE_WRITE_MASK (TRACE_MODE_WRITE & ~TRACE_MODE_READ)
enum trace_type {
__TRACE_FIRST_TYPE = 0,
@@ -483,6 +484,12 @@ extern bool trace_clock_in_ns(struct trace_array *tr);
extern unsigned long trace_adjust_address(struct trace_array *tr, unsigned long addr);
+static inline bool trace_array_is_readonly(struct trace_array *tr)
+{
+ /* backup instance is read only. */
+ return tr->flags & TRACE_ARRAY_FL_VMALLOC;
+}
+
/*
* The global tracer (top) should be the first trace array added,
* but we check the flag anyway.
@@ -681,7 +688,6 @@ struct dentry *trace_create_file(const char *name,
void *data,
const struct file_operations *fops);
-
/**
* tracer_tracing_is_on_cpu - show real state of ring buffer enabled on for a cpu
* @tr : the trace array to know if ring buffer is enabled
diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
index dbe29b4c6a7a..2ca2541c8a58 100644
--- a/kernel/trace/trace_boot.c
+++ b/kernel/trace/trace_boot.c
@@ -61,7 +61,8 @@ trace_boot_set_instance_options(struct trace_array *tr, struct xbc_node *node)
v = memparse(p, NULL);
if (v < PAGE_SIZE)
pr_err("Buffer size is too small: %s\n", p);
- if (tracing_resize_ring_buffer(tr, v, RING_BUFFER_ALL_CPUS) < 0)
+ if (trace_array_is_readonly(tr) ||
+ tracing_resize_ring_buffer(tr, v, RING_BUFFER_ALL_CPUS) < 0)
pr_err("Failed to resize trace buffer to %s\n", p);
}
@@ -597,7 +598,7 @@ trace_boot_enable_tracer(struct trace_array *tr, struct xbc_node *node)
p = xbc_node_find_value(node, "tracer", NULL);
if (p && *p != '\0') {
- if (tracing_set_tracer(tr, p) < 0)
+ if (trace_array_is_readonly(tr) || tracing_set_tracer(tr, p) < 0)
pr_err("Failed to set given tracer: %s\n", p);
}
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 9b07ad9eb284..5a9e03470b03 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1379,6 +1379,9 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
{
int ret;
+ if (trace_array_is_readonly(tr))
+ return -EPERM;
+
mutex_lock(&event_mutex);
ret = __ftrace_set_clr_event_nolock(tr, match, sub, event, set, mod);
mutex_unlock(&event_mutex);
@@ -2817,8 +2820,8 @@ event_subsystem_dir(struct trace_array *tr, const char *name,
} else
__get_system(system);
- /* ftrace only has directories no files */
- if (strcmp(name, "ftrace") == 0)
+ /* ftrace only has directories no files, readonly instance too. */
+ if (strcmp(name, "ftrace") == 0 || trace_array_is_readonly(tr))
nr_entries = 0;
else
nr_entries = ARRAY_SIZE(system_entries);
@@ -2983,28 +2986,30 @@ event_create_dir(struct eventfs_inode *parent, struct trace_event_file *file)
int ret;
static struct eventfs_entry event_entries[] = {
{
- .name = "enable",
+ .name = "format",
.callback = event_callback,
- .release = event_release,
},
+#ifdef CONFIG_PERF_EVENTS
{
- .name = "filter",
+ .name = "id",
.callback = event_callback,
},
+#endif
+#define NR_RO_EVENT_ENTRIES (1 + IS_ENABLED(CONFIG_PERF_EVENTS))
+/* Readonly files must be above this line and counted by NR_RO_EVENT_ENTRIES. */
{
- .name = "trigger",
+ .name = "enable",
.callback = event_callback,
+ .release = event_release,
},
{
- .name = "format",
+ .name = "filter",
.callback = event_callback,
},
-#ifdef CONFIG_PERF_EVENTS
{
- .name = "id",
+ .name = "trigger",
.callback = event_callback,
},
-#endif
#ifdef CONFIG_HIST_TRIGGERS
{
.name = "hist",
@@ -3037,9 +3042,13 @@ event_create_dir(struct eventfs_inode *parent, struct trace_event_file *file)
if (!e_events)
return -ENOMEM;
- nr_entries = ARRAY_SIZE(event_entries);
+ if (trace_array_is_readonly(tr))
+ nr_entries = NR_RO_EVENT_ENTRIES;
+ else
+ nr_entries = ARRAY_SIZE(event_entries);
name = trace_event_name(call);
+
ei = eventfs_create_dir(name, e_events, event_entries, nr_entries, file);
if (IS_ERR(ei)) {
pr_warn("Could not create tracefs '%s' directory\n", name);
@@ -4381,25 +4390,25 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
int nr_entries;
static struct eventfs_entry events_entries[] = {
{
- .name = "enable",
+ .name = "header_page",
.callback = events_callback,
},
{
- .name = "header_page",
+ .name = "header_event",
.callback = events_callback,
},
+#define NR_RO_TOP_ENTRIES 2
+/* Readonly files must be above this line and counted by NR_RO_TOP_ENTRIES. */
{
- .name = "header_event",
+ .name = "enable",
.callback = events_callback,
},
};
- entry = trace_create_file("set_event", TRACE_MODE_WRITE, parent,
- tr, &ftrace_set_event_fops);
- if (!entry)
- return -ENOMEM;
-
- nr_entries = ARRAY_SIZE(events_entries);
+ if (trace_array_is_readonly(tr))
+ nr_entries = NR_RO_TOP_ENTRIES;
+ else
+ nr_entries = ARRAY_SIZE(events_entries);
e_events = eventfs_create_events_dir("events", parent, events_entries,
nr_entries, tr);
@@ -4408,15 +4417,22 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
return -ENOMEM;
}
- /* There are not as crucial, just warn if they are not created */
+ if (!trace_array_is_readonly(tr)) {
- trace_create_file("set_event_pid", TRACE_MODE_WRITE, parent,
- tr, &ftrace_set_event_pid_fops);
+ entry = trace_create_file("set_event", TRACE_MODE_WRITE, parent,
+ tr, &ftrace_set_event_fops);
+ if (!entry)
+ return -ENOMEM;
+
+ /* There are not as crucial, just warn if they are not created */
- trace_create_file("set_event_notrace_pid",
- TRACE_MODE_WRITE, parent, tr,
- &ftrace_set_event_notrace_pid_fops);
+ trace_create_file("set_event_pid", TRACE_MODE_WRITE, parent,
+ tr, &ftrace_set_event_pid_fops);
+ trace_create_file("set_event_notrace_pid",
+ TRACE_MODE_WRITE, parent, tr,
+ &ftrace_set_event_notrace_pid_fops);
+ }
tr->event_dir = e_events;
return 0;
^ permalink raw reply related
* [PATCH v3 0/2] tracing: Remove backup instance after read all
From: Masami Hiramatsu (Google) @ 2026-01-13 6:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
Hi,
Here is the 3rd version of the series to improve backup instances of
the persistent ring buffer. The previous version is here:
https://lore.kernel.org/all/176788217131.1398317.11144318616426272901.stgit@mhiramat.tok.corp.google.com/
In this version, I updated [1/2] to back to checking readonly
flag and consolidate the eventfs_entries.
Since backup instances are a kind of snapshot of the persistent
ring buffer, it should be readonly. And if it is readonly
there is no reason to keep it after reading all data via trace_pipe
because the data has been consumed.
Thus, [1/2] makes backup instances readonly (not able to write any
events, cleanup trace, change buffer size). Also, [2/2] removes the
backup instance after consuming all data via trace_pipe.
With this improvements, even if we makes a backup instance (using
the same amount of memory of the persistent ring buffer), it will
be removed after reading the data automatically.
---
Masami Hiramatsu (Google) (2):
tracing: Make the backup instance readonly
tracing: Add autoremove feature to the backup instance
kernel/trace/trace.c | 164 +++++++++++++++++++++++++++++++++++--------
kernel/trace/trace.h | 14 +++-
kernel/trace/trace_boot.c | 5 +
kernel/trace/trace_events.c | 68 +++++++++++-------
4 files changed, 191 insertions(+), 60 deletions(-)
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/2] tracing: Make the backup instance readonly
From: Masami Hiramatsu @ 2026-01-13 6:04 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel
In-Reply-To: <20260112194722.19164ca7@gandalf.local.home>
On Mon, 12 Jan 2026 19:47:22 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 12 Jan 2026 19:45:51 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > Let's not add any flags to the eventfs_entry. That is created for every
> > event, and events are already too big in size. I don't want to increase the
> > size of each event for this feature.
>
> Actually, there's not many of these. But they are all static variables. How
> are you going to differentiate them?
Since the format/id files are always readonly, so we can put a mode flag on
each entry.
Anyway, I will just share the readonly entries in the same array, but
put them at the beginning of it.
Let me update it to v3.
Thanks,
>
> -- Steve
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 3/3] PCI: dw-rockchip: Add pcie_ltssm_state_transition trace support
From: Shawn Lin @ 2026-01-13 3:55 UTC (permalink / raw)
To: Steven Rostedt
Cc: shawn.lin, Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip,
linux-pci, linux-trace-kernel, linux-doc, Masami Hiramatsu
In-Reply-To: <20260112101644.5c1b772a@gandalf.local.home>
Hi Steven,
在 2026/01/12 星期一 23:16, Steven Rostedt 写道:
> On Mon, 12 Jan 2026 09:20:00 +0800
> Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
>> Rockchip platforms provide a 64x4 bytes debug FIFO to trace the
>> LTSSM history. Any LTSSM change will be recorded. It's userful
>> for debug purpose, for example link failure, etc.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>> Changes in v3:
>> - reorder variables(Mani)
>> - rename loop to i; rename en to enable(Mani)
>> - use FIELD_GET(Mani)
>> - add comment about how the FIFO works(Mani)
>>
>> Changes in v2:
>> - use tracepoint
>>
>> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 104 ++++++++++++++++++++++++++
>> 1 file changed, 104 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> index 352f513..344e0b9 100644
>> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
>> @@ -22,6 +22,8 @@
>> #include <linux/platform_device.h>
>> #include <linux/regmap.h>
>> #include <linux/reset.h>
>> +#include <linux/workqueue.h>
>> +#include <trace/events/pci_controller.h>
>>
>> #include "../../pci.h"
>> #include "pcie-designware.h"
>> @@ -73,6 +75,20 @@
>> #define PCIE_CLIENT_CDM_RASDES_TBA_L1_1 BIT(4)
>> #define PCIE_CLIENT_CDM_RASDES_TBA_L1_2 BIT(5)
>>
>> +/* Debug FIFO information */
>> +#define PCIE_CLIENT_DBG_FIFO_MODE_CON 0x310
>> +#define PCIE_CLIENT_DBG_EN 0xffff0007
>> +#define PCIE_CLIENT_DBG_DIS 0xffff0000
>> +#define PCIE_CLIENT_DBG_FIFO_PTN_HIT_D0 0x320
>> +#define PCIE_CLIENT_DBG_FIFO_PTN_HIT_D1 0x324
>> +#define PCIE_CLIENT_DBG_FIFO_TRN_HIT_D0 0x328
>> +#define PCIE_CLIENT_DBG_FIFO_TRN_HIT_D1 0x32c
>> +#define PCIE_CLIENT_DBG_TRANSITION_DATA 0xffff0000
>> +#define PCIE_CLIENT_DBG_FIFO_STATUS 0x350
>> +#define PCIE_DBG_FIFO_RATE_MASK GENMASK(22, 20)
>> +#define PCIE_DBG_FIFO_L1SUB_MASK GENMASK(10, 8)
>> +#define PCIE_DBG_LTSSM_HISTORY_CNT 64
>> +
>> /* Hot Reset Control Register */
>> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
>> #define PCIE_LTSSM_APP_DLY2_EN BIT(1)
>> @@ -96,6 +112,7 @@ struct rockchip_pcie {
>> struct irq_domain *irq_domain;
>> const struct rockchip_pcie_of_data *data;
>> bool supports_clkreq;
>> + struct delayed_work trace_work;
>> };
>>
>> struct rockchip_pcie_of_data {
>> @@ -206,6 +223,89 @@ static enum dw_pcie_ltssm rockchip_pcie_get_ltssm(struct dw_pcie *pci)
>> return rockchip_pcie_get_ltssm_reg(rockchip) & PCIE_LTSSM_STATUS_MASK;
>> }
>>
>> +#ifdef CONFIG_TRACING
>> +static void rockchip_pcie_ltssm_trace_work(struct work_struct *work)
>> +{
>> + struct rockchip_pcie *rockchip = container_of(work, struct rockchip_pcie,
>> + trace_work.work);
>> + struct dw_pcie *pci = &rockchip->pci;
>> + enum dw_pcie_ltssm state;
>> + u32 i, l1ss, prev_val = DW_PCIE_LTSSM_UNKNOWN, rate, val;
>> +
>> + for (i = 0; i < PCIE_DBG_LTSSM_HISTORY_CNT; i++) {
>> + val = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_DBG_FIFO_STATUS);
>> + rate = FIELD_GET(PCIE_DBG_FIFO_RATE_MASK, val);
>> + l1ss = FIELD_GET(PCIE_DBG_FIFO_L1SUB_MASK, val);
>> + val = FIELD_GET(PCIE_LTSSM_STATUS_MASK, val);
>> +
>> + /*
>> + * Hardware Mechanism: The ring FIFO employs two tracking counters:
>> + * - 'last-read-point': maintains the user's last read position
>> + * - 'last-valid-point': tracks the hardware's last state update
>> + *
>> + * Software Handling: When two consecutive LTSSM states are identical,
>> + * it indicates invalid subsequent data in the FIFO. In this case, we
>> + * skip the remaining entries. The dual-counter design ensures that on
>> + * the next state transition, reading can resume from the last user
>> + * position.
>> + */
>> + if ((i > 0 && val == prev_val) || val > DW_PCIE_LTSSM_RCVRY_EQ3)
>> + break;
>> +
>> + state = prev_val = val;
>> + if (val == DW_PCIE_LTSSM_L1_IDLE) {
>> + if (l1ss == 2)
>> + state = DW_PCIE_LTSSM_L1_2;
>> + else if (l1ss == 1)
>> + state = DW_PCIE_LTSSM_L1_1;
>> + }
>> +
>> + trace_pcie_ltssm_state_transition(dev_name(pci->dev),
>> + dw_pcie_ltssm_status_string(state),
>> + ((rate + 1) > pci->max_link_speed) ?
>> + PCI_SPEED_UNKNOWN : PCIE_SPEED_2_5GT + rate);
>> + }
>
> Does it make sense to call this work function every 5 seconds when the
> tracepoint isn't enabled?
>
That's a good question. We don't need to read fifo and call
trace_pcie_ltssm_state_transition if tracepoint isn't enabled.
Will improve in v4.
> You can add a function callback to when the tracepoint is enabled by defining:
>
> TRACE_EVENT_FN(<name>
> TP_PROTO(..)
> TP_ARGS(..)
> TP_STRUCT__entry(..)
> TP_fast_assign(..)
> TP_printk(..)
>
> reg,
> unreg)
>
> reg() gets called when the tracepoint is first enabled. This could be where
> you can start the work function. And unreg() would stop it.
>
As how to start/stop it may vary from host to host, so I think we could
use reg()/unreg() to set a status to indicate whether this tracepoint is
enabled. Then the host drivers could decide how to implement trace work
based on it.
> You would likely need to also include state variables as I guess you don't
> want to start it if the link is down. Also, if the tracepoint is enabled
> when the link goes up you want to start the work queue.
Frankly, I do want to start it if the link is down as the link may come
up later and that will make us able to dump the transition in time.
>
> I would recommend this so that you don't call this work function when it's
> not doing anything useful.
Sure, very appreciate your suggestion.
Thanks.
>
> -- Steve
>
>
>
>> +
>> + schedule_delayed_work(&rockchip->trace_work, msecs_to_jiffies(5000));
>> +}
>> +
>
^ permalink raw reply
* Re: [PATCH] tracing: Have show_event_trigger/filter format a bit more in columns
From: Aaron Tomlin @ 2026-01-13 1:54 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers
In-Reply-To: <20260112153408.18373e73@gandalf.local.home>
[-- Attachment #1: Type: text/plain, Size: 4539 bytes --]
On Mon, Jan 12, 2026 at 03:34:08PM -0500, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> By doing:
>
> # trace-cmd sqlhist -e -n futex_wait select TIMESTAMP_DELTA_USECS as lat from sys_enter_futex as start join sys_exit_futex as end on start.common_pid = end.common_pid
>
> and
>
> # trace-cmd start -e futex_wait -f 'lat > 100' -e page_pool_state_release -f 'pfn == 1'
>
> The output of the show_event_trigger and show_event_filter files are well
> aligned because of the inconsistent 'tab' spacing:
>
> ~# cat /sys/kernel/tracing/show_event_triggers
> syscalls:sys_exit_futex hist:keys=common_pid:vals=hitcount:__lat_12046_2=common_timestamp.usecs-$__arg_12046_1:sort=hitcount:size=2048:clock=global:onmatch(syscalls.sys_enter_futex).trace(futex_wait,$__lat_12046_2) [active]
> syscalls:sys_enter_futex hist:keys=common_pid:vals=hitcount:__arg_12046_1=common_timestamp.usecs:sort=hitcount:size=2048:clock=global [active]
>
> ~# cat /sys/kernel/tracing/show_event_filters
> synthetic:futex_wait (lat > 100)
> page_pool:page_pool_state_release (pfn == 1)
>
> This makes it not so easy to read. Instead, force the spacing to be at
> least 32 bytes from the beginning (one space if the system:event is longer
> than 30 bytes):
>
> ~# cat /sys/kernel/tracing/show_event_triggers
> syscalls:sys_exit_futex hist:keys=common_pid:vals=hitcount:__lat_8125_2=common_timestamp.usecs-$__arg_8125_1:sort=hitcount:size=2048:clock=global:onmatch(syscalls.sys_enter_futex).trace(futex_wait,$__lat_8125_2) [active]
> syscalls:sys_enter_futex hist:keys=common_pid:vals=hitcount:__arg_8125_1=common_timestamp.usecs:sort=hitcount:size=2048:clock=global [active]
>
> ~# cat /sys/kernel/tracing/show_event_filters
> synthetic:futex_wait (lat > 100)
> page_pool:page_pool_state_release (pfn == 1)
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> [
> This is based on top of these patches:
> https://lore.kernel.org/linux-trace-kernel/20260105142939.2655342-1-atomlin@atomlin.com/
> ]
> kernel/trace/trace_events.c | 26 ++++++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 36936697fa2a..f372a6374164 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1662,6 +1662,18 @@ static void t_stop(struct seq_file *m, void *p)
> mutex_unlock(&event_mutex);
> }
>
> +static int get_call_len(struct trace_event_call *call)
> +{
> + int len;
> +
> + /* Get the length of "<system>:<event>" */
> + len = strlen(call->class->system) + 1;
> + len += strlen(trace_event_name(call));
> +
> + /* Set the index to 32 bytes to separate event from data */
> + return len >= 32 ? 1 : 32 - len;
> +}
> +
> /**
> * t_show_filters - seq_file callback to display active event filters
> * @m: The seq_file interface for formatted output
> @@ -1676,14 +1688,17 @@ static int t_show_filters(struct seq_file *m, void *v)
> struct trace_event_file *file = v;
> struct trace_event_call *call = file->event_call;
> struct event_filter *filter;
> + int len;
>
> guard(rcu)();
> filter = rcu_dereference(file->filter);
> if (!filter || !filter->filter_string)
> return 0;
>
> - seq_printf(m, "%s:%s\t%s\n", call->class->system,
> - trace_event_name(call), filter->filter_string);
> + len = get_call_len(call);
> +
> + seq_printf(m, "%s:%s%*.s%s\n", call->class->system,
> + trace_event_name(call), len, "", filter->filter_string);
>
> return 0;
> }
> @@ -1702,6 +1717,7 @@ static int t_show_triggers(struct seq_file *m, void *v)
> struct trace_event_file *file = v;
> struct trace_event_call *call = file->event_call;
> struct event_trigger_data *data;
> + int len;
>
> /*
> * The event_mutex is held by t_start(), protecting the
> @@ -1710,9 +1726,11 @@ static int t_show_triggers(struct seq_file *m, void *v)
> if (list_empty(&file->triggers))
> return 0;
>
> + len = get_call_len(call);
> +
> list_for_each_entry_rcu(data, &file->triggers, list) {
> - seq_printf(m, "%s:%s\t", call->class->system,
> - trace_event_name(call));
> + seq_printf(m, "%s:%s%*.s", call->class->system,
> + trace_event_name(call), len, "");
>
> data->cmd_ops->print(m, data);
> }
> --
> 2.51.0
>
Nice!
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
--
Aaron Tomlin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] kprobes: Use dedicated kthread for kprobe optimizer
From: Masami Hiramatsu @ 2026-01-13 1:49 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Steven Rostedt, Naveen N Rao, David S . Miller, linux-kernel,
linux-trace-kernel
In-Reply-To: <176805562740.355924.11587476104510049915.stgit@devnote2>
On Sat, 10 Jan 2026 23:33:47 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> +static int kprobe_optimizer_thread(void *data)
> +{
> + set_freezable();
Steve pointed that the kthread usually don't need to be freezable and
the workqueue is not freezable too. So I decided to remove this.
> + while (!kthread_should_stop()) {
> + wait_event_freezable(kprobe_optimizer_wait,
> + atomic_read(&optimizer_state) != OPTIMIZER_ST_IDLE ||
> + kthread_should_stop());
> +
> + if (kthread_should_stop())
> + break;
> +
> + if (try_to_freeze())
> + continue;
> +
> + /*
> + * If it was a normal kick, wait for OPTIMIZE_DELAY.
> + * This wait can be interrupted by a flush request.
> + */
> + if (atomic_read(&optimizer_state) == 1)
> + wait_event_freezable_timeout(kprobe_optimizer_wait,
> + atomic_read(&optimizer_state) == OPTIMIZER_ST_FLUSHING ||
> + kthread_should_stop(),
> + OPTIMIZE_DELAY);
Also, forgot to add kthread_should_stop() check here.
Let me update to v2.
Thanks,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v2] kprobes: Use dedicated kthread for kprobe optimizer
From: Masami Hiramatsu (Google) @ 2026-01-13 1:47 UTC (permalink / raw)
To: Steven Rostedt, Naveen N Rao, David S . Miller, Masami Hiramatsu
Cc: linux-kernel, linux-trace-kernel
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Instead of using generic workqueue, use a dedicated kthread for optimizing
kprobes, because it can wait (sleep) for a long time inside the process
by synchronize_rcu_task(). This means other works can be stopped until it
finishes.
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v2:
- Make the kthread unfreezable same as workqueue.
- Add kthread_should_stop() check right before calling optimizer too.
- Initialize optimizer_state to OPTIMIZER_ST_IDLE instead of 0.
---
kernel/kprobes.c | 104 ++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 84 insertions(+), 20 deletions(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ab8f9fc1f0d1..4778bfca19b8 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -32,6 +32,7 @@
#include <linux/debugfs.h>
#include <linux/sysctl.h>
#include <linux/kdebug.h>
+#include <linux/kthread.h>
#include <linux/memory.h>
#include <linux/ftrace.h>
#include <linux/cpu.h>
@@ -40,6 +41,7 @@
#include <linux/perf_event.h>
#include <linux/execmem.h>
#include <linux/cleanup.h>
+#include <linux/wait.h>
#include <asm/sections.h>
#include <asm/cacheflush.h>
@@ -514,8 +516,17 @@ static LIST_HEAD(optimizing_list);
static LIST_HEAD(unoptimizing_list);
static LIST_HEAD(freeing_list);
-static void kprobe_optimizer(struct work_struct *work);
-static DECLARE_DELAYED_WORK(optimizing_work, kprobe_optimizer);
+static struct task_struct *kprobe_optimizer_task;
+static wait_queue_head_t kprobe_optimizer_wait;
+static atomic_t optimizer_state;
+enum {
+ OPTIMIZER_ST_IDLE = 0,
+ OPTIMIZER_ST_KICKED = 1,
+ OPTIMIZER_ST_FLUSHING = 2,
+};
+
+static DECLARE_COMPLETION(optimizer_completion);
+
#define OPTIMIZE_DELAY 5
/*
@@ -597,14 +608,10 @@ static void do_free_cleaned_kprobes(void)
}
}
-/* Start optimizer after OPTIMIZE_DELAY passed */
-static void kick_kprobe_optimizer(void)
-{
- schedule_delayed_work(&optimizing_work, OPTIMIZE_DELAY);
-}
+static void kick_kprobe_optimizer(void);
/* Kprobe jump optimizer */
-static void kprobe_optimizer(struct work_struct *work)
+static void kprobe_optimizer(void)
{
guard(mutex)(&kprobe_mutex);
@@ -635,9 +642,51 @@ static void kprobe_optimizer(struct work_struct *work)
do_free_cleaned_kprobes();
}
- /* Step 5: Kick optimizer again if needed */
+ /* Step 5: Kick optimizer again if needed. But if there is a flush requested, */
+ if (completion_done(&optimizer_completion))
+ complete(&optimizer_completion);
+
if (!list_empty(&optimizing_list) || !list_empty(&unoptimizing_list))
- kick_kprobe_optimizer();
+ kick_kprobe_optimizer(); /*normal kick*/
+}
+
+static int kprobe_optimizer_thread(void *data)
+{
+ while (!kthread_should_stop()) {
+ wait_event(kprobe_optimizer_wait,
+ atomic_read(&optimizer_state) != OPTIMIZER_ST_IDLE ||
+ kthread_should_stop());
+
+ if (kthread_should_stop())
+ break;
+
+ /*
+ * If it was a normal kick, wait for OPTIMIZE_DELAY.
+ * This wait can be interrupted by a flush request.
+ */
+ if (atomic_read(&optimizer_state) == 1)
+ wait_event_timeout(kprobe_optimizer_wait,
+ atomic_read(&optimizer_state) == OPTIMIZER_ST_FLUSHING ||
+ kthread_should_stop(),
+ OPTIMIZE_DELAY);
+
+ if (kthread_should_stop())
+ break;
+
+ atomic_set(&optimizer_state, OPTIMIZER_ST_IDLE);
+
+ kprobe_optimizer();
+ }
+ return 0;
+}
+
+/* Start optimizer after OPTIMIZE_DELAY passed */
+static void kick_kprobe_optimizer(void)
+{
+ lockdep_assert_held(&kprobe_mutex);
+ if (atomic_cmpxchg(&optimizer_state,
+ OPTIMIZER_ST_IDLE, OPTIMIZER_ST_KICKED) == OPTIMIZER_ST_IDLE)
+ wake_up(&kprobe_optimizer_wait);
}
static void wait_for_kprobe_optimizer_locked(void)
@@ -645,13 +694,17 @@ static void wait_for_kprobe_optimizer_locked(void)
lockdep_assert_held(&kprobe_mutex);
while (!list_empty(&optimizing_list) || !list_empty(&unoptimizing_list)) {
- mutex_unlock(&kprobe_mutex);
-
- /* This will also make 'optimizing_work' execute immmediately */
- flush_delayed_work(&optimizing_work);
- /* 'optimizing_work' might not have been queued yet, relax */
- cpu_relax();
+ init_completion(&optimizer_completion);
+ /*
+ * Set state to OPTIMIZER_ST_FLUSHING and wake up the thread if it's
+ * idle. If it's already kicked, it will see the state change.
+ */
+ if (atomic_xchg_acquire(&optimizer_state,
+ OPTIMIZER_ST_FLUSHING) != OPTIMIZER_ST_FLUSHING)
+ wake_up(&kprobe_optimizer_wait);
+ mutex_unlock(&kprobe_mutex);
+ wait_for_completion(&optimizer_completion);
mutex_lock(&kprobe_mutex);
}
}
@@ -1010,8 +1063,21 @@ static void __disarm_kprobe(struct kprobe *p, bool reopt)
*/
}
+static void __init init_optprobe(void)
+{
+#ifdef __ARCH_WANT_KPROBES_INSN_SLOT
+ /* Init 'kprobe_optinsn_slots' for allocation */
+ kprobe_optinsn_slots.insn_size = MAX_OPTINSN_SIZE;
+#endif
+
+ init_waitqueue_head(&kprobe_optimizer_wait);
+ atomic_set(&optimizer_state, OPTIMIZER_ST_IDLE);
+ kprobe_optimizer_task = kthread_run(kprobe_optimizer_thread, NULL,
+ "kprobe-optimizer");
+}
#else /* !CONFIG_OPTPROBES */
+#define init_optprobe() do {} while (0)
#define optimize_kprobe(p) do {} while (0)
#define unoptimize_kprobe(p, f) do {} while (0)
#define kill_optimized_kprobe(p) do {} while (0)
@@ -2694,10 +2760,8 @@ static int __init init_kprobes(void)
/* By default, kprobes are armed */
kprobes_all_disarmed = false;
-#if defined(CONFIG_OPTPROBES) && defined(__ARCH_WANT_KPROBES_INSN_SLOT)
- /* Init 'kprobe_optinsn_slots' for allocation */
- kprobe_optinsn_slots.insn_size = MAX_OPTINSN_SIZE;
-#endif
+ /* Initialize the optimization infrastructure */
+ init_optprobe();
err = arch_init_kprobes();
if (!err)
^ permalink raw reply related
* Re: [PATCH bpf-next 2/3] bpf: Introduce BPF_BRANCH_SNAPSHOT_F_COPY flag for bpf_get_branch_snapshot helper
From: Leon Hwang @ 2026-01-13 1:43 UTC (permalink / raw)
To: kernel test robot, bpf
Cc: llvm, oe-kbuild-all, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, David S . Miller, David Ahern,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H . Peter Anvin, Matt Bobrowski, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Shuah Khan, netdev, linux-kernel,
linux-trace-kernel
In-Reply-To: <202601122013.hmoeIXXs-lkp@intel.com>
Hi kernel test robot,
Thanks for the test results.
No further testing is needed for this patch series, as it will not be
pursued further and is not expected to be accepted upstream.
Thanks again for the testing effort.
Thanks,
Leon
On 13/1/26 03:22, kernel test robot wrote:
> Hi Leon,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on bpf-next/master]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Leon-Hwang/bpf-x64-Call-perf_snapshot_branch_stack-in-trampoline/20260109-234435
> base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> patch link: https://lore.kernel.org/r/20260109153420.32181-3-leon.hwang%40linux.dev
> patch subject: [PATCH bpf-next 2/3] bpf: Introduce BPF_BRANCH_SNAPSHOT_F_COPY flag for bpf_get_branch_snapshot helper
> config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260112/202601122013.hmoeIXXs-lkp@intel.com/config)
> compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260112/202601122013.hmoeIXXs-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202601122013.hmoeIXXs-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>>> ld.lld: error: undefined symbol: bpf_branch_snapshot
> >>> referenced by bpf_trace.c:1182 (kernel/trace/bpf_trace.c:1182)
> >>> vmlinux.o:(bpf_get_branch_snapshot)
> >>> referenced by bpf_trace.c:0 (kernel/trace/bpf_trace.c:0)
> >>> vmlinux.o:(bpf_get_branch_snapshot)
>
^ permalink raw reply
* Re: [PATCH v3] ftrace: Do not over-allocate ftrace memory
From: Steven Rostedt @ 2026-01-13 1:38 UTC (permalink / raw)
To: Guenter Roeck
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <bde55f7c-7f13-4bb3-8a54-942704d4fe37@roeck-us.net>
On Mon, 12 Jan 2026 16:15:56 -0800
Guenter Roeck <linux@roeck-us.net> wrote:
> > v3: Restore the code calculating the number of unused pages due to skipped
> > entries. Use the actual number of entries per page group when
> > calculating the number of entries in unused page groups.
> > v2: Have ftrace_allocate_pages() return the number of allocated pages,
> > and drop the page count calculation code as well as the associated
> > warnings from ftrace_process_locs().
>
> Any additional feedback / comments / suggestions ?
Yeah, sorry got caught up in other work.
> > -static int ftrace_allocate_records(struct ftrace_page *pg, int count)
> > +static int ftrace_allocate_records(struct ftrace_page *pg, int count,
> > + unsigned long *num_pages)
> > {
> > int order;
> > int pages;
> > @@ -3844,7 +3844,7 @@ static int ftrace_allocate_records(struct ftrace_page *pg, int count)
> > return -EINVAL;
> >
> > /* We want to fill as much as possible, with no empty pages */
> > - pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);
> > + pages = DIV_ROUND_UP(count * ENTRY_SIZE, PAGE_SIZE);
> > order = fls(pages) - 1;
> >
> > again:
> > @@ -3859,6 +3859,7 @@ static int ftrace_allocate_records(struct ftrace_page *pg, int count)
> > }
> >
> > ftrace_number_of_pages += 1 << order;
> > + *num_pages += 1 << order;
> > ftrace_number_of_groups++;
> >
> > cnt = (PAGE_SIZE << order) / ENTRY_SIZE;
> > @@ -3887,12 +3888,14 @@ static void ftrace_free_pages(struct ftrace_page *pages)
> > }
> >
> > static struct ftrace_page *
> > -ftrace_allocate_pages(unsigned long num_to_init)
> > +ftrace_allocate_pages(unsigned long num_to_init, unsigned long *pages)
Small nit.
Can you rename this to num_pages? That way it is consistent with the above.
Both this function and ftrace_allocate_records() has this as a pointer,
whereas below it's an unsigned long. I rather have the pointer names be
consistent than having "pages" be a pointer here and an unsigned long where
it is called.
Thanks,
-- Steve
> > {
> > struct ftrace_page *start_pg;
> > struct ftrace_page *pg;
> > int cnt;
> >
> > + *pages = 0;
> > +
> > if (!num_to_init)
> > return NULL;
> >
> > @@ -3906,7 +3909,7 @@ ftrace_allocate_pages(unsigned long num_to_init)
> > * waste as little space as possible.
> > */
> > for (;;) {
> > - cnt = ftrace_allocate_records(pg, num_to_init);
> > + cnt = ftrace_allocate_records(pg, num_to_init, pages);
> > if (cnt < 0)
> > goto free_pages;
> >
> > @@ -7192,8 +7195,6 @@ static int ftrace_process_locs(struct module *mod,
> > if (!count)
> > return 0;
> >
> > - pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);
> > -
> > /*
> > * Sorting mcount in vmlinux at build time depend on
> > * CONFIG_BUILDTIME_MCOUNT_SORT, while mcount loc in
> > @@ -7206,7 +7207,7 @@ static int ftrace_process_locs(struct module *mod,
> > test_is_sorted(start, count);
> > }
> >
> > - start_pg = ftrace_allocate_pages(count);
> > + start_pg = ftrace_allocate_pages(count, &pages);
> > if (!start_pg)
> > return -ENOMEM;
> >
> > @@ -7305,27 +7306,27 @@ static int ftrace_process_locs(struct module *mod,
> > /* We should have used all pages unless we skipped some */
> > if (pg_unuse) {
> > unsigned long pg_remaining, remaining = 0;
> > - unsigned long skip;
> > + long skip;
> >
> > /* Count the number of entries unused and compare it to skipped. */
> > - pg_remaining = (ENTRIES_PER_PAGE << pg->order) - pg->index;
> > + pg_remaining = (PAGE_SIZE << pg->order) / ENTRY_SIZE - pg->index;
> >
> > if (!WARN(skipped < pg_remaining, "Extra allocated pages for ftrace")) {
> >
> > skip = skipped - pg_remaining;
> >
> > - for (pg = pg_unuse; pg; pg = pg->next)
> > + for (pg = pg_unuse; pg && skip > 0; pg = pg->next) {
> > remaining += 1 << pg->order;
> > + skip -= (PAGE_SIZE << pg->order) / ENTRY_SIZE;
> > + }
> >
> > pages -= remaining;
> >
> > - skip = DIV_ROUND_UP(skip, ENTRIES_PER_PAGE);
> > -
> > /*
> > * Check to see if the number of pages remaining would
> > * just fit the number of entries skipped.
> > */
> > - WARN(skip != remaining, "Extra allocated pages for ftrace: %lu with %lu skipped",
> > + WARN(pg || skip > 0, "Extra allocated pages for ftrace: %lu with %lu skipped",
> > remaining, skipped);
> > }
> > /* Need to synchronize with ftrace_location_range() */
^ permalink raw reply
* Re: [PATCH] function_graph: Fix missing FGRAPH_ENTRY_ARGS() call in print_graph_retval()
From: Masami Hiramatsu @ 2026-01-13 0:48 UTC (permalink / raw)
To: Donglin Peng
Cc: rostedt, mhiramat, linux-trace-kernel, linux-kernel, Donglin Peng
In-Reply-To: <20260111143318.917945-1-dolinux.peng@gmail.com>
On Sun, 11 Jan 2026 22:33:18 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> From: Donglin Peng <pengdonglin@xiaomi.com>
>
> Call FGRAPH_ENTRY_ARGS(entry) to retrieve the correct arguments pointer
> when displaying function parameters in print_graph_retval().
>
> Fixes: f83ac7544fbf ("function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously")
> Signed-off-by: Donglin Peng <pengdonglin@xiaomi.com>
Looks good to me. (other print_function_args() using this macro)
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks!
> ---
> kernel/trace/trace_functions_graph.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index b1e9c9913309..1de6f1573621 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -901,7 +901,7 @@ static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entr
> trace_seq_printf(s, "%ps", func);
>
> if (args_size >= FTRACE_REGS_MAX_ARGS * sizeof(long)) {
> - print_function_args(s, entry->args, (unsigned long)func);
> + print_function_args(s, FGRAPH_ENTRY_ARGS(entry), (unsigned long)func);
> trace_seq_putc(s, ';');
> } else
> trace_seq_puts(s, "();");
> --
> 2.34.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v13 2/3] mm: Fix OOM killer inaccuracy on large many-core systems
From: Mathieu Desnoyers @ 2026-01-13 0:47 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-kernel, Paul E. McKenney, Steven Rostedt,
Masami Hiramatsu, Dennis Zhou, Tejun Heo, Christoph Lameter,
Martin Liu, David Rientjes, christian.koenig, Shakeel Butt,
SeongJae Park, Johannes Weiner, Sweet Tea Dorminy,
Lorenzo Stoakes, Liam R . Howlett, Mike Rapoport,
Suren Baghdasaryan, Vlastimil Babka, Christian Brauner, Wei Yang,
David Hildenbrand, Miaohe Lin, Al Viro, linux-mm,
linux-trace-kernel, Yu Zhao, Roman Gushchin, Mateusz Guzik,
Matthew Wilcox, Baolin Wang, Aboorva Devarajan
In-Reply-To: <aWVP9sPqG8VC8Oq_@tiehlicka>
On 2026-01-12 14:48, Michal Hocko wrote:
> On Mon 12-01-26 14:37:49, Mathieu Desnoyers wrote:
>> On 2026-01-12 03:42, Michal Hocko wrote:
>>> Hi,
>>> sorry to jump in this late but the timing of previous versions didn't
>>> really work well for me.
>>>
>>> On Sun 11-01-26 14:49:57, Mathieu Desnoyers wrote:
>>> [...]
>>>> Here is a (possibly incomplete) list of the prior approaches that were
>>>> used or proposed, along with their downside:
>>>>
>>>> 1) Per-thread rss tracking: large error on many-thread processes.
>>>>
>>>> 2) Per-CPU counters: up to 12% slower for short-lived processes and 9%
>>>> increased system time in make test workloads [1]. Moreover, the
>>>> inaccuracy increases with O(n^2) with the number of CPUs.
>>>>
>>>> 3) Per-NUMA-node counters: requires atomics on fast-path (overhead),
>>>> error is high with systems that have lots of NUMA nodes (32 times
>>>> the number of NUMA nodes).
>>>>
>>>> The approach proposed here is to replace this by the hierarchical
>>>> per-cpu counters, which bounds the inaccuracy based on the system
>>>> topology with O(N*logN).
>>>
>>> The concept of hierarchical pcp counter is interesting and I am
>>> definitely not opposed if there are more users that would benefit.
>>>
>>> From the OOM POV, IIUC the primary problem is that get_mm_counter
>>> (percpu_counter_read_positive) is too imprecise on systems when the task
>>> is moving around a large number of cpus. In the list of alternative
>>> solutions I do not see percpu_counter_sum_positive to be mentioned.
>>> oom_badness() is a really slow path and taking the slow path to
>>> calculate a much more precise value seems acceptable. Have you
>>> considered that option?
>> I must admit I assumed that since there was already a mechanism in place
>> to ensure it's not necessary to sum per-cpu counters when the oom killer
>> is trying to select tasks, it must be because this
>>
>> O(nr_possible_cpus * nr_processes)
>>
>> operation must be too slow for the oom killer requirements.
>>
>> AFAIU, the oom killer is executed when the memory allocator fails to
>> allocate memory, which can be within code paths which need to progress
>> eventually. So even though it's a slow path compared to the allocator
>> fast path, there must be at least _some_ expectations about it
>> completing within a decent amount of time. What would that ballpark be ?
>
> I do not think we have ever promissed more than the oom killer will try
> to unlock the system blocked on memory shortage.
>
>> To give an order of magnitude, I've tried modifying the upstream
>> oom killer to use percpu_counter_sum_positive and compared it to
>> the hierarchical approach:
>>
>> AMD EPYC 9654 96-Core (2 sockets)
>> Within a KVM, configured with 256 logical cpus.
>>
>> nr_processes=40 nr_processes=10000
>> Counter sum: 0.4 ms 81.0 ms
>> HPCC with 2-pass: 0.3 ms 9.3 ms
>
> These are peanuts for the global oom situations. We have had situations
> when soft lockup detector triggered because of the process tree
> traversal so adding 100ms is not really critical.
>
>> So as we scale up the number of processes on large SMP systems,
>> the latency caused by the oom killer task selection greatly
>> increases with the counter sums compared with the hierarchical
>> approach.
>
> Yes, I am not really questioning the hierarchical approach will perform
> much better but I am thinking of a good enough solution and calculating
> the number might be just that stop gap solution (that would be also
> suitable for stable tree backports). I am not ruling out improving on
> top of that by a more clever solution like your hierarchical counters
> approach. Especially if there are more benefits from that elsewhere.
>
Would you be OK with introducing changes in the following order ?
1) Fix the OOM killer inaccuracy by using counter sum (iteration on all
cpu counters) in task selection. This may slow down the oom killer,
but would at least fix its current inaccuracy issues. This could be
backported to stable kernels.
2) Introduce the hierarchical percpu counters on top, as a oom killer
task selection performance optimization (reduce latency of oom kill).
This way, (2) becomes purely a performance optimization, so it's easy
to bissect and revert if it causes issues.
I agree that bringing a fix along with a performance optimization within
a single commit makes it hard to backport to stable, and tricky to
revert if it causes problems.
As for finding other users of the hpcc, I have ideas, but not so much
time available to try them out, as I'm pretty much doing this in my
spare time.
One possibility I see would be to track memory access patterns by
sampling with page fault handlers for NUMA locality tracking.
Using hierarchical counters could help make quick migration decisions
(either task migration or moving memory pages across NUMA nodes)
based on hpcc approximations, including using intermediate counters
within the tree levels.
Another category of use-cases would be tracking resource limits
(e.g. cgroups, network traffic control) through HPCC, and then
using approximated counter values to validate whether the current
usage goes beyond the limit threshold.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ 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