* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Jani Nikula @ 2026-01-05 9:29 UTC (permalink / raw)
To: Yury Norov, Andy Shevchenko
Cc: Joel Fernandes, Steven Rostedt, Andrew Morton, Masami Hiramatsu,
Mathieu Desnoyers, Christophe Leroy, Randy Dunlap, Ingo Molnar,
Joonas Lahtinen, David Laight, Petr Pavlu, Andi Shyti,
Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
dri-devel, linux-modules, linux-trace-kernel
In-Reply-To: <aVkmQ4EGIQgAddZQ@yury>
On Sat, 03 Jan 2026, Yury Norov <yury.norov@gmail.com> wrote:
> On Sat, Jan 03, 2026 at 02:57:58PM +0200, Andy Shevchenko wrote:
>> On Fri, Jan 02, 2026 at 07:50:59PM -0500, Joel Fernandes wrote:
>> > On Mon, Dec 29, 2025 at 11:17:48AM -0500, Steven Rostedt wrote:
>>
>> ...
>>
>> > I use trace_printk() all the time for kernel, particularly RCU development.
>> > One of the key usecases I have is dumping traces on panic (with panic on warn
>> > and stop tracing on warn enabled). This is extremely useful since I can add
>> > custom tracing and dump traces when rare conditions occur. I fixed several
>> > bugs with this technique.
>> >
>> > I also recommend keeping it convenient to use.
>>
>> Okay, you know C, please share your opinion what header is the best to hold the
>> trace_printk.h to be included.
>
> What if we include it on Makefile level, similarly to how W=1 works?
>
> make D=1 // trace_printk() is available
> make D=0 // trace_printk() is not available
> make // trace_printk() is not available
>
> Where D stands for debugging.
>
> D=1 may be a default setting if you prefer, but the most important is
> that every compilation unit will have an access to debugging without
> polluting core headers.
You do realize this means recompiling everything when adding D=1 for
debugging?
BR,
Jani.
--
Jani Nikula, Intel
^ permalink raw reply
* Re: [PATCH net-next v3] page_pool: Add page_pool_release_stalled tracepoint
From: Leon Hwang @ 2026-01-05 6:23 UTC (permalink / raw)
To: Yunsheng Lin, Jesper Dangaard Brouer, netdev
Cc: Ilias Apalodimas, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, kerneljasonxing, lance.yang,
jiayuan.chen, linux-kernel, linux-trace-kernel, Leon Huang Fu,
Dragos Tatulea, kernel-team, Yan Zhai
In-Reply-To: <dfc33064-f99f-4728-858f-95c80300bcff@huawei.com>
On 4/1/26 10:18, Yunsheng Lin wrote:
> On 2026/1/2 19:43, Jesper Dangaard Brouer wrote:
>>
>>
>> On 02/01/2026 08.17, Leon Hwang wrote:
>>> Introduce a new tracepoint to track stalled page pool releases,
>>> providing better observability for page pool lifecycle issues.
>>>
>>
>> In general I like/support adding this tracepoint for "debugability" of
>> page pool lifecycle issues.
>>
>> For "observability" @Kuba added a netlink scheme[1][2] for page_pool[3], which gives us the ability to get events and list page_pools from userspace.
>> I've not used this myself (yet) so I need input from others if this is something that others have been using for page pool lifecycle issues?
>>
>> Need input from @Kuba/others as the "page-pool-get"[4] state that "Only Page Pools associated with a net_device can be listed". Don't we want the ability to list "invisible" page_pool's to allow debugging issues?
>>
>> [1] https://docs.kernel.org/userspace-api/netlink/intro-specs.html
>> [2] https://docs.kernel.org/userspace-api/netlink/index.html
>> [3] https://docs.kernel.org/netlink/specs/netdev.html
>> [4] https://docs.kernel.org/netlink/specs/netdev.html#page-pool-get
>>
>> Looking at the code, I see that NETDEV_CMD_PAGE_POOL_CHANGE_NTF netlink
>> notification is only generated once (in page_pool_destroy) and not when
>> we retry in page_pool_release_retry (like this patch). In that sense,
>> this patch/tracepoint is catching something more than netlink provides.
>> First I though we could add a netlink notification, but I can imagine
>> cases this could generate too many netlink messages e.g. a netdev with
>> 128 RX queues generating these every second for every RX queue.
>>
>> Guess, I've talked myself into liking this change, what do other
>> maintainers think? (e.g. netlink scheme and debugging balance)
>>
>>
>>> Problem:
>>> Currently, when a page pool shutdown is stalled due to inflight pages,
>>> the kernel only logs a warning message via pr_warn(). This has several
>>> limitations:
>>>
>>> 1. The warning floods the kernel log after the initial DEFER_WARN_INTERVAL,
>>> making it difficult to track the progression of stalled releases
>>> 2. There's no structured way to monitor or analyze these events
>>> 3. Debugging tools cannot easily capture and correlate stalled pool
>>> events with other network activity
>>>
>>> Solution:
>>> Add a new tracepoint, page_pool_release_stalled, that fires when a page
>>> pool shutdown is stalled. The tracepoint captures:
>>> - pool: pointer to the stalled page_pool
>>> - inflight: number of pages still in flight
>>> - sec: seconds since the release was deferred
>>>
>>> The implementation also modifies the logging behavior:
>>> - pr_warn() is only emitted during the first warning interval
>>> (DEFER_WARN_INTERVAL to DEFER_WARN_INTERVAL*2)
>>> - The tracepoint is fired always, reducing log noise while still
>>> allowing monitoring tools to track the issue
>
> If the initial log is still present, I don't really see what's the benefit
> of re-triggering logs or tracepoints when the first two fields are unchanged
> and the last two fields can be inspected using some tool? If there are none,
> perhaps we only need to print the first trigger log and a log upon completion
> of page_pool destruction.
>
Even though it is possible to inspect the last two fields via the
workqueue (e.g., by tracing page_pool_release_retry with BPF tools),
this is not a practical approach for routine monitoring or debugging.
With the proposed tracepoint, obtaining these fields becomes
straightforward and lightweight, making it much easier to observe and
reason about stalled page pool releases in real systems.
In the issue I encountered, it was crucial to notice that the inflight
count was gradually decreasing over time. This gave me confidence that
some orphaned pages were eventually being returned to the page pool.
Based on that signal, I was then able to capture the call stack of
page_pool_put_defragged_page (kernel v6.6) to identify the code path
responsible for returning those pages.
Without repeated pr_warn logs or tracepoint events, it would have been
significantly harder to observe this progression and correlate it with
the eventual page returns.
Thanks,
Leon
^ permalink raw reply
* Re: [[PATCH v2] 1/1] kprobes: retry pending optprobe after freeing blocker
From: Masami Hiramatsu @ 2026-01-05 4:58 UTC (permalink / raw)
To: hongao
Cc: naveen, anil.s.keshavamurthy, davem, linux-kernel,
linux-trace-kernel
In-Reply-To: <C5E6295166D4EDE9+20251210033321.2895692-1-hongao@uniontech.com>
Hi Hongao,
Thanks for updating. After a detailed review, we don't need a new boolean
flag for it. Since the "queued unused probe" (which is handled by
do_free_cleaned_kprobes() eventually) is always disarmed. Thus, we only
need to check all probes to reoptimize sibling probes in the
do_free_cleaned_kprobes().
On Wed, 10 Dec 2025 11:33:21 +0800
hongao <hongao@uniontech.com> wrote:
> The freeing_list cleanup now retries optimizing any sibling probe that was
> deferred while this aggregator was being torn down. Track the pending
> address in struct optimized_kprobe so __disarm_kprobe() can defer the
> retry until kprobe_optimizer() finishes disarming.
>
> Signed-off-by: hongao <hongao@uniontech.com>
> ---
> Changes since v1:
> - Replace `kprobe_opcode_t *pending_reopt_addr` with `bool reopt_unblocked_probes`
> in `struct optimized_kprobe` to avoid storing an address and simplify logic.
> - Use `op->kp.addr` when looking up the sibling optimized probe instead of
> keeping a separate stored address.
> - Defer re-optimization by setting/clearing `op->reopt_unblocked_probes` in
> `__disarm_kprobe()` / consuming it in `do_free_cleaned_kprobes()` so the
> retry runs after the worker finishes disarming.
> - Link to v1: https://lore.kernel.org/all/2B0BC73E9D190B7B+20251027130535.2296913-1-hongao@uniontech.com/
> ---
> include/linux/kprobes.h | 1 +
> kernel/kprobes.c | 28 ++++++++++++++++++++++------
> 2 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 8c4f3bb24..4f49925a4 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -338,6 +338,7 @@ DEFINE_INSN_CACHE_OPS(insn);
> struct optimized_kprobe {
> struct kprobe kp;
> struct list_head list; /* list for optimizing queue */
> + bool reopt_unblocked_probes;
> struct arch_optimized_insn optinsn;
> };
This is not needed.
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index da59c68df..799542dff 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -514,6 +514,7 @@ static LIST_HEAD(freeing_list);
>
> static void kprobe_optimizer(struct work_struct *work);
> static DECLARE_DELAYED_WORK(optimizing_work, kprobe_optimizer);
> +static void optimize_kprobe(struct kprobe *p);
> #define OPTIMIZE_DELAY 5
>
> /*
> @@ -591,6 +592,21 @@ static void do_free_cleaned_kprobes(void)
> */
> continue;
> }
> + if (op->reopt_unblocked_probes) {
> + struct kprobe *unblocked;
> +
> + /*
> + * The aggregator was holding back another probe while it sat on the
> + * unoptimizing/freeing lists. Now that the aggregator has been fully
> + * reverted we can safely retry the optimization of that sibling.
> + */
> +
> + unblocked = get_optimized_kprobe(op->kp.addr);
> + if (unlikely(unblocked))
> + optimize_kprobe(unblocked);
> + op->reopt_unblocked_probes = false;
> + }
This is what we need. (but do not need to check/update reopt_unblocked_probes.
> +
> free_aggr_kprobe(&op->kp);
> }
> }
> @@ -1009,13 +1025,13 @@ static void __disarm_kprobe(struct kprobe *p, bool reopt)
> _p = get_optimized_kprobe(p->addr);
> if (unlikely(_p) && reopt)
> optimize_kprobe(_p);
> + } else if (reopt && kprobe_aggrprobe(p)) {
> + struct optimized_kprobe *op =
> + container_of(p, struct optimized_kprobe, kp);
> +
> + /* Defer the re-optimization until the worker finishes disarming. */
> + op->reopt_unblocked_probes = true;
Do not need this.
> }
> - /*
> - * TODO: Since unoptimization and real disarming will be done by
> - * the worker thread, we can not check whether another probe are
> - * unoptimized because of this probe here. It should be re-optimized
> - * by the worker thread.
> - */
Only remove this comment.
Thank you,
> }
>
> #else /* !CONFIG_OPTPROBES */
> --
> 2.47.2
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] ftrace: Make ftrace_graph_ent depth field signed
From: Masami Hiramatsu @ 2026-01-05 2:21 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers,
Dan Carpenter, pengdonglin
In-Reply-To: <20260102143148.251c2e16@gandalf.local.home>
On Fri, 2 Jan 2026 14:31:48 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> The code has integrity checks to make sure that depth never goes below
> zero. But the depth field has recently been converted to unsigned long
> from "int" (for alignment reasons). As unsigned long can never be less
> than zero, the integrity checks no longer work.
>
> Convert depth to long from unsigned long to allow the integrity checks to
> work again.
>
> Cc: stable@vger.kernel.org
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/aS6kGi0maWBl-MjZ@stanley.mountain/
> Fixes: f83ac7544fbf7 ("function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks,
> ---
> include/linux/ftrace.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 770f0dc993cc..a3a8989e3268 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -1167,7 +1167,7 @@ static inline void ftrace_init(void) { }
> */
> struct ftrace_graph_ent {
> unsigned long func; /* Current function */
> - unsigned long depth;
> + long depth; /* signed to check for less than zero */
> } __packed;
>
> /*
> --
> 2.51.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next v3] page_pool: Add page_pool_release_stalled tracepoint
From: Jakub Kicinski @ 2026-01-04 16:43 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Leon Hwang, netdev, Ilias Apalodimas, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, David S . Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, kerneljasonxing,
lance.yang, jiayuan.chen, linux-kernel, linux-trace-kernel,
Leon Huang Fu, Dragos Tatulea, kernel-team, Yan Zhai
In-Reply-To: <011ca15e-107b-4679-8203-f5f821f27900@kernel.org>
On Fri, 2 Jan 2026 12:43:46 +0100 Jesper Dangaard Brouer wrote:
> On 02/01/2026 08.17, Leon Hwang wrote:
> > Introduce a new tracepoint to track stalled page pool releases,
> > providing better observability for page pool lifecycle issues.
>
> In general I like/support adding this tracepoint for "debugability" of
> page pool lifecycle issues.
>
> For "observability" @Kuba added a netlink scheme[1][2] for page_pool[3],
> which gives us the ability to get events and list page_pools from userspace.
> I've not used this myself (yet) so I need input from others if this is
> something that others have been using for page pool lifecycle issues?
My input here is the least valuable (since one may expect the person
who added the code uses it) - but FWIW yes, we do use the PP stats to
monitor PP lifecycle issues at Meta. That said - we only monitor for
accumulation of leaked memory from orphaned pages, as the whole reason
for adding this code was that in practice the page may be sitting in
a socket rx queue (or defer free queue etc.) IOW a PP which is not
getting destroyed for a long time is not necessarily a kernel issue.
> Need input from @Kuba/others as the "page-pool-get"[4] state that "Only
> Page Pools associated with a net_device can be listed". Don't we want
> the ability to list "invisible" page_pool's to allow debugging issues?
>
> [1] https://docs.kernel.org/userspace-api/netlink/intro-specs.html
> [2] https://docs.kernel.org/userspace-api/netlink/index.html
> [3] https://docs.kernel.org/netlink/specs/netdev.html
> [4] https://docs.kernel.org/netlink/specs/netdev.html#page-pool-get
The documentation should probably be updated :(
I think what I meant is that most _drivers_ didn't link their PP to the
netdev via params when the API was added. So if the user doesn't see the
page pools - the driver is probably not well maintained.
In practice only page pools which are not accessible / visible via the
API are page pools from already destroyed network namespaces (assuming
their netdevs were also destroyed and not re-parented to init_net).
Which I'd think is a rare case?
> Looking at the code, I see that NETDEV_CMD_PAGE_POOL_CHANGE_NTF netlink
> notification is only generated once (in page_pool_destroy) and not when
> we retry in page_pool_release_retry (like this patch). In that sense,
> this patch/tracepoint is catching something more than netlink provides.
> First I though we could add a netlink notification, but I can imagine
> cases this could generate too many netlink messages e.g. a netdev with
> 128 RX queues generating these every second for every RX queue.
FWIW yes, we can add more notifications. Tho, as I mentioned at the
start of my reply - the expectation is that page pools waiting for
a long time to be destroyed is something that _will_ happen in
production.
> Guess, I've talked myself into liking this change, what do other
> maintainers think? (e.g. netlink scheme and debugging balance)
We added the Netlink API to mute the pr_warn() in all practical cases.
If Xiang Mei is seeing the pr_warn() I think we should start by asking
what kernel and driver they are using, and what the usage pattern is :(
As I mentioned most commonly the pr_warn() will trigger because driver
doesn't link the pp to a netdev.
^ permalink raw reply
* Re: [BUG/RFC 1/2] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run
From: Masami Hiramatsu @ 2026-01-04 13:34 UTC (permalink / raw)
To: Jiri Olsa, Will Deacon
Cc: Masami Hiramatsu, Steven Rostedt, Peter Zijlstra, bpf,
linux-trace-kernel, linux-arm-kernel, x86, Yonghong Song,
Song Liu, Andrii Nakryiko, Mark Rutland, Mahe Tardy
In-Reply-To: <aVfbqYsWdGXu4lh8@willie-the-truck>
On Fri, 2 Jan 2026 14:52:25 +0000
Will Deacon <will@kernel.org> wrote:
> On Wed, Nov 05, 2025 at 01:59:23PM +0100, Jiri Olsa wrote:
> > hi,
> > Mahe reported issue with bpf_override_return helper not working
> > when executed from kprobe.multi bpf program on arm.
> >
> > The problem seems to be 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.
> >
> > The change below seems to fix the issue, but I have no idea if
> > that's proper fix for arm, thoughts?
> >
> > I'm attaching selftest to actually test bpf_override_return helper
> > functionality, because currently we only test that we are able to
> > attach a program with it, but not the override itself.
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > arch/arm64/include/asm/ftrace.h | 11 +++++++++++
> > include/linux/ftrace.h | 3 +++
> > kernel/trace/bpf_trace.c | 1 +
> > 3 files changed, 15 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
> > index ba7cf7fec5e9..ad6cf587885c 100644
> > --- a/arch/arm64/include/asm/ftrace.h
> > +++ b/arch/arm64/include/asm/ftrace.h
> > @@ -157,6 +157,17 @@ ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs)
> > return regs;
> > }
> >
> > +static __always_inline void
> > +ftrace_partial_regs_fix(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];
> > + }
> > +}
>
> 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()?
Thank you,
>
> Will
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [BUG/RFC 1/2] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run
From: Jiri Olsa @ 2026-01-04 11:56 UTC (permalink / raw)
To: Will Deacon
Cc: Masami Hiramatsu, Steven Rostedt, Peter Zijlstra, bpf,
linux-trace-kernel, linux-arm-kernel, x86, Yonghong Song,
Song Liu, Andrii Nakryiko, Mark Rutland, Mahe Tardy
In-Reply-To: <aVfbqYsWdGXu4lh8@willie-the-truck>
On Fri, Jan 02, 2026 at 02:52:25PM +0000, Will Deacon wrote:
> On Wed, Nov 05, 2025 at 01:59:23PM +0100, Jiri Olsa wrote:
> > hi,
> > Mahe reported issue with bpf_override_return helper not working
> > when executed from kprobe.multi bpf program on arm.
> >
> > The problem seems to be 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.
> >
> > The change below seems to fix the issue, but I have no idea if
> > that's proper fix for arm, thoughts?
> >
> > I'm attaching selftest to actually test bpf_override_return helper
> > functionality, because currently we only test that we are able to
> > attach a program with it, but not the override itself.
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > arch/arm64/include/asm/ftrace.h | 11 +++++++++++
> > include/linux/ftrace.h | 3 +++
> > kernel/trace/bpf_trace.c | 1 +
> > 3 files changed, 15 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
> > index ba7cf7fec5e9..ad6cf587885c 100644
> > --- a/arch/arm64/include/asm/ftrace.h
> > +++ b/arch/arm64/include/asm/ftrace.h
> > @@ -157,6 +157,17 @@ ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs)
> > return regs;
> > }
> >
> > +static __always_inline void
> > +ftrace_partial_regs_fix(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];
> > + }
> > +}
>
> 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 knew I forgot some change.. thanks for replying
ftrace_partial_regs is overloaded in arm64 and because of that we need
to propagate the change to pt_regs, so I think the ftrace_partial_regs_fix
code is arm64 specific, so can't see that in core code
also wrt ftrace_partial_regs_fix name, I was thinking it might be better
to have begin/end functions, like:
ftrace_partial_regs_begin
ftrace_partial_regs_end
thanks,
jirka
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2560,10 +2560,11 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
}
rcu_read_lock();
- regs = ftrace_partial_regs(fregs, bpf_kprobe_multi_pt_regs_ptr());
+ regs = ftrace_partial_regs_begin(fregs, bpf_kprobe_multi_pt_regs_ptr());
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_end(fregs, bpf_kprobe_multi_pt_regs_ptr());
rcu_read_unlock();
out:
^ permalink raw reply
* Re: [PATCH net-next v3] page_pool: Add page_pool_release_stalled tracepoint
From: Yunsheng Lin @ 2026-01-04 2:18 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Leon Hwang, netdev
Cc: Ilias Apalodimas, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, kerneljasonxing, lance.yang,
jiayuan.chen, linux-kernel, linux-trace-kernel, Leon Huang Fu,
Dragos Tatulea, kernel-team, Yan Zhai
In-Reply-To: <011ca15e-107b-4679-8203-f5f821f27900@kernel.org>
On 2026/1/2 19:43, Jesper Dangaard Brouer wrote:
>
>
> On 02/01/2026 08.17, Leon Hwang wrote:
>> Introduce a new tracepoint to track stalled page pool releases,
>> providing better observability for page pool lifecycle issues.
>>
>
> In general I like/support adding this tracepoint for "debugability" of
> page pool lifecycle issues.
>
> For "observability" @Kuba added a netlink scheme[1][2] for page_pool[3], which gives us the ability to get events and list page_pools from userspace.
> I've not used this myself (yet) so I need input from others if this is something that others have been using for page pool lifecycle issues?
>
> Need input from @Kuba/others as the "page-pool-get"[4] state that "Only Page Pools associated with a net_device can be listed". Don't we want the ability to list "invisible" page_pool's to allow debugging issues?
>
> [1] https://docs.kernel.org/userspace-api/netlink/intro-specs.html
> [2] https://docs.kernel.org/userspace-api/netlink/index.html
> [3] https://docs.kernel.org/netlink/specs/netdev.html
> [4] https://docs.kernel.org/netlink/specs/netdev.html#page-pool-get
>
> Looking at the code, I see that NETDEV_CMD_PAGE_POOL_CHANGE_NTF netlink
> notification is only generated once (in page_pool_destroy) and not when
> we retry in page_pool_release_retry (like this patch). In that sense,
> this patch/tracepoint is catching something more than netlink provides.
> First I though we could add a netlink notification, but I can imagine
> cases this could generate too many netlink messages e.g. a netdev with
> 128 RX queues generating these every second for every RX queue.
>
> Guess, I've talked myself into liking this change, what do other
> maintainers think? (e.g. netlink scheme and debugging balance)
>
>
>> Problem:
>> Currently, when a page pool shutdown is stalled due to inflight pages,
>> the kernel only logs a warning message via pr_warn(). This has several
>> limitations:
>>
>> 1. The warning floods the kernel log after the initial DEFER_WARN_INTERVAL,
>> making it difficult to track the progression of stalled releases
>> 2. There's no structured way to monitor or analyze these events
>> 3. Debugging tools cannot easily capture and correlate stalled pool
>> events with other network activity
>>
>> Solution:
>> Add a new tracepoint, page_pool_release_stalled, that fires when a page
>> pool shutdown is stalled. The tracepoint captures:
>> - pool: pointer to the stalled page_pool
>> - inflight: number of pages still in flight
>> - sec: seconds since the release was deferred
>>
>> The implementation also modifies the logging behavior:
>> - pr_warn() is only emitted during the first warning interval
>> (DEFER_WARN_INTERVAL to DEFER_WARN_INTERVAL*2)
>> - The tracepoint is fired always, reducing log noise while still
>> allowing monitoring tools to track the issue
If the initial log is still present, I don't really see what's the benefit
of re-triggering logs or tracepoints when the first two fields are unchanged
and the last two fields can be inspected using some tool? If there are none,
perhaps we only need to print the first trigger log and a log upon completion
of page_pool destruction.
>>
>> This allows developers and system administrators to:
>> - Use tools like perf, ftrace, or eBPF to monitor stalled releases
>> - Correlate page pool issues with network driver behavior
>> - Analyze patterns without parsing kernel logs
>> - Track the progression of inflight page counts over time
>>
>> Signed-off-by: Leon Huang Fu <leon.huangfu@shopee.com>
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
^ permalink raw reply
* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Andy Shevchenko @ 2026-01-04 0:20 UTC (permalink / raw)
To: Joel Fernandes
Cc: Steven Rostedt, Andrew Morton, Yury Norov, Masami Hiramatsu,
Mathieu Desnoyers, Christophe Leroy, Randy Dunlap, Ingo Molnar,
Jani Nikula, Joonas Lahtinen, David Laight, Petr Pavlu,
Andi Shyti, Vivi Rodrigo, Tvrtko Ursulin, Daniel Gomez,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-modules@vger.kernel.org,
linux-trace-kernel@vger.kernel.org
In-Reply-To: <937926D0-00DC-499B-9FD8-D921C903882D@nvidia.com>
On Sat, Jan 03, 2026 at 03:36:44PM +0000, Joel Fernandes wrote:
> > On Jan 3, 2026, at 7:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, Jan 02, 2026 at 07:50:59PM -0500, Joel Fernandes wrote:
> >> On Mon, Dec 29, 2025 at 11:17:48AM -0500, Steven Rostedt wrote:
...
> >> I use trace_printk() all the time for kernel, particularly RCU development.
> >> One of the key usecases I have is dumping traces on panic (with panic on warn
> >> and stop tracing on warn enabled). This is extremely useful since I can add
> >> custom tracing and dump traces when rare conditions occur. I fixed several
> >> bugs with this technique.
> >>
> >> I also recommend keeping it convenient to use.
> >
> > Okay, you know C, please share your opinion what header is the best to hold the
> > trace_printk.h to be included.
>
> I do not think it is necessary to move it.
I'm not talking about move, I'm talking about the C 101 thingy. Any custom API
should be included before use, otherwise compiler won't see it. Which header do
you want to include to have this API being provided? Note, it's really bad
situation right now with the header to be included implicitly via non-obvious
or obscure path. The discussion moved as far as I see it towards the finding a
good place for the trace_printk.h.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Joel Fernandes @ 2026-01-03 15:36 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Steven Rostedt, Andrew Morton, Yury Norov, Masami Hiramatsu,
Mathieu Desnoyers, Christophe Leroy, Randy Dunlap, Ingo Molnar,
Jani Nikula, Joonas Lahtinen, David Laight, Petr Pavlu,
Andi Shyti, Vivi Rodrigo, Tvrtko Ursulin, Daniel Gomez,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-modules@vger.kernel.org,
linux-trace-kernel@vger.kernel.org
In-Reply-To: <aVkSVk2L6VH9MYGz@smile.fi.intel.com>
> On Jan 3, 2026, at 7:58 AM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Jan 02, 2026 at 07:50:59PM -0500, Joel Fernandes wrote:
>> On Mon, Dec 29, 2025 at 11:17:48AM -0500, Steven Rostedt wrote:
>
> ...
>
>> I use trace_printk() all the time for kernel, particularly RCU development.
>> One of the key usecases I have is dumping traces on panic (with panic on warn
>> and stop tracing on warn enabled). This is extremely useful since I can add
>> custom tracing and dump traces when rare conditions occur. I fixed several
>> bugs with this technique.
>>
>> I also recommend keeping it convenient to use.
>
> Okay, you know C, please share your opinion what header is the best to hold the
> trace_printk.h to be included.
I do not think it is necessary to move it.
- Joel
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Yury Norov @ 2026-01-03 14:22 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Joel Fernandes, Steven Rostedt, Andrew Morton, Masami Hiramatsu,
Mathieu Desnoyers, Christophe Leroy, Randy Dunlap, Ingo Molnar,
Jani Nikula, Joonas Lahtinen, David Laight, Petr Pavlu,
Andi Shyti, Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
linux-kernel, intel-gfx, dri-devel, linux-modules,
linux-trace-kernel
In-Reply-To: <aVkSVk2L6VH9MYGz@smile.fi.intel.com>
On Sat, Jan 03, 2026 at 02:57:58PM +0200, Andy Shevchenko wrote:
> On Fri, Jan 02, 2026 at 07:50:59PM -0500, Joel Fernandes wrote:
> > On Mon, Dec 29, 2025 at 11:17:48AM -0500, Steven Rostedt wrote:
>
> ...
>
> > I use trace_printk() all the time for kernel, particularly RCU development.
> > One of the key usecases I have is dumping traces on panic (with panic on warn
> > and stop tracing on warn enabled). This is extremely useful since I can add
> > custom tracing and dump traces when rare conditions occur. I fixed several
> > bugs with this technique.
> >
> > I also recommend keeping it convenient to use.
>
> Okay, you know C, please share your opinion what header is the best to hold the
> trace_printk.h to be included.
What if we include it on Makefile level, similarly to how W=1 works?
make D=1 // trace_printk() is available
make D=0 // trace_printk() is not available
make // trace_printk() is not available
Where D stands for debugging.
D=1 may be a default setting if you prefer, but the most important is
that every compilation unit will have an access to debugging without
polluting core headers.
^ permalink raw reply
* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Andy Shevchenko @ 2026-01-03 12:57 UTC (permalink / raw)
To: Joel Fernandes
Cc: Steven Rostedt, Andrew Morton, Yury Norov (NVIDIA),
Masami Hiramatsu, Mathieu Desnoyers, Christophe Leroy,
Randy Dunlap, Ingo Molnar, Jani Nikula, Joonas Lahtinen,
David Laight, Petr Pavlu, Andi Shyti, Rodrigo Vivi,
Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
dri-devel, linux-modules, linux-trace-kernel
In-Reply-To: <20260103005059.GA11015@joelbox2>
On Fri, Jan 02, 2026 at 07:50:59PM -0500, Joel Fernandes wrote:
> On Mon, Dec 29, 2025 at 11:17:48AM -0500, Steven Rostedt wrote:
...
> I use trace_printk() all the time for kernel, particularly RCU development.
> One of the key usecases I have is dumping traces on panic (with panic on warn
> and stop tracing on warn enabled). This is extremely useful since I can add
> custom tracing and dump traces when rare conditions occur. I fixed several
> bugs with this technique.
>
> I also recommend keeping it convenient to use.
Okay, you know C, please share your opinion what header is the best to hold the
trace_printk.h to be included.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 0/5] uprobes: transition from kmap_atomic to kmap_local_page
From: Oleg Nesterov @ 2026-01-03 10:56 UTC (permalink / raw)
To: Keke Ming
Cc: mhiramat, peterz, linux, catalin.marinas, will, tsbogend, pjw,
palmer, aou, akpm, linux-kernel, linux-trace-kernel, linux-mm,
linux-arm-kernel, linux-mips, linux-riscv
In-Reply-To: <20260103084243.195125-1-ming.jvle@gmail.com>
On 01/03, Keke Ming wrote:
>
> Keke Ming (5):
> riscv/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
> arm64/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
> mips/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
> arm/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
> uprobes: use kmap_local_page() for temporary page mappings
>
> arch/arm/probes/uprobes/core.c | 4 ++--
> arch/arm64/kernel/probes/uprobes.c | 4 ++--
> arch/mips/kernel/uprobes.c | 4 ++--
> arch/riscv/kernel/probes/uprobes.c | 4 ++--
> kernel/events/uprobes.c | 12 ++++++------
> 5 files changed, 14 insertions(+), 14 deletions(-)
Thanks,
Acked-by: Oleg Nesterov <oleg@redhat.com>
^ permalink raw reply
* [PATCH 5/5] uprobes: use kmap_local_page() for temporary page mappings
From: Keke Ming @ 2026-01-03 8:42 UTC (permalink / raw)
To: mhiramat, oleg, peterz
Cc: linux, catalin.marinas, will, tsbogend, pjw, palmer, aou, akpm,
linux-kernel, linux-trace-kernel, linux-mm, linux-arm-kernel,
linux-mips, linux-riscv, Keke Ming
In-Reply-To: <20260103084243.195125-1-ming.jvle@gmail.com>
Replace deprecated kmap_atomic() with kmap_local_page().
Signed-off-by: Keke Ming <ming.jvle@gmail.com>
---
kernel/events/uprobes.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index d546d32390a8..a7d7d83ca1d7 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -179,16 +179,16 @@ bool __weak is_trap_insn(uprobe_opcode_t *insn)
void uprobe_copy_from_page(struct page *page, unsigned long vaddr, void *dst, int len)
{
- void *kaddr = kmap_atomic(page);
+ void *kaddr = kmap_local_page(page);
memcpy(dst, kaddr + (vaddr & ~PAGE_MASK), len);
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
}
static void copy_to_page(struct page *page, unsigned long vaddr, const void *src, int len)
{
- void *kaddr = kmap_atomic(page);
+ void *kaddr = kmap_local_page(page);
memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
}
static int verify_opcode(struct page *page, unsigned long vaddr, uprobe_opcode_t *insn,
@@ -323,7 +323,7 @@ __update_ref_ctr(struct mm_struct *mm, unsigned long vaddr, short d)
return ret == 0 ? -EBUSY : ret;
}
- kaddr = kmap_atomic(page);
+ kaddr = kmap_local_page(page);
ptr = kaddr + (vaddr & ~PAGE_MASK);
if (unlikely(*ptr + d < 0)) {
@@ -336,7 +336,7 @@ __update_ref_ctr(struct mm_struct *mm, unsigned long vaddr, short d)
*ptr += d;
ret = 0;
out:
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
put_page(page);
return ret;
}
--
2.43.0
^ permalink raw reply related
* [PATCH 4/5] arm/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
From: Keke Ming @ 2026-01-03 8:42 UTC (permalink / raw)
To: mhiramat, oleg, peterz
Cc: linux, catalin.marinas, will, tsbogend, pjw, palmer, aou, akpm,
linux-kernel, linux-trace-kernel, linux-mm, linux-arm-kernel,
linux-mips, linux-riscv, Keke Ming
In-Reply-To: <20260103084243.195125-1-ming.jvle@gmail.com>
Replace deprecated kmap_atomic() with kmap_local_page().
Signed-off-by: Keke Ming <ming.jvle@gmail.com>
---
arch/arm/probes/uprobes/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/probes/uprobes/core.c b/arch/arm/probes/uprobes/core.c
index 3d96fb41d624..0e1c6b9e7e54 100644
--- a/arch/arm/probes/uprobes/core.c
+++ b/arch/arm/probes/uprobes/core.c
@@ -113,7 +113,7 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
void *src, unsigned long len)
{
- void *xol_page_kaddr = kmap_atomic(page);
+ void *xol_page_kaddr = kmap_local_page(page);
void *dst = xol_page_kaddr + (vaddr & ~PAGE_MASK);
preempt_disable();
@@ -126,7 +126,7 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
preempt_enable();
- kunmap_atomic(xol_page_kaddr);
+ kunmap_local(xol_page_kaddr);
}
--
2.43.0
^ permalink raw reply related
* [PATCH 3/5] mips/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
From: Keke Ming @ 2026-01-03 8:42 UTC (permalink / raw)
To: mhiramat, oleg, peterz
Cc: linux, catalin.marinas, will, tsbogend, pjw, palmer, aou, akpm,
linux-kernel, linux-trace-kernel, linux-mm, linux-arm-kernel,
linux-mips, linux-riscv, Keke Ming
In-Reply-To: <20260103084243.195125-1-ming.jvle@gmail.com>
Replace deprecated kmap_atomic() with kmap_local_page().
Signed-off-by: Keke Ming <ming.jvle@gmail.com>
---
arch/mips/kernel/uprobes.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
index 401b148f8917..05cfc320992b 100644
--- a/arch/mips/kernel/uprobes.c
+++ b/arch/mips/kernel/uprobes.c
@@ -214,11 +214,11 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
unsigned long kaddr, kstart;
/* Initialize the slot */
- kaddr = (unsigned long)kmap_atomic(page);
+ kaddr = (unsigned long)kmap_local_page(page);
kstart = kaddr + (vaddr & ~PAGE_MASK);
memcpy((void *)kstart, src, len);
flush_icache_range(kstart, kstart + len);
- kunmap_atomic((void *)kaddr);
+ kunmap_local((void *)kaddr);
}
/**
--
2.43.0
^ permalink raw reply related
* [PATCH 2/5] arm64/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
From: Keke Ming @ 2026-01-03 8:42 UTC (permalink / raw)
To: mhiramat, oleg, peterz
Cc: linux, catalin.marinas, will, tsbogend, pjw, palmer, aou, akpm,
linux-kernel, linux-trace-kernel, linux-mm, linux-arm-kernel,
linux-mips, linux-riscv, Keke Ming
In-Reply-To: <20260103084243.195125-1-ming.jvle@gmail.com>
Replace deprecated kmap_atomic() with kmap_local_page().
Signed-off-by: Keke Ming <ming.jvle@gmail.com>
---
arch/arm64/kernel/probes/uprobes.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/probes/uprobes.c b/arch/arm64/kernel/probes/uprobes.c
index 941668800aea..4c55bf832ec3 100644
--- a/arch/arm64/kernel/probes/uprobes.c
+++ b/arch/arm64/kernel/probes/uprobes.c
@@ -15,7 +15,7 @@
void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
void *src, unsigned long len)
{
- void *xol_page_kaddr = kmap_atomic(page);
+ void *xol_page_kaddr = kmap_local_page(page);
void *dst = xol_page_kaddr + (vaddr & ~PAGE_MASK);
/*
@@ -32,7 +32,7 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
sync_icache_aliases((unsigned long)dst, (unsigned long)dst + len);
done:
- kunmap_atomic(xol_page_kaddr);
+ kunmap_local(xol_page_kaddr);
}
unsigned long uprobe_get_swbp_addr(struct pt_regs *regs)
--
2.43.0
^ permalink raw reply related
* [PATCH 1/5] riscv/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
From: Keke Ming @ 2026-01-03 8:42 UTC (permalink / raw)
To: mhiramat, oleg, peterz
Cc: linux, catalin.marinas, will, tsbogend, pjw, palmer, aou, akpm,
linux-kernel, linux-trace-kernel, linux-mm, linux-arm-kernel,
linux-mips, linux-riscv, Keke Ming
In-Reply-To: <20260103084243.195125-1-ming.jvle@gmail.com>
Replace deprecated kmap_atomic() with kmap_local_page().
Signed-off-by: Keke Ming <ming.jvle@gmail.com>
---
arch/riscv/kernel/probes/uprobes.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kernel/probes/uprobes.c b/arch/riscv/kernel/probes/uprobes.c
index cc15f7ca6cc1..f0d0691a8688 100644
--- a/arch/riscv/kernel/probes/uprobes.c
+++ b/arch/riscv/kernel/probes/uprobes.c
@@ -165,7 +165,7 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
void *src, unsigned long len)
{
/* Initialize the slot */
- void *kaddr = kmap_atomic(page);
+ void *kaddr = kmap_local_page(page);
void *dst = kaddr + (vaddr & ~PAGE_MASK);
unsigned long start = (unsigned long)dst;
@@ -178,5 +178,5 @@ void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
}
flush_icache_range(start, start + len);
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
}
--
2.43.0
^ permalink raw reply related
* [PATCH 0/5] uprobes: transition from kmap_atomic to kmap_local_page
From: Keke Ming @ 2026-01-03 8:42 UTC (permalink / raw)
To: mhiramat, oleg, peterz
Cc: linux, catalin.marinas, will, tsbogend, pjw, palmer, aou, akpm,
linux-kernel, linux-trace-kernel, linux-mm, linux-arm-kernel,
linux-mips, linux-riscv, Keke Ming
The use of kmap_atomic/kunmap_atomic is deprecated. The purpose of
kmap-like functions is to create temporary mappings.
kmap_atomic() typically disables preemption, while kmap_local_page()
allows preemption.
According to the documentation, kmap_atomic() is primarily necessary
for contexts that cannot sleep.
> kmap_atomic() may also be used by interrupt contexts, since it does
> not sleep and the callers too may not sleep until after
> kunmap_atomic() is called.
> kunmap_atomic() may implicitly depend on the side effects of atomic
> mappings, i.e. disabling page faults or preemption, or both. In that
> case, explicit calls to pagefault_disable() or preempt_disable() or
> both must be made in conjunction with the use of kmap_local_page().
Link: https://docs.kernel.org/mm/highmem.html#temporary-virtual-mappings
Link: https://lwn.net/Articles/836144/
Keke Ming (5):
riscv/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
arm64/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
mips/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
arm/uprobes: use kmap_local_page() in arch_uprobe_copy_ixol()
uprobes: use kmap_local_page() for temporary page mappings
arch/arm/probes/uprobes/core.c | 4 ++--
arch/arm64/kernel/probes/uprobes.c | 4 ++--
arch/mips/kernel/uprobes.c | 4 ++--
arch/riscv/kernel/probes/uprobes.c | 4 ++--
kernel/events/uprobes.c | 12 ++++++------
5 files changed, 14 insertions(+), 14 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v4 7/7] kernel.h: drop trace_printk.h
From: Joel Fernandes @ 2026-01-03 0:50 UTC (permalink / raw)
To: Steven Rostedt
Cc: Andrew Morton, Yury Norov (NVIDIA), Masami Hiramatsu,
Mathieu Desnoyers, Andy Shevchenko, Christophe Leroy,
Randy Dunlap, Ingo Molnar, Jani Nikula, Joonas Lahtinen,
David Laight, Petr Pavlu, Andi Shyti, Rodrigo Vivi,
Tvrtko Ursulin, Daniel Gomez, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, linux-kernel, intel-gfx,
dri-devel, linux-modules, linux-trace-kernel
In-Reply-To: <20251229111748.3ba66311@gandalf.local.home>
On Mon, Dec 29, 2025 at 11:17:48AM -0500, Steven Rostedt wrote:
> On Sun, 28 Dec 2025 13:31:50 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
>
> > > trace_printk() should be as available to the kernel as printk() is.
> >
> > um, why? trace_printk is used 1% as often as is printk. Seems
> > reasonable to include a header file to access such a rarely-used(!) and
> > specialized thing?
>
> It will waste a lot of kernel developers time. Go to conferences and talk
> with developers. trace_printk() is now one of the most common ways to debug
> your code. Having to add "#include <linux/trace_printk.h>" in every file
> that you use trace_printk() (and after your build fails because you forgot
> to include that file **WILL** slow down kernel debugging for hundreds of
> developers! It *is* used more than printk() for debugging today. Because
> it's fast and can be used in any context (NMI, interrupt handlers, etc).
>
> But sure, if you want to save the few minutes that is added to "make
> allyesconfig" by sacrificing minutes of kernel developer's time. Go ahead
> and make this change.
>
> I don't know how much you debug and develop today, but lots of people I
> talk to at conferences thank me for trace_printk() because it makes
> debugging their code so much easier.
>
> The "shotgun" approach is very common. That is, you add:
>
> trace_printk("%s:%d\n", __func__, __LINE__);
>
> all over your code to find out where things are going wrong. With the
> persistent ring buffer, you can even extract that information after a
> crash and reboot.
I use trace_printk() all the time for kernel, particularly RCU development.
One of the key usecases I have is dumping traces on panic (with panic on warn
and stop tracing on warn enabled). This is extremely useful since I can add
custom tracing and dump traces when rare conditions occur. I fixed several
bugs with this technique.
I also recommend keeping it convenient to use.
thanks,
- Joel
^ permalink raw reply
* Re: [PATCH] tracing: Add show_event_filters to expose active event filters
From: Aaron Tomlin @ 2026-01-02 20:19 UTC (permalink / raw)
To: Steven Rostedt
Cc: mhiramat, mark.rutland, mathieu.desnoyers, corbet, neelx, sean,
linux-kernel, linux-trace-kernel, linux-doc
In-Reply-To: <20260102133619.39d3e323@gandalf.local.home>
[-- Attachment #1: Type: text/plain, Size: 2500 bytes --]
On Fri, Jan 02, 2026 at 01:36:19PM -0500, Steven Rostedt wrote:
> Nice. Perhaps we should do something similar for event triggers.
Hi Steve,
Thanks for the review; I’m glad you find the addition useful.
That is an excellent suggestion. Consolidating the active triggers
alongside the filters would certainly provide a more comprehensive
overview.
I shall incorporate a similar mechanism for event triggers and submit the
update as a formal patch series in the next revision.
> Enabled? Or just events with filters. I think this should just say:
>
> A list of events that have filters. This shows the system/event
> pair along with the filter that is attached to the event.
Acknowledged.
> This doesn't traverse the trace_array. The seq_file does the traversing.
> Just state that this is part of the seq_file output and shows events with
> filters.
Acknowledged.
> > + * Uses RCU to safely dereference the volatile filter pointer.
>
> This is internal to the function and should not be part of the kerneldoc.
Acknowledged.
> > + */
> > +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;
> > +
> > + rcu_read_lock();
>
> Use:
>
> guard(rcu)();
>
> instead.
>
> > + filter = rcu_dereference(file->filter);
> > + if (filter && filter->filter_string) {
> > + seq_printf(m, "%s:%s\t%s\n",
> > + call->class->system,
> > + trace_event_name(call),
> > + filter->filter_string);
> > + }
> > + rcu_read_unlock();
>
> And remove the rcu_read_unlock().
>
> Actually, the function may be better by just doing:
>
> 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);
>
> return 0;
Agreed. Using guard(rcu)() is indeed much cleaner and aligns well with the
current preference for scoped-based resource management in the kernel.
Your proposed refactoring also helpfully reduces the indentation level,
which improves readability. I shall adopt this approach and incorporate it
into the next version of the patch.
> Why not just:
>
> return ftrace_event_open(inode, file, &show_show_event_filters_seq_ops);
>
> ?
Acknowledged.
Kind regards,
--
Aaron Tomlin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] ftrace: Make ftrace_graph_ent depth field signed
From: Steven Rostedt @ 2026-01-02 19:31 UTC (permalink / raw)
To: LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers, Dan Carpenter, pengdonglin
From: Steven Rostedt <rostedt@goodmis.org>
The code has integrity checks to make sure that depth never goes below
zero. But the depth field has recently been converted to unsigned long
from "int" (for alignment reasons). As unsigned long can never be less
than zero, the integrity checks no longer work.
Convert depth to long from unsigned long to allow the integrity checks to
work again.
Cc: stable@vger.kernel.org
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aS6kGi0maWBl-MjZ@stanley.mountain/
Fixes: f83ac7544fbf7 ("function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/ftrace.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 770f0dc993cc..a3a8989e3268 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1167,7 +1167,7 @@ static inline void ftrace_init(void) { }
*/
struct ftrace_graph_ent {
unsigned long func; /* Current function */
- unsigned long depth;
+ long depth; /* signed to check for less than zero */
} __packed;
/*
--
2.51.0
^ permalink raw reply related
* Re: [bug report] function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously
From: Steven Rostedt @ 2026-01-02 18:58 UTC (permalink / raw)
To: Dan Carpenter; +Cc: pengdonglin, linux-trace-kernel
In-Reply-To: <20260102135630.0a5d91fb@gandalf.local.home>
On Fri, 2 Jan 2026 13:56:30 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> > 1031 /* Save this function pointer to see if the exit matches */
> > 1032 if (call->depth < FTRACE_RETFUNC_DEPTH &&
> > --> 1033 !WARN_ON_ONCE(call->depth < 0))
> > ^^^^^^^^^^^^^^^
> > The patch changed call->depth from int to unsigned long.
>
> Yep, I'm fixing this with:
>
> !WARN_ON_ONCE((long)call->depth < 0))
>
Thinking about this more. I think it is more robust to change depth to
"long" from "unsigned long" in case there's other locations that expect
depth to be signed.
-- Steve
^ permalink raw reply
* Re: [bug report] function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously
From: Steven Rostedt @ 2026-01-02 18:56 UTC (permalink / raw)
To: Dan Carpenter; +Cc: pengdonglin, linux-trace-kernel
In-Reply-To: <aS6kGi0maWBl-MjZ@stanley.mountain>
On Tue, 2 Dec 2025 11:32:26 +0300
Dan Carpenter <dan.carpenter@linaro.org> wrote:
I just noticed this email.
> Hello pengdonglin,
>
> Commit f83ac7544fbf ("function_graph: Enable funcgraph-args and
> funcgraph-retaddr to work simultaneously") from Nov 25, 2025
> (linux-next), leads to the following Smatch static checker warning:
>
> kernel/trace/trace_functions_graph.c:1033 print_graph_entry_nested() warn: unsigned 'call->depth' is never less than zero.
> kernel/trace/trace_functions_graph.c:975 print_graph_entry_leaf() warn: unsigned 'call->depth' is never less than zero.
> kernel/trace/trace.h:1130 ftrace_graph_ignore_func() warn: unsigned 'trace->depth' is never less than zero.
>
> kernel/trace/trace_functions_graph.c
> 1012 static enum print_line_t
> 1013 print_graph_entry_nested(struct trace_iterator *iter,
> 1014 struct ftrace_graph_ent_entry *entry,
> 1015 struct trace_seq *s, int cpu, u32 flags)
> 1016 {
> 1017 struct ftrace_graph_ent *call = &entry->graph_ent;
> 1018 struct fgraph_data *data = iter->private;
> 1019 struct trace_array *tr = iter->tr;
> 1020 unsigned long func;
> 1021 int args_size;
> 1022 int i;
> 1023
> 1024 if (data) {
> 1025 struct fgraph_cpu_data *cpu_data;
> 1026 int cpu = iter->cpu;
> 1027
> 1028 cpu_data = per_cpu_ptr(data->cpu_data, cpu);
> 1029 cpu_data->depth = call->depth;
> 1030
> 1031 /* Save this function pointer to see if the exit matches */
> 1032 if (call->depth < FTRACE_RETFUNC_DEPTH &&
> --> 1033 !WARN_ON_ONCE(call->depth < 0))
> ^^^^^^^^^^^^^^^
> The patch changed call->depth from int to unsigned long.
Yep, I'm fixing this with:
!WARN_ON_ONCE((long)call->depth < 0))
Thanks!
-- Steve
>
> 1034 cpu_data->enter_funcs[call->depth] = call->func;
> 1035 }
> 1036
> 1037 /* No time */
> 1038 print_graph_duration(tr, 0, s, flags | FLAGS_FILL_FULL);
>
> regards,
> dan carpenter
^ permalink raw reply
* Re: [PATCH] tracing: Add show_event_filters to expose active event filters
From: Steven Rostedt @ 2026-01-02 18:36 UTC (permalink / raw)
To: Aaron Tomlin
Cc: mhiramat, mark.rutland, mathieu.desnoyers, corbet, neelx, sean,
linux-kernel, linux-trace-kernel, linux-doc
In-Reply-To: <20260101233414.2476973-1-atomlin@atomlin.com>
On Thu, 1 Jan 2026 18:34:14 -0500
Aaron Tomlin <atomlin@atomlin.com> wrote:
> Currently, to audit active Ftrace event filters, userspace must
> recursively traverse the events/ directory and read each individual
> filter file. This is inefficient for monitoring tools and debugging.
>
> Introduce "show_event_filters" at the trace root directory. This file
> displays all events that currently have a filter applied, alongside the
> actual filter string, in a consolidated system:event [tab] filter
> format.
>
> The implementation reuses the existing trace_event_file iterators to
> ensure atomic traversal of the event list and utilises rcu_read_lock()
> to safely access volatile filter strings.
Nice. Perhaps we should do something similar for event triggers.
>
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
> Documentation/trace/ftrace.rst | 6 ++++
> kernel/trace/trace_events.c | 62 ++++++++++++++++++++++++++++++++++
> 2 files changed, 68 insertions(+)
>
> diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
> index 639f4d95732f..27ea54bfbc52 100644
> --- a/Documentation/trace/ftrace.rst
> +++ b/Documentation/trace/ftrace.rst
> @@ -684,6 +684,12 @@ of ftrace. Here is a list of some of the key files:
>
> See events.rst for more information.
>
> + show_event_filters:
> +
> + A list of events that are enabled and have a filter applied.
Enabled? Or just events with filters. I think this should just say:
A list of events that have filters. This shows the system/event
pair along with the filter that is attached to the event.
> +
> + See events.rst for more information.
> +
> available_events:
>
> A list of events that can be enabled in tracing.
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index b16a5a158040..f578ee2e5c12 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1661,6 +1661,34 @@ static void t_stop(struct seq_file *m, void *p)
> mutex_unlock(&event_mutex);
> }
>
> +/**
> + * t_show_filters - seq_file callback to display active event filters
> + * @m: The seq_file instance
> + * @v: The current trace_event_file being iterated
> + *
> + * Traverses the trace_array event list and prints the system, name,
> + * and filter string for any event with an active filter.
This doesn't traverse the trace_array. The seq_file does the traversing.
Just state that this is part of the seq_file output and shows events with
filters.
> + * Uses RCU to safely dereference the volatile filter pointer.
This is internal to the function and should not be part of the kerneldoc.
> + */
> +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;
> +
> + rcu_read_lock();
Use:
guard(rcu)();
instead.
> + filter = rcu_dereference(file->filter);
> + if (filter && filter->filter_string) {
> + seq_printf(m, "%s:%s\t%s\n",
> + call->class->system,
> + trace_event_name(call),
> + filter->filter_string);
> + }
> + rcu_read_unlock();
And remove the rcu_read_unlock().
Actually, the function may be better by just doing:
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);
return 0;
> +
> + return 0;
> +}
> +
> #ifdef CONFIG_MODULES
> static int s_show(struct seq_file *m, void *v)
> {
> @@ -2488,6 +2516,7 @@ ftrace_event_npid_write(struct file *filp, const char __user *ubuf,
>
> static int ftrace_event_avail_open(struct inode *inode, struct file *file);
> static int ftrace_event_set_open(struct inode *inode, struct file *file);
> +static int ftrace_event_show_filters_open(struct inode *inode, struct file *file);
> static int ftrace_event_set_pid_open(struct inode *inode, struct file *file);
> static int ftrace_event_set_npid_open(struct inode *inode, struct file *file);
> static int ftrace_event_release(struct inode *inode, struct file *file);
> @@ -2506,6 +2535,13 @@ static const struct seq_operations show_set_event_seq_ops = {
> .stop = s_stop,
> };
>
> +static const struct seq_operations show_show_event_filters_seq_ops = {
> + .start = t_start,
> + .next = t_next,
> + .show = t_show_filters,
> + .stop = t_stop,
> +};
> +
> static const struct seq_operations show_set_pid_seq_ops = {
> .start = p_start,
> .next = p_next,
> @@ -2535,6 +2571,13 @@ static const struct file_operations ftrace_set_event_fops = {
> .release = ftrace_event_release,
> };
>
> +static const struct file_operations ftrace_show_event_filters_fops = {
> + .open = ftrace_event_show_filters_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = seq_release,
> +};
> +
> static const struct file_operations ftrace_set_event_pid_fops = {
> .open = ftrace_event_set_pid_open,
> .read = seq_read,
> @@ -2679,6 +2722,22 @@ ftrace_event_set_open(struct inode *inode, struct file *file)
> return ret;
> }
>
> +/**
> + * ftrace_event_show_filters_open - open interface for set_event_filters
> + * @inode: the inode of the file
> + * @file: the file being opened
> + *
> + * Connects the set_event_filters file to the sequence operations
> + * required to iterate over and display active event filters.
> + */
> +static int
> +ftrace_event_show_filters_open(struct inode *inode, struct file *file)
> +{
> + const struct seq_operations *seq_ops = &show_show_event_filters_seq_ops;
> +
> + return ftrace_event_open(inode, file, seq_ops);
Why not just:
return ftrace_event_open(inode, file, &show_show_event_filters_seq_ops);
?
> +}
> +
> static int
> ftrace_event_set_pid_open(struct inode *inode, struct file *file)
> {
> @@ -4399,6 +4458,9 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
> if (!entry)
> return -ENOMEM;
>
> + trace_create_file("show_event_filters", TRACE_MODE_READ, parent, tr,
> + &ftrace_show_event_filters_fops);
> +
> nr_entries = ARRAY_SIZE(events_entries);
>
> e_events = eventfs_create_events_dir("events", parent, events_entries,
Thanks,
-- Steve
^ 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