* Re: [PATCH RFC bpf-next 5/7] bpf: introduce bpf_arch_text_poke_type
From: Menglong Dong @ 2025-11-15 2:26 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, bpf,
LKML, linux-trace-kernel
In-Reply-To: <CAADnVQLemtF-m5et+c5pWppNZoWnWBehtMCHVJU9Yagvi+dRZA@mail.gmail.com>
On Sat, Nov 15, 2025 at 2:42 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Nov 14, 2025 at 1:25 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
> >
> > Introduce the function bpf_arch_text_poke_type(), which is able to specify
> > both the current and new opcode. If it is not implemented by the arch,
> > bpf_arch_text_poke() will be called directly if the current opcode is the
> > same as the new one. Otherwise, -EOPNOTSUPP will be returned.
> >
> > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > ---
> > include/linux/bpf.h | 4 ++++
> > kernel/bpf/core.c | 10 ++++++++++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index d65a71042aa3..aec7c65539f5 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -3711,6 +3711,10 @@ enum bpf_text_poke_type {
> > BPF_MOD_JUMP,
> > };
> >
> > +int bpf_arch_text_poke_type(void *ip, enum bpf_text_poke_type old_t,
> > + enum bpf_text_poke_type new_t, void *addr1,
> > + void *addr2);
> > +
>
> Instead of adding a new helper, I think, it's cleaner to change
> the existing bpf_arch_text_poke() across all archs in one patch,
> and also do:
>
> enum bpf_text_poke_type {
> + BPF_MOD_NOP,
> BPF_MOD_CALL,
> BPF_MOD_JUMP,
> };
>
> and use that instead of addr[12] = !NULL to indicate
> the transition.
>
> The callsites will be easier to read when they will look like:
> bpf_arch_text_poke(ip, BPF_MOD_CALL, BPF_MOD_CALL, old_addr, new_addr);
>
> bpf_arch_text_poke(ip, BPF_MOD_NOP, BPF_MOD_CALL, NULL, new_addr);
>
> bpf_arch_text_poke(ip, BPF_MOD_JMP, BPF_MOD_CALL, old_addr, new_addr);
Yeah, much clearer. The new helper also makes me feel a bit
dizzy.
Thanks!
Menglong Dong
^ permalink raw reply
* Re: [PATCH RFC bpf-next 4/7] bpf,x86: adjust the "jmp" mode for bpf trampoline
From: Menglong Dong @ 2025-11-15 2:14 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, bpf,
LKML, linux-trace-kernel
In-Reply-To: <CAADnVQJU39q9amZMuVLzsg7CK5MLT_xFr0K4Bx9zp7P5C6MCRw@mail.gmail.com>
On Sat, Nov 15, 2025 at 2:22 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Nov 14, 2025 at 1:25 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
> >
> > In the origin call case, if BPF_TRAMP_F_SKIP_FRAME is not set, it means
> > that the trampoline is not called, but "jmp".
> >
> > Introduce the function bpf_trampoline_need_jmp() to check if the
> > trampoline is in "jmp" mode.
> >
> > Do some adjustment on the "jmp" mode for the x86_64. The main adjustment
> > that we make is for the stack parameter passing case, as the stack
> > alignment logic changes in the "jmp" mode without the "rip". What's more,
> > the location of the parameters on the stack also changes.
> >
> > Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> > ---
> > arch/x86/net/bpf_jit_comp.c | 15 ++++++++++-----
> > include/linux/bpf.h | 12 ++++++++++++
> > 2 files changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > index 2d300ab37cdd..21ce2b8457ec 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -2830,7 +2830,7 @@ static int get_nr_used_regs(const struct btf_func_model *m)
> > }
> >
> > static void save_args(const struct btf_func_model *m, u8 **prog,
> > - int stack_size, bool for_call_origin)
> > + int stack_size, bool for_call_origin, bool jmp)
>
> I have an allergy to bool args.
>
> Please pass flags and do
> boll jmp_based_tramp = bpf_trampoline_uses_jmp(flags);
Ok, I'll do this way in the next patch.
>
> I think bpf_trampoline_uses_jmp() is more descriptive than
> bpf_trampoline_need_jmp().
ACK.
>
> The actual math lgtm.
^ permalink raw reply
* Re: [PATCH RFC bpf-next 2/7] x86/ftrace: implement DYNAMIC_FTRACE_WITH_JMP
From: Menglong Dong @ 2025-11-15 2:12 UTC (permalink / raw)
To: Steven Rostedt
Cc: ast, daniel, john.fastabend, andrii, martin.lau, eddyz87, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, mhiramat,
mark.rutland, mathieu.desnoyers, bpf, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251114113924.723f6fde@gandalf.local.home>
On Sat, Nov 15, 2025 at 12:39 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 14 Nov 2025 17:24:45 +0800
> Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> > --- a/arch/x86/kernel/ftrace_64.S
> > +++ b/arch/x86/kernel/ftrace_64.S
> > @@ -285,8 +285,18 @@ SYM_INNER_LABEL(ftrace_regs_caller_end, SYM_L_GLOBAL)
> > ANNOTATE_NOENDBR
> > RET
> >
> > +1:
> > + testb $1, %al
> > + jz 2f
> > + andq $0xfffffffffffffffe, %rax
> > + movq %rax, MCOUNT_REG_SIZE+8(%rsp)
> > + restore_mcount_regs
> > + /* Restore flags */
> > + popfq
> > + RET
> > +
> > /* Swap the flags with orig_rax */
> > -1: movq MCOUNT_REG_SIZE(%rsp), %rdi
> > +2: movq MCOUNT_REG_SIZE(%rsp), %rdi
> > movq %rdi, MCOUNT_REG_SIZE-8(%rsp)
> > movq %rax, MCOUNT_REG_SIZE(%rsp)
> >
>
> So in this case we have:
>
> original_caller:
> call foo -> foo:
> call fentry -> fentry:
> [do ftrace callbacks ]
> move tramp_addr to stack
> RET -> tramp_addr
> tramp_addr:
> [..]
> call foo_body -> foo_body:
> [..]
> RET -> back to tramp_addr
> [..]
> RET -> back to original_caller
Nice flow chart, which I think we can put in the commit log.
>
> I guess that looks balanced.
Yes, it is balanced.
>
> -- Steve
>
>
^ permalink raw reply
* Re: [RFC PATCH v1 06/37] KVM: guest_memfd: Update kvm_gmem_populate() to use gmem attributes
From: Ackerley Tng @ 2025-11-15 0:52 UTC (permalink / raw)
To: Yan Zhao
Cc: cgroups, kvm, linux-doc, linux-fsdevel, linux-kernel,
linux-kselftest, linux-mm, linux-trace-kernel, x86, akpm,
binbin.wu, bp, brauner, chao.p.peng, chenhuacai, corbet,
dave.hansen, dave.hansen, david, dmatlack, erdemaktas, fan.du,
fvdl, haibo1.xu, hannes, hch, hpa, hughd, ira.weiny,
isaku.yamahata, jack, james.morse, jarkko, jgg, jgowans, jhubbard,
jthoughton, jun.miao, kai.huang, keirf, kent.overstreet,
liam.merwick, maciej.wieczor-retman, mail, maobibo,
mathieu.desnoyers, maz, mhiramat, mhocko, mic, michael.roth,
mingo, mlevitsk, mpe, muchun.song, nikunj, nsaenz, oliver.upton,
palmer, pankaj.gupta, paul.walmsley, pbonzini, peterx, pgonda,
prsampat, pvorel, qperret, richard.weiyang, rick.p.edgecombe,
rientjes, rostedt, roypat, rppt, seanjc, shakeel.butt, shuah,
steven.price, suzuki.poulose, tabba, tglx, thomas.lendacky,
vannapurve, vbabka, viro, vkuznets, will, willy, wyihan,
xiaoyao.li, yilun.xu, yuzenghui
In-Reply-To: <aRG35j3OhMvQo85n@yzhao56-desk.sh.intel.com>
Yan Zhao <yan.y.zhao@intel.com> writes:
>> #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_POPULATE
>> +static bool kvm_gmem_range_is_private(struct gmem_inode *gi, pgoff_t index,
>> + size_t nr_pages, struct kvm *kvm, gfn_t gfn)
>> +{
>> + pgoff_t end = index + nr_pages - 1;
>> + void *entry;
>> +
>> + if (vm_memory_attributes)
>> + return kvm_range_has_vm_memory_attributes(kvm, gfn, gfn + nr_pages,
>> + KVM_MEMORY_ATTRIBUTE_PRIVATE,
>> + KVM_MEMORY_ATTRIBUTE_PRIVATE);
> Can't compile kvm_range_has_vm_memory_attributes() if
> CONFIG_KVM_VM_MEMORY_ATTRIBUTES is not set.
Thanks! I will fix this in the next revision.
We've been discussing HugeTLB support in the guest_memfd upstream calls
and I'd like to add a quick follow up here, with code, for anyone who
might be interested. Here's a WIP tree:
https://github.com/googleprodkernel/linux-cc/tree/wip-gmem-conversions-hugetlb-restructuring
This tree was based off kvm-next (as of 2025-10-08), and includes
+ Mmap fixes from Sean
+ NUMA mempolicy support from Shivank Garg (AMD)
+ Some cleanup patches from Sean
+ Conversion series [1] with some cleanups (Thanks for the comments and
reviews on this series! Haven't had time to figure out all of it,
addressed some first)
+ st_blocks fix for guest_memfd, which was discussed at the guest_memfd
upstream call: slides [2]
+ HugeTLB support without conversion: this stage does not yet take into
account comments from the upstream call. This stage provides support
for HugeTLB to be used through guest_memfd for private memory. This
has to be used with the KVM parameter vm_memory_attributes set to
true. This stage can be used to test Yan's TDX huge page support by
setting up guest_memfd with HugeTLB just for private memory, with
shared memory being taken from elsewhere.
+ HugeTLB support with conversion and folio restructuring: this stage
also does not take into account comments from the upstream call, so it
still disables the INIT_SHARED flag, although HugeTLB can now be used
with in-place conversion for both shared and private memory. This can
be used to test Yan's TDX huge page support.
[1] https://lore.kernel.org/all/cover.1760731772.git.ackerleytng@google.com/T/
[2] https://lpc.events/event/18/contributions/1764/attachments/1409/3715/2025-10-30%20guest_memfd%20upstream%20call_%20guest_memfd%20HugeTLB%20support%20overview.pdf
^ permalink raw reply
* Re: [RFC PATCH v1 11/37] KVM: guest_memfd: Add support for KVM_SET_MEMORY_ATTRIBUTES
From: Ackerley Tng @ 2025-11-15 0:46 UTC (permalink / raw)
To: Yan Zhao
Cc: cgroups, kvm, linux-doc, linux-fsdevel, linux-kernel,
linux-kselftest, linux-mm, linux-trace-kernel, x86, akpm,
binbin.wu, bp, brauner, chao.p.peng, chenhuacai, corbet,
dave.hansen, dave.hansen, david, dmatlack, erdemaktas, fan.du,
fvdl, haibo1.xu, hannes, hch, hpa, hughd, ira.weiny,
isaku.yamahata, jack, james.morse, jarkko, jgg, jgowans, jhubbard,
jroedel, jthoughton, jun.miao, kai.huang, keirf, kent.overstreet,
liam.merwick, maciej.wieczor-retman, mail, maobibo,
mathieu.desnoyers, maz, mhiramat, mhocko, mic, michael.roth,
mingo, mlevitsk, mpe, muchun.song, nikunj, nsaenz, oliver.upton,
palmer, pankaj.gupta, paul.walmsley, pbonzini, peterx, pgonda,
prsampat, pvorel, qperret, richard.weiyang, rick.p.edgecombe,
rientjes, rostedt, roypat, rppt, seanjc, shakeel.butt, shuah,
steven.price, steven.sistare, suzuki.poulose, tabba, tglx,
thomas.lendacky, vannapurve, vbabka, viro, vkuznets, wei.w.wang,
will, willy, wyihan, xiaoyao.li, yilun.xu, yuzenghui, zhiquan1.li
In-Reply-To: <aQnGJ5agTohMijj8@yzhao56-desk.sh.intel.com>
Yan Zhao <yan.y.zhao@intel.com> writes:
> On Fri, Oct 17, 2025 at 01:11:52PM -0700, Ackerley Tng wrote:
>> For shared to private conversions, if refcounts on any of the folios
>> within the range are elevated, fail the conversion with -EAGAIN.
>>
>> At the point of shared to private conversion, all folios in range are
>> also unmapped. The filemap_invalidate_lock() is held, so no faulting
>> can occur. Hence, from that point on, only transient refcounts can be
>> taken on the folios associated with that guest_memfd.
>>
>> Hence, it is safe to do the conversion from shared to private.
>>
>> After conversion is complete, refcounts may become elevated, but that
>> is fine since users of transient refcounts don't actually access
>> memory.
>>
>> For private to shared conversions, there are no refcount checks. any
>> transient refcounts are expected to drop their refcounts soon. The
>> conversion process will spin waiting for these transient refcounts to
>> go away.
> Where's the code to spin?
>
Thanks, I will fix the commit message for the next revision.
>> +/*
>> + * Preallocate memory for attributes to be stored on a maple tree, pointed to
>> + * by mas. Adjacent ranges with attributes identical to the new attributes
>> + * will be merged. Also sets mas's bounds up for storing attributes.
>> + *
>> + * This maintains the invariant that ranges with the same attributes will
>> + * always be merged.
>> + */
>> +static int kvm_gmem_mas_preallocate(struct ma_state *mas, u64 attributes,
>> + pgoff_t start, size_t nr_pages)
>> +{
>> + pgoff_t end = start + nr_pages;
>> + pgoff_t last = end - 1;
>> + void *entry;
>> +
>> + /* Try extending range. entry is NULL on overflow/wrap-around. */
>> + mas_set_range(mas, end, end);
>> + entry = mas_find(mas, end);
>> + if (entry && xa_to_value(entry) == attributes)
>> + last = mas->last;
>> +
>> + mas_set_range(mas, start - 1, start - 1);
> Check start == 0 ?
>
Thanks!
>> + entry = mas_find(mas, start - 1);
>> + if (entry && xa_to_value(entry) == attributes)
>> + start = mas->index;
>> +
>> + mas_set_range(mas, start, last);
>> + return mas_preallocate(mas, xa_mk_value(attributes), GFP_KERNEL);
>> +}
> ...
>
>> +static long kvm_gmem_set_attributes(struct file *file, void __user *argp)
>> +{
>> + struct gmem_file *f = file->private_data;
>> + struct inode *inode = file_inode(file);
>> + struct kvm_memory_attributes2 attrs;
>> + pgoff_t err_index;
>> + size_t nr_pages;
>> + pgoff_t index;
>> + int r;
>> +
>> + if (copy_from_user(&attrs, argp, sizeof(attrs)))
>> + return -EFAULT;
>> +
>> + if (attrs.flags)
>> + return -EINVAL;
>> + if (attrs.attributes & ~kvm_supported_mem_attributes(f->kvm))
>> + return -EINVAL;
>> + if (attrs.size == 0 || attrs.offset + attrs.size < attrs.offset)
>> + return -EINVAL;
>> + if (!PAGE_ALIGNED(attrs.offset) || !PAGE_ALIGNED(attrs.offset))
> Should be
> if (!PAGE_ALIGNED(attrs.offset) || !PAGE_ALIGNED(attrs.size))
> ?
>
Thanks!
>> + return -EINVAL;
>> +
>> + if (attrs.offset > inode->i_size ||
> Should be
> if (attrs.offset >= inode->i_size ||
> ?
Thanks!
>> + attrs.offset + attrs.size > inode->i_size)
>> + return -EINVAL;
>> +
>> + nr_pages = attrs.size >> PAGE_SHIFT;
>> + index = attrs.offset >> PAGE_SHIFT;
>> + r = __kvm_gmem_set_attributes(inode, index, nr_pages, attrs.attributes,
>> + &err_index);
>> + if (r) {
>> + attrs.error_offset = err_index << PAGE_SHIFT;
>> +
>> + if (copy_to_user(argp, &attrs, sizeof(attrs)))
>> + return -EFAULT;
>> + }
>> +
>> + return r;
>> +}
^ permalink raw reply
* Re: [PATCH v7 00/31] context_tracking,x86: Defer some IPIs until a user->kernel transition
From: Andy Lutomirski @ 2025-11-15 0:29 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Valentin Schneider, Linux Kernel Mailing List, linux-mm, rcu,
the arch/x86 maintainers, linux-arm-kernel, loongarch,
linux-riscv, linux-arch, linux-trace-kernel, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin,
Peter Zijlstra (Intel), Arnaldo Carvalho de Melo, Josh Poimboeuf,
Paolo Bonzini, Arnd Bergmann, Frederic Weisbecker, Jason Baron,
Steven Rostedt, Ard Biesheuvel, Sami Tolvanen, David S. Miller,
Neeraj Upadhyay, Joel Fernandes, Josh Triplett, Boqun Feng,
Uladzislau Rezki, Mathieu Desnoyers, Mel Gorman, Andrew Morton,
Masahiro Yamada, Han Shen, Rik van Riel, Jann Horn, Dan Carpenter,
Oleg Nesterov, Juri Lelli, Clark Williams, Yair Podemsky,
Marcelo Tosatti, Daniel Wagner, Petr Tesarik, Shrikanth Hegde
In-Reply-To: <b3472cb3-86fa-4687-bfce-3b9a1bf4ff36@paulmck-laptop>
On Fri, Nov 14, 2025, at 12:03 PM, Paul E. McKenney wrote:
> On Fri, Nov 14, 2025 at 10:45:08AM -0800, Andy Lutomirski wrote:
>>
>>
>> On Fri, Nov 14, 2025, at 10:14 AM, Paul E. McKenney wrote:
>> > On Fri, Nov 14, 2025 at 09:22:35AM -0800, Andy Lutomirski wrote:
>> >>
>>
>> >> > Oh, any another primitive would be possible: one CPU could plausibly
>> >> > execute another CPU's interrupts or soft-irqs or whatever by taking a
>> >> > special lock that would effectively pin the remote CPU in user mode --
>> >> > you'd set a flag in the target cpu_flags saying "pin in USER mode" and
>> >> > the transition on that CPU to kernel mode would then spin on entry to
>> >> > kernel mode and wait for the lock to be released. This could plausibly
>> >> > get a lot of the on_each_cpu callers to switch over in one fell swoop:
>> >> > anything that needs to synchronize to the remote CPU but does not need
>> >> > to poke its actual architectural state could be executed locally while
>> >> > the remote CPU is pinned.
>> >
>> > It would be necessary to arrange for the remote CPU to remain pinned
>> > while the local CPU executed on its behalf. Does the above approach
>> > make that happen without re-introducing our current context-tracking
>> > overhead and complexity?
>>
>> Using the pseudo-implementation farther down, I think this would be like:
>>
>> if (my_state->status == INDETERMINATE) {
>> // we were lazy and we never promised to do work atomically.
>> atomic_set(&my_state->status, KERNEL);
>> this_entry_work = 0;
>> /* we are definitely not pinned in this path */
>> } else {
>> // we were not lazy and we promised we would do work atomically
>> atomic exchange the entire state to { .work = 0, .status = KERNEL }
>> this_entry_work = (whatever we just read);
>> if (this_entry_work & PINNED) {
>> u32 this_cpu_pin_count = this_cpu_ptr(pin_count);
>> while (atomic_read(&this_cpu_pin_count)) {
>> cpu_relax();
>> }
>> }
>> }
>>
>> and we'd have something like:
>>
>> bool try_pin_remote_cpu(int cpu)
>> {
>> u32 *remote_pin_count = ...;
>> struct fancy_cpu_state *remote_state = ...;
>> atomic_inc(remote_pin_count); // optimistic
>>
>> // Hmm, we do not want that read to get reordered with the inc, so we probably
>> // need a full barrier or seq_cst. How does Linux spell that? C++ has atomic::load
>> // with seq_cst and maybe the optimizer can do the right thing. Maybe it's:
>> smp_mb__after_atomic();
>>
>> if (atomic_read(&remote_state->status) == USER) {
>> // Okay, it's genuinely pinned.
>> return true;
>>
>> // egads, if this is some arch with very weak ordering,
>> // do we need to be concerned that we just took a lock but we
>> // just did a relaxed read and therefore a subsequent access
>> // that thinks it's locked might appear to precede the load and therefore
>> // somehow get surprisingly seen out of order by the target cpu?
>> // maybe we wanted atomic_read_acquire above instead?
>> } else {
>> // We might not have successfully pinned it
>> atomic_dec(remote_pin_count);
>> }
>> }
>>
>> void unpin_remote_cpu(int cpu)
>> {
>> atomic_dec(remote_pin_count();
>> }
>>
>> and we'd use it like:
>>
>> if (try_pin_remote_cpu(cpu)) {
>> // do something useful
>> } else {
>> send IPI;
>> }
>>
>> but we'd really accumulate the set of CPUs that need the IPIs and do them all at once.
>>
>> I ran the theorem prover that lives inside my head on this code using the assumption that the machine is a well-behaved x86 system and it said "yeah, looks like it might be correct". I trust an actual formalized system or someone like you who is genuinely very good at this stuff much more than I trust my initial impression :)
>
> Let's start with requirements, non-traditional though that might be. ;-)
That's ridiculous! :-)
>
> An "RCU idle" CPU is either in deep idle or executing in nohz_full
> userspace.
>
> 1. If the RCU grace-period kthread sees an RCU-idle CPU, then:
>
> a. Everything that this CPU did before entering RCU-idle
> state must be visible to sufficiently later code executed
> by the grace-period kthread, and:
>
> b. Everything that the CPU will do after exiting RCU-idle
> state must *not* have been visible to the grace-period
> kthread sufficiently prior to having sampled this
> CPU's state.
>
> 2. If the RCU grace-period kthread sees an RCU-nonidle CPU, then
> it depends on whether this is the same nonidle sojourn as was
> initially seen. (If the kthread initially saw the CPU in an
> RCU-idle state, it would not have bothered resampling.)
>
> a. If this is the same nonidle sojourn, then there are no
> ordering requirements. RCU must continue to wait on
> this CPU.
>
> b. Otherwise, everything that this CPU did before entering
> its last RCU-idle state must be visible to sufficiently
> later code executed by the grace-period kthread.
> Similar to (1a) above.
>
> 3. If a given CPU quickly switches into and out of RCU-idle
> state, and it is always in RCU-nonidle state whenever the RCU
> grace-period kthread looks, RCU must still realize that this
> CPU has passed through at least one quiescent state.
>
> This is why we have a counter for RCU rather than just a
> simple state.
...
> I am not seeing how the third requirement above is met, though. I have
> not verified the sampling code that the RCU grace-period kthread is
> supposed to use because I am not seeing it right off-hand.
This is ct_rcu_watching_cpu_acquire, right? Lemme think. Maybe there's even a way to do everything I'm suggesting without changing that interface.
>> It wouldn't be outrageous to have real-time imply the full USER transition.
>
> We could have special code for RT, but this of course increases the
> complexity. Which might be justified by sufficient speedup.
I'm thinking that the code would be arranged so that going to user mode in the USER or the INTERMEDIATE state would be fully valid and would just have different performance characteristics. So the only extra complexity here would be the actual logic to choose which state to go to. and...
>
> And yes, many distros enable NO_HZ_FULL by default. I will refrain from
> suggesting additional static branches. ;-)
I'm not even suggesting a static branch. I think the potential performance wins are big enough to justify a bona fide ordinary if statement or two :) I'm also contemplating whether it could make sense to make this whole thing be unconditionally configured in if the performance in the case where no one uses it (i.e. everything is INTERMEDIATE instead of USER) is good enough.
I will ponder.
^ permalink raw reply
* Re: [PATCH v7 00/31] context_tracking,x86: Defer some IPIs until a user->kernel transition
From: Thomas Gleixner @ 2025-11-14 20:06 UTC (permalink / raw)
To: Andy Lutomirski, Paul E. McKenney
Cc: Valentin Schneider, Linux Kernel Mailing List, linux-mm, rcu,
the arch/x86 maintainers, linux-arm-kernel, loongarch,
linux-riscv, linux-arch, linux-trace-kernel, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin,
Peter Zijlstra (Intel), Arnaldo Carvalho de Melo, Josh Poimboeuf,
Paolo Bonzini, Arnd Bergmann, Frederic Weisbecker, Jason Baron,
Steven Rostedt, Ard Biesheuvel, Sami Tolvanen, David S. Miller,
Neeraj Upadhyay, Joel Fernandes, Josh Triplett, Boqun Feng,
Uladzislau Rezki, Mathieu Desnoyers, Mel Gorman, Andrew Morton,
Masahiro Yamada, Han Shen, Rik van Riel, Jann Horn, Dan Carpenter,
Oleg Nesterov, Juri Lelli, Clark Williams, Yair Podemsky,
Marcelo Tosatti, Daniel Wagner, Petr Tesarik, Shrikanth Hegde
In-Reply-To: <1a911310-4ca5-45a4-b9bf-5f37c6ab238e@app.fastmail.com>
On Fri, Nov 14 2025 at 10:45, Andy Lutomirski wrote:
> But I did that test on a machine that would absolutely benefit from
> the IPI suppression that the OP is talking about here, and I think it
> would be really quite nice if a default distro kernel with a more or
> less default distribution could be easily convinced to run a user
> thread without interrupting it. It's a little bit had that NO_HZ_FULL
> is still an exotic non-default thing, IMO.
Feel free to help with getting it over the finish line. Feeling sad/bad
(or whatever you wanted to say with 'had') does not change anything as
you should know :)
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH v7 00/31] context_tracking,x86: Defer some IPIs until a user->kernel transition
From: Paul E. McKenney @ 2025-11-14 20:03 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Valentin Schneider, Linux Kernel Mailing List, linux-mm, rcu,
the arch/x86 maintainers, linux-arm-kernel, loongarch,
linux-riscv, linux-arch, linux-trace-kernel, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin,
Peter Zijlstra (Intel), Arnaldo Carvalho de Melo, Josh Poimboeuf,
Paolo Bonzini, Arnd Bergmann, Frederic Weisbecker, Jason Baron,
Steven Rostedt, Ard Biesheuvel, Sami Tolvanen, David S. Miller,
Neeraj Upadhyay, Joel Fernandes, Josh Triplett, Boqun Feng,
Uladzislau Rezki, Mathieu Desnoyers, Mel Gorman, Andrew Morton,
Masahiro Yamada, Han Shen, Rik van Riel, Jann Horn, Dan Carpenter,
Oleg Nesterov, Juri Lelli, Clark Williams, Yair Podemsky,
Marcelo Tosatti, Daniel Wagner, Petr Tesarik, Shrikanth Hegde
In-Reply-To: <1a911310-4ca5-45a4-b9bf-5f37c6ab238e@app.fastmail.com>
On Fri, Nov 14, 2025 at 10:45:08AM -0800, Andy Lutomirski wrote:
>
>
> On Fri, Nov 14, 2025, at 10:14 AM, Paul E. McKenney wrote:
> > On Fri, Nov 14, 2025 at 09:22:35AM -0800, Andy Lutomirski wrote:
> >>
>
> >> > Oh, any another primitive would be possible: one CPU could plausibly
> >> > execute another CPU's interrupts or soft-irqs or whatever by taking a
> >> > special lock that would effectively pin the remote CPU in user mode --
> >> > you'd set a flag in the target cpu_flags saying "pin in USER mode" and
> >> > the transition on that CPU to kernel mode would then spin on entry to
> >> > kernel mode and wait for the lock to be released. This could plausibly
> >> > get a lot of the on_each_cpu callers to switch over in one fell swoop:
> >> > anything that needs to synchronize to the remote CPU but does not need
> >> > to poke its actual architectural state could be executed locally while
> >> > the remote CPU is pinned.
> >
> > It would be necessary to arrange for the remote CPU to remain pinned
> > while the local CPU executed on its behalf. Does the above approach
> > make that happen without re-introducing our current context-tracking
> > overhead and complexity?
>
> Using the pseudo-implementation farther down, I think this would be like:
>
> if (my_state->status == INDETERMINATE) {
> // we were lazy and we never promised to do work atomically.
> atomic_set(&my_state->status, KERNEL);
> this_entry_work = 0;
> /* we are definitely not pinned in this path */
> } else {
> // we were not lazy and we promised we would do work atomically
> atomic exchange the entire state to { .work = 0, .status = KERNEL }
> this_entry_work = (whatever we just read);
> if (this_entry_work & PINNED) {
> u32 this_cpu_pin_count = this_cpu_ptr(pin_count);
> while (atomic_read(&this_cpu_pin_count)) {
> cpu_relax();
> }
> }
> }
>
> and we'd have something like:
>
> bool try_pin_remote_cpu(int cpu)
> {
> u32 *remote_pin_count = ...;
> struct fancy_cpu_state *remote_state = ...;
> atomic_inc(remote_pin_count); // optimistic
>
> // Hmm, we do not want that read to get reordered with the inc, so we probably
> // need a full barrier or seq_cst. How does Linux spell that? C++ has atomic::load
> // with seq_cst and maybe the optimizer can do the right thing. Maybe it's:
> smp_mb__after_atomic();
>
> if (atomic_read(&remote_state->status) == USER) {
> // Okay, it's genuinely pinned.
> return true;
>
> // egads, if this is some arch with very weak ordering,
> // do we need to be concerned that we just took a lock but we
> // just did a relaxed read and therefore a subsequent access
> // that thinks it's locked might appear to precede the load and therefore
> // somehow get surprisingly seen out of order by the target cpu?
> // maybe we wanted atomic_read_acquire above instead?
> } else {
> // We might not have successfully pinned it
> atomic_dec(remote_pin_count);
> }
> }
>
> void unpin_remote_cpu(int cpu)
> {
> atomic_dec(remote_pin_count();
> }
>
> and we'd use it like:
>
> if (try_pin_remote_cpu(cpu)) {
> // do something useful
> } else {
> send IPI;
> }
>
> but we'd really accumulate the set of CPUs that need the IPIs and do them all at once.
>
> I ran the theorem prover that lives inside my head on this code using the assumption that the machine is a well-behaved x86 system and it said "yeah, looks like it might be correct". I trust an actual formalized system or someone like you who is genuinely very good at this stuff much more than I trust my initial impression :)
Let's start with requirements, non-traditional though that might be. ;-)
An "RCU idle" CPU is either in deep idle or executing in nohz_full
userspace.
1. If the RCU grace-period kthread sees an RCU-idle CPU, then:
a. Everything that this CPU did before entering RCU-idle
state must be visible to sufficiently later code executed
by the grace-period kthread, and:
b. Everything that the CPU will do after exiting RCU-idle
state must *not* have been visible to the grace-period
kthread sufficiently prior to having sampled this
CPU's state.
2. If the RCU grace-period kthread sees an RCU-nonidle CPU, then
it depends on whether this is the same nonidle sojourn as was
initially seen. (If the kthread initially saw the CPU in an
RCU-idle state, it would not have bothered resampling.)
a. If this is the same nonidle sojourn, then there are no
ordering requirements. RCU must continue to wait on
this CPU.
b. Otherwise, everything that this CPU did before entering
its last RCU-idle state must be visible to sufficiently
later code executed by the grace-period kthread.
Similar to (1a) above.
3. If a given CPU quickly switches into and out of RCU-idle
state, and it is always in RCU-nonidle state whenever the RCU
grace-period kthread looks, RCU must still realize that this
CPU has passed through at least one quiescent state.
This is why we have a counter for RCU rather than just a
simple state.
The usual way to handle (1a) and (2b) is make the update marking entry
to the RCU-idle state have release semantics and to make the operation
that the RCU grace-period kthread uses to sample the CPU's state have
acquire semantics.
The usual way to handle (1b) is to have a full barrier after the update
marking exit from the RCU-idle state and another full barrier before the
operation that the RCU grace-period kthread uses to sample the CPU's
state. The "sufficiently" allows some wiggle room on the placement
of both full barriers. A full barrier can be smp_mb() or some fully
ordered atomic operation.
I will let Valentin and Frederic check the current code. ;-)
> >> Following up, I think that x86 can do this all with a single atomic (in the common case) per usermode round trip. Imagine:
> >>
> >> struct fancy_cpu_state {
> >> u32 work; // <-- writable by any CPU
> >> u32 status; // <-- readable anywhere; writable locally
> >> };
> >>
> >> status includes KERNEL, USER, and maybe INDETERMINATE. (INDETERMINATE means USER but we're not committing to doing work.)
> >>
> >> Exit to user mode:
> >>
> >> atomic_set(&my_state->status, USER);
> >
> > We need ordering in the RCU nohz_full case. If the grace-period kthread
> > sees the status as USER, all the preceding KERNEL code's effects must
> > be visible to the grace-period kthread.
>
> Sorry, I'm speaking lazy x86 programmer here. Maybe I mean atomic_set_release. I want, roughly, the property that anyone who remotely observes USER can rely on the target cpu subsequently going through the atomic exchange path above. I think even relaxed ought to be good enough for that one most architectures, but there are some potentially nasty complications involving that fact that this mixes operations on a double word and a single word that's part of the double word.
Please see the requirements laid out above.
> >> (or, in the lazy case, set to INDETERMINATE instead.)
> >>
> >> Entry from user mode, with IRQs off, before switching to kernel CR3:
> >>
> >> if (my_state->status == INDETERMINATE) {
> >> // we were lazy and we never promised to do work atomically.
> >> atomic_set(&my_state->status, KERNEL);
> >> this_entry_work = 0;
> >> } else {
> >> // we were not lazy and we promised we would do work atomically
> >> atomic exchange the entire state to { .work = 0, .status = KERNEL }
> >> this_entry_work = (whatever we just read);
> >> }
> >
> > If this atomic exchange is fully ordered (as opposed to, say, _relaxed),
> > then this works in that if the grace-period kthread sees USER, its prior
> > references are guaranteed not to see later kernel-mode references from
> > that CPU.
>
> Yep, that's the intent of my pseudocode. On x86 this would be a plain 64-bit lock xchg -- I don't think cmpxchg is needed. (I like to write lock even when it's implicit to avoid needing to trust myself to remember precisely which instructions imply it.)
OK, then the atomic exchange provides all the ordering that the update
ever needs.
> >> if (PTI) {
> >> switch to kernel CR3 *and flush if this_entry_work says to flush*
> >> } else {
> >> flush if this_entry_work says to flush;
> >> }
> >>
> >> do the rest of the work;
> >>
> >>
> >>
> >> I suppose that a lot of the stuff in ti_flags could merge into here, but it could be done one bit at a time when people feel like doing so. And I imagine, but I'm very far from confident, that RCU could use this instead of the current context tracking code.
> >
> > RCU currently needs pretty heavy-duty ordering to reliably detect the
> > other CPUs' quiescent states without needing to wake them from idle, or,
> > in the nohz_full case, interrupt their userspace execution. Not saying
> > it is impossible, but it will need extreme care.
>
> Is the thingy above heavy-duty enough? Perhaps more relevantly, could RCU do this *instead* of the current CT hooks on architectures that implement it, and/or could RCU arrange for the ct hooks to be cheap no-ops on architectures that support the thingy above. By "could" I mean "could be done without absolutely massive refactoring and without the resulting code being an unmaintainable disaster? I'm sure any sufficiently motivated human or LLM could pull off the unmaintainable disaster version. I bet I could even do it myself!)
I write broken and unmaintainable code all the time, having more than 50
years of experience doing so. This is one reason we have rcutorture. ;-)
RCU used to do its own CT-like hooks. Merging them into the actual
context-tracking code reduced the entry/exit overhead significantly.
I am not seeing how the third requirement above is met, though. I have
not verified the sampling code that the RCU grace-period kthread is
supposed to use because I am not seeing it right off-hand.
> >> The idea behind INDETERMINATE is that there are plenty of workloads that frequently switch between user and kernel mode and that would rather accept a few IPIs to avoid the heavyweight atomic operation on user -> kernel transitions. So the default behavior could be to do KERNEL -> INDETERMINATE instead of KERNEL -> USER, but code that wants to be in user mode for a long time could go all the way to USER. We could make it sort of automatic by noticing that we're returning from an IRQ without a context switch and go to USER (so we would get at most one unneeded IPI per normal user entry), and we could have some nice API for a program that intends to hang out in user mode for a very long time (cpu isolation users, for example) to tell the kernel to go immediately into USER mode. (Don't we already have something that could be used for this purpose?)
> >
> > RCU *could* do an smp_call_function_single() when the CPU failed
> > to respond, perhaps in a manner similar to how it already forces a
> > given CPU out of nohz_full state if that CPU has been executing in the
> > kernel for too long. The real-time guys might not be amused, though.
> > Especially those real-time guys hitting sub-microsecond latencies.
>
> It wouldn't be outrageous to have real-time imply the full USER transition.
We could have special code for RT, but this of course increases the
complexity. Which might be justified by sufficient speedup.
> >> Hmm, now I wonder if it would make sense for the default behavior of Linux to be like that. We could call it ONEHZ. It's like NOHZ_FULL except that user threads that don't do syscalls get one single timer tick instead of many or none.
> >>
> >>
> >> Anyway, I think my proposal is pretty good *if* RCU could be made to use it -- the existing context tracking code is fairly expensive, and I don't think we want to invent a new context-tracking-like mechanism if we still need to do the existing thing.
> >
> > If you build with CONFIG_NO_HZ_FULL=n, do you still get the heavyweight
> > operations when transitioning between kernel and user execution?
>
> No. And I even tested approximately this a couple weeks ago for unrelated reasons. The only unnecessary heavy-weight thing we're doing in a syscall loop with mitigations off is the RDTSC for stack randomization.
OK, so why not just build your kernels with CONFIG_NO_HZ_FULL=n and be happy?
> But I did that test on a machine that would absolutely benefit from the IPI suppression that the OP is talking about here, and I think it would be really quite nice if a default distro kernel with a more or less default distribution could be easily convinced to run a user thread without interrupting it. It's a little bit had that NO_HZ_FULL is still an exotic non-default thing, IMO.
And yes, many distros enable NO_HZ_FULL by default. I will refrain from
suggesting additional static branches. ;-)
Thanx, Paul
^ permalink raw reply
* [PATCH 4/4] tracing: Convert function graph set_flags() to use a switch() statement
From: Steven Rostedt @ 2025-11-14 19:22 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20251114192229.829042264@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
Currently the set_flags() of the function graph tracer has a bunch of:
if (bit == FLAG1) {
[..]
}
if (bit == FLAG2) {
[..]
}
To clean it up a bit, convert it over to a switch statement.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_functions_graph.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 12315eb65925..44d5dc5031e2 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -1664,7 +1664,8 @@ func_graph_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
if (!!set == !!(tr->current_trace_flags->val & bit))
return 0;
- if (bit == TRACE_GRAPH_SLEEP_TIME) {
+ switch (bit) {
+ case TRACE_GRAPH_SLEEP_TIME:
if (set) {
fgraph_no_sleep_time--;
if (WARN_ON_ONCE(fgraph_no_sleep_time < 0))
@@ -1672,19 +1673,20 @@ func_graph_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
} else {
fgraph_no_sleep_time++;
}
- }
+ break;
- if (bit == TRACE_GRAPH_PRINT_IRQS) {
+ case TRACE_GRAPH_PRINT_IRQS:
if (set)
ftrace_graph_skip_irqs--;
else
ftrace_graph_skip_irqs++;
if (WARN_ON_ONCE(ftrace_graph_skip_irqs < 0))
ftrace_graph_skip_irqs = 0;
- }
+ break;
- if (bit == TRACE_GRAPH_ARGS)
+ case TRACE_GRAPH_ARGS:
return ftrace_graph_trace_args(tr, set);
+ }
return 0;
}
--
2.51.0
^ permalink raw reply related
* [PATCH 2/4] tracing: Move graph-time out of function graph options
From: Steven Rostedt @ 2025-11-14 19:22 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20251114192229.829042264@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
The option "graph-time" affects the function profiler when it is using the
function graph infrastructure. It has nothing to do with the function
graph tracer itself. The option only affects the global function profiler
and does nothing to the function graph tracer.
Move it out of the function graph tracer options and make it a global
option that is only available at the top level instance.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace.c | 14 ++++++++++----
kernel/trace/trace.h | 13 ++++++++++++-
kernel/trace/trace_functions_graph.c | 10 +---------
3 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 9268489d2ce8..8ae95800592d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -509,10 +509,10 @@ EXPORT_SYMBOL_GPL(unregister_ftrace_export);
/* trace_flags holds trace_options default values */
#define TRACE_DEFAULT_FLAGS \
- (FUNCTION_DEFAULT_FLAGS | \
- TRACE_ITER(PRINT_PARENT) | TRACE_ITER(PRINTK) | \
+ (FUNCTION_DEFAULT_FLAGS | FPROFILE_DEFAULT_FLAGS | \
+ TRACE_ITER(PRINT_PARENT) | TRACE_ITER(PRINTK) | \
TRACE_ITER(ANNOTATE) | TRACE_ITER(CONTEXT_INFO) | \
- TRACE_ITER(RECORD_CMD) | TRACE_ITER(OVERWRITE) | \
+ TRACE_ITER(RECORD_CMD) | TRACE_ITER(OVERWRITE) | \
TRACE_ITER(IRQ_INFO) | TRACE_ITER(MARKERS) | \
TRACE_ITER(HASH_PTR) | TRACE_ITER(TRACE_PRINTK) | \
TRACE_ITER(COPY_MARKER))
@@ -520,7 +520,7 @@ EXPORT_SYMBOL_GPL(unregister_ftrace_export);
/* trace_options that are only supported by global_trace */
#define TOP_LEVEL_TRACE_FLAGS (TRACE_ITER(PRINTK) | \
TRACE_ITER(PRINTK_MSGONLY) | TRACE_ITER(RECORD_CMD) | \
- TRACE_ITER(PROF_TEXT_OFFSET))
+ TRACE_ITER(PROF_TEXT_OFFSET) | FPROFILE_DEFAULT_FLAGS)
/* trace_flags that are default zero for instances */
#define ZEROED_TRACE_FLAGS \
@@ -5331,6 +5331,12 @@ int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
trace_printk_start_stop_comm(enabled);
trace_printk_control(enabled);
break;
+
+#if defined(CONFIG_FUNCTION_PROFILER) && defined(CONFIG_FUNCTION_GRAPH_TRACER)
+ case TRACE_GRAPH_GRAPH_TIME:
+ ftrace_graph_graph_time_control(enabled);
+ break;
+#endif
}
return 0;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 299862aad66c..41b416a22450 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1368,8 +1368,18 @@ extern int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
#ifdef CONFIG_FUNCTION_PROFILER
# define PROFILER_FLAGS \
C(PROF_TEXT_OFFSET, "prof-text-offset"),
+# ifdef CONFIG_FUNCTION_GRAPH_TRACER
+# define FPROFILE_FLAGS \
+ C(GRAPH_TIME, "graph-time"),
+# define FPROFILE_DEFAULT_FLAGS TRACE_ITER(GRAPH_TIME)
+# else
+# define FPROFILE_FLAGS
+# define FPROFILE_DEFAULT_FLAGS 0UL
+# endif
#else
# define PROFILER_FLAGS
+# define FPROFILE_FLAGS
+# define FPROFILE_DEFAULT_FLAGS 0UL
# define TRACE_ITER_PROF_TEXT_OFFSET_BIT -1
#endif
@@ -1412,7 +1422,8 @@ extern int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
FGRAPH_FLAGS \
STACK_FLAGS \
BRANCH_FLAGS \
- PROFILER_FLAGS
+ PROFILER_FLAGS \
+ FPROFILE_FLAGS
/*
* By defining C, we can make TRACE_FLAGS a list of bit names
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 3f55b49cf64e..53adbe4bfedb 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -85,11 +85,6 @@ static struct tracer_opt trace_opts[] = {
/* Include sleep time (scheduled out) between entry and return */
{ TRACER_OPT(sleep-time, TRACE_GRAPH_SLEEP_TIME) },
-#ifdef CONFIG_FUNCTION_PROFILER
- /* Include time within nested functions */
- { TRACER_OPT(graph-time, TRACE_GRAPH_GRAPH_TIME) },
-#endif
-
{ } /* Empty entry */
};
@@ -97,7 +92,7 @@ static struct tracer_flags tracer_flags = {
/* Don't display overruns, proc, or tail by default */
.val = TRACE_GRAPH_PRINT_CPU | TRACE_GRAPH_PRINT_OVERHEAD |
TRACE_GRAPH_PRINT_DURATION | TRACE_GRAPH_PRINT_IRQS |
- TRACE_GRAPH_SLEEP_TIME | TRACE_GRAPH_GRAPH_TIME,
+ TRACE_GRAPH_SLEEP_TIME,
.opts = trace_opts
};
@@ -1627,9 +1622,6 @@ func_graph_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
if (bit == TRACE_GRAPH_SLEEP_TIME)
ftrace_graph_sleep_time_control(set);
- if (bit == TRACE_GRAPH_GRAPH_TIME)
- ftrace_graph_graph_time_control(set);
-
/* Do nothing if the current tracer is not this tracer */
if (tr->current_trace != &graph_trace)
return 0;
--
2.51.0
^ permalink raw reply related
* [PATCH 3/4] tracing: Have function graph tracer option sleep-time be per instance
From: Steven Rostedt @ 2025-11-14 19:22 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20251114192229.829042264@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
Currently the option to have function graph tracer to ignore time spent
when a task is sleeping is global when the interface is per-instance.
Changing the value in one instance will affect the results of another
instance that is also running the function graph tracer. This can lead to
confusing results.
Fixes: c132be2c4fcc1 ("function_graph: Have the instances use their own ftrace_ops for filtering")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/fgraph.c | 10 +----
kernel/trace/ftrace.c | 4 +-
kernel/trace/trace.h | 5 +--
kernel/trace/trace_functions_graph.c | 64 +++++++++++++++++++++++-----
4 files changed, 60 insertions(+), 23 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 484ad7a18463..7fb9b169d6d4 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -498,9 +498,6 @@ void *fgraph_retrieve_parent_data(int idx, int *size_bytes, int depth)
return get_data_type_data(current, offset);
}
-/* Both enabled by default (can be cleared by function_graph tracer flags */
-bool fgraph_sleep_time = true;
-
#ifdef CONFIG_DYNAMIC_FTRACE
/*
* archs can override this function if they must do something
@@ -1023,11 +1020,6 @@ void fgraph_init_ops(struct ftrace_ops *dst_ops,
#endif
}
-void ftrace_graph_sleep_time_control(bool enable)
-{
- fgraph_sleep_time = enable;
-}
-
/*
* Simply points to ftrace_stub, but with the proper protocol.
* Defined by the linker script in linux/vmlinux.lds.h
@@ -1098,7 +1090,7 @@ ftrace_graph_probe_sched_switch(void *ignore, bool preempt,
* Does the user want to count the time a function was asleep.
* If so, do not update the time stamps.
*/
- if (fgraph_sleep_time)
+ if (!fgraph_no_sleep_time)
return;
timestamp = trace_clock_local();
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index ab601cd9638b..7c3bbebeec7a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -862,6 +862,8 @@ static int profile_graph_entry(struct ftrace_graph_ent *trace,
return 1;
}
+bool fprofile_no_sleep_time;
+
static void profile_graph_return(struct ftrace_graph_ret *trace,
struct fgraph_ops *gops,
struct ftrace_regs *fregs)
@@ -887,7 +889,7 @@ static void profile_graph_return(struct ftrace_graph_ret *trace,
calltime = rettime - profile_data->calltime;
- if (!fgraph_sleep_time) {
+ if (fprofile_no_sleep_time) {
if (current->ftrace_sleeptime)
calltime -= current->ftrace_sleeptime - profile_data->sleeptime;
}
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 41b416a22450..58be6d741d72 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -943,8 +943,6 @@ static __always_inline bool ftrace_hash_empty(struct ftrace_hash *hash)
#define TRACE_GRAPH_PRINT_FILL_SHIFT 28
#define TRACE_GRAPH_PRINT_FILL_MASK (0x3 << TRACE_GRAPH_PRINT_FILL_SHIFT)
-extern void ftrace_graph_sleep_time_control(bool enable);
-
#ifdef CONFIG_FUNCTION_PROFILER
extern void ftrace_graph_graph_time_control(bool enable);
#else
@@ -1115,7 +1113,8 @@ static inline void ftrace_graph_addr_finish(struct fgraph_ops *gops, struct ftra
#endif /* CONFIG_DYNAMIC_FTRACE */
extern unsigned int fgraph_max_depth;
-extern bool fgraph_sleep_time;
+extern unsigned int fgraph_no_sleep_time;
+extern bool fprofile_no_sleep_time;
static inline bool
ftrace_graph_ignore_func(struct fgraph_ops *gops, struct ftrace_graph_ent *trace)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 53adbe4bfedb..12315eb65925 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -19,6 +19,9 @@
/* When set, irq functions might be ignored */
static int ftrace_graph_skip_irqs;
+/* Do not record function time when task is sleeping */
+unsigned int fgraph_no_sleep_time;
+
struct fgraph_cpu_data {
pid_t last_pid;
int depth;
@@ -239,13 +242,14 @@ static int graph_entry(struct ftrace_graph_ent *trace,
if (ftrace_graph_ignore_irqs(tr))
return 0;
- if (fgraph_sleep_time) {
- /* Only need to record the calltime */
- ftimes = fgraph_reserve_data(gops->idx, sizeof(ftimes->calltime));
- } else {
+ if (fgraph_no_sleep_time &&
+ !tracer_flags_is_set(tr, TRACE_GRAPH_SLEEP_TIME)) {
ftimes = fgraph_reserve_data(gops->idx, sizeof(*ftimes));
if (ftimes)
ftimes->sleeptime = current->ftrace_sleeptime;
+ } else {
+ /* Only need to record the calltime */
+ ftimes = fgraph_reserve_data(gops->idx, sizeof(ftimes->calltime));
}
if (!ftimes)
return 0;
@@ -331,11 +335,15 @@ void __trace_graph_return(struct trace_array *tr,
trace_buffer_unlock_commit_nostack(buffer, event);
}
-static void handle_nosleeptime(struct ftrace_graph_ret *trace,
+static void handle_nosleeptime(struct trace_array *tr,
+ struct ftrace_graph_ret *trace,
struct fgraph_times *ftimes,
int size)
{
- if (fgraph_sleep_time || size < sizeof(*ftimes))
+ if (size < sizeof(*ftimes))
+ return;
+
+ if (!fgraph_no_sleep_time || tracer_flags_is_set(tr, TRACE_GRAPH_SLEEP_TIME))
return;
ftimes->calltime += current->ftrace_sleeptime - ftimes->sleeptime;
@@ -364,7 +372,7 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
if (!ftimes)
return;
- handle_nosleeptime(trace, ftimes, size);
+ handle_nosleeptime(tr, trace, ftimes, size);
calltime = ftimes->calltime;
@@ -377,6 +385,7 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
struct ftrace_regs *fregs)
{
struct fgraph_times *ftimes;
+ struct trace_array *tr;
int size;
ftrace_graph_addr_finish(gops, trace);
@@ -390,7 +399,8 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
if (!ftimes)
return;
- handle_nosleeptime(trace, ftimes, size);
+ tr = gops->private;
+ handle_nosleeptime(tr, trace, ftimes, size);
if (tracing_thresh &&
(trace_clock_local() - ftimes->calltime < tracing_thresh))
@@ -452,6 +462,9 @@ static int graph_trace_init(struct trace_array *tr)
if (!tracer_flags_is_set(tr, TRACE_GRAPH_PRINT_IRQS))
ftrace_graph_skip_irqs++;
+ if (!tracer_flags_is_set(tr, TRACE_GRAPH_SLEEP_TIME))
+ fgraph_no_sleep_time++;
+
/* Make gops functions visible before we start tracing */
smp_mb();
@@ -494,6 +507,11 @@ static void graph_trace_reset(struct trace_array *tr)
if (WARN_ON_ONCE(ftrace_graph_skip_irqs < 0))
ftrace_graph_skip_irqs = 0;
+ if (!tracer_flags_is_set(tr, TRACE_GRAPH_SLEEP_TIME))
+ fgraph_no_sleep_time--;
+ if (WARN_ON_ONCE(fgraph_no_sleep_time < 0))
+ fgraph_no_sleep_time = 0;
+
tracing_stop_cmdline_record();
unregister_ftrace_graph(tr->gops);
}
@@ -1619,8 +1637,24 @@ void graph_trace_close(struct trace_iterator *iter)
static int
func_graph_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
{
- if (bit == TRACE_GRAPH_SLEEP_TIME)
- ftrace_graph_sleep_time_control(set);
+/*
+ * The function profiler gets updated even if function graph
+ * isn't the current tracer. Handle it separately.
+ */
+#ifdef CONFIG_FUNCTION_PROFILER
+ if (bit == TRACE_GRAPH_SLEEP_TIME && (tr->flags & TRACE_ARRAY_FL_GLOBAL) &&
+ !!set == fprofile_no_sleep_time) {
+ if (set) {
+ fgraph_no_sleep_time--;
+ if (WARN_ON_ONCE(fgraph_no_sleep_time < 0))
+ fgraph_no_sleep_time = 0;
+ fprofile_no_sleep_time = false;
+ } else {
+ fgraph_no_sleep_time++;
+ fprofile_no_sleep_time = true;
+ }
+ }
+#endif
/* Do nothing if the current tracer is not this tracer */
if (tr->current_trace != &graph_trace)
@@ -1630,6 +1664,16 @@ func_graph_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
if (!!set == !!(tr->current_trace_flags->val & bit))
return 0;
+ if (bit == TRACE_GRAPH_SLEEP_TIME) {
+ if (set) {
+ fgraph_no_sleep_time--;
+ if (WARN_ON_ONCE(fgraph_no_sleep_time < 0))
+ fgraph_no_sleep_time = 0;
+ } else {
+ fgraph_no_sleep_time++;
+ }
+ }
+
if (bit == TRACE_GRAPH_PRINT_IRQS) {
if (set)
ftrace_graph_skip_irqs--;
--
2.51.0
^ permalink raw reply related
* [PATCH 1/4] tracing: Have function graph tracer option funcgraph-irqs be per instance
From: Steven Rostedt @ 2025-11-14 19:22 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20251114192229.829042264@kernel.org>
From: Steven Rostedt <rostedt@goodmis.org>
Currently the option to trace interrupts in the function graph tracer is
global when the interface is per-instance. Changing the value in one
instance will affect the results of another instance that is also running
the function graph tracer. This can lead to confusing results.
Fixes: c132be2c4fcc1 ("function_graph: Have the instances use their own ftrace_ops for filtering")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_functions_graph.c | 41 +++++++++++++++++++++-------
1 file changed, 31 insertions(+), 10 deletions(-)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 4e86adf6dd4d..3f55b49cf64e 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -16,7 +16,7 @@
#include "trace.h"
#include "trace_output.h"
-/* When set, irq functions will be ignored */
+/* When set, irq functions might be ignored */
static int ftrace_graph_skip_irqs;
struct fgraph_cpu_data {
@@ -190,11 +190,14 @@ int __trace_graph_retaddr_entry(struct trace_array *tr,
}
#endif
-static inline int ftrace_graph_ignore_irqs(void)
+static inline int ftrace_graph_ignore_irqs(struct trace_array *tr)
{
if (!ftrace_graph_skip_irqs || trace_recursion_test(TRACE_IRQ_BIT))
return 0;
+ if (tracer_flags_is_set(tr, TRACE_GRAPH_PRINT_IRQS))
+ return 0;
+
return in_hardirq();
}
@@ -238,7 +241,7 @@ static int graph_entry(struct ftrace_graph_ent *trace,
if (ftrace_graph_ignore_func(gops, trace))
return 0;
- if (ftrace_graph_ignore_irqs())
+ if (ftrace_graph_ignore_irqs(tr))
return 0;
if (fgraph_sleep_time) {
@@ -451,6 +454,9 @@ static int graph_trace_init(struct trace_array *tr)
else
tr->gops->retfunc = trace_graph_return;
+ if (!tracer_flags_is_set(tr, TRACE_GRAPH_PRINT_IRQS))
+ ftrace_graph_skip_irqs++;
+
/* Make gops functions visible before we start tracing */
smp_mb();
@@ -468,10 +474,6 @@ static int ftrace_graph_trace_args(struct trace_array *tr, int set)
{
trace_func_graph_ent_t entry;
- /* Do nothing if the current tracer is not this tracer */
- if (tr->current_trace != &graph_trace)
- return 0;
-
if (set)
entry = trace_graph_entry_args;
else
@@ -492,6 +494,11 @@ static int ftrace_graph_trace_args(struct trace_array *tr, int set)
static void graph_trace_reset(struct trace_array *tr)
{
+ if (!tracer_flags_is_set(tr, TRACE_GRAPH_PRINT_IRQS))
+ ftrace_graph_skip_irqs--;
+ if (WARN_ON_ONCE(ftrace_graph_skip_irqs < 0))
+ ftrace_graph_skip_irqs = 0;
+
tracing_stop_cmdline_record();
unregister_ftrace_graph(tr->gops);
}
@@ -1617,15 +1624,29 @@ void graph_trace_close(struct trace_iterator *iter)
static int
func_graph_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
{
- if (bit == TRACE_GRAPH_PRINT_IRQS)
- ftrace_graph_skip_irqs = !set;
-
if (bit == TRACE_GRAPH_SLEEP_TIME)
ftrace_graph_sleep_time_control(set);
if (bit == TRACE_GRAPH_GRAPH_TIME)
ftrace_graph_graph_time_control(set);
+ /* Do nothing if the current tracer is not this tracer */
+ if (tr->current_trace != &graph_trace)
+ return 0;
+
+ /* Do nothing if already set. */
+ if (!!set == !!(tr->current_trace_flags->val & bit))
+ return 0;
+
+ if (bit == TRACE_GRAPH_PRINT_IRQS) {
+ if (set)
+ ftrace_graph_skip_irqs--;
+ else
+ ftrace_graph_skip_irqs++;
+ if (WARN_ON_ONCE(ftrace_graph_skip_irqs < 0))
+ ftrace_graph_skip_irqs = 0;
+ }
+
if (bit == TRACE_GRAPH_ARGS)
return ftrace_graph_trace_args(tr, set);
--
2.51.0
^ permalink raw reply related
* [PATCH 0/4] tracing: Make more function graph tracer options per-instance
From: Steven Rostedt @ 2025-11-14 19:22 UTC (permalink / raw)
To: linux-kernel, linux-trace-kernel
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton
Convert the function graph tracer options sleep-time and funcgraph-irqs
into per instance options (as they currently are not consistent in
affecting all instances).
Also make graph-time a top level option only, as it only affects the
function profiler and not the function graph tracer itself.
Clean up the function graph tracer set_flags() to use a switch instead
of a bunch of if statements.
Steven Rostedt (4):
tracing: Have function graph tracer option funcgraph-irqs be per instance
tracing: Move graph-time out of function graph options
tracing: Have function graph tracer option sleep-time be per instance
tracing: Convert function graph set_flags() to use a switch() statement
----
kernel/trace/fgraph.c | 10 +--
kernel/trace/ftrace.c | 4 +-
kernel/trace/trace.c | 14 +++--
kernel/trace/trace.h | 18 ++++--
kernel/trace/trace_functions_graph.c | 115 ++++++++++++++++++++++++++---------
5 files changed, 115 insertions(+), 46 deletions(-)
^ permalink raw reply
* Re: [PATCH v3 3/5] perf record: Enable defer_callchain for user callchains
From: Namhyung Kim @ 2025-11-14 19:20 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ian Rogers, Arnaldo Carvalho de Melo, James Clark, Jiri Olsa,
Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Josh Poimboeuf, Indu Bhagat, Jens Remus,
Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251114133009.7dd97625@gandalf.local.home>
On Fri, Nov 14, 2025 at 01:30:09PM -0500, Steven Rostedt wrote:
> On Fri, 14 Nov 2025 10:09:26 -0800
> Ian Rogers <irogers@google.com> wrote:
>
> > Just to be clear. I don't think the behavior of using frame pointers
> > should change. Deferral has downsides, for example:
> >
> > $ perf record -g -a sleep 1
>
> The biggest advantage of the deferred callstack is that there's much less
> duplication of data in the ring buffer. Especially when you have deep
> stacks and long system calls.
>
> Now, if we have frame pointers enabled, we could possibly add a feature to
> the deferred unwinder where it could try to do the deferred immediately and
> if it faults it then waits until going back to user space.
This would be great if it can share the callstack with later samples
before going to user space.
> This means that
> the frame pointer version should work (unless the user space stack was
> swapped out).
>
> >
> > Without deferral kernel stack traces will contain both kernel and user
> > traces. With deferral the user stack trace is only generated when the
> > system call returns and so there is a chance for kernel stack traces
> > to be missing their user part. An obvious behavioral change.
Right, this is one of my concerns too. For system-wide profiling, the
chances are high it can have some tasks sleeping in the kernel and perf
finishes the profiling before they return to user space.
Thanks,
Namhyung
> > I think
> > for what you are doing here we can have an option something like:
> >
> > $ perf record --call-graph fp-deferred -a sleep 1
>
> I would be OK with this but I would prefer a much shorter name. Adding 20
> characters to the command line will likely keep people from using it.
>
> -- Steve
^ permalink raw reply
* Re: [PATCH v3 3/5] perf record: Enable defer_callchain for user callchains
From: Namhyung Kim @ 2025-11-14 19:15 UTC (permalink / raw)
To: Ian Rogers
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users,
Steven Rostedt, Josh Poimboeuf, Indu Bhagat, Jens Remus,
Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <CAP-5=fVptEdzt363LpuZzzm=BJFFkB_xkOLW=x-2-TZa+cvS0g@mail.gmail.com>
On Fri, Nov 14, 2025 at 10:12:34AM -0800, Ian Rogers wrote:
> On Fri, Nov 14, 2025 at 10:09 AM Ian Rogers <irogers@google.com> wrote:
> >
> > On Fri, Nov 14, 2025 at 9:59 AM Ian Rogers <irogers@google.com> wrote:
> > >
> > > On Thu, Nov 13, 2025 at 11:01 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > >
> > > > And add the missing feature detection logic to clear the flag on old
> > > > kernels.
> > > >
> > > > $ perf record -g -vv true
> > > > ...
> > > > ------------------------------------------------------------
> > > > perf_event_attr:
> > > > type 0 (PERF_TYPE_HARDWARE)
> > > > size 136
> > > > config 0 (PERF_COUNT_HW_CPU_CYCLES)
> > > > { sample_period, sample_freq } 4000
> > > > sample_type IP|TID|TIME|CALLCHAIN|PERIOD
> > > > read_format ID|LOST
> > > > disabled 1
> > > > inherit 1
> > > > mmap 1
> > > > comm 1
> > > > freq 1
> > > > enable_on_exec 1
> > > > task 1
> > > > sample_id_all 1
> > > > mmap2 1
> > > > comm_exec 1
> > > > ksymbol 1
> > > > bpf_event 1
> > > > defer_callchain 1
> > > > defer_output 1
> > > > ------------------------------------------------------------
> > > > sys_perf_event_open: pid 162755 cpu 0 group_fd -1 flags 0x8
> > > > sys_perf_event_open failed, error -22
> > > > switching off deferred callchain support
> > > >
> > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > ---
> > > > tools/perf/util/evsel.c | 24 ++++++++++++++++++++++++
> > > > tools/perf/util/evsel.h | 1 +
> > > > 2 files changed, 25 insertions(+)
> > > >
> > > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > > > index 244b3e44d090d413..f5652d00b457d096 100644
> > > > --- a/tools/perf/util/evsel.c
> > > > +++ b/tools/perf/util/evsel.c
> > > > @@ -1061,6 +1061,14 @@ static void __evsel__config_callchain(struct evsel *evsel, struct record_opts *o
> > > > }
> > > > }
> > > >
> > > > + if (param->record_mode == CALLCHAIN_FP && !attr->exclude_callchain_user) {
> > > > + /*
> > > > + * Enable deferred callchains optimistically. It'll be switched
> > > > + * off later if the kernel doesn't support it.
> > > > + */
> > > > + attr->defer_callchain = 1;
> > > > + }
> > >
> > > If a user has requested frame pointer call chains why would they want
> > > deferred call chains? The point of deferral to my understanding is to
> > > allow the paging in of debug data, but frame pointers don't need that
> > > as the stack should be in the page cache.
> > >
> > > Is this being done for code coverage reasons so that deferral is known
> > > to work for later addition of SFrames? In which case this should be an
> > > opt-in not default behavior. When there is a record_mode of
> > > CALLCHAIN_SFRAME then making deferral the default for that mode makes
> > > sense, but not for frame pointers IMO.
> >
> > Just to be clear. I don't think the behavior of using frame pointers
> > should change. Deferral has downsides, for example:
> >
> > $ perf record -g -a sleep 1
> >
> > Without deferral kernel stack traces will contain both kernel and user
> > traces. With deferral the user stack trace is only generated when the
> > system call returns and so there is a chance for kernel stack traces
> > to be missing their user part. An obvious behavioral change. I think
> > for what you are doing here we can have an option something like:
> >
> > $ perf record --call-graph fp-deferred -a sleep 1
> >
> > Which would need a man page update, etc. What is happening with the
> > other call-graph modes and deferral? Could the option be something
> > like `--call-graph fp,deferred` so that the option is a common one and
> > say stack snapshots for dwarf be somehow improved?
>
> Also, making deferral the norm will generate new perf events that
> tools, other than perf, processing perf.data files will fail to
> consume. So this change would break quite a lot of stuff, so it should
> not just be made the default.
Thanks a lot for your input! Yeah I agree it'd be better to make it
optional. Having separate `--call-graph fp,defer` sounds good. I can
add a config option to control deferred callchains as well.
Thanks,
Namhyung
^ permalink raw reply
* Re: [PATCH v3 2/5] perf tools: Minimal DEFERRED_CALLCHAIN support
From: Namhyung Kim @ 2025-11-14 19:07 UTC (permalink / raw)
To: Ian Rogers
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users,
Steven Rostedt, Josh Poimboeuf, Indu Bhagat, Jens Remus,
Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <CAP-5=fVrpBjsJ7=BZQmhXKcaN+OYTY5_gOVj-Qs+33cH0gft7Q@mail.gmail.com>
On Fri, Nov 14, 2025 at 09:52:41AM -0800, Ian Rogers wrote:
> On Thu, Nov 13, 2025 at 11:00 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Add a new event type for deferred callchains and a new callback for the
> > struct perf_tool. For now it doesn't actually handle the deferred
> > callchains but it just marks the sample if it has the PERF_CONTEXT_
> > USER_DEFFERED in the callchain array.
> >
> > At least, perf report can dump the raw data with this change. Actually
> > this requires the next commit to enable attr.defer_callchain, but if you
> > already have a data file, it'll show the following result.
> >
> > $ perf report -D
> > ...
> > 0x2158@perf.data [0x40]: event: 22
> > .
> > . ... raw event: size 64 bytes
> > . 0000: 16 00 00 00 02 00 40 00 06 00 00 00 0b 00 00 00 ......@.........
> > . 0010: 03 00 00 00 00 00 00 00 a7 7f 33 fe 18 7f 00 00 ..........3.....
> > . 0020: 0f 0e 33 fe 18 7f 00 00 48 14 33 fe 18 7f 00 00 ..3.....H.3.....
> > . 0030: 08 09 00 00 08 09 00 00 e6 7a e7 35 1c 00 00 00 .........z.5....
> >
> > 121163447014 0x2158 [0x40]: PERF_RECORD_CALLCHAIN_DEFERRED(IP, 0x2): 2312/2312: 0xb00000006
> > ... FP chain: nr:3
> > ..... 0: 00007f18fe337fa7
> > ..... 1: 00007f18fe330e0f
> > ..... 2: 00007f18fe331448
> > : unhandled!
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/lib/perf/include/perf/event.h | 8 ++++++++
> > tools/perf/util/event.c | 1 +
> > tools/perf/util/evsel.c | 19 +++++++++++++++++++
> > tools/perf/util/machine.c | 1 +
> > tools/perf/util/perf_event_attr_fprintf.c | 2 ++
> > tools/perf/util/sample.h | 2 ++
> > tools/perf/util/session.c | 20 ++++++++++++++++++++
> > tools/perf/util/tool.c | 1 +
> > tools/perf/util/tool.h | 3 ++-
> > 9 files changed, 56 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h
> > index aa1e91c97a226e1a..769bc48ca85c0eb8 100644
> > --- a/tools/lib/perf/include/perf/event.h
> > +++ b/tools/lib/perf/include/perf/event.h
> > @@ -151,6 +151,13 @@ struct perf_record_switch {
> > __u32 next_prev_tid;
> > };
> >
> > +struct perf_record_callchain_deferred {
> > + struct perf_event_header header;
> > + __u64 cookie;
>
> Could we add a comment that this value is used to match user and
> kernel stack traces together? I don't believe that intent is
> immediately obvious from the word "cookie".
Sounds good, will add.
>
> > + __u64 nr;
> > + __u64 ips[];
> > +};
> > +
> > struct perf_record_header_attr {
> > struct perf_event_header header;
> > struct perf_event_attr attr;
> > @@ -523,6 +530,7 @@ union perf_event {
> > struct perf_record_read read;
> > struct perf_record_throttle throttle;
> > struct perf_record_sample sample;
> > + struct perf_record_callchain_deferred callchain_deferred;
> > struct perf_record_bpf_event bpf;
> > struct perf_record_ksymbol ksymbol;
> > struct perf_record_text_poke_event text_poke;
> > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> > index fcf44149feb20c35..4c92cc1a952c1d9f 100644
> > --- a/tools/perf/util/event.c
> > +++ b/tools/perf/util/event.c
> > @@ -61,6 +61,7 @@ static const char *perf_event__names[] = {
> > [PERF_RECORD_CGROUP] = "CGROUP",
> > [PERF_RECORD_TEXT_POKE] = "TEXT_POKE",
> > [PERF_RECORD_AUX_OUTPUT_HW_ID] = "AUX_OUTPUT_HW_ID",
> > + [PERF_RECORD_CALLCHAIN_DEFERRED] = "CALLCHAIN_DEFERRED",
> > [PERF_RECORD_HEADER_ATTR] = "ATTR",
> > [PERF_RECORD_HEADER_EVENT_TYPE] = "EVENT_TYPE",
> > [PERF_RECORD_HEADER_TRACING_DATA] = "TRACING_DATA",
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index 989c56d4a23f74f4..244b3e44d090d413 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -3089,6 +3089,20 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
> > data->data_src = PERF_MEM_DATA_SRC_NONE;
> > data->vcpu = -1;
> >
> > + if (event->header.type == PERF_RECORD_CALLCHAIN_DEFERRED) {
> > + const u64 max_callchain_nr = UINT64_MAX / sizeof(u64);
> > +
> > + data->callchain = (struct ip_callchain *)&event->callchain_deferred.nr;
> > + if (data->callchain->nr > max_callchain_nr)
> > + return -EFAULT;
> > +
> > + data->deferred_cookie = event->callchain_deferred.cookie;
> > +
> > + if (evsel->core.attr.sample_id_all)
> > + perf_evsel__parse_id_sample(evsel, event, data);
> > + return 0;
> > + }
> > +
> > if (event->header.type != PERF_RECORD_SAMPLE) {
> > if (!evsel->core.attr.sample_id_all)
> > return 0;
> > @@ -3219,6 +3233,11 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
> > if (data->callchain->nr > max_callchain_nr)
> > return -EFAULT;
> > sz = data->callchain->nr * sizeof(u64);
> > + if (evsel->core.attr.defer_callchain && data->callchain->nr >= 2 &&
> > + data->callchain->ips[data->callchain->nr - 2] == PERF_CONTEXT_USER_DEFERRED) {
> > + data->deferred_cookie = data->callchain->ips[data->callchain->nr - 1];
> > + data->deferred_callchain = true;
> > + }
>
> It'd be nice to have a comment saying what is going on here. I can see
> that if there are 2 stack slots and the 2nd is a magic value then the
> first should be read as the "cookie". At a first look this code is
> difficult to parse so a comment would add value.
Will add the comment.
Thanks,
Namhyung
> > OVERFLOW_CHECK(array, sz, max_size);
> > array = (void *)array + sz;
> > }
^ permalink raw reply
* Re: [PATCH RFC bpf-next 7/7] bpf: implement "jmp" mode for trampoline
From: Alexei Starovoitov @ 2025-11-14 18:50 UTC (permalink / raw)
To: Menglong Dong
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, bpf,
LKML, linux-trace-kernel
In-Reply-To: <20251114092450.172024-8-dongml2@chinatelecom.cn>
On Fri, Nov 14, 2025 at 1:25 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> Implement the "jmp" mode for the bpf trampoline. For the ftrace_managed
> case, we need only to set the FTRACE_OPS_FL_JMP on the tr->fops if "jmp"
> is needed.
>
> For the bpf poke case, the new flag BPF_TRAMP_F_JMPED is introduced to
> store and check if the trampoline is in the "jmp" mode.
>
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> ---
> include/linux/bpf.h | 6 +++++
> kernel/bpf/trampoline.c | 53 ++++++++++++++++++++++++++++++++++-------
> 2 files changed, 50 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index aec7c65539f5..3598785ac8d1 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1201,6 +1201,12 @@ struct btf_func_model {
> */
> #define BPF_TRAMP_F_INDIRECT BIT(8)
>
> +/*
> + * Indicate that the trampoline is using "jmp" instead of "call". This flag
> + * is only used in the !ftrace_managed case.
> + */
> +#define BPF_TRAMP_F_JMPED BIT(9)
> +
> /* Each call __bpf_prog_enter + call bpf_func + call __bpf_prog_exit is ~50
> * bytes on x86.
> */
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 5949095e51c3..02a9f33d8f6c 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -175,15 +175,37 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key)
> return tr;
> }
>
> -static int unregister_fentry(struct bpf_trampoline *tr, void *old_addr)
> +static int bpf_text_poke(struct bpf_trampoline *tr, void *old_addr,
> + void *new_addr)
The bpf_text_poke is a generic name. It really doesn't fit here.
Use bpf_trampoline_update_fentry() or something along those lines.
> {
> + enum bpf_text_poke_type new_t = BPF_MOD_CALL, old_t = BPF_MOD_CALL;
> void *ip = tr->func.addr;
> int ret;
>
> + if (bpf_trampoline_need_jmp(tr->flags))
> + new_t = BPF_MOD_JUMP;
> + if (tr->flags & BPF_TRAMP_F_JMPED)
> + old_t = BPF_MOD_JUMP;
Now I see why you picked _need_jmp().. to alternate with F_JMPED ?
_uses_jmp() suggestions isn't quite right.
How about bpf_trampoline_must_jmp() ?
and drop if (!ret) fallback and BPF_TRAMP_F_JMPED bit.
It doesn't look to be necessary.
^ permalink raw reply
* Re: [PATCH v3 3/5] perf record: Enable defer_callchain for user callchains
From: Ian Rogers @ 2025-11-14 18:49 UTC (permalink / raw)
To: Steven Rostedt
Cc: Namhyung Kim, Arnaldo Carvalho de Melo, James Clark, Jiri Olsa,
Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Josh Poimboeuf, Indu Bhagat, Jens Remus,
Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251114133009.7dd97625@gandalf.local.home>
On Fri, Nov 14, 2025 at 10:29 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 14 Nov 2025 10:09:26 -0800
> Ian Rogers <irogers@google.com> wrote:
>
> > Just to be clear. I don't think the behavior of using frame pointers
> > should change. Deferral has downsides, for example:
> >
> > $ perf record -g -a sleep 1
>
> The biggest advantage of the deferred callstack is that there's much less
> duplication of data in the ring buffer. Especially when you have deep
> stacks and long system calls.
I've never had anybody raise this as a concern with fp stack traces,
especially given the stack snapshot approach being far more space
consuming - but okay.
> Now, if we have frame pointers enabled, we could possibly add a feature to
> the deferred unwinder where it could try to do the deferred immediately and
> if it faults it then waits until going back to user space. This means that
> the frame pointer version should work (unless the user space stack was
> swapped out).
>
> >
> > Without deferral kernel stack traces will contain both kernel and user
> > traces. With deferral the user stack trace is only generated when the
> > system call returns and so there is a chance for kernel stack traces
> > to be missing their user part. An obvious behavioral change. I think
> > for what you are doing here we can have an option something like:
> >
> > $ perf record --call-graph fp-deferred -a sleep 1
>
> I would be OK with this but I would prefer a much shorter name. Adding 20
> characters to the command line will likely keep people from using it.
Fwiw, with buildid-mmap we just (v6.18) flipped the default when the
kernel has the feature to use it. The kernel feature was added in
v5.12.
https://lore.kernel.org/r/20250724163302.596743-9-irogers@google.com
I don't oppose a shorter name, callchain option, .. Unfortunately with
`perf record` -d is taken for saying record data mmaps, -D is taken
for a start-up delay option, and -G is a cgroup option. Perhaps '-f'
for "frame" and have it mirror '-g' except that deferred is default
true rather than false.
Thanks,
Ian
> -- Steve
^ permalink raw reply
* Re: [PATCH v7 00/31] context_tracking,x86: Defer some IPIs until a user->kernel transition
From: Andy Lutomirski @ 2025-11-14 18:45 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Valentin Schneider, Linux Kernel Mailing List, linux-mm, rcu,
the arch/x86 maintainers, linux-arm-kernel, loongarch,
linux-riscv, linux-arch, linux-trace-kernel, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin,
Peter Zijlstra (Intel), Arnaldo Carvalho de Melo, Josh Poimboeuf,
Paolo Bonzini, Arnd Bergmann, Frederic Weisbecker, Jason Baron,
Steven Rostedt, Ard Biesheuvel, Sami Tolvanen, David S. Miller,
Neeraj Upadhyay, Joel Fernandes, Josh Triplett, Boqun Feng,
Uladzislau Rezki, Mathieu Desnoyers, Mel Gorman, Andrew Morton,
Masahiro Yamada, Han Shen, Rik van Riel, Jann Horn, Dan Carpenter,
Oleg Nesterov, Juri Lelli, Clark Williams, Yair Podemsky,
Marcelo Tosatti, Daniel Wagner, Petr Tesarik, Shrikanth Hegde
In-Reply-To: <89398bdf-4dad-4976-8eb9-1e86032c8794@paulmck-laptop>
On Fri, Nov 14, 2025, at 10:14 AM, Paul E. McKenney wrote:
> On Fri, Nov 14, 2025 at 09:22:35AM -0800, Andy Lutomirski wrote:
>>
>> > Oh, any another primitive would be possible: one CPU could plausibly
>> > execute another CPU's interrupts or soft-irqs or whatever by taking a
>> > special lock that would effectively pin the remote CPU in user mode --
>> > you'd set a flag in the target cpu_flags saying "pin in USER mode" and
>> > the transition on that CPU to kernel mode would then spin on entry to
>> > kernel mode and wait for the lock to be released. This could plausibly
>> > get a lot of the on_each_cpu callers to switch over in one fell swoop:
>> > anything that needs to synchronize to the remote CPU but does not need
>> > to poke its actual architectural state could be executed locally while
>> > the remote CPU is pinned.
>
> It would be necessary to arrange for the remote CPU to remain pinned
> while the local CPU executed on its behalf. Does the above approach
> make that happen without re-introducing our current context-tracking
> overhead and complexity?
Using the pseudo-implementation farther down, I think this would be like:
if (my_state->status == INDETERMINATE) {
// we were lazy and we never promised to do work atomically.
atomic_set(&my_state->status, KERNEL);
this_entry_work = 0;
/* we are definitely not pinned in this path */
} else {
// we were not lazy and we promised we would do work atomically
atomic exchange the entire state to { .work = 0, .status = KERNEL }
this_entry_work = (whatever we just read);
if (this_entry_work & PINNED) {
u32 this_cpu_pin_count = this_cpu_ptr(pin_count);
while (atomic_read(&this_cpu_pin_count)) {
cpu_relax();
}
}
}
and we'd have something like:
bool try_pin_remote_cpu(int cpu)
{
u32 *remote_pin_count = ...;
struct fancy_cpu_state *remote_state = ...;
atomic_inc(remote_pin_count); // optimistic
// Hmm, we do not want that read to get reordered with the inc, so we probably
// need a full barrier or seq_cst. How does Linux spell that? C++ has atomic::load
// with seq_cst and maybe the optimizer can do the right thing. Maybe it's:
smp_mb__after_atomic();
if (atomic_read(&remote_state->status) == USER) {
// Okay, it's genuinely pinned.
return true;
// egads, if this is some arch with very weak ordering,
// do we need to be concerned that we just took a lock but we
// just did a relaxed read and therefore a subsequent access
// that thinks it's locked might appear to precede the load and therefore
// somehow get surprisingly seen out of order by the target cpu?
// maybe we wanted atomic_read_acquire above instead?
} else {
// We might not have successfully pinned it
atomic_dec(remote_pin_count);
}
}
void unpin_remote_cpu(int cpu)
{
atomic_dec(remote_pin_count();
}
and we'd use it like:
if (try_pin_remote_cpu(cpu)) {
// do something useful
} else {
send IPI;
}
but we'd really accumulate the set of CPUs that need the IPIs and do them all at once.
I ran the theorem prover that lives inside my head on this code using the assumption that the machine is a well-behaved x86 system and it said "yeah, looks like it might be correct". I trust an actual formalized system or someone like you who is genuinely very good at this stuff much more than I trust my initial impression :)
>
>> Following up, I think that x86 can do this all with a single atomic (in the common case) per usermode round trip. Imagine:
>>
>> struct fancy_cpu_state {
>> u32 work; // <-- writable by any CPU
>> u32 status; // <-- readable anywhere; writable locally
>> };
>>
>> status includes KERNEL, USER, and maybe INDETERMINATE. (INDETERMINATE means USER but we're not committing to doing work.)
>>
>> Exit to user mode:
>>
>> atomic_set(&my_state->status, USER);
>
> We need ordering in the RCU nohz_full case. If the grace-period kthread
> sees the status as USER, all the preceding KERNEL code's effects must
> be visible to the grace-period kthread.
Sorry, I'm speaking lazy x86 programmer here. Maybe I mean atomic_set_release. I want, roughly, the property that anyone who remotely observes USER can rely on the target cpu subsequently going through the atomic exchange path above. I think even relaxed ought to be good enough for that one most architectures, but there are some potentially nasty complications involving that fact that this mixes operations on a double word and a single word that's part of the double word.
>
>> (or, in the lazy case, set to INDETERMINATE instead.)
>>
>> Entry from user mode, with IRQs off, before switching to kernel CR3:
>>
>> if (my_state->status == INDETERMINATE) {
>> // we were lazy and we never promised to do work atomically.
>> atomic_set(&my_state->status, KERNEL);
>> this_entry_work = 0;
>> } else {
>> // we were not lazy and we promised we would do work atomically
>> atomic exchange the entire state to { .work = 0, .status = KERNEL }
>> this_entry_work = (whatever we just read);
>> }
>
> If this atomic exchange is fully ordered (as opposed to, say, _relaxed),
> then this works in that if the grace-period kthread sees USER, its prior
> references are guaranteed not to see later kernel-mode references from
> that CPU.
Yep, that's the intent of my pseudocode. On x86 this would be a plain 64-bit lock xchg -- I don't think cmpxchg is needed. (I like to write lock even when it's implicit to avoid needing to trust myself to remember precisely which instructions imply it.)
>
>> if (PTI) {
>> switch to kernel CR3 *and flush if this_entry_work says to flush*
>> } else {
>> flush if this_entry_work says to flush;
>> }
>>
>> do the rest of the work;
>>
>>
>>
>> I suppose that a lot of the stuff in ti_flags could merge into here, but it could be done one bit at a time when people feel like doing so. And I imagine, but I'm very far from confident, that RCU could use this instead of the current context tracking code.
>
> RCU currently needs pretty heavy-duty ordering to reliably detect the
> other CPUs' quiescent states without needing to wake them from idle, or,
> in the nohz_full case, interrupt their userspace execution. Not saying
> it is impossible, but it will need extreme care.
Is the thingy above heavy-duty enough? Perhaps more relevantly, could RCU do this *instead* of the current CT hooks on architectures that implement it, and/or could RCU arrange for the ct hooks to be cheap no-ops on architectures that support the thingy above. By "could" I mean "could be done without absolutely massive refactoring and without the resulting code being an unmaintainable disaster? I'm sure any sufficiently motivated human or LLM could pull off the unmaintainable disaster version. I bet I could even do it myself!)
>
>> The idea behind INDETERMINATE is that there are plenty of workloads that frequently switch between user and kernel mode and that would rather accept a few IPIs to avoid the heavyweight atomic operation on user -> kernel transitions. So the default behavior could be to do KERNEL -> INDETERMINATE instead of KERNEL -> USER, but code that wants to be in user mode for a long time could go all the way to USER. We could make it sort of automatic by noticing that we're returning from an IRQ without a context switch and go to USER (so we would get at most one unneeded IPI per normal user entry), and we could have some nice API for a program that intends to hang out in user mode for a very long time (cpu isolation users, for example) to tell the kernel to go immediately into USER mode. (Don't we already have something that could be used for this purpose?)
>
> RCU *could* do an smp_call_function_single() when the CPU failed
> to respond, perhaps in a manner similar to how it already forces a
> given CPU out of nohz_full state if that CPU has been executing in the
> kernel for too long. The real-time guys might not be amused, though.
> Especially those real-time guys hitting sub-microsecond latencies.
It wouldn't be outrageous to have real-time imply the full USER transition.
>
>> Hmm, now I wonder if it would make sense for the default behavior of Linux to be like that. We could call it ONEHZ. It's like NOHZ_FULL except that user threads that don't do syscalls get one single timer tick instead of many or none.
>>
>>
>> Anyway, I think my proposal is pretty good *if* RCU could be made to use it -- the existing context tracking code is fairly expensive, and I don't think we want to invent a new context-tracking-like mechanism if we still need to do the existing thing.
>
> If you build with CONFIG_NO_HZ_FULL=n, do you still get the heavyweight
> operations when transitioning between kernel and user execution?
No. And I even tested approximately this a couple weeks ago for unrelated reasons. The only unnecessary heavy-weight thing we're doing in a syscall loop with mitigations off is the RDTSC for stack randomization.
But I did that test on a machine that would absolutely benefit from the IPI suppression that the OP is talking about here, and I think it would be really quite nice if a default distro kernel with a more or less default distribution could be easily convinced to run a user thread without interrupting it. It's a little bit had that NO_HZ_FULL is still an exotic non-default thing, IMO.
--Andy
^ permalink raw reply
* Re: [PATCH RFC bpf-next 5/7] bpf: introduce bpf_arch_text_poke_type
From: Alexei Starovoitov @ 2025-11-14 18:41 UTC (permalink / raw)
To: Menglong Dong
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, bpf,
LKML, linux-trace-kernel
In-Reply-To: <20251114092450.172024-6-dongml2@chinatelecom.cn>
On Fri, Nov 14, 2025 at 1:25 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> Introduce the function bpf_arch_text_poke_type(), which is able to specify
> both the current and new opcode. If it is not implemented by the arch,
> bpf_arch_text_poke() will be called directly if the current opcode is the
> same as the new one. Otherwise, -EOPNOTSUPP will be returned.
>
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> ---
> include/linux/bpf.h | 4 ++++
> kernel/bpf/core.c | 10 ++++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index d65a71042aa3..aec7c65539f5 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -3711,6 +3711,10 @@ enum bpf_text_poke_type {
> BPF_MOD_JUMP,
> };
>
> +int bpf_arch_text_poke_type(void *ip, enum bpf_text_poke_type old_t,
> + enum bpf_text_poke_type new_t, void *addr1,
> + void *addr2);
> +
Instead of adding a new helper, I think, it's cleaner to change
the existing bpf_arch_text_poke() across all archs in one patch,
and also do:
enum bpf_text_poke_type {
+ BPF_MOD_NOP,
BPF_MOD_CALL,
BPF_MOD_JUMP,
};
and use that instead of addr[12] = !NULL to indicate
the transition.
The callsites will be easier to read when they will look like:
bpf_arch_text_poke(ip, BPF_MOD_CALL, BPF_MOD_CALL, old_addr, new_addr);
bpf_arch_text_poke(ip, BPF_MOD_NOP, BPF_MOD_CALL, NULL, new_addr);
bpf_arch_text_poke(ip, BPF_MOD_JMP, BPF_MOD_CALL, old_addr, new_addr);
^ permalink raw reply
* Re: [PATCH v3 5/5] perf tools: Merge deferred user callchains
From: Ian Rogers @ 2025-11-14 18:36 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users,
Steven Rostedt, Josh Poimboeuf, Indu Bhagat, Jens Remus,
Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251114070018.160330-6-namhyung@kernel.org>
On Thu, Nov 13, 2025 at 11:00 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Save samples with deferred callchains in a separate list and deliver
> them after merging the user callchains. If users don't want to merge
> they can set tool->merge_deferred_callchains to false to prevent the
> behavior.
>
> With previous result, now perf script will show the merged callchains.
>
> $ perf script
> ...
> pwd 2312 121.163435: 249113 cpu/cycles/P:
> ffffffff845b78d8 __build_id_parse.isra.0+0x218 ([kernel.kallsyms])
> ffffffff83bb5bf6 perf_event_mmap+0x2e6 ([kernel.kallsyms])
> ffffffff83c31959 mprotect_fixup+0x1e9 ([kernel.kallsyms])
> ffffffff83c31dc5 do_mprotect_pkey+0x2b5 ([kernel.kallsyms])
> ffffffff83c3206f __x64_sys_mprotect+0x1f ([kernel.kallsyms])
> ffffffff845e6692 do_syscall_64+0x62 ([kernel.kallsyms])
> ffffffff8360012f entry_SYSCALL_64_after_hwframe+0x76 ([kernel.kallsyms])
> 7f18fe337fa7 mprotect+0x7 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> 7f18fe330e0f _dl_sysdep_start+0x7f (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> 7f18fe331448 _dl_start_user+0x0 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> ...
>
> The old output can be get using --no-merge-callchain option.
> Also perf report can get the user callchain entry at the end.
>
> $ perf report --no-children --stdio -q -S __build_id_parse.isra.0
> # symbol: __build_id_parse.isra.0
> 8.40% pwd [kernel.kallsyms]
> |
> ---__build_id_parse.isra.0
> perf_event_mmap
> mprotect_fixup
> do_mprotect_pkey
> __x64_sys_mprotect
> do_syscall_64
> entry_SYSCALL_64_after_hwframe
> mprotect
> _dl_sysdep_start
> _dl_start_user
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/Documentation/perf-script.txt | 5 ++
> tools/perf/builtin-inject.c | 1 +
> tools/perf/builtin-report.c | 1 +
> tools/perf/builtin-script.c | 5 +-
> tools/perf/util/callchain.c | 29 ++++++++++
> tools/perf/util/callchain.h | 3 ++
> tools/perf/util/evlist.c | 1 +
> tools/perf/util/evlist.h | 2 +
> tools/perf/util/session.c | 67 +++++++++++++++++++++++-
> tools/perf/util/tool.c | 1 +
> tools/perf/util/tool.h | 1 +
> 11 files changed, 114 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> index 28bec7e78bc858ba..03d1129606328d6d 100644
> --- a/tools/perf/Documentation/perf-script.txt
> +++ b/tools/perf/Documentation/perf-script.txt
> @@ -527,6 +527,11 @@ include::itrace.txt[]
> The known limitations include exception handing such as
> setjmp/longjmp will have calls/returns not match.
>
> +--merge-callchains::
> + Enable merging deferred user callchains if available. This is the
> + default behavior. If you want to see separate CALLCHAIN_DEFERRED
> + records for some reason, use --no-merge-callchains explicitly.
> +
> :GMEXAMPLECMD: script
> :GMEXAMPLESUBCMD:
> include::guest-files.txt[]
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 044074080aa53abd..30ae38212f57580a 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -2542,6 +2542,7 @@ int cmd_inject(int argc, const char **argv)
> inject.tool.auxtrace = perf_event__repipe_auxtrace;
> inject.tool.bpf_metadata = perf_event__repipe_op2_synth;
> inject.tool.dont_split_sample_group = true;
> + inject.tool.merge_deferred_callchains = false;
> inject.session = __perf_session__new(&data, &inject.tool,
> /*trace_event_repipe=*/inject.output.is_pipe,
> /*host_env=*/NULL);
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 2bc269f5fcef8023..add6b1c2aaf04270 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -1614,6 +1614,7 @@ int cmd_report(int argc, const char **argv)
> report.tool.event_update = perf_event__process_event_update;
> report.tool.feature = process_feature_event;
> report.tool.ordering_requires_timestamps = true;
> + report.tool.merge_deferred_callchains = !dump_trace;
>
> session = perf_session__new(&data, &report.tool);
> if (IS_ERR(session)) {
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 3b2896350bad2924..2374c7a1684028cc 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -4025,6 +4025,7 @@ int cmd_script(int argc, const char **argv)
> bool header_only = false;
> bool script_started = false;
> bool unsorted_dump = false;
> + bool merge_deferred_callchains = true;
> char *rec_script_path = NULL;
> char *rep_script_path = NULL;
> struct perf_session *session;
> @@ -4178,6 +4179,8 @@ int cmd_script(int argc, const char **argv)
> "Guest code can be found in hypervisor process"),
> OPT_BOOLEAN('\0', "stitch-lbr", &script.stitch_lbr,
> "Enable LBR callgraph stitching approach"),
> + OPT_BOOLEAN('\0', "merge-callchains", &merge_deferred_callchains,
> + "Enable merge deferred user callchains"),
> OPTS_EVSWITCH(&script.evswitch),
> OPT_END()
> };
> @@ -4434,7 +4437,7 @@ int cmd_script(int argc, const char **argv)
> script.tool.throttle = process_throttle_event;
> script.tool.unthrottle = process_throttle_event;
> script.tool.ordering_requires_timestamps = true;
> - script.tool.merge_deferred_callchains = false;
> + script.tool.merge_deferred_callchains = merge_deferred_callchains;
> session = perf_session__new(&data, &script.tool);
> if (IS_ERR(session))
> return PTR_ERR(session);
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index d7b7eef740b9d6ed..a0a0e6784420d478 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -1828,3 +1828,32 @@ int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
> }
> return 0;
> }
> +
> +int sample__merge_deferred_callchain(struct perf_sample *sample_orig,
> + struct perf_sample *sample_callchain)
Would sample_kernel and sample_user be clearer names here? Perhaps
sample_deferred_kernel.
> +{
> + u64 nr_orig = sample_orig->callchain->nr - 1;
> + u64 nr_deferred = sample_callchain->callchain->nr;
> + struct ip_callchain *callchain;
> +
> + if (sample_orig->callchain->nr < 2) {
> + sample_orig->deferred_callchain = false;
> + return -EINVAL;
> + }
> +
> + callchain = calloc(1 + nr_orig + nr_deferred, sizeof(u64));
> + if (callchain == NULL) {
> + sample_orig->deferred_callchain = false;
> + return -ENOMEM;
> + }
> +
> + callchain->nr = nr_orig + nr_deferred;
> + /* copy original including PERF_CONTEXT_USER_DEFERRED (but the cookie) */
I don't follow "but the cookie", do you mean "but not the cookie" ?
> + memcpy(callchain->ips, sample_orig->callchain->ips, nr_orig * sizeof(u64));
> + /* copy deferred user callchains */
> + memcpy(&callchain->ips[nr_orig], sample_callchain->callchain->ips,
> + nr_deferred * sizeof(u64));
> +
assert(sample_orig->callchain == NULL);
> + sample_orig->callchain = callchain;
> + return 0;
> +}
> diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
> index 86ed9e4d04f9ee7b..89785125ed25783d 100644
> --- a/tools/perf/util/callchain.h
> +++ b/tools/perf/util/callchain.h
> @@ -317,4 +317,7 @@ int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
> struct perf_sample *sample, int max_stack,
> bool symbols, callchain_iter_fn cb, void *data);
>
> +int sample__merge_deferred_callchain(struct perf_sample *sample_orig,
> + struct perf_sample *sample_callchain);
> +
> #endif /* __PERF_CALLCHAIN_H */
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index e8217efdda5323c6..03674d2cbd015e4f 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -85,6 +85,7 @@ void evlist__init(struct evlist *evlist, struct perf_cpu_map *cpus,
> evlist->ctl_fd.pos = -1;
> evlist->nr_br_cntr = -1;
> metricgroup__rblist_init(&evlist->metric_events);
> + INIT_LIST_HEAD(&evlist->deferred_samples);
> }
>
> struct evlist *evlist__new(void)
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 5e71e3dc60423079..911834ae7c2a6f76 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -92,6 +92,8 @@ struct evlist {
> * of struct metric_expr.
> */
> struct rblist metric_events;
> + /* samples with deferred_callchain would wait here. */
> + struct list_head deferred_samples;
Please document which struct is on the list.
> };
>
> struct evsel_str_handler {
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 361e15c1f26a96d0..2e777fd1bcf6707b 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1285,6 +1285,60 @@ static int evlist__deliver_sample(struct evlist *evlist, const struct perf_tool
> per_thread);
> }
>
> +struct deferred_event {
> + struct list_head list;
> + union perf_event *event;
> +};
Given the list is in evlist.h, could we put this at the top of the
file so that you don't need to search down for it. Comments on where
the list is held and life time of the event would be useful.
> +
> +static int evlist__deliver_deferred_samples(struct evlist *evlist,
> + const struct perf_tool *tool,
> + union perf_event *event,
> + struct perf_sample *sample,
> + struct machine *machine)
> +{
> + struct deferred_event *de, *tmp;
> + struct evsel *evsel;
> + int ret = 0;
> +
> + if (!tool->merge_deferred_callchains) {
> + evsel = evlist__id2evsel(evlist, sample->id);
> + return tool->callchain_deferred(tool, event, sample,
> + evsel, machine);
> + }
> +
> + list_for_each_entry_safe(de, tmp, &evlist->deferred_samples, list) {
> + struct perf_sample orig_sample;
> +
> + ret = evlist__parse_sample(evlist, de->event, &orig_sample);
> + if (ret < 0) {
> + pr_err("failed to parse original sample\n");
> + break;
> + }
> +
> + if (sample->tid != orig_sample.tid)
> + continue;
> +
> + if (event->callchain_deferred.cookie == orig_sample.deferred_cookie)
> + sample__merge_deferred_callchain(&orig_sample, sample);
> + else
> + orig_sample.deferred_callchain = false;
> +
> + evsel = evlist__id2evsel(evlist, orig_sample.id);
> + ret = evlist__deliver_sample(evlist, tool, de->event,
> + &orig_sample, evsel, machine);
> +
> + if (orig_sample.deferred_callchain)
> + free(orig_sample.callchain);
> +
> + list_del(&de->list);
> + free(de);
> +
> + if (ret)
> + break;
> + }
> + return ret;
> +}
> +
> static int machines__deliver_event(struct machines *machines,
> struct evlist *evlist,
> union perf_event *event,
> @@ -1313,6 +1367,16 @@ static int machines__deliver_event(struct machines *machines,
> return 0;
> }
> dump_sample(evsel, event, sample, perf_env__arch(machine->env));
> + if (sample->deferred_callchain && tool->merge_deferred_callchains) {
> + struct deferred_event *de = malloc(sizeof(*de));
> +
> + if (de == NULL)
> + return -ENOMEM;
> +
> + de->event = event;
I'm not sure how this is safe. Don't you need to copy the event at
this point? An event may be in a ring buffer, file, or some memory.
Generally the event can only be used in the context of the tool
callback and not for longer. Similarly with the perf_sample as that
has pointers into the event.
Thanks,
Ian
> + list_add_tail(&de->list, &evlist->deferred_samples);
> + return 0;
> + }
> return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine);
> case PERF_RECORD_MMAP:
> return tool->mmap(tool, event, sample, machine);
> @@ -1372,7 +1436,8 @@ static int machines__deliver_event(struct machines *machines,
> return tool->aux_output_hw_id(tool, event, sample, machine);
> case PERF_RECORD_CALLCHAIN_DEFERRED:
> dump_deferred_callchain(evsel, event, sample);
> - return tool->callchain_deferred(tool, event, sample, evsel, machine);
> + return evlist__deliver_deferred_samples(evlist, tool, event,
> + sample, machine);
> default:
> ++evlist->stats.nr_unknown_events;
> return -1;
> diff --git a/tools/perf/util/tool.c b/tools/perf/util/tool.c
> index f732d33e7f895ed4..c5d3b464b2a433b3 100644
> --- a/tools/perf/util/tool.c
> +++ b/tools/perf/util/tool.c
> @@ -266,6 +266,7 @@ void perf_tool__init(struct perf_tool *tool, bool ordered_events)
> tool->cgroup_events = false;
> tool->no_warn = false;
> tool->show_feat_hdr = SHOW_FEAT_NO_HEADER;
> + tool->merge_deferred_callchains = true;
>
> tool->sample = process_event_sample_stub;
> tool->mmap = process_event_stub;
> diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
> index 9b9f0a8cbf3de4b5..e96b69d25a5b737d 100644
> --- a/tools/perf/util/tool.h
> +++ b/tools/perf/util/tool.h
> @@ -90,6 +90,7 @@ struct perf_tool {
> bool cgroup_events;
> bool no_warn;
> bool dont_split_sample_group;
> + bool merge_deferred_callchains;
> enum show_feature_header show_feat_hdr;
> };
>
> --
> 2.52.0.rc1.455.g30608eb744-goog
>
^ permalink raw reply
* Re: [PATCH v3 3/5] perf record: Enable defer_callchain for user callchains
From: Steven Rostedt @ 2025-11-14 18:30 UTC (permalink / raw)
To: Ian Rogers
Cc: Namhyung Kim, Arnaldo Carvalho de Melo, James Clark, Jiri Olsa,
Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Josh Poimboeuf, Indu Bhagat, Jens Remus,
Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <CAP-5=fU33sEARn0tc1hSMahBVCvs0cy+Cu-J6+BG0Cm-nuwKnA@mail.gmail.com>
On Fri, 14 Nov 2025 10:09:26 -0800
Ian Rogers <irogers@google.com> wrote:
> Just to be clear. I don't think the behavior of using frame pointers
> should change. Deferral has downsides, for example:
>
> $ perf record -g -a sleep 1
The biggest advantage of the deferred callstack is that there's much less
duplication of data in the ring buffer. Especially when you have deep
stacks and long system calls.
Now, if we have frame pointers enabled, we could possibly add a feature to
the deferred unwinder where it could try to do the deferred immediately and
if it faults it then waits until going back to user space. This means that
the frame pointer version should work (unless the user space stack was
swapped out).
>
> Without deferral kernel stack traces will contain both kernel and user
> traces. With deferral the user stack trace is only generated when the
> system call returns and so there is a chance for kernel stack traces
> to be missing their user part. An obvious behavioral change. I think
> for what you are doing here we can have an option something like:
>
> $ perf record --call-graph fp-deferred -a sleep 1
I would be OK with this but I would prefer a much shorter name. Adding 20
characters to the command line will likely keep people from using it.
-- Steve
^ permalink raw reply
* Re: [PATCH RFC bpf-next 3/7] bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME
From: Alexei Starovoitov @ 2025-11-14 18:23 UTC (permalink / raw)
To: Menglong Dong
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, bpf,
LKML, linux-trace-kernel
In-Reply-To: <20251114092450.172024-4-dongml2@chinatelecom.cn>
On Fri, Nov 14, 2025 at 1:25 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> Some places calculate the origin_call by checking if
> BPF_TRAMP_F_SKIP_FRAME is set. However, it should use
> BPF_TRAMP_F_ORIG_STACK for this propose. Just fix them.
>
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> ---
> arch/riscv/net/bpf_jit_comp64.c | 2 +-
> arch/x86/net/bpf_jit_comp.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 45cbc7c6fe49..21c70ae3296b 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -1131,7 +1131,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
> store_args(nr_arg_slots, args_off, ctx);
>
> /* skip to actual body of traced function */
> - if (flags & BPF_TRAMP_F_SKIP_FRAME)
> + if (flags & BPF_TRAMP_F_ORIG_STACK)
> orig_call += RV_FENTRY_NINSNS * 4;
>
> if (flags & BPF_TRAMP_F_CALL_ORIG) {
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index de5083cb1d37..2d300ab37cdd 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -3272,7 +3272,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im
>
> arg_stack_off = stack_size;
>
> - if (flags & BPF_TRAMP_F_SKIP_FRAME) {
> + if (flags & BPF_TRAMP_F_CALL_ORIG) {
Good catch. Ack. Pls carry it in respin, so I don't
forget that I looked at it.
^ permalink raw reply
* Re: [PATCH RFC bpf-next 4/7] bpf,x86: adjust the "jmp" mode for bpf trampoline
From: Alexei Starovoitov @ 2025-11-14 18:22 UTC (permalink / raw)
To: Menglong Dong
Cc: Alexei Starovoitov, Steven Rostedt, Daniel Borkmann,
John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo,
Jiri Olsa, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, bpf,
LKML, linux-trace-kernel
In-Reply-To: <20251114092450.172024-5-dongml2@chinatelecom.cn>
On Fri, Nov 14, 2025 at 1:25 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> In the origin call case, if BPF_TRAMP_F_SKIP_FRAME is not set, it means
> that the trampoline is not called, but "jmp".
>
> Introduce the function bpf_trampoline_need_jmp() to check if the
> trampoline is in "jmp" mode.
>
> Do some adjustment on the "jmp" mode for the x86_64. The main adjustment
> that we make is for the stack parameter passing case, as the stack
> alignment logic changes in the "jmp" mode without the "rip". What's more,
> the location of the parameters on the stack also changes.
>
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> ---
> arch/x86/net/bpf_jit_comp.c | 15 ++++++++++-----
> include/linux/bpf.h | 12 ++++++++++++
> 2 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 2d300ab37cdd..21ce2b8457ec 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2830,7 +2830,7 @@ static int get_nr_used_regs(const struct btf_func_model *m)
> }
>
> static void save_args(const struct btf_func_model *m, u8 **prog,
> - int stack_size, bool for_call_origin)
> + int stack_size, bool for_call_origin, bool jmp)
I have an allergy to bool args.
Please pass flags and do
boll jmp_based_tramp = bpf_trampoline_uses_jmp(flags);
I think bpf_trampoline_uses_jmp() is more descriptive than
bpf_trampoline_need_jmp().
The actual math lgtm.
^ permalink raw reply
* Re: [PATCH v3 4/5] perf script: Display PERF_RECORD_CALLCHAIN_DEFERRED
From: Ian Rogers @ 2025-11-14 18:18 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users,
Steven Rostedt, Josh Poimboeuf, Indu Bhagat, Jens Remus,
Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251114070018.160330-5-namhyung@kernel.org>
On Thu, Nov 13, 2025 at 11:02 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Handle the deferred callchains in the script output.
>
> $ perf script
> ...
> pwd 2312 121.163435: 249113 cpu/cycles/P:
> ffffffff845b78d8 __build_id_parse.isra.0+0x218 ([kernel.kallsyms])
> ffffffff83bb5bf6 perf_event_mmap+0x2e6 ([kernel.kallsyms])
> ffffffff83c31959 mprotect_fixup+0x1e9 ([kernel.kallsyms])
> ffffffff83c31dc5 do_mprotect_pkey+0x2b5 ([kernel.kallsyms])
> ffffffff83c3206f __x64_sys_mprotect+0x1f ([kernel.kallsyms])
> ffffffff845e6692 do_syscall_64+0x62 ([kernel.kallsyms])
> ffffffff8360012f entry_SYSCALL_64_after_hwframe+0x76 ([kernel.kallsyms])
> b00000006 [unknown] ([unknown])
Does this unknown value correspond to the cookie? Can the cookie and
deferred data be added to the kernel stack trace output?
> pwd 2312 121.163447: DEFERRED CALLCHAIN
> 7f18fe337fa7 mprotect+0x7 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> 7f18fe330e0f _dl_sysdep_start+0x7f (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> 7f18fe331448 _dl_start_user+0x0 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
Can we display the cookie here so that the user callchain can be
matched with the kernel part?
Thanks,
Ian
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-script.c | 89 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 89 insertions(+)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index cf0040bbaba9cbc9..3b2896350bad2924 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -2719,6 +2719,93 @@ static int process_sample_event(const struct perf_tool *tool,
> return ret;
> }
>
> +static int process_deferred_sample_event(const struct perf_tool *tool,
> + union perf_event *event,
> + struct perf_sample *sample,
> + struct evsel *evsel,
> + struct machine *machine)
> +{
> + struct perf_script *scr = container_of(tool, struct perf_script, tool);
> + struct perf_event_attr *attr = &evsel->core.attr;
> + struct evsel_script *es = evsel->priv;
> + unsigned int type = output_type(attr->type);
> + struct addr_location al;
> + FILE *fp = es->fp;
> + int ret = 0;
> +
> + if (output[type].fields == 0)
> + return 0;
> +
> + /* Set thread to NULL to indicate addr_al and al are not initialized */
> + addr_location__init(&al);
> +
> + if (perf_time__ranges_skip_sample(scr->ptime_range, scr->range_num,
> + sample->time)) {
> + goto out_put;
> + }
> +
> + if (debug_mode) {
> + if (sample->time < last_timestamp) {
> + pr_err("Samples misordered, previous: %" PRIu64
> + " this: %" PRIu64 "\n", last_timestamp,
> + sample->time);
> + nr_unordered++;
> + }
> + last_timestamp = sample->time;
> + goto out_put;
> + }
> +
> + if (filter_cpu(sample))
> + goto out_put;
> +
> + if (machine__resolve(machine, &al, sample) < 0) {
> + pr_err("problem processing %d event, skipping it.\n",
> + event->header.type);
> + ret = -1;
> + goto out_put;
> + }
> +
> + if (al.filtered)
> + goto out_put;
> +
> + if (!show_event(sample, evsel, al.thread, &al, NULL))
> + goto out_put;
> +
> + if (evswitch__discard(&scr->evswitch, evsel))
> + goto out_put;
> +
> + perf_sample__fprintf_start(scr, sample, al.thread, evsel,
> + PERF_RECORD_CALLCHAIN_DEFERRED, fp);
> + fprintf(fp, "DEFERRED CALLCHAIN");
> +
> + if (PRINT_FIELD(IP)) {
> + struct callchain_cursor *cursor = NULL;
> +
> + if (symbol_conf.use_callchain && sample->callchain) {
> + cursor = get_tls_callchain_cursor();
> + if (thread__resolve_callchain(al.thread, cursor, evsel,
> + sample, NULL, NULL,
> + scripting_max_stack)) {
> + pr_info("cannot resolve deferred callchains\n");
> + cursor = NULL;
> + }
> + }
> +
> + fputc(cursor ? '\n' : ' ', fp);
> + sample__fprintf_sym(sample, &al, 0, output[type].print_ip_opts,
> + cursor, symbol_conf.bt_stop_list, fp);
> + }
> +
> + fprintf(fp, "\n");
> +
> + if (verbose > 0)
> + fflush(fp);
> +
> +out_put:
> + addr_location__exit(&al);
> + return ret;
> +}
> +
> // Used when scr->per_event_dump is not set
> static struct evsel_script es_stdout;
>
> @@ -4320,6 +4407,7 @@ int cmd_script(int argc, const char **argv)
>
> perf_tool__init(&script.tool, !unsorted_dump);
> script.tool.sample = process_sample_event;
> + script.tool.callchain_deferred = process_deferred_sample_event;
> script.tool.mmap = perf_event__process_mmap;
> script.tool.mmap2 = perf_event__process_mmap2;
> script.tool.comm = perf_event__process_comm;
> @@ -4346,6 +4434,7 @@ int cmd_script(int argc, const char **argv)
> script.tool.throttle = process_throttle_event;
> script.tool.unthrottle = process_throttle_event;
> script.tool.ordering_requires_timestamps = true;
> + script.tool.merge_deferred_callchains = false;
> session = perf_session__new(&data, &script.tool);
> if (IS_ERR(session))
> return PTR_ERR(session);
> --
> 2.52.0.rc1.455.g30608eb744-goog
>
>
^ 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