* Re: [PATCH] ring-buffer: Use a housekeeping CPU to wake up waiters
From: Steven Rostedt @ 2026-01-06 22:04 UTC (permalink / raw)
To: Petr Tesarik
Cc: Masami Hiramatsu, Mathieu Desnoyers, Sebastian Andrzej Siewior,
Clark Williams, linux-kernel, linux-trace-kernel, linux-rt-devel
In-Reply-To: <20260106091039.2012108-1-ptesarik@suse.com>
On Tue, 6 Jan 2026 10:10:39 +0100
Petr Tesarik <ptesarik@suse.com> wrote:
> Avoid running the wakeup irq_work on an isolated CPU. Since the wakeup can
> run on any CPU, let's pick a housekeeping CPU to do the job.
>
> This change reduces additional noise when tracing isolated CPUs. For
> example, the following ipi_send_cpu stack trace was captured with
> nohz_full=2 on the isolated CPU:
>
> <idle>-0 [002] d.h4. 1255.379293: ipi_send_cpu: cpu=2 callsite=irq_work_queue+0x2d/0x50 callback=rb_wake_up_waiters+0x0/0x80
> <idle>-0 [002] d.h4. 1255.379329: <stack trace>
> => trace_event_raw_event_ipi_send_cpu
> => __irq_work_queue_local
> => irq_work_queue
> => ring_buffer_unlock_commit
> => trace_buffer_unlock_commit_regs
> => trace_event_buffer_commit
> => trace_event_raw_event_x86_irq_vector
> => __sysvec_apic_timer_interrupt
> => sysvec_apic_timer_interrupt
> => asm_sysvec_apic_timer_interrupt
> => pv_native_safe_halt
> => default_idle
> => default_idle_call
> => do_idle
> => cpu_startup_entry
> => start_secondary
> => common_startup_64
I take it that even with this patch you would still get the above events.
The only difference would be the "cpu=" in the event info will not be the
same as the CPU it executed on, right?
>
> The IRQ work interrupt alone adds considerable noise, but the impact can
> get even worse with PREEMPT_RT, because the IRQ work interrupt is then
> handled by a separate kernel thread. This requires a task switch and makes
> tracing useless for analyzing latency on an isolated CPU.
>
> Signed-off-by: Petr Tesarik <ptesarik@suse.com>
LGTM,
I'll queue it up for the next merge window.
-- Steve
^ permalink raw reply
* Re: [PATCH v12] dma-buf: add some tracepoints to debug.
From: Steven Rostedt @ 2026-01-06 22:10 UTC (permalink / raw)
To: Xiang Gao
Cc: sumit.semwal, christian.koenig, mhiramat, linux-media, dri-devel,
linux-kernel, mathieu.desnoyers, dhowells, kuba, brauner, akpm,
linux-trace-kernel, gaoxiang17
In-Reply-To: <20251229031547.59272-1-gxxa03070307@gmail.com>
On Mon, 29 Dec 2025 11:15:47 +0800
Xiang Gao <gxxa03070307@gmail.com> wrote:
> From: gaoxiang17 <gaoxiang17@xiaomi.com>
>
> Since we can only inspect dmabuf by iterating over process FDs or the
> dmabuf_list, we need to add our own tracepoints to track its status in
> real time in production.
>
> For example:
> binder:3016_1-3102 [006] ...1. 255.126521: dma_buf_export: exp_name=qcom,system size=12685312 ino=2738
> binder:3016_1-3102 [006] ...1. 255.126528: dma_buf_fd: exp_name=qcom,system size=12685312 ino=2738 fd=8
> binder:3016_1-3102 [006] ...1. 255.126642: dma_buf_mmap_internal: exp_name=qcom,system size=28672 ino=2739
> kworker/6:1-86 [006] ...1. 255.127194: dma_buf_put: exp_name=qcom,system size=12685312 ino=2738
> RenderThread-9293 [006] ...1. 316.618179: dma_buf_get: exp_name=qcom,system size=12771328 ino=2762 fd=176
> RenderThread-9293 [006] ...1. 316.618195: dma_buf_dynamic_attach: exp_name=qcom,system size=12771328 ino=2762 attachment:ffffff880a18dd00 is_dynamic=0 dev_name=kgsl-3d0
> RenderThread-9293 [006] ...1. 318.878220: dma_buf_detach: exp_name=qcom,system size=12771328 ino=2762 attachment:ffffff880a18dd00 is_dynamic=0 dev_name=kgsl-3d0
>
> Signed-off-by: Xiang Gao <gaoxiang17@xiaomi.com>
From a tracing POV:
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH] ftrace: Do not over-allocate ftrace memory
From: Guenter Roeck @ 2026-01-06 22:26 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260106161844.02ab387f@gandalf.local.home>
Hi Steven,
On Tue, Jan 06, 2026 at 04:18:44PM -0500, Steven Rostedt wrote:
> On Tue, 6 Jan 2026 12:35:33 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
> > 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.
>
> Nice catch! I guess you have a machine that allows much higher order
> allocations than I do ;-)
>
I have actually seen the problem when the system tried to allocate 341
entries. 2 x 170 = 340, so the code allocated three pages, and things
went downhill from there.
Here is a specific example of the allocation sequence with debug
information.
ftrace_allocate_records: Requesting 52904 entries: 310 pages [order 8]. Alternate: 312 pages [order 8]
Allocated 43690 records, 9214 remaining
ftrace_allocate_records: Requesting 9214 entries: 54 pages [order 5]. Alternate: 55 pages [order 5]
Allocated 5461 records, 3753 remaining
ftrace_allocate_records: Requesting 3753 entries: 22 pages [order 4]. Alternate: 23 pages [order 4]
Allocated 2730 records, 1023 remaining
ftrace_allocate_records: Requesting 1023 entries: 6 pages [order 2]. Alternate: 7 pages [order 2]
Allocated 682 records, 341 remaining
ftrace_allocate_records: Requesting 341 entries: 2 pages [order 1]. Alternate: 3 pages [order 1]
Allocated 341 records, 0 remaining
The description above is just a more extreme example.
> >
> > 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 if too many pages
> > have been allocated to solve the problem. Also use the actual capacity
> > of a page group to determine the number of pages needed to avoid over-
> > allocations in ftrace_allocate_records().
> >
> > 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>
> > ---
> > kernel/trace/ftrace.c | 25 +++++++++++++++++--------
> > 1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index ef2d5dca6f70..211ec7a04f7e 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -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:
> > @@ -7308,24 +7308,33 @@ static int ftrace_process_locs(struct module *mod,
> > unsigned 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")) {
> > + unsigned long space = 0;
> >
> > skip = skipped - pg_remaining;
> >
> > - for (pg = pg_unuse; pg; pg = pg->next)
> > + for (pg = pg_unuse; pg; pg = pg->next) {
> > remaining += 1 << pg->order;
> > + /*
> > + * The capacity of a page group is
> > + * (PAGE_SIZE << order) / ENTRY_SIZE
> > + * Accumulate the total capacity of unused pages.
> > + */
> > + space += (PAGE_SIZE << pg->order) / ENTRY_SIZE;
> > + }
> >
> > pages -= remaining;
>
> I think pages is meaningless here, as it was set in the beginning with:
>
> pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);
>
> Which is incorrect. I wonder if we should set it via:
>
I know, I just didn't want to make more changes than absolutely
necessary.
> /*
> * Use ftrace_number_of_pages to determine how many pages were
> * allocated
> */
> pages = ftrace_number_of_pages;
>
> start_pg = ftrace_allocate_pages(count);
> if (!start_pg)
> return -ENOMEM;
>
> /* ftrace_allocate_pages() increments ftrace_number_of_pages */
> pages = ftrace_number_of_pages - pages;
>
That might work, assuming that the code updating ftrace_number_of_pages
is (mutex) protected. I don't immediately see that, and the
"mutex_lock(&ftrace_lock);" right after the above code makes me a bit
concerned.
> This will make pages equal the number of pages that were allocated. Then
> I'm not sure we need this extra logic.
>
Which extra logic do you refer to ? Everything in "if (pg_unused)"
except for the calls to synchronize_rcu() and ftrace_free_pages() ?
If so I'll be more than happy to drop that.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH] ftrace: Do not over-allocate ftrace memory
From: Guenter Roeck @ 2026-01-06 23:05 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <dc80ae8e-54f6-45f0-bf2f-69012c3b47aa@roeck-us.net>
On Tue, Jan 06, 2026 at 02:26:23PM -0800, Guenter Roeck wrote:
> > >
> > > pages -= remaining;
> >
> > I think pages is meaningless here, as it was set in the beginning with:
> >
> > pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);
> >
> > Which is incorrect. I wonder if we should set it via:
> >
> I know, I just didn't want to make more changes than absolutely
> necessary.
>
> > /*
> > * Use ftrace_number_of_pages to determine how many pages were
> > * allocated
> > */
> > pages = ftrace_number_of_pages;
> >
> > start_pg = ftrace_allocate_pages(count);
> > if (!start_pg)
> > return -ENOMEM;
> >
> > /* ftrace_allocate_pages() increments ftrace_number_of_pages */
> > pages = ftrace_number_of_pages - pages;
> >
>
> That might work, assuming that the code updating ftrace_number_of_pages
> is (mutex) protected. I don't immediately see that, and the
> "mutex_lock(&ftrace_lock);" right after the above code makes me a bit
> concerned.
>
One way to avoid the locking problem without potentially risky code changes
would be to pass a pointer to pages to ftrace_allocate_pages() and to
ftrace_allocate_records(), and to update it from there. I tested that and
confirmed that it works.
Guenter
^ permalink raw reply
* [PATCH] trace: ftrace_dump_on_oops[] is not exported, make it static
From: Ben Dooks @ 2026-01-06 23:10 UTC (permalink / raw)
To: linux-trace-kernel
Cc: linux-kernel, mhiramat, mathieu.desnoyers, rostedt, Ben Dooks
The ftrace_dump_on_oops string is not used outside of trace.c so
make it static to avoid the export warning from sparse:
kernel/trace/trace.c:141:6: warning: symbol 'ftrace_dump_on_oops' was not declared. Should it be static?
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
kernel/trace/trace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6f2148df14d9..5c196ea93768 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -138,7 +138,7 @@ cpumask_var_t __read_mostly tracing_buffer_mask;
* by commas.
*/
/* Set to string format zero to disable by default */
-char ftrace_dump_on_oops[MAX_TRACER_SIZE] = "0";
+static char ftrace_dump_on_oops[MAX_TRACER_SIZE] = "0";
/* When set, tracing will stop when a WARN*() is hit */
static int __disable_trace_on_warning;
--
2.37.2.352.g3c44437643
^ permalink raw reply related
* Re: [PATCH] ftrace: Do not over-allocate ftrace memory
From: Steven Rostedt @ 2026-01-06 23:38 UTC (permalink / raw)
To: Guenter Roeck
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <0a9cd12e-6db8-4b41-979e-95f06e9390f6@roeck-us.net>
On Tue, 6 Jan 2026 15:05:25 -0800
Guenter Roeck <linux@roeck-us.net> wrote:
> > > /*
> > > * Use ftrace_number_of_pages to determine how many pages were
> > > * allocated
> > > */
> > > pages = ftrace_number_of_pages;
> > >
> > > start_pg = ftrace_allocate_pages(count);
> > > if (!start_pg)
> > > return -ENOMEM;
> > >
> > > /* ftrace_allocate_pages() increments ftrace_number_of_pages */
> > > pages = ftrace_number_of_pages - pages;
> > >
> >
> > That might work, assuming that the code updating ftrace_number_of_pages
> > is (mutex) protected. I don't immediately see that, and the
> > "mutex_lock(&ftrace_lock);" right after the above code makes me a bit
> > concerned.
> >
>
> One way to avoid the locking problem without potentially risky code changes
> would be to pass a pointer to pages to ftrace_allocate_pages() and to
> ftrace_allocate_records(), and to update it from there. I tested that and
> confirmed that it works.
I was originally going to suggest that, but when looking at the code, I
noticed that these variables could be useful. They are only updated on boot
up, module load, module unload and when module memory is freed.
But looking into the module code, these updates are done outside of the
module_mutex. This means these values need to be converted to atomics as
they are updated without any protection.
Yeah, better to just get the value from passing in a parameter to both
ftrace_allocate_pages() and to ftrace_allocate_records().
Something like:
unsigend long pages = 0;
[..]
start_pg = ftrace_allocate_pages(count, &pages);
[..]
ftrace_allocate_pages(unsigned long num_to_init, unsigned long *num_pages) {
[..]
cnt = ftrace_allocate_records(pg, num_to_init, num_pages);
And have ftrace_allocte_records() have:
pages = 1 << order;
*num_pages += pages;
ftrace_number_of_pages += pages;
And I'll add another patch on top of this to make the variables atomic.
Thanks,
-- Steve
^ permalink raw reply
* Re: [PATCH] trace: ftrace_dump_on_oops[] is not exported, make it static
From: Steven Rostedt @ 2026-01-06 23:41 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-trace-kernel, linux-kernel, mhiramat, mathieu.desnoyers
In-Reply-To: <20260106231054.84270-1-ben.dooks@codethink.co.uk>
On Tue, 6 Jan 2026 23:10:54 +0000
Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> The ftrace_dump_on_oops string is not used outside of trace.c so
> make it static to avoid the export warning from sparse:
>
> kernel/trace/trace.c:141:6: warning: symbol 'ftrace_dump_on_oops' was not declared. Should it be static?
>
This could probably have:
Fixes: dd293df6395a2 ("tracing: Move trace sysctls into trace.c")
as that was the commit to move the usage of that variable back into the
defining file.
-- Steve
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> kernel/trace/trace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 6f2148df14d9..5c196ea93768 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -138,7 +138,7 @@ cpumask_var_t __read_mostly tracing_buffer_mask;
> * by commas.
> */
> /* Set to string format zero to disable by default */
> -char ftrace_dump_on_oops[MAX_TRACER_SIZE] = "0";
> +static char ftrace_dump_on_oops[MAX_TRACER_SIZE] = "0";
>
> /* When set, tracing will stop when a WARN*() is hit */
> static int __disable_trace_on_warning;
^ permalink raw reply
* Re: [PATCH] ftrace: Do not over-allocate ftrace memory
From: Guenter Roeck @ 2026-01-06 23:48 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260106183815.33fa41f9@gandalf.local.home>
Hi Steve,
On Tue, Jan 06, 2026 at 06:38:15PM -0500, Steven Rostedt wrote:
> On Tue, 6 Jan 2026 15:05:25 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
> > > > /*
> > > > * Use ftrace_number_of_pages to determine how many pages were
> > > > * allocated
> > > > */
> > > > pages = ftrace_number_of_pages;
> > > >
> > > > start_pg = ftrace_allocate_pages(count);
> > > > if (!start_pg)
> > > > return -ENOMEM;
> > > >
> > > > /* ftrace_allocate_pages() increments ftrace_number_of_pages */
> > > > pages = ftrace_number_of_pages - pages;
> > > >
> > >
> > > That might work, assuming that the code updating ftrace_number_of_pages
> > > is (mutex) protected. I don't immediately see that, and the
> > > "mutex_lock(&ftrace_lock);" right after the above code makes me a bit
> > > concerned.
> > >
> >
> > One way to avoid the locking problem without potentially risky code changes
> > would be to pass a pointer to pages to ftrace_allocate_pages() and to
> > ftrace_allocate_records(), and to update it from there. I tested that and
> > confirmed that it works.
>
> I was originally going to suggest that, but when looking at the code, I
> noticed that these variables could be useful. They are only updated on boot
> up, module load, module unload and when module memory is freed.
>
> But looking into the module code, these updates are done outside of the
> module_mutex. This means these values need to be converted to atomics as
> they are updated without any protection.
>
> Yeah, better to just get the value from passing in a parameter to both
> ftrace_allocate_pages() and to ftrace_allocate_records().
>
> Something like:
>
> unsigend long pages = 0;
>
> [..]
> start_pg = ftrace_allocate_pages(count, &pages);
>
> [..]
> ftrace_allocate_pages(unsigned long num_to_init, unsigned long *num_pages) {
> [..]
> cnt = ftrace_allocate_records(pg, num_to_init, num_pages);
>
> And have ftrace_allocte_records() have:
>
> pages = 1 << order;
> *num_pages += pages;
> ftrace_number_of_pages += pages;
>
That is exactly what I tested. With that, I assume I can drop the
code to update pages in ftrace_process_locs(), inclusing the warning
backtraces. I'll send v2 with those changes.
Thanks,
Guenter
^ permalink raw reply
* [PATCH v2] ftrace: Do not over-allocate ftrace memory
From: Guenter Roeck @ 2026-01-07 0:24 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. Drop the code calculating the
actual page count since it is no longer needed, and with it the warning
backtraces. Also drop the ENTRIES_PER_PAGE definition since it is no longer
needed either.
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>
---
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 | 41 +++++++++--------------------------------
1 file changed, 9 insertions(+), 32 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index ef2d5dca6f70..755a11f13808 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 *pages)
{
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;
@@ -7304,30 +7305,6 @@ 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;
-
- /* Count the number of entries unused and compare it to skipped. */
- pg_remaining = (ENTRIES_PER_PAGE << pg->order) - pg->index;
-
- if (!WARN(skipped < pg_remaining, "Extra allocated pages for ftrace")) {
-
- skip = skipped - pg_remaining;
-
- for (pg = pg_unuse; pg; pg = pg->next)
- remaining += 1 << pg->order;
-
- 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",
- remaining, skipped);
- }
/* Need to synchronize with ftrace_location_range() */
synchronize_rcu();
ftrace_free_pages(pg_unuse);
--
2.45.2
^ permalink raw reply related
* Re: [PATCH v2] ftrace: Do not over-allocate ftrace memory
From: Steven Rostedt @ 2026-01-07 0:43 UTC (permalink / raw)
To: Guenter Roeck
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260107002428.2904852-1-linux@roeck-us.net>
On Tue, 6 Jan 2026 16:24:28 -0800
Guenter Roeck <linux@roeck-us.net> wrote:
> 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. Drop the code calculating the
> actual page count since it is no longer needed, and with it the warning
> backtraces. Also drop the ENTRIES_PER_PAGE definition since it is no longer
> needed either.
>
> 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>
> ---
> 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 | 41 +++++++++--------------------------------
> 1 file changed, 9 insertions(+), 32 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index ef2d5dca6f70..755a11f13808 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 *pages)
> {
> 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;
>
> @@ -7304,30 +7305,6 @@ 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;
> -
> - /* Count the number of entries unused and compare it to skipped. */
> - pg_remaining = (ENTRIES_PER_PAGE << pg->order) - pg->index;
> -
> - if (!WARN(skipped < pg_remaining, "Extra allocated pages for ftrace")) {
> -
> - skip = skipped - pg_remaining;
> -
> - for (pg = pg_unuse; pg; pg = pg->next)
> - remaining += 1 << pg->order;
> -
> - 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",
> - remaining, skipped);
> - }
Don't remove this block. It's still needed. A lot of entries are
skipped when adding the records. Weak functions and zero'd pointers
that were part of the count are skipped. This is the code that handles
that. It has nothing to do with rounding errors.
-- Steve
> /* Need to synchronize with ftrace_location_range() */
> synchronize_rcu();
> ftrace_free_pages(pg_unuse);
^ permalink raw reply
* Re: [PATCH v2] ftrace: Do not over-allocate ftrace memory
From: Steven Rostedt @ 2026-01-07 0:52 UTC (permalink / raw)
To: Guenter Roeck
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260107002428.2904852-1-linux@roeck-us.net>
On Tue, 6 Jan 2026 16:24:28 -0800
Guenter Roeck <linux@roeck-us.net> wrote:
> if (pg_unuse) {
> - unsigned long pg_remaining, remaining = 0;
> - unsigned long skip;
> -
> - /* Count the number of entries unused and compare it to skipped. */
> - pg_remaining = (ENTRIES_PER_PAGE << pg->order) - pg->index;
> -
The above needs to be fixed though.
-- Steve
^ permalink raw reply
* Re: [PATCH v2] ftrace: Do not over-allocate ftrace memory
From: Guenter Roeck @ 2026-01-07 0:54 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260106194355.66cf869e@fedora>
On Tue, Jan 06, 2026 at 07:43:55PM -0500, Steven Rostedt wrote:
> >
> > @@ -7304,30 +7305,6 @@ 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;
> > -
> > - /* Count the number of entries unused and compare it to skipped. */
> > - pg_remaining = (ENTRIES_PER_PAGE << pg->order) - pg->index;
> > -
> > - if (!WARN(skipped < pg_remaining, "Extra allocated pages for ftrace")) {
> > -
> > - skip = skipped - pg_remaining;
> > -
> > - for (pg = pg_unuse; pg; pg = pg->next)
> > - remaining += 1 << pg->order;
> > -
> > - 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",
> > - remaining, skipped);
> > - }
>
> Don't remove this block. It's still needed. A lot of entries are
> skipped when adding the records. Weak functions and zero'd pointers
> that were part of the count are skipped. This is the code that handles
> that. It has nothing to do with rounding errors.
>
Sorry, misunderstanding. I thought that is what you meant with "This will
make pages equal the number of pages that were allocated. Then I'm not sure
we need this extra logic."
What is the no longer needed extra logic ?
Thanks,
Guenter
^ permalink raw reply
* Re: [v2 PATCH 2/2] tracing: Add show_event_triggers to expose active event triggers
From: Aaron Tomlin @ 2026-01-07 1:15 UTC (permalink / raw)
To: Randy Dunlap
Cc: Steven Rostedt, mhiramat, mark.rutland, mathieu.desnoyers, corbet,
neelx, sean, linux-kernel, linux-trace-kernel, linux-doc
In-Reply-To: <b08ad926-8582-4a43-ba17-3a987b7a0ca6@infradead.org>
[-- Attachment #1: Type: text/plain, Size: 240 bytes --]
On Tue, Jan 06, 2026 at 08:08:17AM -0800, Randy Dunlap wrote:
> > It is simply a s/filter/trigger/g difference though.
>
> Ack. Thanks.
Hi Randy,
Sorry about the delay. Indeed, as per Steve.
Kind regards,
--
Aaron Tomlin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 1/2] rcu: Add rcu_read_lock_notrace()
From: Joel Fernandes @ 2026-01-07 1:26 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: paulmck, Steven Rostedt, Sebastian Andrzej Siewior, Boqun Feng,
linux-rt-devel, rcu, linux-trace-kernel, Frederic Weisbecker,
Joel Fernandes, Josh Triplett, Lai Jiangshan, Masami Hiramatsu,
Neeraj Upadhyay, Thomas Gleixner, Uladzislau Rezki, Zqiang, bpf
In-Reply-To: <5dc49f5a-ddda-422b-a8af-c662ee53d503@efficios.com>
On Thu, Jul 17, 2025 at 03:36:46PM -0400, Mathieu Desnoyers wrote:
> On 2025-07-17 11:18, Paul E. McKenney wrote:
> > On Thu, Jul 17, 2025 at 10:46:46AM -0400, Mathieu Desnoyers wrote:
> > > On 2025-07-17 09:14, Mathieu Desnoyers wrote:
> > > > On 2025-07-16 18:54, Paul E. McKenney wrote:
> > > [...]
> > > >
> > > > 2) I think I'm late to the party in reviewing srcu-fast, I'll
> > > > go have a look :)
> > >
> > > OK, I'll bite. :) Please let me know where I'm missing something:
> > >
> > > Looking at srcu-lite and srcu-fast, I understand that they fundamentally
> > > depend on a trick we published here https://lwn.net/Articles/573497/
> > > "The RCU-barrier menagerie" that allows turning, e.g. this Dekker:
> > >
> > > volatile int x = 0, y = 0
> > >
> > > CPU 0 CPU 1
> > >
> > > x = 1 y = 1
> > > smp_mb smp_mb
> > > r2 = y r4 = x
> > >
> > > BUG_ON(r2 == 0 && r4 == 0)
> > >
> > > into
> > >
> > > volatile int x = 0, y = 0
> > >
> > > CPU 0 CPU 1
> > >
> > > rcu_read_lock()
> > > x = 1 y = 1
> > > synchronize_rcu()
> > > r2 = y r4 = x
> > > rcu_read_unlock()
> > >
> > > BUG_ON(r2 == 0 && r4 == 0)
> > >
> > > So looking at srcu-fast, we have:
> > >
> > > * Note that both this_cpu_inc() and atomic_long_inc() are RCU read-side
> > > * critical sections either because they disables interrupts, because they
> > > * are a single instruction, or because they are a read-modify-write atomic
> > > * operation, depending on the whims of the architecture.
> > >
> > > It appears to be pairing, as RCU read-side:
> > >
> > > - irq off/on implied by this_cpu_inc
> > > - atomic
> > > - single instruction
> > >
> > > with synchronize_rcu within the grace period, and hope that this behaves as a
> > > smp_mb pairing preventing the srcu read-side critical section from leaking
> > > out of the srcu read lock/unlock.
> > >
> > > I note that there is a validation that rcu_is_watching() within
> > > __srcu_read_lock_fast, but it's one thing to have rcu watching, but
> > > another to have an actual read-side critical section. Note that
> > > preemption, irqs, softirqs can very well be enabled when calling
> > > __srcu_read_lock_fast.
> > >
> > > My understanding of the how memory barriers implemented with RCU
> > > work is that we need to surround the memory accesses on the fast-path
> > > (where we turn smp_mb into barrier) with an RCU read-side critical
> > > section to make sure it does not spawn across a synchronize_rcu.
> > >
> > > What I am missing here is how can a RCU side-side that only consist
> > > of the irq off/on or atomic or single instruction cover all memory
> > > accesses we are trying to order, namely those within the srcu
> > > critical section after the compiler barrier() ? Is having RCU
> > > watching sufficient to guarantee this ?
> >
> > Good eyes!!!
> >
> > The trick is that this "RCU read-side critical section" consists only of
> > either this_cpu_inc() or atomic_long_inc(), with the latter only happening
> > in systems that have NMIs, but don't have NMI-safe per-CPU operations.
> > Neither this_cpu_inc() nor atomic_long_inc() can be interrupted, and
> > thus both act as an interrupts-disabled RCU read-side critical section.
> >
> > Therefore, if the SRCU grace-period computation fails to see an
> > srcu_read_lock_fast() increment, its earlier code is guaranteed to
> > happen before the corresponding critical section. Similarly, if the SRCU
> > grace-period computation sees an srcu_read_unlock_fast(), its subsequent
> > code is guaranteed to happen after the corresponding critical section.
> >
> > Does that help? If so, would you be interested and nominating a comment?
> >
> > Or am I missing something subtle here?
>
> Here is the root of my concern: considering a single instruction
> as an RCU-barrier "read-side" for a classic Dekker would not work,
> because the read-side would not cover both memory accesses that need
> to be ordered.
I had similar questions, so let me share, assuming I even got your question
correct. I think what might help is to note that for synchronize_rcu(), any
transition to and from RCU watching is a full memory barrier. And if I
understand correctly, because SRCU fast is not allowed when RCU is not
watching, the memory ordering Required for RCU correctness is automatically
taken care of because the update side uses synchronize_rcu().
That is my understanding. Could you clarify how your question is related to the
single instruction thing? I think if you apply the
classical RCU reasoning where RCU (regular RCU) read-side critical sections
do not have any memory barriers, then you will see the similarity of that
with srcu-fast.
thanks,
- Joel
>
> I cannot help but notice the similarity between this pattern of
> barrier vs synchronize_rcu and what we allow userspace to do with
> barrier vs sys_membarrier, which has one implementation
> based on synchronize_rcu (except for TICK_NOHZ_FULL). Originally
> when membarrier was introduced, this was based on synchronize_sched(),
> and I recall that this was OK because userspace execution acted as
> a read-side critical section from the perspective of synchronize_sched().
> As commented in kernel v4.10 near synchronize_sched():
>
> * Note that this guarantee implies further memory-ordering guarantees.
> * On systems with more than one CPU, when synchronize_sched() returns,
> * each CPU is guaranteed to have executed a full memory barrier since the
> * end of its last RCU-sched read-side critical section whose beginning
> * preceded the call to synchronize_sched(). In addition, each CPU having
> * an RCU read-side critical section that extends beyond the return from
> * synchronize_sched() is guaranteed to have executed a full memory barrier
> * after the beginning of synchronize_sched() and before the beginning of
> * that RCU read-side critical section. Note that these guarantees include
> * CPUs that are offline, idle, or executing in user mode, as well as CPUs
> * that are executing in the kernel.
>
> So even though I see how synchronize_rcu() nowadays is still a good
> choice to implement sys_membarrier, it only apply to RCU read side
> critical sections, which covers userspace code and the specific
> read-side critical sections in the kernel.
>
> But what I don't get is how synchronize_rcu() can help us promote
> the barrier() in SRCU-fast to smp_mb when outside of any RCU read-side
> critical section tracked by the synchronize_rcu grace period,
> mainly because unlike the sys_membarrier scenario, this is *not*
> userspace code.
>
> And what we want to order here on the read-side is the lock/unlock
> increments vs the memory accesses within the critical section, but
> there is no RCU read-side that contain all those memory accesses
> that match those synchronize_rcu calls, so the promotion from barrier
> to smp_mb don't appear to be valid.
>
> But perhaps there is something more that is specific to the SRCU
> algorithm that I missing here ?
>
> Thanks,
>
> Mathieu
>
> >
> > Either way, many thanks for digging into this!!!
> >
> > Thanx, Paul
> >
> > > Thanks,
> > >
> > > Mathieu
> > >
> > > --
> > > Mathieu Desnoyers
> > > EfficiOS Inc.
> > > https://www.efficios.com
>
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com
^ permalink raw reply
* Re: [PATCH v2] ftrace: Do not over-allocate ftrace memory
From: Steven Rostedt @ 2026-01-07 1:33 UTC (permalink / raw)
To: Guenter Roeck
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <b13af4e1-3510-4240-9347-31459d5f0ef6@roeck-us.net>
On Tue, 6 Jan 2026 16:54:27 -0800
Guenter Roeck <linux@roeck-us.net> wrote:
> > Don't remove this block. It's still needed. A lot of entries are
> > skipped when adding the records. Weak functions and zero'd pointers
> > that were part of the count are skipped. This is the code that handles
> > that. It has nothing to do with rounding errors.
> >
> Sorry, misunderstanding. I thought that is what you meant with "This will
> make pages equal the number of pages that were allocated. Then I'm not sure
> we need this extra logic."
>
> What is the no longer needed extra logic ?
I meant the added logic you had there, as I was a bit confused by the
"spaces" part.
The logic still needs to be updated, but I think it can be done with the
following:
if (pg_unuse) {
unsigned long pg_remaining, remaining = 0;
long skip;
/* Count the number of entries unused and compare it to skipped. */
pg_remaining = (PAGE_SIZE << pg->order) / ENTRIES_SIZE - pg->index;
if (!WARN(skipped < pg_remaining, "Extra allocated pages for ftrace")) {
skip = skipped - pg_remaining;
for (pg = pg_unuse; pg && skip > 0; pg = pg->next) {
remaining += 1 << pg->order;
skip -= (PAGE_SIZE << pg->order) / ENTRIES_SIZE;
}
pages -= remaining;
/*
* Check to see if the number of pages remaining would
* just fit the number of entries skipped.
*/
WARN(pg || skip > 0, "Extra allocated pages for ftrace: %lu with %lu skipped",
remaining, skipped);
}
/* Need to synchronize with ftrace_location_range() */
synchronize_rcu();
ftrace_free_pages(pg_unuse);
}
-- Steve
^ permalink raw reply
* Re: [PATCH v2] ftrace: Do not over-allocate ftrace memory
From: Guenter Roeck @ 2026-01-07 1:47 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260106195241.166f4115@fedora>
On Tue, Jan 06, 2026 at 07:52:41PM -0500, Steven Rostedt wrote:
> On Tue, 6 Jan 2026 16:24:28 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
> > if (pg_unuse) {
> > - unsigned long pg_remaining, remaining = 0;
> > - unsigned long skip;
> > -
> > - /* Count the number of entries unused and compare it to skipped. */
> > - pg_remaining = (ENTRIES_PER_PAGE << pg->order) - pg->index;
> > -
>
> The above needs to be fixed though.
Yes, and
skip = DIV_ROUND_UP(skip, ENTRIES_PER_PAGE);
is also wrong. That means
WARN(skip != remaining, "Extra allocated pages for ftrace: %lu with %lu skipped",
remaining, skipped);
will be hit because because 'skip' may be inaccurate. Example with added
debug log:
ftrace: skipped: 654 pg_remaining: 313 skip: 341 (3) pages: 310 remaining: 2
------------[ cut here ]------------
Extra allocated pages for ftrace: 2 with 654 skipped
WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:7299 ftrace_process_locs+0x5a8/0x5c0
...
ftrace: allocating 52250 entries in 308 pages
ftrace: allocated 308 pages with 4 groups
The warning is seen because the number of pages needed for 341 entries is
mis-calculated as 3 instead of 2.
That means I'll need the added code from v1 to calculate the correct value
for the number of pages needed, at least if you want to keep the validation
code with the warning backtraces.
Thanks,
Guenter
^ permalink raw reply
* [PATCH] fgraph: Make sure fregs is non-null when accessing
From: sunliming @ 2026-01-07 1:51 UTC (permalink / raw)
To: rostedt, mhiramat
Cc: mark.rutland, mathieu.desnoyers, linux-kernel, linux-trace-kernel,
sunliming, kernel test robot, Dan Carpenter
From: sunliming <sunliming@kylinos.cn>
Fix below smatch warnings:
kernel/trace/fgraph.c:834 __ftrace_return_to_handler() error: we previously
assumed 'fregs' could be null (see line 830)
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/r/202512131657.JQUt5fXQ-lkp@intel.com/
Signed-off-by: sunliming <sunliming@kylinos.cn>
---
kernel/trace/fgraph.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index cc48d16be43e..0f50a55032c5 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -837,7 +837,8 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
goto out;
#ifdef CONFIG_FUNCTION_GRAPH_RETVAL
- trace.retval = ftrace_regs_get_return_value(fregs);
+ if (fregs)
+ trace.retval = ftrace_regs_get_return_value(fregs);
#endif
bitmap = get_bitmap_bits(current, offset);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] ftrace: Do not over-allocate ftrace memory
From: Guenter Roeck @ 2026-01-07 1:56 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260106203352.539b9548@gandalf.local.home>
On Tue, Jan 06, 2026 at 08:33:52PM -0500, Steven Rostedt wrote:
> On Tue, 6 Jan 2026 16:54:27 -0800
> Guenter Roeck <linux@roeck-us.net> wrote:
>
> > > Don't remove this block. It's still needed. A lot of entries are
> > > skipped when adding the records. Weak functions and zero'd pointers
> > > that were part of the count are skipped. This is the code that handles
> > > that. It has nothing to do with rounding errors.
> > >
> > Sorry, misunderstanding. I thought that is what you meant with "This will
> > make pages equal the number of pages that were allocated. Then I'm not sure
> > we need this extra logic."
> >
> > What is the no longer needed extra logic ?
>
> I meant the added logic you had there, as I was a bit confused by the
> "spaces" part.
>
That was just to avoid "skip" going negative in the for loop.
The code below should work as well. I'll give it a try.
Thanks,
Guenter
> The logic still needs to be updated, but I think it can be done with the
> following:
>
> if (pg_unuse) {
> unsigned long pg_remaining, remaining = 0;
> long skip;
>
> /* Count the number of entries unused and compare it to skipped. */
> pg_remaining = (PAGE_SIZE << pg->order) / ENTRIES_SIZE - pg->index;
>
> if (!WARN(skipped < pg_remaining, "Extra allocated pages for ftrace")) {
>
> skip = skipped - pg_remaining;
>
> for (pg = pg_unuse; pg && skip > 0; pg = pg->next) {
> remaining += 1 << pg->order;
> skip -= (PAGE_SIZE << pg->order) / ENTRIES_SIZE;
> }
>
> pages -= remaining;
>
> /*
> * Check to see if the number of pages remaining would
> * just fit the number of entries skipped.
> */
> WARN(pg || skip > 0, "Extra allocated pages for ftrace: %lu with %lu skipped",
> remaining, skipped);
> }
> /* Need to synchronize with ftrace_location_range() */
> synchronize_rcu();
> ftrace_free_pages(pg_unuse);
> }
>
> -- Steve
^ permalink raw reply
* build error on CONFIG_MEMCG=n "error: invalid use of undefined type 'struct mem_cgroup'"
From: Harry Yoo @ 2026-01-07 1:56 UTC (permalink / raw)
To: Thomas Ballasi; +Cc: akpm, linux-mm, linux-trace-kernel, mhiramat, rostedt
In-Reply-To: <20260105160423.23708-2-tballasi@linux.microsoft.com>
On Mon, Jan 05, 2026 at 08:04:22AM -0800, Thomas Ballasi wrote:
> Memory reclaim events are currently difficult to attribute to
> specific cgroups, making debugging memory pressure issues
> challenging. This patch adds memory cgroup ID (memcg_id) to key
> vmscan tracepoints to enable better correlation and analysis.
>
> For operations not associated with a specific cgroup, the field
> is defaulted to 0.
>
> Signed-off-by: Thomas Ballasi <tballasi@linux.microsoft.com>
> ---
> include/trace/events/vmscan.h | 79 ++++++++++++++++++++---------------
> mm/shrinker.c | 2 +-
> mm/vmscan.c | 17 ++++----
> 3 files changed, 56 insertions(+), 42 deletions(-)
>
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index 490958fa10dee..93a9a9ba9405d 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -208,31 +215,34 @@ TRACE_EVENT(mm_shrink_slab_start,
> TP_fast_assign(
> __entry->shr = shr;
> __entry->shrink = shr->scan_objects;
> - __entry->nid = sc->nid;
> __entry->nr_objects_to_shrink = nr_objects_to_shrink;
> __entry->gfp_flags = (__force unsigned long)sc->gfp_mask;
> __entry->cache_items = cache_items;
> __entry->delta = delta;
> __entry->total_scan = total_scan;
> __entry->priority = priority;
> + __entry->nid = sc->nid;
> + __entry->memcg_id = sc->memcg ? cgroup_id(sc->memcg->css.cgroup) : 0;
Hi Thomas, this is breaking CONFIG_MEMCG=n builds.
In file included from ./include/trace/define_trace.h:132,
from ./include/trace/events/vmscan.h:569,
from mm/vmscan.c:73:
./include/trace/events/vmscan.h: In function ‘do_trace_event_raw_event_mm_shrink_slab_start’:
./include/trace/events/vmscan.h:248:68: error: invalid use of undefined type ‘struct mem_cgroup’
248 | __entry->memcg_id = sc->memcg ? cgroup_id(sc->memcg->css.cgroup) : 0;
| ^~
./include/trace/trace_events.h:427:11: note: in definition of macro ‘__DECLARE_EVENT_CLASS’
427 | { assign; } \
| ^~~~~~
./include/trace/trace_events.h:435:23: note: in expansion of macro ‘PARAMS’
435 | PARAMS(assign), PARAMS(print)) \
| ^~~~~~
./include/trace/trace_events.h:40:9: note: in expansion of macro ‘DECLARE_EVENT_CLASS’
40 | DECLARE_EVENT_CLASS(name, \
| ^~~~~~~~~~~~~~~~~~~
./include/trace/trace_events.h:44:30: note: in expansion of macro ‘PARAMS’
44 | PARAMS(assign), \
| ^~~~~~
./include/trace/events/vmscan.h:214:1: note: in expansion of macro ‘TRACE_EVENT’
214 | TRACE_EVENT(mm_shrink_slab_start,
| ^~~~~~~~~~~
./include/trace/events/vmscan.h:237:9: note: in expansion of macro ‘TP_fast_assign’
237 | TP_fast_assign(
| ^~~~~~~~~~~~~~
./include/trace/events/vmscan.h: In function ‘do_trace_event_raw_event_mm_shrink_slab_end’:
./include/trace/events/vmscan.h:293:56: error: invalid use of undefined type ‘struct mem_cgroup’
293 | __entry->memcg_id = cgroup_id(sc->memcg->css.cgroup);
| ^~
./include/trace/trace_events.h:427:11: note: in definition of macro ‘__DECLARE_EVENT_CLASS’
427 | { assign; } \
| ^~~~~~
./include/trace/trace_events.h:435:23: note: in expansion of macro ‘PARAMS’
435 | PARAMS(assign), PARAMS(print)) \
| ^~~~~~
./include/trace/trace_events.h:40:9: note: in expansion of macro ‘DECLARE_EVENT_CLASS’
40 | DECLARE_EVENT_CLASS(name, \
| ^~~~~~~~~~~~~~~~~~~
./include/trace/trace_events.h:44:30: note: in expansion of macro ‘PARAMS’
44 | PARAMS(assign), \
| ^~~~~~
./include/trace/events/vmscan.h:266:1: note: in expansion of macro ‘TRACE_EVENT’
266 | TRACE_EVENT(mm_shrink_slab_end,
| ^~~~~~~~~~~
./include/trace/events/vmscan.h:285:9: note: in expansion of macro ‘TP_fast_assign’
285 | TP_fast_assign(
| ^~~~~~~~~~~~~~
In file included from ./include/trace/define_trace.h:133:
./include/trace/events/vmscan.h: In function ‘do_perf_trace_mm_shrink_slab_start’:
./include/trace/events/vmscan.h:248:68: error: invalid use of undefined type ‘struct mem_cgroup’
248 | __entry->memcg_id = sc->memcg ? cgroup_id(sc->memcg->css.cgroup) : 0;
| ^~
./include/trace/perf.h:51:11: note: in definition of macro ‘__DECLARE_EVENT_CLASS’
51 | { assign; } \
| ^~~~~~
./include/trace/perf.h:67:23: note: in expansion of macro ‘PARAMS’
67 | PARAMS(assign), PARAMS(print)) \
| ^~~~~~
./include/trace/trace_events.h:40:9: note: in expansion of macro ‘DECLARE_EVENT_CLASS’
40 | DECLARE_EVENT_CLASS(name, \
| ^~~~~~~~~~~~~~~~~~~
./include/trace/trace_events.h:44:30: note: in expansion of macro ‘PARAMS’
44 | PARAMS(assign), \
| ^~~~~~
./include/trace/events/vmscan.h:214:1: note: in expansion of macro ‘TRACE_EVENT’
214 | TRACE_EVENT(mm_shrink_slab_start,
| ^~~~~~~~~~~
./include/trace/events/vmscan.h:237:9: note: in expansion of macro ‘TP_fast_assign’
237 | TP_fast_assign(
| ^~~~~~~~~~~~~~
./include/trace/events/vmscan.h: In function ‘do_perf_trace_mm_shrink_slab_end’:
./include/trace/events/vmscan.h:293:56: error: invalid use of undefined type ‘struct mem_cgroup’
293 | __entry->memcg_id = cgroup_id(sc->memcg->css.cgroup);
| ^~
./include/trace/perf.h:51:11: note: in definition of macro ‘__DECLARE_EVENT_CLASS’
51 | { assign; } \
| ^~~~~~
./include/trace/perf.h:67:23: note: in expansion of macro ‘PARAMS’
67 | PARAMS(assign), PARAMS(print)) \
| ^~~~~~
./include/trace/trace_events.h:40:9: note: in expansion of macro ‘DECLARE_EVENT_CLASS’
40 | DECLARE_EVENT_CLASS(name, \
| ^~~~~~~~~~~~~~~~~~~
./include/trace/trace_events.h:44:30: note: in expansion of macro ‘PARAMS’
44 | PARAMS(assign), \
| ^~~~~~
./include/trace/events/vmscan.h:266:1: note: in expansion of macro ‘TRACE_EVENT’
266 | TRACE_EVENT(mm_shrink_slab_end,
| ^~~~~~~~~~~
./include/trace/events/vmscan.h:285:9: note: in expansion of macro ‘TP_fast_assign’
285 | TP_fast_assign(
| ^~~~~~~~~~~~~~
CC arch/x86/mm/extable.o
make[3]: *** [scripts/Makefile.build:287: mm/vmscan.o] Error 1
make[2]: *** [scripts/Makefile.build:556: mm] Error 2
make[2]: *** Waiting for unfinished jobs....
--
Cheers,
Harry / Hyeonggon
> ),
>
> - TP_printk("%pS %p: nid: %d objects to shrink %ld gfp_flags %s cache items %ld delta %lld total_scan %ld priority %d",
> + TP_printk("%pS %p: nid: %d memcg_id: %u objects to shrink %ld gfp_flags %s cache items %ld delta %lld total_scan %ld priority %d",
> __entry->shrink,
> __entry->shr,
> __entry->nid,
> + __entry->memcg_id,
> __entry->nr_objects_to_shrink,
> show_gfp_flags(__entry->gfp_flags),
> __entry->cache_items,
> @@ -242,36 +252,39 @@ TRACE_EVENT(mm_shrink_slab_start,
> );
>
> TRACE_EVENT(mm_shrink_slab_end,
> - TP_PROTO(struct shrinker *shr, int nid, int shrinker_retval,
> + TP_PROTO(struct shrinker *shr, struct shrink_control *sc, int shrinker_retval,
> long unused_scan_cnt, long new_scan_cnt, long total_scan),
>
> - TP_ARGS(shr, nid, shrinker_retval, unused_scan_cnt, new_scan_cnt,
> + TP_ARGS(shr, sc, shrinker_retval, unused_scan_cnt, new_scan_cnt,
> total_scan),
>
> TP_STRUCT__entry(
> __field(struct shrinker *, shr)
> - __field(int, nid)
> __field(void *, shrink)
> __field(long, unused_scan)
> __field(long, new_scan)
> - __field(int, retval)
> __field(long, total_scan)
> + __field(int, nid)
> + __field(int, retval)
> + __field(unsigned short, memcg_id)
> ),
>
> TP_fast_assign(
> __entry->shr = shr;
> - __entry->nid = nid;
> __entry->shrink = shr->scan_objects;
> __entry->unused_scan = unused_scan_cnt;
> __entry->new_scan = new_scan_cnt;
> - __entry->retval = shrinker_retval;
> __entry->total_scan = total_scan;
> + __entry->nid = sc->nid;
> + __entry->retval = shrinker_retval;
> + __entry->memcg_id = cgroup_id(sc->memcg->css.cgroup);
> ),
>
> - TP_printk("%pS %p: nid: %d unused scan count %ld new scan count %ld total_scan %ld last shrinker return val %d",
> + TP_printk("%pS %p: nid: %d memcg_id: %u unused scan count %ld new scan count %ld total_scan %ld last shrinker return val %d",
> __entry->shrink,
> __entry->shr,
> __entry->nid,
> + __entry->memcg_id,
> __entry->unused_scan,
> __entry->new_scan,
> __entry->total_scan,
> @@ -504,9 +517,9 @@ TRACE_EVENT(mm_vmscan_node_reclaim_begin,
>
> DEFINE_EVENT(mm_vmscan_direct_reclaim_end_template, mm_vmscan_node_reclaim_end,
>
> - TP_PROTO(unsigned long nr_reclaimed),
> + TP_PROTO(unsigned long nr_reclaimed, unsigned short memcg_id),
>
> - TP_ARGS(nr_reclaimed)
> + TP_ARGS(nr_reclaimed, memcg_id)
> );
>
> TRACE_EVENT(mm_vmscan_throttled,
^ permalink raw reply
* Re: build error on CONFIG_MEMCG=n "error: invalid use of undefined type 'struct mem_cgroup'"
From: Andrew Morton @ 2026-01-07 2:17 UTC (permalink / raw)
To: Harry Yoo; +Cc: Thomas Ballasi, linux-mm, linux-trace-kernel, mhiramat, rostedt
In-Reply-To: <aV29Xj2wo786xVn1@hyeyoo>
On Wed, 7 Jan 2026 10:56:46 +0900 Harry Yoo <harry.yoo@oracle.com> wrote:
> > --- a/include/trace/events/vmscan.h
> > +++ b/include/trace/events/vmscan.h
> > @@ -208,31 +215,34 @@ TRACE_EVENT(mm_shrink_slab_start,
> > TP_fast_assign(
> > __entry->shr = shr;
> > __entry->shrink = shr->scan_objects;
> > - __entry->nid = sc->nid;
> > __entry->nr_objects_to_shrink = nr_objects_to_shrink;
> > __entry->gfp_flags = (__force unsigned long)sc->gfp_mask;
> > __entry->cache_items = cache_items;
> > __entry->delta = delta;
> > __entry->total_scan = total_scan;
> > __entry->priority = priority;
> > + __entry->nid = sc->nid;
> > + __entry->memcg_id = sc->memcg ? cgroup_id(sc->memcg->css.cgroup) : 0;
>
> Hi Thomas, this is breaking CONFIG_MEMCG=n builds.
yup, thanks, that series has been removed from mm.git.
^ permalink raw reply
* [PATCH v3] ftrace: Do not over-allocate ftrace memory
From: Guenter Roeck @ 2026-01-07 2: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>
---
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..2c9692b45215 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 *pages)
{
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() */
--
2.45.2
^ permalink raw reply related
* Re: [PATCH v2] selftests/tracing: Fix test_multiple_writes stall
From: Fushuai Wang @ 2026-01-07 3:49 UTC (permalink / raw)
To: fushuai.wang
Cc: linux-kernel, linux-kselftest, linux-trace-kernel,
mathieu.desnoyers, mhiramat, rostedt, shuah, wangfushuai
In-Reply-To: <20251227041821.75504-1-fushuai.wang@linux.dev>
> When /sys/kernel/tracing/buffer_size_kb is less than 12KB,
> the test_multiple_writes test will stall and wait for more
> input due to insufficient buffer space.
>
> Check current buffer_size_kb value before the test. If it is
> less than 12KB, it temporarily increase the buffer to 12KB,
> and restore the original value after the tests are completed.
>
> Fixes: 37f46601383a ("selftests/tracing: Add basic test for trace_marker_raw file")
> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
Gentle ping.
---
Regards,
WANG
^ permalink raw reply
* Re: [PATCH] ring-buffer: Use a housekeeping CPU to wake up waiters
From: Petr Tesarik @ 2026-01-07 7:50 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, Sebastian Andrzej Siewior,
Clark Williams, linux-kernel, linux-trace-kernel, linux-rt-devel
In-Reply-To: <20260106170405.425f469e@gandalf.local.home>
On Tue, 6 Jan 2026 17:04:05 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 6 Jan 2026 10:10:39 +0100
> Petr Tesarik <ptesarik@suse.com> wrote:
>
> > Avoid running the wakeup irq_work on an isolated CPU. Since the wakeup can
> > run on any CPU, let's pick a housekeeping CPU to do the job.
> >
> > This change reduces additional noise when tracing isolated CPUs. For
> > example, the following ipi_send_cpu stack trace was captured with
> > nohz_full=2 on the isolated CPU:
> >
> > <idle>-0 [002] d.h4. 1255.379293: ipi_send_cpu: cpu=2 callsite=irq_work_queue+0x2d/0x50 callback=rb_wake_up_waiters+0x0/0x80
> > <idle>-0 [002] d.h4. 1255.379329: <stack trace>
> > => trace_event_raw_event_ipi_send_cpu
> > => __irq_work_queue_local
> > => irq_work_queue
> > => ring_buffer_unlock_commit
> > => trace_buffer_unlock_commit_regs
> > => trace_event_buffer_commit
> > => trace_event_raw_event_x86_irq_vector
> > => __sysvec_apic_timer_interrupt
> > => sysvec_apic_timer_interrupt
> > => asm_sysvec_apic_timer_interrupt
> > => pv_native_safe_halt
> > => default_idle
> > => default_idle_call
> > => do_idle
> > => cpu_startup_entry
> > => start_secondary
> > => common_startup_64
>
> I take it that even with this patch you would still get the above events.
> The only difference would be the "cpu=" in the event info will not be the
> same as the CPU it executed on, right?
Yes, this is trace of a similar event after applying the patch:
<idle>-0 [002] d.h4. 313.334367: ipi_send_cpu: cpu=1 callsite=irq_work_queue_on+0x55/0x90 callback=generic_smp_call_function_single_interrupt+0x0/0x20
<idle>-0 [002] d.h4. 313.334390: <stack trace>
=> trace_event_raw_event_ipi_send_cpu
=> __smp_call_single_queue
=> irq_work_queue_on
=> ring_buffer_unlock_commit
=> trace_buffer_unlock_commit_regs
=> trace_event_buffer_commit
=> trace_event_raw_event_x86_irq_vector
=> __sysvec_apic_timer_interrupt
=> sysvec_apic_timer_interrupt
=> asm_sysvec_apic_timer_interrupt
=> pv_native_safe_halt
=> default_idle
=> default_idle_call
=> do_idle
=> cpu_startup_entry
=> start_secondary
=> common_startup_64
The callback function in the trace event is different. That's because
send_call_function_single_ipi() always uses this value. Maybe it can be
improved, and I can look into it, but that's clearly a very separate
issue.
> > The IRQ work interrupt alone adds considerable noise, but the impact can
> > get even worse with PREEMPT_RT, because the IRQ work interrupt is then
> > handled by a separate kernel thread. This requires a task switch and makes
> > tracing useless for analyzing latency on an isolated CPU.
> >
> > Signed-off-by: Petr Tesarik <ptesarik@suse.com>
>
> LGTM,
>
> I'll queue it up for the next merge window.
Thank you!
Petr T
^ permalink raw reply
* Re: [BUG/RFC 1/2] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run
From: Jiri Olsa @ 2026-01-07 8:23 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu (Google), Will Deacon, Peter Zijlstra, bpf,
linux-trace-kernel, linux-arm-kernel, x86, Yonghong Song,
Song Liu, Andrii Nakryiko, Mark Rutland, Mahe Tardy
In-Reply-To: <20260105162220.6ba5129a@gandalf.local.home>
On Mon, Jan 05, 2026 at 04:22:20PM -0500, Steven Rostedt wrote:
> On Sun, 4 Jan 2026 22:34:15 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
>
> > > This looks a bit grotty to me and presumably other architectures would
> > > need similar treatement. Wouldn't it be cleaner to reuse the existing
> > > API instead? For example, by calling ftrace_regs_set_instruction_pointer()
> > > and ftrace_regs_set_return_value() to update the relevant registers from
> > > the core code?
> >
> > I agreed with using the generic APIs. Also, ftrace_partial_regs_fix() is
> > not self-explained. Maybe ftrace_regs_set_by_regs()?
>
> Or perhaps: ftrace_partial_regs_update() where you call it if you need to
> update the regs.
>
> /*
> * ftrace_partial_regs_update - update the original ftrace_regs from regs
> * @fregs: The ftrace_regs to update from @regs
> * @regs: The partial regs from ftrace_partial_regs() that was updated
> *
> * Some architectures have the partial regs living in the ftrace_regs
> * structure, whereas other architectures need to make a different copy
> * of the @regs. If a partial @regs is retrieved by ftrace_partial_regs() and
> * if the code using @regs updates a field (like the instruction pointer or
> * stack pointer) it may need to propagate that change to the original @fregs
> * it retrieved the partial @regs from. Use this function to guarantee that
> * update happens.
> */
> static __always_inline void
> ftrace_partial_regs_update(const struct ftrace_regs *fregs, struct pt_regs *regs) {
> struct __arch_ftrace_regs *afregs = arch_ftrace_regs(fregs);
>
> if (afregs->pc != regs->pc) {
> afregs->pc = regs->pc;
> afregs->regs[0] = regs->regs[0];
> }
> }
>
> -- Steve
lgtm, the full change is below, I'll send new version of the patchset
thanks,
jirka
---
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
index 1621c84f44b3..177c7bbf3b84 100644
--- a/arch/arm64/include/asm/ftrace.h
+++ b/arch/arm64/include/asm/ftrace.h
@@ -157,6 +157,30 @@ ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs)
return regs;
}
+/*
+ * ftrace_partial_regs_update - update the original ftrace_regs from regs
+ * @fregs: The ftrace_regs to update from @regs
+ * @regs: The partial regs from ftrace_partial_regs() that was updated
+ *
+ * Some architectures have the partial regs living in the ftrace_regs
+ * structure, whereas other architectures need to make a different copy
+ * of the @regs. If a partial @regs is retrieved by ftrace_partial_regs() and
+ * if the code using @regs updates a field (like the instruction pointer or
+ * stack pointer) it may need to propagate that change to the original @fregs
+ * it retrieved the partial @regs from. Use this function to guarantee that
+ * update happens.
+ */
+static __always_inline void
+ftrace_partial_regs_update(const struct ftrace_regs *fregs, struct pt_regs *regs)
+{
+ struct __arch_ftrace_regs *afregs = arch_ftrace_regs(fregs);
+
+ if (afregs->pc != regs->pc) {
+ afregs->pc = regs->pc;
+ afregs->regs[0] = regs->regs[0];
+ }
+}
+
#define arch_ftrace_fill_perf_regs(fregs, _regs) do { \
(_regs)->pc = arch_ftrace_regs(fregs)->pc; \
(_regs)->regs[29] = arch_ftrace_regs(fregs)->fp; \
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 770f0dc993cc..ae22559b4099 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -213,6 +213,9 @@ ftrace_partial_regs(struct ftrace_regs *fregs, struct pt_regs *regs)
return regs;
}
+static __always_inline void
+ftrace_partial_regs_update(struct ftrace_regs *fregs, struct pt_regs *regs) { }
+
#endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS */
#ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 6e076485bf70..3a17f79b20c2 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2564,6 +2564,7 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
old_run_ctx = bpf_set_run_ctx(&run_ctx.session_ctx.run_ctx);
err = bpf_prog_run(link->link.prog, regs);
bpf_reset_run_ctx(old_run_ctx);
+ ftrace_partial_regs_update(fregs, bpf_kprobe_multi_pt_regs_ptr());
rcu_read_unlock();
out:
--
2.52.0
^ permalink raw reply related
* [PATCH bpf-next 1/2] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run
From: Jiri Olsa @ 2026-01-07 9:32 UTC (permalink / raw)
To: Masami Hiramatsu, Steven Rostedt, Will Deacon
Cc: Mahe Tardy, Peter Zijlstra, bpf, linux-trace-kernel,
linux-arm-kernel, x86, Yonghong Song, Song Liu, Andrii Nakryiko,
Mark Rutland
Mahe reported issue with bpf_override_return helper not working when
executed from kprobe.multi bpf program on arm.
The problem is that on arm we use alternate storage for pt_regs object
that is passed to bpf_prog_run and if any register is changed (which
is the case of bpf_override_return) it's not propagated back to actual
pt_regs object.
Fixing this by introducing and calling ftrace_partial_regs_update function
to propagate the values of changed registers (ip and stack).
Fixes: b9b55c8912ce ("tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs")
Reported-by: Mahe Tardy <mahe.tardy@gmail.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
v1 changes:
- used ftrace_partial_regs_update with comments from Steven
arch/arm64/include/asm/ftrace.h | 24 ++++++++++++++++++++++++
include/linux/ftrace.h | 3 +++
kernel/trace/bpf_trace.c | 1 +
3 files changed, 28 insertions(+)
diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
index 1621c84f44b3..177c7bbf3b84 100644
--- a/arch/arm64/include/asm/ftrace.h
+++ b/arch/arm64/include/asm/ftrace.h
@@ -157,6 +157,30 @@ ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs)
return regs;
}
+/*
+ * ftrace_partial_regs_update - update the original ftrace_regs from regs
+ * @fregs: The ftrace_regs to update from @regs
+ * @regs: The partial regs from ftrace_partial_regs() that was updated
+ *
+ * Some architectures have the partial regs living in the ftrace_regs
+ * structure, whereas other architectures need to make a different copy
+ * of the @regs. If a partial @regs is retrieved by ftrace_partial_regs() and
+ * if the code using @regs updates a field (like the instruction pointer or
+ * stack pointer) it may need to propagate that change to the original @fregs
+ * it retrieved the partial @regs from. Use this function to guarantee that
+ * update happens.
+ */
+static __always_inline void
+ftrace_partial_regs_update(const struct ftrace_regs *fregs, struct pt_regs *regs)
+{
+ struct __arch_ftrace_regs *afregs = arch_ftrace_regs(fregs);
+
+ if (afregs->pc != regs->pc) {
+ afregs->pc = regs->pc;
+ afregs->regs[0] = regs->regs[0];
+ }
+}
+
#define arch_ftrace_fill_perf_regs(fregs, _regs) do { \
(_regs)->pc = arch_ftrace_regs(fregs)->pc; \
(_regs)->regs[29] = arch_ftrace_regs(fregs)->fp; \
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 770f0dc993cc..ae22559b4099 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -213,6 +213,9 @@ ftrace_partial_regs(struct ftrace_regs *fregs, struct pt_regs *regs)
return regs;
}
+static __always_inline void
+ftrace_partial_regs_update(struct ftrace_regs *fregs, struct pt_regs *regs) { }
+
#endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS */
#ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 6e076485bf70..3a17f79b20c2 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2564,6 +2564,7 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
old_run_ctx = bpf_set_run_ctx(&run_ctx.session_ctx.run_ctx);
err = bpf_prog_run(link->link.prog, regs);
bpf_reset_run_ctx(old_run_ctx);
+ ftrace_partial_regs_update(fregs, bpf_kprobe_multi_pt_regs_ptr());
rcu_read_unlock();
out:
--
2.52.0
^ permalink raw reply related
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