* Re: [PATCHv5 bpf-next 6/9] ftrace: Add update_ftrace_direct_mod function
From: Steven Rostedt @ 2025-12-18 15:41 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Florent Revest, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <20251218101942.0716efd6@gandalf.local.home>
On Thu, 18 Dec 2025 10:19:42 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 15 Dec 2025 22:13:59 +0100
> Jiri Olsa <jolsa@kernel.org> wrote:
>
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 48dc0de5f2ce..95a38fb18ed7 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -6489,6 +6489,78 @@ int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
> > return err;
> > }
> >
>
> Kerneldoc needed.
>
> > +int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash *hash, bool do_direct_lock)
> > +{
> > + struct ftrace_func_entry *entry, *tmp;
> > + static struct ftrace_ops tmp_ops = {
> > + .func = ftrace_stub,
> > + .flags = FTRACE_OPS_FL_STUB,
> > + };
> > + struct ftrace_hash *orig_hash;
> > + unsigned long size, i;
> > + int err = -EINVAL;
> > +
> > + if (!hash_count(hash))
> > + return -EINVAL;
> > + if (check_direct_multi(ops))
> > + return -EINVAL;
> > + if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
> > + return -EINVAL;
> > + if (direct_functions == EMPTY_HASH)
> > + return -EINVAL;
> > +
> > + if (do_direct_lock)
> > + mutex_lock(&direct_mutex);
>
> This optional taking of the direct_mutex lock needs some serious rationale
> and documentation.
>
> > +
> > + orig_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
> > + if (!orig_hash)
> > + goto unlock;
> > +
> > + /* Enable the tmp_ops to have the same functions as the direct ops */
Add to the comments here:
* In order to modify the direct callers, all the functions need to
* first be calling the ftrace_ops_list_func() and not be connected
* to any direct callers. To do that, create a temporary ops that
* attach to the same functions as the direct ops, and attach that
* first. Then when adding the direct ops, it will use the
* ftrace_ops_list_func(), and this can safely modify what the
* direct ops call.
Or something like that. I want this code to be as clear as day to what it
is doing. In a year or two, we will forget, and this will be very confusing
to newcomers.
> > + ftrace_ops_init(&tmp_ops);
> > + tmp_ops.func_hash = ops->func_hash;
> > +
> > + err = register_ftrace_function_nolock(&tmp_ops);
> > + if (err)
> > + goto unlock;
> > +
> > + /*
> > + * Call __ftrace_hash_update_ipmodify() here, so that we can call
> > + * ops->ops_func for the ops. This is needed because the above
> > + * register_ftrace_function_nolock() worked on tmp_ops.
> > + */
> > + err = __ftrace_hash_update_ipmodify(ops, orig_hash, orig_hash, true);
> > + if (err)
> > + goto out;
> > +
> > + /*
> > + * Now the ftrace_ops_list_func() is called to do the direct callers.
> > + * We can safely change the direct functions attached to each entry.
> > + */
> > + mutex_lock(&ftrace_lock);
>
> I'm going to need some time staring at this code. It looks like it may be
> relying on some internals here.
>
> -- Steve
>
>
> > +
I would add a comment here:
/* Now update the direct functions to point to the new callbacks */
-- Steve
> > + size = 1 << hash->size_bits;
> > + for (i = 0; i < size; i++) {
> > + hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
> > + tmp = __ftrace_lookup_ip(direct_functions, entry->ip);
> > + if (!tmp)
> > + continue;
> > + tmp->direct = entry->direct;
> > + }
> > + }
> > +
> > + mutex_unlock(&ftrace_lock);
> > +
> > +out:
> > + /* Removing the tmp_ops will add the updated direct callers to the functions */
> > + unregister_ftrace_function(&tmp_ops);
> > +
> > +unlock:
> > + if (do_direct_lock)
> > + mutex_unlock(&direct_mutex);
> > + return err;
> > +}
> > +
> > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
> >
> > /**
^ permalink raw reply
* Re: [PATCH v4 1/7] LoongArch: ftrace: Refactor register restoration in ftrace_common_return
From: Chris Mason @ 2025-12-18 15:26 UTC (permalink / raw)
To: Chenghao Duan, bot+bpf-ci
Cc: yangtiezhu, rostedt, mhiramat, mark.rutland, hengqi.chen,
chenhuacai, kernel, zhangtianyang, masahiroy, linux-kernel,
loongarch, bpf, youling.tang, jianghaoran, vincent.mc.li,
linux-trace-kernel, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, ihor.solodrai
In-Reply-To: <20251218012656.GA75051@chenghao-pc>
On 12/17/25 8:26 PM, Chenghao Duan wrote:
> On Wed, Dec 17, 2025 at 06:55:00AM +0000, bot+bpf-ci@kernel.org wrote:
>>> diff --git a/arch/loongarch/kernel/mcount_dyn.S b/arch/loongarch/kernel/mcount_dyn.S
>>> index d6b474ad1d5e..5729c20e5b8b 100644
>>> --- a/arch/loongarch/kernel/mcount_dyn.S
>>> +++ b/arch/loongarch/kernel/mcount_dyn.S
>>> @@ -94,7 +94,6 @@ SYM_INNER_LABEL(ftrace_graph_call, SYM_L_GLOBAL)
>>> * at the callsite, so there is no need to restore the T series regs.
>>> */
>>> ftrace_common_return:
>>> - PTR_L ra, sp, PT_R1
>>> PTR_L a0, sp, PT_R4
>>> PTR_L a1, sp, PT_R5
>>> PTR_L a2, sp, PT_R6
>>> @@ -104,12 +103,17 @@ ftrace_common_return:
>>> PTR_L a6, sp, PT_R10
>>> PTR_L a7, sp, PT_R11
>>> PTR_L fp, sp, PT_R22
>>> - PTR_L t0, sp, PT_ERA
>>> PTR_L t1, sp, PT_R13
>>> - PTR_ADDI sp, sp, PT_SIZE
>>> bnez t1, .Ldirect
>>> +
>>> + PTR_L ra, sp, PT_R1
>>> + PTR_L t0, sp, PT_ERA
>>> + PTR_ADDI sp, sp, PT_SIZE
>>> jr t0
>>> .Ldirect:
>>> + PTR_L t0, sp, PT_R1
>>> + PTR_L ra, sp, PT_ERA
>> ^^^^^^^^^^^^^^^^^^^
>> Does this break existing ftrace direct trampolines?
>>
>> Before this change, direct trampolines received:
>> ra = parent return address (from PT_R1)
>> t0 = traced function address (from PT_ERA)
>>
>> After this change, direct trampolines receive:
>> t0 = parent return address (from PT_R1)
>> ra = traced function address (from PT_ERA)
>>
>> The sample trampolines in samples/ftrace/ were written for the old
>> convention. For example, samples/ftrace/ftrace-direct.c does:
>>
>> my_tramp:
>> st.d $t0, $sp, 8 # Save what it thinks is traced func addr
>> st.d $ra, $sp, 16 # Save what it thinks is parent addr
>> bl my_direct_func
>> ld.d $t0, $sp, 8 # Restore
>> ld.d $ra, $sp, 16 # Restore
>> jr $t0 # Jump to what it thinks is traced function
>>
>> With the new convention, the sample would save parent address in the
>> t0 slot, restore it to t0, and then jump to the parent instead of the
>> traced function, skipping the traced function entirely.
>>
>> This was fixed later in commit 055e392f3be7 "LoongArch: ftrace: Adjust
>> register stack restore order in direct call trampolines" which updated
>> all the samples to match the new convention. However, any out-of-tree
>> direct trampolines written before this commit would be broken by this
>> change.
>>
>> Was this ABI break intentional? The commit message mentions matching
>> "the state when ftrace was entered" but doesn't explain why breaking
>> compatibility was necessary. RISC-V uses the opposite convention
>> (t0 = traced func, ra = parent), which the original LoongArch code
>> matched.
>
> The patch "LoongArch: ftrace: Refactor register restoration in
> ftrace_common_return" changed the conventions for t0 and ra after
> entering the direct call function, which is exactly the reason forthe
> changes made in this patch.
I'll try to adjust the kinds of ABI breakage AI comments on. It did
catch the other related changes from this series, but the additional
commentary wasn't useful.
Thanks,
Chris
^ permalink raw reply
* Re: [PATCHv5 bpf-next 6/9] ftrace: Add update_ftrace_direct_mod function
From: Steven Rostedt @ 2025-12-18 15:19 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Florent Revest, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <20251215211402.353056-7-jolsa@kernel.org>
On Mon, 15 Dec 2025 22:13:59 +0100
Jiri Olsa <jolsa@kernel.org> wrote:
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 48dc0de5f2ce..95a38fb18ed7 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6489,6 +6489,78 @@ int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
> return err;
> }
>
Kerneldoc needed.
> +int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash *hash, bool do_direct_lock)
> +{
> + struct ftrace_func_entry *entry, *tmp;
> + static struct ftrace_ops tmp_ops = {
> + .func = ftrace_stub,
> + .flags = FTRACE_OPS_FL_STUB,
> + };
> + struct ftrace_hash *orig_hash;
> + unsigned long size, i;
> + int err = -EINVAL;
> +
> + if (!hash_count(hash))
> + return -EINVAL;
> + if (check_direct_multi(ops))
> + return -EINVAL;
> + if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
> + return -EINVAL;
> + if (direct_functions == EMPTY_HASH)
> + return -EINVAL;
> +
> + if (do_direct_lock)
> + mutex_lock(&direct_mutex);
This optional taking of the direct_mutex lock needs some serious rationale
and documentation.
> +
> + orig_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
> + if (!orig_hash)
> + goto unlock;
> +
> + /* Enable the tmp_ops to have the same functions as the direct ops */
> + ftrace_ops_init(&tmp_ops);
> + tmp_ops.func_hash = ops->func_hash;
> +
> + err = register_ftrace_function_nolock(&tmp_ops);
> + if (err)
> + goto unlock;
> +
> + /*
> + * Call __ftrace_hash_update_ipmodify() here, so that we can call
> + * ops->ops_func for the ops. This is needed because the above
> + * register_ftrace_function_nolock() worked on tmp_ops.
> + */
> + err = __ftrace_hash_update_ipmodify(ops, orig_hash, orig_hash, true);
> + if (err)
> + goto out;
> +
> + /*
> + * Now the ftrace_ops_list_func() is called to do the direct callers.
> + * We can safely change the direct functions attached to each entry.
> + */
> + mutex_lock(&ftrace_lock);
I'm going to need some time staring at this code. It looks like it may be
relying on some internals here.
-- Steve
> +
> + size = 1 << hash->size_bits;
> + for (i = 0; i < size; i++) {
> + hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
> + tmp = __ftrace_lookup_ip(direct_functions, entry->ip);
> + if (!tmp)
> + continue;
> + tmp->direct = entry->direct;
> + }
> + }
> +
> + mutex_unlock(&ftrace_lock);
> +
> +out:
> + /* Removing the tmp_ops will add the updated direct callers to the functions */
> + unregister_ftrace_function(&tmp_ops);
> +
> +unlock:
> + if (do_direct_lock)
> + mutex_unlock(&direct_mutex);
> + return err;
> +}
> +
> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
>
> /**
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/6] bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME
From: Menglong Dong @ 2025-12-18 15:05 UTC (permalink / raw)
To: Andreas Schwab
Cc: ast, rostedt, daniel, john.fastabend, andrii, martin.lau, eddyz87,
song, yonghong.song, kpsingh, sdf, haoluo, jolsa, mhiramat,
mark.rutland, mathieu.desnoyers, jiang.biao, bpf, linux-kernel,
linux-trace-kernel, linux-riscv
In-Reply-To: <874ipnkfvt.fsf@igel.home>
On Thu, Dec 18, 2025 at 10:45 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Nov 18 2025, Menglong Dong 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>
> > Acked-by: Alexei Starovoitov <ast@kernel.org>
>
> This breaks RISC-V:
It's weird, as the 2 flags should be set together all the time
in RISC-V. Sorry that I'm already in bed, I'll check it tomorrow
morning.
Thanks!
Menglong Dong
>
> [ 8.584381][ T1] systemd[1]: bpf-restrict-fs: LSM BPF program attached
> [ 8.588359][ T1] Insufficient stack space to handle exception!
> [ 8.588823][ T1] Task stack: [0xff20000000010000..0xff20000000014000]
> [ 8.589219][ T1] Overflow stack: [0xff600000ffdad070..0xff600000ffdae070]
> [ 8.590133][ T1] CPU: 1 UID: 0 PID: 1 Comm: systemd Not tainted 6.18.0-rc5+ #15 PREEMPT(voluntary) c900881ed1c1988ec5cf3e914d0edeb1b4d83ca3
> [ 8.590898][ T1] Hardware name: riscv-virtio qemu/qemu, BIOS 2025.10 10/01/2025
> [ 8.591494][ T1] epc : copy_from_kernel_nofault+0xa/0x198
> [ 8.592292][ T1] ra : bpf_probe_read_kernel+0x20/0x60
> [ 8.592658][ T1] epc : ffffffff802b732a ra : ffffffff801e6070 sp : ff2000000000ffe0
> [ 8.593121][ T1] gp : ffffffff82262ed0 tp : 0000000000000000 t0 : ffffffff80022320
> [ 8.593566][ T1] t1 : ffffffff801e6056 t2 : 0000000000000000 s0 : ff20000000010040
> [ 8.593997][ T1] s1 : 0000000000000008 a0 : ff20000000010050 a1 : ff60000083b3d320
> [ 8.594446][ T1] a2 : 0000000000000008 a3 : 0000000000000097 a4 : 0000000000000000
> [ 8.594940][ T1] a5 : 0000000000000000 a6 : 0000000000000021 a7 : 0000000000000003
> [ 8.595396][ T1] s2 : ff20000000010050 s3 : ff6000008459fc18 s4 : ff60000083b3d340
> [ 8.595831][ T1] s5 : ff20000000010060 s6 : 0000000000000000 s7 : ff20000000013aa8
> [ 8.596215][ T1] s8 : 0000000000000000 s9 : 0000000000008000 s10: 000000000058dcb0
> [ 8.596641][ T1] s11: 000000000058dca7 t3 : 000000006925116d t4 : ff6000008090f026
> [ 8.597065][ T1] t5 : 00007fff9b0cbaa8 t6 : 0000000000000016
> [ 8.597363][ T1] status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
> [ 8.598033][ T1] Kernel panic - not syncing: Kernel stack overflow
> [ 8.598597][ T1] CPU: 1 UID: 0 PID: 1 Comm: systemd Not tainted 6.18.0-rc5+ #15 PREEMPT(voluntary) c900881ed1c1988ec5cf3e914d0edeb1b4d83ca3
> [ 8.599244][ T1] Hardware name: riscv-virtio qemu/qemu, BIOS 2025.10 10/01/2025
> [ 8.599659][ T1] Call Trace:
> [ 8.600117][ T1] [<ffffffff8001a1f8>] dump_backtrace+0x28/0x38
> [ 8.600517][ T1] [<ffffffff80002502>] show_stack+0x3a/0x50
> [ 8.600844][ T1] [<ffffffff800122be>] dump_stack_lvl+0x56/0x80
> [ 8.601176][ T1] [<ffffffff80012300>] dump_stack+0x18/0x22
> [ 8.601518][ T1] [<ffffffff80002abe>] vpanic+0xf6/0x328
> [ 8.601819][ T1] [<ffffffff80002d2e>] panic+0x3e/0x40
> [ 8.602088][ T1] [<ffffffff80019ef0>] handle_bad_stack+0x98/0xa0
> [ 8.602395][ T1] [<ffffffff801e6070>] bpf_probe_read_kernel+0x20/0x60
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
> "And now for something completely different."
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/6] bpf: fix the usage of BPF_TRAMP_F_SKIP_FRAME
From: Andreas Schwab @ 2025-12-18 14:45 UTC (permalink / raw)
To: Menglong Dong
Cc: ast, rostedt, daniel, john.fastabend, andrii, martin.lau, eddyz87,
song, yonghong.song, kpsingh, sdf, haoluo, jolsa, mhiramat,
mark.rutland, mathieu.desnoyers, jiang.biao, bpf, linux-kernel,
linux-trace-kernel, linux-riscv
In-Reply-To: <20251118123639.688444-4-dongml2@chinatelecom.cn>
On Nov 18 2025, Menglong Dong 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>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
This breaks RISC-V:
[ 8.584381][ T1] systemd[1]: bpf-restrict-fs: LSM BPF program attached
[ 8.588359][ T1] Insufficient stack space to handle exception!
[ 8.588823][ T1] Task stack: [0xff20000000010000..0xff20000000014000]
[ 8.589219][ T1] Overflow stack: [0xff600000ffdad070..0xff600000ffdae070]
[ 8.590133][ T1] CPU: 1 UID: 0 PID: 1 Comm: systemd Not tainted 6.18.0-rc5+ #15 PREEMPT(voluntary) c900881ed1c1988ec5cf3e914d0edeb1b4d83ca3
[ 8.590898][ T1] Hardware name: riscv-virtio qemu/qemu, BIOS 2025.10 10/01/2025
[ 8.591494][ T1] epc : copy_from_kernel_nofault+0xa/0x198
[ 8.592292][ T1] ra : bpf_probe_read_kernel+0x20/0x60
[ 8.592658][ T1] epc : ffffffff802b732a ra : ffffffff801e6070 sp : ff2000000000ffe0
[ 8.593121][ T1] gp : ffffffff82262ed0 tp : 0000000000000000 t0 : ffffffff80022320
[ 8.593566][ T1] t1 : ffffffff801e6056 t2 : 0000000000000000 s0 : ff20000000010040
[ 8.593997][ T1] s1 : 0000000000000008 a0 : ff20000000010050 a1 : ff60000083b3d320
[ 8.594446][ T1] a2 : 0000000000000008 a3 : 0000000000000097 a4 : 0000000000000000
[ 8.594940][ T1] a5 : 0000000000000000 a6 : 0000000000000021 a7 : 0000000000000003
[ 8.595396][ T1] s2 : ff20000000010050 s3 : ff6000008459fc18 s4 : ff60000083b3d340
[ 8.595831][ T1] s5 : ff20000000010060 s6 : 0000000000000000 s7 : ff20000000013aa8
[ 8.596215][ T1] s8 : 0000000000000000 s9 : 0000000000008000 s10: 000000000058dcb0
[ 8.596641][ T1] s11: 000000000058dca7 t3 : 000000006925116d t4 : ff6000008090f026
[ 8.597065][ T1] t5 : 00007fff9b0cbaa8 t6 : 0000000000000016
[ 8.597363][ T1] status: 0000000200000120 badaddr: 0000000000000000 cause: 8000000000000005
[ 8.598033][ T1] Kernel panic - not syncing: Kernel stack overflow
[ 8.598597][ T1] CPU: 1 UID: 0 PID: 1 Comm: systemd Not tainted 6.18.0-rc5+ #15 PREEMPT(voluntary) c900881ed1c1988ec5cf3e914d0edeb1b4d83ca3
[ 8.599244][ T1] Hardware name: riscv-virtio qemu/qemu, BIOS 2025.10 10/01/2025
[ 8.599659][ T1] Call Trace:
[ 8.600117][ T1] [<ffffffff8001a1f8>] dump_backtrace+0x28/0x38
[ 8.600517][ T1] [<ffffffff80002502>] show_stack+0x3a/0x50
[ 8.600844][ T1] [<ffffffff800122be>] dump_stack_lvl+0x56/0x80
[ 8.601176][ T1] [<ffffffff80012300>] dump_stack+0x18/0x22
[ 8.601518][ T1] [<ffffffff80002abe>] vpanic+0xf6/0x328
[ 8.601819][ T1] [<ffffffff80002d2e>] panic+0x3e/0x40
[ 8.602088][ T1] [<ffffffff80019ef0>] handle_bad_stack+0x98/0xa0
[ 8.602395][ T1] [<ffffffff801e6070>] bpf_probe_read_kernel+0x20/0x60
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH 2/2] bpf: Implement kretprobe fallback for kprobe multi link
From: bot+bpf-ci @ 2025-12-18 13:33 UTC (permalink / raw)
To: liujing.root, ast, daniel, andrii, mhiramat, martin.lau, eddyz87,
song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa
Cc: bpf, linux-kernel, linux-trace-kernel, liujing40, ast, andrii,
daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
In-Reply-To: <20251218130629.365398-3-liujing40@xiaomi.com>
[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 1fd07c10378f..426a1c627508 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
[ ... ]
> +int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> +{
> + struct bpf_kprobe_multi_link *link = NULL;
> + struct bpf_link_primer link_primer;
> + struct bpf_kprobe *kprobes = NULL;
> + u32 flags, cnt;
> + u64 __user *ucookies;
> + unsigned long __user *uaddrs;
> + unsigned long __user *usyms;
> + int err, i;
[ ... ]
> + for (i = 0; i < cnt; i++) {
> + unsigned long addr;
> +
> + if (uaddrs) {
> + if (__get_user(addr, uaddrs + i)) {
> + err = -EFAULT;
> + goto error;
> + }
> + } else {
> + unsigned long __user usymbol;
> + char buf[KSYM_NAME_LEN];
> +
> + if (__get_user(usymbol, usyms + i)) {
> + err = -EFAULT;
> + goto error;
> + }
> + err = strncpy_from_user(buf, (const char __user *) usymbol, KSYM_NAME_LEN);
> + if (err == KSYM_NAME_LEN)
> + err = -E2BIG;
> + if (err < 0)
> + goto error;
> +
> + addr = kallsyms_lookup_name(buf);
> + if (!addr)
> + goto error;
When kallsyms_lookup_name() returns 0 (symbol not found), is err being set
to an error code before jumping to the error label? It looks like err will
still hold the positive return value from strncpy_from_user() (the string
length), which would be returned to the caller as a success indication.
The error label at the end of the function does:
error:
kvfree(kprobes);
kfree(link);
return err;
This could cause the syscall to return a positive value to userspace instead
of an error, and also leak a prog reference since link_create() only calls
bpf_prog_put() when ret < 0.
> + }
> + if (prog->kprobe_override && !within_error_injection_list(addr)) {
> + err = -EINVAL;
> + goto error;
> + }
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20338242683
^ permalink raw reply
* [PATCH 2/2] bpf: Implement kretprobe fallback for kprobe multi link
From: liujing40 @ 2025-12-18 13:06 UTC (permalink / raw)
To: ast, daniel, andrii, mhiramat, martin.lau, eddyz87, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa
Cc: bpf, linux-kernel, linux-trace-kernel, liujing40
In-Reply-To: <20251218130629.365398-1-liujing40@xiaomi.com>
When fprobe is not available, provide a fallback implementation of
kprobe_multi using the traditional kretprobe API.
Uses kretprobe's entry_handler and handler callbacks to simulate fprobe's
entry/exit functionality.
Signed-off-by: Jing Liu <liujing40@xiaomi.com>
---
kernel/trace/bpf_trace.c | 307 +++++++++++++++++++++++++++++++++++++--
1 file changed, 295 insertions(+), 12 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 1fd07c10378f..426a1c627508 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2274,12 +2274,44 @@ struct bpf_session_run_ctx {
void *data;
};
-#ifdef CONFIG_FPROBE
+#if defined(CONFIG_FPROBE) || defined(CONFIG_KRETPROBES)
+#ifndef CONFIG_FPROBE
+struct bpf_kprobe {
+ struct bpf_kprobe_multi_link *link;
+ u64 cookie;
+ struct kretprobe rp;
+};
+
+static void bpf_kprobe_unregister(struct bpf_kprobe *kps, u32 cnt)
+{
+ for (int i = 0; i < cnt; i++)
+ unregister_kretprobe(&kps[i].rp);
+}
+
+static int bpf_kprobe_register(struct bpf_kprobe *kps, u32 cnt)
+{
+ int ret = 0, i;
+
+ for (i = 0; i < cnt; i++) {
+ ret = register_kretprobe(&kps[i].rp);
+ if (ret < 0) {
+ bpf_kprobe_unregister(kps, i);
+ break;
+ }
+ }
+ return ret;
+}
+#endif
+
struct bpf_kprobe_multi_link {
struct bpf_link link;
+#ifdef CONFIG_FPROBE
struct fprobe fp;
unsigned long *addrs;
u64 *cookies;
+#else
+ struct bpf_kprobe *kprobes;
+#endif
u32 cnt;
u32 mods_cnt;
struct module **mods;
@@ -2287,7 +2319,11 @@ struct bpf_kprobe_multi_link {
struct bpf_kprobe_multi_run_ctx {
struct bpf_session_run_ctx session_ctx;
+#ifdef CONFIG_FPROBE
struct bpf_kprobe_multi_link *link;
+#else
+ struct bpf_kprobe *kprobe;
+#endif
unsigned long entry_ip;
};
@@ -2304,7 +2340,11 @@ static void bpf_kprobe_multi_link_release(struct bpf_link *link)
struct bpf_kprobe_multi_link *kmulti_link;
kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
+#ifdef CONFIG_FPROBE
unregister_fprobe(&kmulti_link->fp);
+#else
+ bpf_kprobe_unregister(kmulti_link->kprobes, kmulti_link->cnt);
+#endif
kprobe_multi_put_modules(kmulti_link->mods, kmulti_link->mods_cnt);
}
@@ -2313,8 +2353,12 @@ static void bpf_kprobe_multi_link_dealloc(struct bpf_link *link)
struct bpf_kprobe_multi_link *kmulti_link;
kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
+#ifdef CONFIG_FPROBE
kvfree(kmulti_link->addrs);
kvfree(kmulti_link->cookies);
+#else
+ kvfree(kmulti_link->kprobes);
+#endif
kfree(kmulti_link->mods);
kfree(kmulti_link);
}
@@ -2326,6 +2370,7 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
u64 __user *uaddrs = u64_to_user_ptr(info->kprobe_multi.addrs);
struct bpf_kprobe_multi_link *kmulti_link;
u32 ucount = info->kprobe_multi.count;
+ bool kallsyms_show = kallsyms_show_value(current_cred());
int err = 0, i;
if (!uaddrs ^ !ucount)
@@ -2336,7 +2381,12 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
info->kprobe_multi.count = kmulti_link->cnt;
info->kprobe_multi.flags = kmulti_link->link.flags;
+#ifdef CONFIG_FPROBE
info->kprobe_multi.missed = kmulti_link->fp.nmissed;
+#else
+ for (i = 0; i < kmulti_link->cnt; i++)
+ info->kprobe_multi.missed += kmulti_link->kprobes[i].rp.nmissed;
+#endif
if (!uaddrs)
return 0;
@@ -2345,6 +2395,7 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
else
ucount = kmulti_link->cnt;
+#ifdef CONFIG_FPROBE
if (ucookies) {
if (kmulti_link->cookies) {
if (copy_to_user(ucookies, kmulti_link->cookies, ucount * sizeof(u64)))
@@ -2357,7 +2408,7 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
}
}
- if (kallsyms_show_value(current_cred())) {
+ if (kallsyms_show) {
if (copy_to_user(uaddrs, kmulti_link->addrs, ucount * sizeof(u64)))
return -EFAULT;
} else {
@@ -2366,6 +2417,16 @@ static int bpf_kprobe_multi_link_fill_link_info(const struct bpf_link *link,
return -EFAULT;
}
}
+#else
+ for (i = 0; i < ucount; i++) {
+ if (ucookies && put_user(kmulti_link->kprobes[i].cookie, ucookies + i))
+ return -EFAULT;
+
+ if (put_user(kallsyms_show ? (uintptr_t)kmulti_link->kprobes[i].rp.kp.addr : 0,
+ uaddrs + i))
+ return -EFAULT;
+ }
+#endif
return err;
}
@@ -2374,21 +2435,32 @@ static void bpf_kprobe_multi_show_fdinfo(const struct bpf_link *link,
struct seq_file *seq)
{
struct bpf_kprobe_multi_link *kmulti_link;
+ unsigned long kprobe_multi_missed = 0;
kmulti_link = container_of(link, struct bpf_kprobe_multi_link, link);
+#ifdef CONFIG_FPROBE
+ kprobe_multi_missed = kmulti_link->fp.nmissed;
+#else
+ for (int i = 0; i < kmulti_link->cnt; i++)
+ kprobe_multi_missed += kmulti_link->kprobes[i].rp.nmissed;
+#endif
seq_printf(seq,
"kprobe_cnt:\t%u\n"
"missed:\t%lu\n",
kmulti_link->cnt,
- kmulti_link->fp.nmissed);
+ kprobe_multi_missed);
seq_printf(seq, "%s\t %s\n", "cookie", "func");
for (int i = 0; i < kmulti_link->cnt; i++) {
- seq_printf(seq,
- "%llu\t %pS\n",
- kmulti_link->cookies[i],
- (void *)kmulti_link->addrs[i]);
+#ifdef CONFIG_FPROBE
+ u64 cookie = kmulti_link->cookies[i];
+ void *addr = (void *)kmulti_link->addrs[i];
+#else
+ u64 cookie = kmulti_link->kprobes[i].cookie;
+ void *addr = (void *)kmulti_link->kprobes[i].rp.kp.addr;
+#endif
+ seq_printf(seq, "%llu\t %pS\n", cookie, addr);
}
}
#endif
@@ -2445,17 +2517,22 @@ static bool has_module(struct modules_array *arr, struct module *mod)
return false;
}
-static int get_modules_for_addrs(struct module ***mods, unsigned long *addrs, u32 addrs_cnt)
+static int get_modules_for_addrs(struct bpf_kprobe_multi_link *link)
{
struct modules_array arr = {};
u32 i, err = 0;
- for (i = 0; i < addrs_cnt; i++) {
+ for (i = 0; i < link->cnt; i++) {
bool skip_add = false;
struct module *mod;
+#ifdef CONFIG_FPROBE
+ unsigned long addr = link->addrs[i];
+#else
+ unsigned long addr = (unsigned long)link->kprobes[i].rp.kp.addr;
+#endif
scoped_guard(rcu) {
- mod = __module_address(addrs[i]);
+ mod = __module_address(addr);
/* Either no module or it's already stored */
if (!mod || has_module(&arr, mod)) {
skip_add = true;
@@ -2483,10 +2560,11 @@ static int get_modules_for_addrs(struct module ***mods, unsigned long *addrs, u3
}
/* or number of modules found if everything is ok. */
- *mods = arr.mods;
+ link->mods = arr.mods;
return arr.mods_cnt;
}
+#ifdef CONFIG_FPROBE
struct user_syms {
const char **syms;
char *buf;
@@ -2843,7 +2921,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
link);
}
- err = get_modules_for_addrs(&link->mods, addrs, cnt);
+ err = get_modules_for_addrs(link);
if (err < 0) {
bpf_link_cleanup(&link_primer);
return err;
@@ -2866,6 +2944,211 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
return err;
}
#else /* !CONFIG_FPROBE */
+static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx)
+{
+ struct bpf_kprobe_multi_run_ctx *run_ctx;
+
+ run_ctx = container_of(current->bpf_ctx, struct bpf_kprobe_multi_run_ctx,
+ session_ctx.run_ctx);
+ return run_ctx->kprobe->cookie;
+}
+
+static __always_inline int
+kprobe_multi_link_prog_run(struct bpf_kprobe *kprobe, unsigned long entry_ip,
+ struct pt_regs *regs, bool is_return, void *data)
+{
+ struct bpf_kprobe_multi_link *link = kprobe->link;
+ struct bpf_kprobe_multi_run_ctx run_ctx = {
+ .session_ctx = {
+ .is_return = is_return,
+ .data = data,
+ },
+ .kprobe = kprobe,
+ .entry_ip = entry_ip,
+ };
+ struct bpf_run_ctx *old_run_ctx;
+ int err;
+
+ cant_sleep();
+
+ if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
+ bpf_prog_inc_misses_counter(link->link.prog);
+ err = 1;
+ goto out;
+ }
+
+ rcu_read_lock();
+ migrate_disable();
+ old_run_ctx = bpf_set_run_ctx(&run_ctx.session_ctx.run_ctx);
+ err = bpf_prog_run(link->link.prog, regs);
+ bpf_reset_run_ctx(old_run_ctx);
+ migrate_enable();
+ rcu_read_unlock();
+
+ out:
+ __this_cpu_dec(bpf_prog_active);
+ return err;
+}
+
+static int
+kprobe_multi_link_handler(struct kretprobe_instance *ri, struct pt_regs *regs)
+{
+ struct kretprobe *rp = get_kretprobe(ri);
+ struct bpf_kprobe *kprobe;
+ int err;
+
+ if (unlikely(!rp))
+ return 1;
+
+ kprobe = container_of(rp, struct bpf_kprobe, rp);
+ err = kprobe_multi_link_prog_run(kprobe, get_entry_ip((uintptr_t)rp->kp.addr),
+ regs, false, ri->data);
+ return is_kprobe_session(kprobe->link->link.prog) ? err : 0;
+}
+
+static int
+kprobe_multi_link_exit_handler(struct kretprobe_instance *ri, struct pt_regs *regs)
+{
+ struct kretprobe *rp = get_kretprobe(ri);
+ struct bpf_kprobe *kprobe;
+
+ if (unlikely(!rp))
+ return 0;
+
+ kprobe = container_of(rp, struct bpf_kprobe, rp);
+ kprobe_multi_link_prog_run(kprobe, get_entry_ip((uintptr_t)rp->kp.addr),
+ regs, true, ri->data);
+ return 0;
+}
+
+int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+ struct bpf_kprobe_multi_link *link = NULL;
+ struct bpf_link_primer link_primer;
+ struct bpf_kprobe *kprobes = NULL;
+ u32 flags, cnt;
+ u64 __user *ucookies;
+ unsigned long __user *uaddrs;
+ unsigned long __user *usyms;
+ int err, i;
+
+ /* no support for 32bit archs yet */
+ if (sizeof(u64) != sizeof(void *))
+ return -EOPNOTSUPP;
+
+ if (attr->link_create.flags)
+ return -EINVAL;
+
+ if (!is_kprobe_multi(prog))
+ return -EINVAL;
+
+ /* Writing to context is not allowed for kprobes. */
+ if (prog->aux->kprobe_write_ctx)
+ return -EINVAL;
+
+ flags = attr->link_create.kprobe_multi.flags;
+ if (flags & ~BPF_F_KPROBE_MULTI_RETURN)
+ return -EINVAL;
+
+ uaddrs = u64_to_user_ptr(attr->link_create.kprobe_multi.addrs);
+ usyms = u64_to_user_ptr(attr->link_create.kprobe_multi.syms);
+ if (!!uaddrs == !!usyms)
+ return -EINVAL;
+
+ cnt = attr->link_create.kprobe_multi.cnt;
+ if (!cnt)
+ return -EINVAL;
+ if (cnt > MAX_KPROBE_MULTI_CNT)
+ return -E2BIG;
+
+ ucookies = u64_to_user_ptr(attr->link_create.kprobe_multi.cookies);
+ kprobes = kvcalloc(cnt, sizeof(*kprobes), GFP_KERNEL);
+ link = kzalloc(sizeof(*link), GFP_KERNEL);
+ if (!link || !kprobes) {
+ err = -ENOMEM;
+ goto error;
+ }
+
+ for (i = 0; i < cnt; i++) {
+ unsigned long addr;
+
+ if (uaddrs) {
+ if (__get_user(addr, uaddrs + i)) {
+ err = -EFAULT;
+ goto error;
+ }
+ } else {
+ unsigned long __user usymbol;
+ char buf[KSYM_NAME_LEN];
+
+ if (__get_user(usymbol, usyms + i)) {
+ err = -EFAULT;
+ goto error;
+ }
+ err = strncpy_from_user(buf, (const char __user *) usymbol, KSYM_NAME_LEN);
+ if (err == KSYM_NAME_LEN)
+ err = -E2BIG;
+ if (err < 0)
+ goto error;
+
+ addr = kallsyms_lookup_name(buf);
+ if (!addr)
+ goto error;
+ }
+ if (prog->kprobe_override && !within_error_injection_list(addr)) {
+ err = -EINVAL;
+ goto error;
+ }
+ if (ucookies && __get_user(kprobes[i].cookie, ucookies + i)) {
+ err = -EFAULT;
+ goto error;
+ }
+
+ kprobes[i].link = link;
+ kprobes[i].rp.kp.addr = (kprobe_opcode_t *)addr;
+
+ if (!(flags & BPF_F_KPROBE_MULTI_RETURN))
+ kprobes[i].rp.entry_handler = kprobe_multi_link_handler;
+ if ((flags & BPF_F_KPROBE_MULTI_RETURN) || is_kprobe_session(prog))
+ kprobes[i].rp.handler = kprobe_multi_link_exit_handler;
+ if (is_kprobe_session(prog))
+ kprobes[i].rp.data_size = sizeof(u64);
+ }
+
+ bpf_link_init(&link->link, BPF_LINK_TYPE_KPROBE_MULTI,
+ &bpf_kprobe_multi_link_lops, prog, attr->link_create.attach_type);
+
+ err = bpf_link_prime(&link->link, &link_primer);
+ if (err)
+ goto error;
+
+ link->kprobes = kprobes;
+ link->cnt = cnt;
+ link->link.flags = flags;
+
+ err = get_modules_for_addrs(link);
+ if (err < 0) {
+ bpf_link_cleanup(&link_primer);
+ return err;
+ }
+ link->mods_cnt = err;
+
+ err = bpf_kprobe_register(kprobes, cnt);
+ if (err) {
+ kprobe_multi_put_modules(link->mods, link->mods_cnt);
+ bpf_link_cleanup(&link_primer);
+ return err;
+ }
+
+ return bpf_link_settle(&link_primer);
+
+error:
+ kvfree(kprobes);
+ kfree(link);
+ return err;
+}
+#endif
+#else /* !CONFIG_FPROBE && !CONFIG_KRETPROBES*/
int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
{
return -EOPNOTSUPP;
--
2.25.1
^ permalink raw reply related
* [PATCH 1/2] bpf: Prepare for kprobe multi link fallback patch
From: liujing40 @ 2025-12-18 13:06 UTC (permalink / raw)
To: ast, daniel, andrii, mhiramat, martin.lau, eddyz87, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa
Cc: bpf, linux-kernel, linux-trace-kernel, liujing40
In-Reply-To: <20251218130629.365398-1-liujing40@xiaomi.com>
This moves some functions and structs around to make the following patch
easier to read.
Signed-off-by: Jing Liu <liujing40@xiaomi.com>
---
kernel/trace/bpf_trace.c | 304 +++++++++++++++++++--------------------
1 file changed, 152 insertions(+), 152 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index fe28d86f7c35..1fd07c10378f 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2291,67 +2291,6 @@ struct bpf_kprobe_multi_run_ctx {
unsigned long entry_ip;
};
-struct user_syms {
- const char **syms;
- char *buf;
-};
-
-#ifndef CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS
-static DEFINE_PER_CPU(struct pt_regs, bpf_kprobe_multi_pt_regs);
-#define bpf_kprobe_multi_pt_regs_ptr() this_cpu_ptr(&bpf_kprobe_multi_pt_regs)
-#else
-#define bpf_kprobe_multi_pt_regs_ptr() (NULL)
-#endif
-
-static unsigned long ftrace_get_entry_ip(unsigned long fentry_ip)
-{
- unsigned long ip = ftrace_get_symaddr(fentry_ip);
-
- return ip ? : fentry_ip;
-}
-
-static int copy_user_syms(struct user_syms *us, unsigned long __user *usyms, u32 cnt)
-{
- unsigned long __user usymbol;
- const char **syms = NULL;
- char *buf = NULL, *p;
- int err = -ENOMEM;
- unsigned int i;
-
- syms = kvmalloc_array(cnt, sizeof(*syms), GFP_KERNEL);
- if (!syms)
- goto error;
-
- buf = kvmalloc_array(cnt, KSYM_NAME_LEN, GFP_KERNEL);
- if (!buf)
- goto error;
-
- for (p = buf, i = 0; i < cnt; i++) {
- if (__get_user(usymbol, usyms + i)) {
- err = -EFAULT;
- goto error;
- }
- err = strncpy_from_user(p, (const char __user *) usymbol, KSYM_NAME_LEN);
- if (err == KSYM_NAME_LEN)
- err = -E2BIG;
- if (err < 0)
- goto error;
- syms[i] = p;
- p += err + 1;
- }
-
- us->syms = syms;
- us->buf = buf;
- return 0;
-
-error:
- if (err) {
- kvfree(syms);
- kvfree(buf);
- }
- return err;
-}
-
static void kprobe_multi_put_modules(struct module **mods, u32 cnt)
{
u32 i;
@@ -2360,12 +2299,6 @@ static void kprobe_multi_put_modules(struct module **mods, u32 cnt)
module_put(mods[i]);
}
-static void free_user_syms(struct user_syms *us)
-{
- kvfree(us->syms);
- kvfree(us->buf);
-}
-
static void bpf_kprobe_multi_link_release(struct bpf_link *link)
{
struct bpf_kprobe_multi_link *kmulti_link;
@@ -2469,6 +2402,152 @@ static const struct bpf_link_ops bpf_kprobe_multi_link_lops = {
#endif
};
+static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
+{
+ struct bpf_kprobe_multi_run_ctx *run_ctx;
+
+ run_ctx = container_of(current->bpf_ctx, struct bpf_kprobe_multi_run_ctx,
+ session_ctx.run_ctx);
+ return run_ctx->entry_ip;
+}
+
+struct modules_array {
+ struct module **mods;
+ int mods_cnt;
+ int mods_cap;
+};
+
+static int add_module(struct modules_array *arr, struct module *mod)
+{
+ struct module **mods;
+
+ if (arr->mods_cnt == arr->mods_cap) {
+ arr->mods_cap = max(16, arr->mods_cap * 3 / 2);
+ mods = krealloc_array(arr->mods, arr->mods_cap, sizeof(*mods), GFP_KERNEL);
+ if (!mods)
+ return -ENOMEM;
+ arr->mods = mods;
+ }
+
+ arr->mods[arr->mods_cnt] = mod;
+ arr->mods_cnt++;
+ return 0;
+}
+
+static bool has_module(struct modules_array *arr, struct module *mod)
+{
+ int i;
+
+ for (i = arr->mods_cnt - 1; i >= 0; i--) {
+ if (arr->mods[i] == mod)
+ return true;
+ }
+ return false;
+}
+
+static int get_modules_for_addrs(struct module ***mods, unsigned long *addrs, u32 addrs_cnt)
+{
+ struct modules_array arr = {};
+ u32 i, err = 0;
+
+ for (i = 0; i < addrs_cnt; i++) {
+ bool skip_add = false;
+ struct module *mod;
+
+ scoped_guard(rcu) {
+ mod = __module_address(addrs[i]);
+ /* Either no module or it's already stored */
+ if (!mod || has_module(&arr, mod)) {
+ skip_add = true;
+ break; /* scoped_guard */
+ }
+ if (!try_module_get(mod))
+ err = -EINVAL;
+ }
+ if (skip_add)
+ continue;
+ if (err)
+ break;
+ err = add_module(&arr, mod);
+ if (err) {
+ module_put(mod);
+ break;
+ }
+ }
+
+ /* We return either err < 0 in case of error, ... */
+ if (err) {
+ kprobe_multi_put_modules(arr.mods, arr.mods_cnt);
+ kfree(arr.mods);
+ return err;
+ }
+
+ /* or number of modules found if everything is ok. */
+ *mods = arr.mods;
+ return arr.mods_cnt;
+}
+
+struct user_syms {
+ const char **syms;
+ char *buf;
+};
+
+#ifndef CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS
+static DEFINE_PER_CPU(struct pt_regs, bpf_kprobe_multi_pt_regs);
+#define bpf_kprobe_multi_pt_regs_ptr() this_cpu_ptr(&bpf_kprobe_multi_pt_regs)
+#else
+#define bpf_kprobe_multi_pt_regs_ptr() (NULL)
+#endif
+
+static unsigned long ftrace_get_entry_ip(unsigned long fentry_ip)
+{
+ unsigned long ip = ftrace_get_symaddr(fentry_ip);
+
+ return ip ? : fentry_ip;
+}
+
+static int copy_user_syms(struct user_syms *us, unsigned long __user *usyms, u32 cnt)
+{
+ unsigned long __user usymbol;
+ const char **syms = NULL;
+ char *buf = NULL, *p;
+ int err = -ENOMEM;
+ unsigned int i;
+
+ syms = kvmalloc_array(cnt, sizeof(*syms), GFP_KERNEL);
+ if (!syms)
+ goto error;
+
+ buf = kvmalloc_array(cnt, KSYM_NAME_LEN, GFP_KERNEL);
+ if (!buf)
+ goto error;
+
+ for (p = buf, i = 0; i < cnt; i++) {
+ if (__get_user(usymbol, usyms + i)) {
+ err = -EFAULT;
+ goto error;
+ }
+ err = strncpy_from_user(p, (const char __user *) usymbol, KSYM_NAME_LEN);
+ if (err == KSYM_NAME_LEN)
+ err = -E2BIG;
+ if (err < 0)
+ goto error;
+ syms[i] = p;
+ p += err + 1;
+ }
+
+ us->syms = syms;
+ us->buf = buf;
+ return 0;
+
+error:
+ if (err) {
+ kvfree(syms);
+ kvfree(buf);
+ }
+ return err;
+}
+
static void bpf_kprobe_multi_cookie_swap(void *a, void *b, int size, const void *priv)
{
const struct bpf_kprobe_multi_link *link = priv;
@@ -2520,15 +2599,6 @@ static u64 bpf_kprobe_multi_cookie(struct bpf_run_ctx *ctx)
return *cookie;
}
-static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
-{
- struct bpf_kprobe_multi_run_ctx *run_ctx;
-
- run_ctx = container_of(current->bpf_ctx, struct bpf_kprobe_multi_run_ctx,
- session_ctx.run_ctx);
- return run_ctx->entry_ip;
-}
-
static __always_inline int
kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
unsigned long entry_ip, struct ftrace_regs *fregs,
@@ -2597,6 +2667,12 @@ kprobe_multi_link_exit_handler(struct fprobe *fp, unsigned long fentry_ip,
fregs, true, data);
}
+static void free_user_syms(struct user_syms *us)
+{
+ kvfree(us->syms);
+ kvfree(us->buf);
+}
+
static int symbols_cmp_r(const void *a, const void *b, const void *priv)
{
const char **str_a = (const char **) a;
@@ -2627,82 +2703,6 @@ static void symbols_swap_r(void *a, void *b, int size, const void *priv)
}
}
-struct modules_array {
- struct module **mods;
- int mods_cnt;
- int mods_cap;
-};
-
-static int add_module(struct modules_array *arr, struct module *mod)
-{
- struct module **mods;
-
- if (arr->mods_cnt == arr->mods_cap) {
- arr->mods_cap = max(16, arr->mods_cap * 3 / 2);
- mods = krealloc_array(arr->mods, arr->mods_cap, sizeof(*mods), GFP_KERNEL);
- if (!mods)
- return -ENOMEM;
- arr->mods = mods;
- }
-
- arr->mods[arr->mods_cnt] = mod;
- arr->mods_cnt++;
- return 0;
-}
-
-static bool has_module(struct modules_array *arr, struct module *mod)
-{
- int i;
-
- for (i = arr->mods_cnt - 1; i >= 0; i--) {
- if (arr->mods[i] == mod)
- return true;
- }
- return false;
-}
-
-static int get_modules_for_addrs(struct module ***mods, unsigned long *addrs, u32 addrs_cnt)
-{
- struct modules_array arr = {};
- u32 i, err = 0;
-
- for (i = 0; i < addrs_cnt; i++) {
- bool skip_add = false;
- struct module *mod;
-
- scoped_guard(rcu) {
- mod = __module_address(addrs[i]);
- /* Either no module or it's already stored */
- if (!mod || has_module(&arr, mod)) {
- skip_add = true;
- break; /* scoped_guard */
- }
- if (!try_module_get(mod))
- err = -EINVAL;
- }
- if (skip_add)
- continue;
- if (err)
- break;
- err = add_module(&arr, mod);
- if (err) {
- module_put(mod);
- break;
- }
- }
-
- /* We return either err < 0 in case of error, ... */
- if (err) {
- kprobe_multi_put_modules(arr.mods, arr.mods_cnt);
- kfree(arr.mods);
- return err;
- }
-
- /* or number of modules found if everything is ok. */
- *mods = arr.mods;
- return arr.mods_cnt;
-}
-
static int addrs_check_error_injection_list(unsigned long *addrs, u32 cnt)
{
u32 i;
--
2.25.1
^ permalink raw reply related
* [PATCH 0/2] bpf: Add kretprobe fallback for kprobe multi link
From: liujing40 @ 2025-12-18 13:06 UTC (permalink / raw)
To: ast, daniel, andrii, mhiramat, martin.lau, eddyz87, song,
yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa
Cc: bpf, linux-kernel, linux-trace-kernel, liujing40
This patch provides a fallback implementation of kprobe multi link
using the traditional kretprobe API When fprobe is not available.
This ensures compatibility with older kernels or platforms where
fprobe support is not compiled in.
Uses kretprobe's entry_handler and handler callbacks to simulate
fprobe's entry/exit functionality.The API remains identical to
fprobe-based implementation, allowing userspace tools to work
transparently with either backend. Cookie support, both entry
and return probes, and session handling are fully supported.
Jing Liu (2):
bpf: Prepare for kprobe multi link fallback patch
bpf: Implement kretprobe fallback for kprobe multi link
kernel/trace/bpf_trace.c | 603 ++++++++++++++++++++++++++++-----------
1 file changed, 443 insertions(+), 160 deletions(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH v8] dma-buf: add some tracepoints to debug.
From: Christian König @ 2025-12-18 10:25 UTC (permalink / raw)
To: Xiang Gao, sumit.semwal, rostedt, mhiramat
Cc: linux-media, dri-devel, linux-kernel, mathieu.desnoyers, dhowells,
kuba, brauner, akpm, linux-trace-kernel, gaoxiang17
In-Reply-To: <20251218062853.819744-1-gxxa03070307@gmail.com>
On 12/18/25 07:28, Xiang Gao wrote:
> From: gaoxiang17 <gaoxiang17@xiaomi.com>
>
> Since we can only inspect dmabuf by iterating over process FDs or the
> dmabuf_list, we need to add our own tracepoints to track its status in
> real time in production.
>
> For example:
> binder:3016_1-3102 [006] ...1. 255.126521: dma_buf_export: exp_name=qcom,system size=12685312 ino=2738
> binder:3016_1-3102 [006] ...1. 255.126528: dma_buf_fd: exp_name=qcom,system size=12685312 ino=2738 fd=8
> binder:3016_1-3102 [006] ...1. 255.126642: dma_buf_mmap_internal: exp_name=qcom,system size=28672 ino=2739
> kworker/6:1-86 [006] ...1. 255.127194: dma_buf_put: exp_name=qcom,system size=12685312 ino=2738
> RenderThread-9293 [006] ...1. 316.618179: dma_buf_get: exp_name=qcom,system size=12771328 ino=2762 fd=176
> RenderThread-9293 [006] ...1. 316.618195: dma_buf_dynamic_attach: exp_name=qcom,system size=12771328 ino=2762 attachment:ffffff880a18dd00 is_dynamic=0 dev_name=kgsl-3d0
> RenderThread-9293 [006] ...1. 318.878220: dma_buf_detach: exp_name=qcom,system size=12771328 ino=2762 attachment:ffffff880a18dd00 is_dynamic=0 dev_name=kgsl-3d0
>
> Signed-off-by: Xiang Gao <gaoxiang17@xiaomi.com>
This patch was still not properly rebased on drm-misc-next and there was some whitespace issues with the macro.
I've reviewed the patch, fixed the merge conflict and whitespace issues and pushed the result to drm-misc-next.
Thanks,
Christian.
> ---
> drivers/dma-buf/dma-buf.c | 51 ++++++++++-
> include/trace/events/dma_buf.h | 157 +++++++++++++++++++++++++++++++++
> 2 files changed, 206 insertions(+), 2 deletions(-)
> create mode 100644 include/trace/events/dma_buf.h
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index edaa9e4ee4ae..59a5261cb26b 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -35,6 +35,28 @@
>
> #include "dma-buf-sysfs-stats.h"
>
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/dma_buf.h>
> +
> +/*
> + * dmabuf->name must be accessed with holding dmabuf->name_lock.
> + * we need to take the lock around the tracepoint call itself where
> + * it is called in the code.
> + *
> + * Note: FUNC##_enabled() is a static branch that will only
> + * be set when the trace event is enabled.
> + */
> +#define DMA_BUF_TRACE(FUNC, ...) \
> + do { \
> + if (FUNC##_enabled()) { \
> + guard(spinlock)(&dmabuf->name_lock); \
> + FUNC(__VA_ARGS__); \
> + } else if (IS_ENABLED(CONFIG_LOCKDEP)) { \
> + /* Expose this lock when lockdep is enabled */ \
> + guard(spinlock)(&dmabuf->name_lock); \
> + } \
> + } while (0)
> +
> static inline int is_dma_buf_file(struct file *);
>
> static DEFINE_MUTEX(dmabuf_list_mutex);
> @@ -220,6 +242,8 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
> dmabuf->size >> PAGE_SHIFT)
> return -EINVAL;
>
> + DMA_BUF_TRACE(trace_dma_buf_mmap_internal, dmabuf);
> +
> return dmabuf->ops->mmap(dmabuf, vma);
> }
>
> @@ -745,6 +769,8 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>
> __dma_buf_list_add(dmabuf);
>
> + DMA_BUF_TRACE(trace_dma_buf_export, dmabuf);
> +
> return dmabuf;
>
> err_dmabuf:
> @@ -768,10 +794,16 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_export, "DMA_BUF");
> */
> int dma_buf_fd(struct dma_buf *dmabuf, int flags)
> {
> + int fd;
> +
> if (!dmabuf || !dmabuf->file)
> return -EINVAL;
>
> - return FD_ADD(flags, dmabuf->file);
> + fd = FD_ADD(flags, dmabuf->file);
> + if (fd >= 0)
> + DMA_BUF_TRACE(trace_dma_buf_fd, dmabuf, fd);
> +
> + return fd;
> }
> EXPORT_SYMBOL_NS_GPL(dma_buf_fd, "DMA_BUF");
>
> @@ -786,6 +818,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_fd, "DMA_BUF");
> struct dma_buf *dma_buf_get(int fd)
> {
> struct file *file;
> + struct dma_buf *dmabuf;
>
> file = fget(fd);
>
> @@ -797,7 +830,11 @@ struct dma_buf *dma_buf_get(int fd)
> return ERR_PTR(-EINVAL);
> }
>
> - return file->private_data;
> + dmabuf = file->private_data;
> +
> + DMA_BUF_TRACE(trace_dma_buf_get, dmabuf, fd);
> +
> + return dmabuf;
> }
> EXPORT_SYMBOL_NS_GPL(dma_buf_get, "DMA_BUF");
>
> @@ -817,6 +854,8 @@ void dma_buf_put(struct dma_buf *dmabuf)
> return;
>
> fput(dmabuf->file);
> +
> + DMA_BUF_TRACE(trace_dma_buf_put, dmabuf);
> }
> EXPORT_SYMBOL_NS_GPL(dma_buf_put, "DMA_BUF");
>
> @@ -971,6 +1010,9 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> list_add(&attach->node, &dmabuf->attachments);
> dma_resv_unlock(dmabuf->resv);
>
> + DMA_BUF_TRACE(trace_dma_buf_dynamic_attach, dmabuf, attach,
> + dma_buf_attachment_is_dynamic(attach), dev);
> +
> return attach;
>
> err_attach:
> @@ -1015,6 +1057,9 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
> if (dmabuf->ops->detach)
> dmabuf->ops->detach(dmabuf, attach);
>
> + DMA_BUF_TRACE(trace_dma_buf_detach, dmabuf, attach,
> + dma_buf_attachment_is_dynamic(attach), attach->dev);
> +
> kfree(attach);
> }
> EXPORT_SYMBOL_NS_GPL(dma_buf_detach, "DMA_BUF");
> @@ -1480,6 +1525,8 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
> vma_set_file(vma, dmabuf->file);
> vma->vm_pgoff = pgoff;
>
> + DMA_BUF_TRACE(trace_dma_buf_mmap, dmabuf);
> +
> return dmabuf->ops->mmap(dmabuf, vma);
> }
> EXPORT_SYMBOL_NS_GPL(dma_buf_mmap, "DMA_BUF");
> diff --git a/include/trace/events/dma_buf.h b/include/trace/events/dma_buf.h
> new file mode 100644
> index 000000000000..35f8140095f4
> --- /dev/null
> +++ b/include/trace/events/dma_buf.h
> @@ -0,0 +1,157 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM dma_buf
> +
> +#if !defined(_TRACE_DMA_BUF_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_DMA_BUF_H
> +
> +#include <linux/dma-buf.h>
> +#include <linux/tracepoint.h>
> +
> +DECLARE_EVENT_CLASS(dma_buf,
> +
> + TP_PROTO(struct dma_buf *dmabuf),
> +
> + TP_ARGS(dmabuf),
> +
> + TP_STRUCT__entry(
> + __string(exp_name, dmabuf->exp_name)
> + __field(size_t, size)
> + __field(ino_t, ino)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(exp_name);
> + __entry->size = dmabuf->size;
> + __entry->ino = dmabuf->file->f_inode->i_ino;
> + ),
> +
> + TP_printk("exp_name=%s size=%zu ino=%lu",
> + __get_str(exp_name),
> + __entry->size,
> + __entry->ino)
> +);
> +
> +DECLARE_EVENT_CLASS(dma_buf_attach_dev,
> +
> + TP_PROTO(struct dma_buf *dmabuf, struct dma_buf_attachment *attach,
> + bool is_dynamic, struct device *dev),
> +
> + TP_ARGS(dmabuf, attach, is_dynamic, dev),
> +
> + TP_STRUCT__entry(
> + __string(dev_name, dev_name(dev))
> + __string(exp_name, dmabuf->exp_name)
> + __field(size_t, size)
> + __field(ino_t, ino)
> + __field(struct dma_buf_attachment *, attach)
> + __field(bool, is_dynamic)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(dev_name);
> + __assign_str(exp_name);
> + __entry->size = dmabuf->size;
> + __entry->ino = dmabuf->file->f_inode->i_ino;
> + __entry->is_dynamic = is_dynamic;
> + __entry->attach = attach;
> + ),
> +
> + TP_printk("exp_name=%s size=%zu ino=%lu attachment:%p is_dynamic=%d dev_name=%s",
> + __get_str(exp_name),
> + __entry->size,
> + __entry->ino,
> + __entry->attach,
> + __entry->is_dynamic,
> + __get_str(dev_name))
> +);
> +
> +DECLARE_EVENT_CLASS(dma_buf_fd,
> +
> + TP_PROTO(struct dma_buf *dmabuf, int fd),
> +
> + TP_ARGS(dmabuf, fd),
> +
> + TP_STRUCT__entry(
> + __string(exp_name, dmabuf->exp_name)
> + __field(size_t, size)
> + __field(ino_t, ino)
> + __field(int, fd)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(exp_name);
> + __entry->size = dmabuf->size;
> + __entry->ino = dmabuf->file->f_inode->i_ino;
> + __entry->fd = fd;
> + ),
> +
> + TP_printk("exp_name=%s size=%zu ino=%lu fd=%d",
> + __get_str(exp_name),
> + __entry->size,
> + __entry->ino,
> + __entry->fd)
> +);
> +
> +DEFINE_EVENT(dma_buf, dma_buf_export,
> +
> + TP_PROTO(struct dma_buf *dmabuf),
> +
> + TP_ARGS(dmabuf)
> +);
> +
> +DEFINE_EVENT(dma_buf, dma_buf_mmap_internal,
> +
> + TP_PROTO(struct dma_buf *dmabuf),
> +
> + TP_ARGS(dmabuf)
> +);
> +
> +DEFINE_EVENT(dma_buf, dma_buf_mmap,
> +
> + TP_PROTO(struct dma_buf *dmabuf),
> +
> + TP_ARGS(dmabuf)
> +);
> +
> +DEFINE_EVENT(dma_buf, dma_buf_put,
> +
> + TP_PROTO(struct dma_buf *dmabuf),
> +
> + TP_ARGS(dmabuf)
> +);
> +
> +DEFINE_EVENT(dma_buf_attach_dev, dma_buf_dynamic_attach,
> +
> + TP_PROTO(struct dma_buf *dmabuf, struct dma_buf_attachment *attach,
> + bool is_dynamic, struct device *dev),
> +
> + TP_ARGS(dmabuf, attach, is_dynamic, dev)
> +);
> +
> +DEFINE_EVENT(dma_buf_attach_dev, dma_buf_detach,
> +
> + TP_PROTO(struct dma_buf *dmabuf, struct dma_buf_attachment *attach,
> + bool is_dynamic, struct device *dev),
> +
> + TP_ARGS(dmabuf, attach, is_dynamic, dev)
> +);
> +
> +DEFINE_EVENT(dma_buf_fd, dma_buf_fd,
> +
> + TP_PROTO(struct dma_buf *dmabuf, int fd),
> +
> + TP_ARGS(dmabuf, fd)
> +);
> +
> +DEFINE_EVENT(dma_buf_fd, dma_buf_get,
> +
> + TP_PROTO(struct dma_buf *dmabuf, int fd),
> +
> + TP_ARGS(dmabuf, fd)
> +);
> +
> +#endif /* _TRACE_DMA_BUF_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
^ permalink raw reply
* Re: [PATCH 3/3] mm/khugepaged: make khugepaged_collapse_control static
From: David Hildenbrand (Red Hat) @ 2025-12-18 9:18 UTC (permalink / raw)
To: Shivank Garg, Andrew Morton, Lorenzo Stoakes
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, linux-mm, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251216111139.95438-8-shivankg@amd.com>
On 12/16/25 12:11, Shivank Garg wrote:
> The global variable 'khugepaged_collapse_control' is not used outside of
> mm/khugepaged.c. Make it static to limit its scope.
>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
--
Cheers
David
^ permalink raw reply
* Re: [PATCH 2/3] mm/khugepaged: use enum scan_result for result variables
From: David Hildenbrand (Red Hat) @ 2025-12-18 9:17 UTC (permalink / raw)
To: Garg, Shivank, Zi Yan
Cc: Andrew Morton, Lorenzo Stoakes, Baolin Wang, Liam R . Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
linux-mm, linux-kernel, linux-trace-kernel
In-Reply-To: <79fb5cb2-8a24-4260-a9e9-bdf2c5ef602f@amd.com>
On 12/18/25 06:40, Garg, Shivank wrote:
>
>
> On 12/16/2025 9:08 PM, Zi Yan wrote:
>> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>>
>
>>
>>> int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>>> bool install_pmd)
>>> {
>>
>> And here.
> Since this function is declared in khugepaged.h, I need to
> move the enum definition to that header. I see two options for handling
> the CONFIG_TRANSPARENT_HUGEPAGE check:
>
> 1. Define enum OUTSIDE the ifdef: This allows the static inline stub
> to also return enum scan_result, keeping the API consistent.
> 2. Define enum INSIDE the ifdef: The enum is hidden when THP is disabled,
> forcing the stub to return int 0 instead.
>
> The only external caller (uprobes.c) of collapse_pte_mapped_thp currently
> ignores the return value.
Probably best to not expose that enum (especially when nobody cares ...)
and instead expose a new void function for uprobe purposes.
Maybe
void collapse_pte_mapped_thp(...)
{
try_collapse_pte_mapped_thp();
}
Maybe something like that?
--
Cheers
David
^ permalink raw reply
* Re: [PATCH 1/3] mm/khugepaged: remove unnecessary goto 'skip' label
From: David Hildenbrand (Red Hat) @ 2025-12-18 9:14 UTC (permalink / raw)
To: Shivank Garg, Andrew Morton, Lorenzo Stoakes
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, linux-mm, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251216111139.95438-4-shivankg@amd.com>
On 12/16/25 12:11, Shivank Garg wrote:
> Replace 'goto skip' with actual logic for better code readability.
>
> No functional change.
>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
> mm/khugepaged.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 6c8c35d3e0c9..107146f012b1 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -2442,14 +2442,15 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages, int *result,
> break;
> }
> if (!thp_vma_allowable_order(vma, vma->vm_flags, TVA_KHUGEPAGED, PMD_ORDER)) {
> -skip:
> progress++;
> continue;
> }
> hstart = round_up(vma->vm_start, HPAGE_PMD_SIZE);
> hend = round_down(vma->vm_end, HPAGE_PMD_SIZE);
> - if (khugepaged_scan.address > hend)
> - goto skip;
> + if (khugepaged_scan.address > hend) {
> + progress++;
> + continue;
> + }
> if (khugepaged_scan.address < hstart)
> khugepaged_scan.address = hstart;
> VM_BUG_ON(khugepaged_scan.address & ~HPAGE_PMD_MASK);
Oh yes, please
Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
--
Cheers
David
^ permalink raw reply
* Re: [PATCH v3 0/7] kallsyms: Prevent invalid access when showing module buildid
From: Petr Mladek @ 2025-12-18 9:09 UTC (permalink / raw)
To: Andrew Morton
Cc: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Kees Cook,
Aaron Tomlin, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel
In-Reply-To: <20251217130904.33163c243172324a5308efe9@linux-foundation.org>
On Wed 2025-12-17 13:09:04, Andrew Morton wrote:
> On Fri, 28 Nov 2025 14:59:13 +0100 Petr Mladek <pmladek@suse.com> wrote:
>
> > This patchset is cleaning up kallsyms code related to module buildid.
> > It is fixing an invalid access when printing backtraces, see [v1] for
> > more details:
> >
> > ...
> >
> > [v1] https://lore.kernel.org/r/20251105142319.1139183-1-pmladek@suse.com
> > [v2] https://lore.kernel.org/r/20251112142003.182062-1-pmladek@suse.com
> >
>
> It's best to avoid sending people off to the WWW to understand a
> patchset - better that the git history be self-contained.
I see. I'll do better next time.
> So when
> staging this for mm.git I scooped the relevant material from [1] and
> added it to your cover letter, as below. Looks OK?
It looks OK to me. Thanks for taking the patchset.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v3 10/13] rv: Add support for per-object monitors in DA/HA
From: Nam Cao @ 2025-12-18 7:26 UTC (permalink / raw)
To: Gabriele Monaco, linux-kernel, Steven Rostedt, Gabriele Monaco,
linux-trace-kernel
Cc: Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
In-Reply-To: <20251205131621.135513-11-gmonaco@redhat.com>
Gabriele Monaco <gmonaco@redhat.com> writes:
> RV deterministic and hybrid automata currently only support global,
> per-cpu and per-task monitors. It isn't possible to write a model that
> would follow some different type of object, like a deadline entity or a
> lock.
>
> Define the generic per-object monitor implementation which shares part
> of the implementation with the per-task monitors.
> The user needs to provide an id for the object (e.g. pid for tasks) and
> define the data type for the monitor_target (e.g. struct task_struct *
> for tasks). Both are supplied to the event handlers, as the id may not
> be easily available in the target.
>
> The monitor storage (e.g. the rv monitor, pointer to the target, etc.)
> is stored in a hash table indexed by id. Monitor storage objects are
> automatically allocated unless specified otherwise (e.g. if the creation
> context is unsafe for allocation).
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
^ permalink raw reply
* Re: [PATCH v3 07/13] rv: Convert the opid monitor to a hybrid automaton
From: Nam Cao @ 2025-12-18 6:50 UTC (permalink / raw)
To: Gabriele Monaco, linux-kernel, Steven Rostedt, Gabriele Monaco,
Jonathan Corbet, Masami Hiramatsu, linux-trace-kernel, linux-doc
Cc: Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
In-Reply-To: <20251205131621.135513-8-gmonaco@redhat.com>
Gabriele Monaco <gmonaco@redhat.com> writes:
> The opid monitor validates that wakeup and need_resched events only
> occur with interrupts and preemption disabled by following the
> preemptirq tracepoints.
> As reported in [1], those tracepoints might be inaccurate in some
> situations (e.g. NMIs).
>
> Since the monitor doesn't validate other ordering properties, remove the
> dependency on preemptirq tracepoints and convert the monitor to a hybrid
> automaton to validate the constraint during event handling.
> This makes the monitor more robust by also removing the workaround for
> interrupts missing the preemption tracepoints, which was working on
> PREEMPT_RT only and allows the monitor to be built on kernels without
> the preemptirqs tracepoints.
>
> [1] - https://lore.kernel.org/lkml/20250625120823.60600-1-gmonaco@redhat.com
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
^ permalink raw reply
* Re: [PATCH v3 06/13] rv: Add sample hybrid monitors stall
From: Nam Cao @ 2025-12-18 6:32 UTC (permalink / raw)
To: Gabriele Monaco, linux-kernel, Steven Rostedt, Jonathan Corbet,
Gabriele Monaco, Masami Hiramatsu, linux-doc, linux-trace-kernel
Cc: Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
In-Reply-To: <20251205131621.135513-7-gmonaco@redhat.com>
Gabriele Monaco <gmonaco@redhat.com> writes:
> Add a sample monitor to showcase hybrid/timed automata.
> The stall monitor identifies tasks stalled for longer than a threshold
> and reacts when that happens.
>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
^ permalink raw reply
* [PATCH v8] dma-buf: add some tracepoints to debug.
From: Xiang Gao @ 2025-12-18 6:28 UTC (permalink / raw)
To: sumit.semwal, christian.koenig, rostedt, mhiramat
Cc: linux-media, dri-devel, linux-kernel, mathieu.desnoyers, dhowells,
kuba, brauner, akpm, linux-trace-kernel, gaoxiang17
From: gaoxiang17 <gaoxiang17@xiaomi.com>
Since we can only inspect dmabuf by iterating over process FDs or the
dmabuf_list, we need to add our own tracepoints to track its status in
real time in production.
For example:
binder:3016_1-3102 [006] ...1. 255.126521: dma_buf_export: exp_name=qcom,system size=12685312 ino=2738
binder:3016_1-3102 [006] ...1. 255.126528: dma_buf_fd: exp_name=qcom,system size=12685312 ino=2738 fd=8
binder:3016_1-3102 [006] ...1. 255.126642: dma_buf_mmap_internal: exp_name=qcom,system size=28672 ino=2739
kworker/6:1-86 [006] ...1. 255.127194: dma_buf_put: exp_name=qcom,system size=12685312 ino=2738
RenderThread-9293 [006] ...1. 316.618179: dma_buf_get: exp_name=qcom,system size=12771328 ino=2762 fd=176
RenderThread-9293 [006] ...1. 316.618195: dma_buf_dynamic_attach: exp_name=qcom,system size=12771328 ino=2762 attachment:ffffff880a18dd00 is_dynamic=0 dev_name=kgsl-3d0
RenderThread-9293 [006] ...1. 318.878220: dma_buf_detach: exp_name=qcom,system size=12771328 ino=2762 attachment:ffffff880a18dd00 is_dynamic=0 dev_name=kgsl-3d0
Signed-off-by: Xiang Gao <gaoxiang17@xiaomi.com>
---
drivers/dma-buf/dma-buf.c | 51 ++++++++++-
include/trace/events/dma_buf.h | 157 +++++++++++++++++++++++++++++++++
2 files changed, 206 insertions(+), 2 deletions(-)
create mode 100644 include/trace/events/dma_buf.h
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index edaa9e4ee4ae..59a5261cb26b 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -35,6 +35,28 @@
#include "dma-buf-sysfs-stats.h"
+#define CREATE_TRACE_POINTS
+#include <trace/events/dma_buf.h>
+
+/*
+ * dmabuf->name must be accessed with holding dmabuf->name_lock.
+ * we need to take the lock around the tracepoint call itself where
+ * it is called in the code.
+ *
+ * Note: FUNC##_enabled() is a static branch that will only
+ * be set when the trace event is enabled.
+ */
+#define DMA_BUF_TRACE(FUNC, ...) \
+ do { \
+ if (FUNC##_enabled()) { \
+ guard(spinlock)(&dmabuf->name_lock); \
+ FUNC(__VA_ARGS__); \
+ } else if (IS_ENABLED(CONFIG_LOCKDEP)) { \
+ /* Expose this lock when lockdep is enabled */ \
+ guard(spinlock)(&dmabuf->name_lock); \
+ } \
+ } while (0)
+
static inline int is_dma_buf_file(struct file *);
static DEFINE_MUTEX(dmabuf_list_mutex);
@@ -220,6 +242,8 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
dmabuf->size >> PAGE_SHIFT)
return -EINVAL;
+ DMA_BUF_TRACE(trace_dma_buf_mmap_internal, dmabuf);
+
return dmabuf->ops->mmap(dmabuf, vma);
}
@@ -745,6 +769,8 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
__dma_buf_list_add(dmabuf);
+ DMA_BUF_TRACE(trace_dma_buf_export, dmabuf);
+
return dmabuf;
err_dmabuf:
@@ -768,10 +794,16 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_export, "DMA_BUF");
*/
int dma_buf_fd(struct dma_buf *dmabuf, int flags)
{
+ int fd;
+
if (!dmabuf || !dmabuf->file)
return -EINVAL;
- return FD_ADD(flags, dmabuf->file);
+ fd = FD_ADD(flags, dmabuf->file);
+ if (fd >= 0)
+ DMA_BUF_TRACE(trace_dma_buf_fd, dmabuf, fd);
+
+ return fd;
}
EXPORT_SYMBOL_NS_GPL(dma_buf_fd, "DMA_BUF");
@@ -786,6 +818,7 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_fd, "DMA_BUF");
struct dma_buf *dma_buf_get(int fd)
{
struct file *file;
+ struct dma_buf *dmabuf;
file = fget(fd);
@@ -797,7 +830,11 @@ struct dma_buf *dma_buf_get(int fd)
return ERR_PTR(-EINVAL);
}
- return file->private_data;
+ dmabuf = file->private_data;
+
+ DMA_BUF_TRACE(trace_dma_buf_get, dmabuf, fd);
+
+ return dmabuf;
}
EXPORT_SYMBOL_NS_GPL(dma_buf_get, "DMA_BUF");
@@ -817,6 +854,8 @@ void dma_buf_put(struct dma_buf *dmabuf)
return;
fput(dmabuf->file);
+
+ DMA_BUF_TRACE(trace_dma_buf_put, dmabuf);
}
EXPORT_SYMBOL_NS_GPL(dma_buf_put, "DMA_BUF");
@@ -971,6 +1010,9 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
list_add(&attach->node, &dmabuf->attachments);
dma_resv_unlock(dmabuf->resv);
+ DMA_BUF_TRACE(trace_dma_buf_dynamic_attach, dmabuf, attach,
+ dma_buf_attachment_is_dynamic(attach), dev);
+
return attach;
err_attach:
@@ -1015,6 +1057,9 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
if (dmabuf->ops->detach)
dmabuf->ops->detach(dmabuf, attach);
+ DMA_BUF_TRACE(trace_dma_buf_detach, dmabuf, attach,
+ dma_buf_attachment_is_dynamic(attach), attach->dev);
+
kfree(attach);
}
EXPORT_SYMBOL_NS_GPL(dma_buf_detach, "DMA_BUF");
@@ -1480,6 +1525,8 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
vma_set_file(vma, dmabuf->file);
vma->vm_pgoff = pgoff;
+ DMA_BUF_TRACE(trace_dma_buf_mmap, dmabuf);
+
return dmabuf->ops->mmap(dmabuf, vma);
}
EXPORT_SYMBOL_NS_GPL(dma_buf_mmap, "DMA_BUF");
diff --git a/include/trace/events/dma_buf.h b/include/trace/events/dma_buf.h
new file mode 100644
index 000000000000..35f8140095f4
--- /dev/null
+++ b/include/trace/events/dma_buf.h
@@ -0,0 +1,157 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM dma_buf
+
+#if !defined(_TRACE_DMA_BUF_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_DMA_BUF_H
+
+#include <linux/dma-buf.h>
+#include <linux/tracepoint.h>
+
+DECLARE_EVENT_CLASS(dma_buf,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf),
+
+ TP_STRUCT__entry(
+ __string(exp_name, dmabuf->exp_name)
+ __field(size_t, size)
+ __field(ino_t, ino)
+ ),
+
+ TP_fast_assign(
+ __assign_str(exp_name);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ ),
+
+ TP_printk("exp_name=%s size=%zu ino=%lu",
+ __get_str(exp_name),
+ __entry->size,
+ __entry->ino)
+);
+
+DECLARE_EVENT_CLASS(dma_buf_attach_dev,
+
+ TP_PROTO(struct dma_buf *dmabuf, struct dma_buf_attachment *attach,
+ bool is_dynamic, struct device *dev),
+
+ TP_ARGS(dmabuf, attach, is_dynamic, dev),
+
+ TP_STRUCT__entry(
+ __string(dev_name, dev_name(dev))
+ __string(exp_name, dmabuf->exp_name)
+ __field(size_t, size)
+ __field(ino_t, ino)
+ __field(struct dma_buf_attachment *, attach)
+ __field(bool, is_dynamic)
+ ),
+
+ TP_fast_assign(
+ __assign_str(dev_name);
+ __assign_str(exp_name);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->is_dynamic = is_dynamic;
+ __entry->attach = attach;
+ ),
+
+ TP_printk("exp_name=%s size=%zu ino=%lu attachment:%p is_dynamic=%d dev_name=%s",
+ __get_str(exp_name),
+ __entry->size,
+ __entry->ino,
+ __entry->attach,
+ __entry->is_dynamic,
+ __get_str(dev_name))
+);
+
+DECLARE_EVENT_CLASS(dma_buf_fd,
+
+ TP_PROTO(struct dma_buf *dmabuf, int fd),
+
+ TP_ARGS(dmabuf, fd),
+
+ TP_STRUCT__entry(
+ __string(exp_name, dmabuf->exp_name)
+ __field(size_t, size)
+ __field(ino_t, ino)
+ __field(int, fd)
+ ),
+
+ TP_fast_assign(
+ __assign_str(exp_name);
+ __entry->size = dmabuf->size;
+ __entry->ino = dmabuf->file->f_inode->i_ino;
+ __entry->fd = fd;
+ ),
+
+ TP_printk("exp_name=%s size=%zu ino=%lu fd=%d",
+ __get_str(exp_name),
+ __entry->size,
+ __entry->ino,
+ __entry->fd)
+);
+
+DEFINE_EVENT(dma_buf, dma_buf_export,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf)
+);
+
+DEFINE_EVENT(dma_buf, dma_buf_mmap_internal,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf)
+);
+
+DEFINE_EVENT(dma_buf, dma_buf_mmap,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf)
+);
+
+DEFINE_EVENT(dma_buf, dma_buf_put,
+
+ TP_PROTO(struct dma_buf *dmabuf),
+
+ TP_ARGS(dmabuf)
+);
+
+DEFINE_EVENT(dma_buf_attach_dev, dma_buf_dynamic_attach,
+
+ TP_PROTO(struct dma_buf *dmabuf, struct dma_buf_attachment *attach,
+ bool is_dynamic, struct device *dev),
+
+ TP_ARGS(dmabuf, attach, is_dynamic, dev)
+);
+
+DEFINE_EVENT(dma_buf_attach_dev, dma_buf_detach,
+
+ TP_PROTO(struct dma_buf *dmabuf, struct dma_buf_attachment *attach,
+ bool is_dynamic, struct device *dev),
+
+ TP_ARGS(dmabuf, attach, is_dynamic, dev)
+);
+
+DEFINE_EVENT(dma_buf_fd, dma_buf_fd,
+
+ TP_PROTO(struct dma_buf *dmabuf, int fd),
+
+ TP_ARGS(dmabuf, fd)
+);
+
+DEFINE_EVENT(dma_buf_fd, dma_buf_get,
+
+ TP_PROTO(struct dma_buf *dmabuf, int fd),
+
+ TP_ARGS(dmabuf, fd)
+);
+
+#endif /* _TRACE_DMA_BUF_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 4/4] tracing: move tracing declarations from kernel.h to a dedicated header
From: Randy Dunlap @ 2025-12-18 5:59 UTC (permalink / raw)
To: Yury Norov
Cc: Andrew Morton, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Andy Shevchenko, Christophe Leroy, Ingo Molnar,
Jani Nikula, Joonas Lahtinen, David Laight, Petr Pavlu,
Andi Shyti, Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
linux-kernel, intel-gfx, dri-devel, linux-modules,
linux-trace-kernel, Kees Cook
In-Reply-To: <aUN8Hm377C5A0ILX@yury>
On 12/17/25 7:59 PM, Yury Norov wrote:
> On Tue, Dec 16, 2025 at 09:24:55PM -0800, Randy Dunlap wrote:
>> [adding Kees]
>>
>> On 12/16/25 4:13 PM, Andrew Morton wrote:
>>> On Fri, 5 Dec 2025 12:52:35 -0500 "Yury Norov (NVIDIA)" <yury.norov@gmail.com> wrote:
>>>
>>>> Tracing is a half of the kernel.h in terms of LOCs, although it's
>>>> a self-consistent part. It is intended for quick debugging purposes
>>>> and isn't used by the normal tracing utilities.
>>>>
>>>> Move it to a separate header. If someone needs to just throw a
>>>> trace_printk() in their driver, they will not have to pull all
>>>> the heavy tracing machinery.
>>>>
>>>> This is a pure move, except for removing a few 'extern's.
>>>>
>>
>> Hm, for a pure move, this shouldn't be necessary. Anyway, not using
>> FORTIFY in purgatory.o fixes this build error.
>> Or maybe there's a better answer.
>>
>> ---
>> arch/x86/purgatory/Makefile | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- a/arch/x86/purgatory/Makefile
>> +++ b/arch/x86/purgatory/Makefile
>> @@ -62,7 +62,7 @@ PURGATORY_CFLAGS_REMOVE += $(CC_FLAGS_C
>> endif
>>
>> CFLAGS_REMOVE_purgatory.o += $(PURGATORY_CFLAGS_REMOVE)
>> -CFLAGS_purgatory.o += $(PURGATORY_CFLAGS)
>> +CFLAGS_purgatory.o += $(PURGATORY_CFLAGS) -D__NO_FORTIFY
>>
>> CFLAGS_REMOVE_sha256.o += $(PURGATORY_CFLAGS_REMOVE)
>> CFLAGS_sha256.o += $(PURGATORY_CFLAGS)
>
> That happened because the new trace_printk.h includes string.h for
> strlen(), so all kernel.h users now indirectly include it, and it
> causes, seemingly, a circular dependency if FORTIFY is enabled.
>
> A fix would be dropping trace_printk.h from kernel.h, or switching the
> only user of string.h, trace_puts(), to __builtin_strlen().
>
> Notice, Andy has concerned about this on the previous round, and also
> suggested __builtin_strlen():
>
> https://lkml.org/lkml/2025/12/3/910
>
> I deem to drop trace_printk.h from kernel.h - it is more aligned with
> the idea of unloading the header. The original motivation to keep
> trace_printk.h in kernel.h was just because a similar printk.h is living
> there. But after all, this is a purely debugging header, so no need for
> almost every C file to bear debugging stuff.
>
> I can actually do both - switch to an intrinsic and drop the header.
>
> Guys, please let me know what do you thing.
There are some problems with using __builtin_mem{cpy,set} -- don't
know about __builtin_str{whatever}. See
commit 4ce97317f41d
Author: Nick Desaulniers <nick.desaulniers+lkml@gmail.com>
Date: Wed Aug 7 15:15:32 2019 -0700
x86/purgatory: Do not use __builtin_memcpy and __builtin_memset
We should drop the header from kernel.h soon anyway, whether now or
in a few weeks/months. IMHO.
--
~Randy
^ permalink raw reply
* Re: [PATCH v3 02/13] rv: Add Hybrid Automata monitor type
From: Nam Cao @ 2025-12-18 5:57 UTC (permalink / raw)
To: Gabriele Monaco, linux-kernel, Steven Rostedt, Gabriele Monaco,
Masami Hiramatsu, linux-trace-kernel
Cc: Tomas Glozar, Juri Lelli, Clark Williams, John Kacur
In-Reply-To: <20251205131621.135513-3-gmonaco@redhat.com>
Gabriele Monaco <gmonaco@redhat.com> writes:
> +static inline void ha_start_timer_ns(struct ha_monitor *ha_mon, enum envs env,
> + u64 expire, u64 time_ns)
> +{
> + u64 passed = ha_invariant_passed_ns(ha_mon, env, expire, time_ns);
> +
> + ha_start_timer_jiffy(ha_mon, ENV_MAX_STORED,
> + nsecs_to_jiffies(expire - passed) + 1, time_ns);
What is the +1 for? I guess to prevent it being rounded down to zero? If
so, maybe we should do
nsecs_to_jiffies(expire - passed + TICK_NSEC - 1)
instead.
Anyway, no big deal:
Reviewed-by: Nam Cao <namcao@linutronix.de>
^ permalink raw reply
* Re: [PATCH 2/3] mm/khugepaged: use enum scan_result for result variables
From: Garg, Shivank @ 2025-12-18 5:40 UTC (permalink / raw)
To: Zi Yan
Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Baolin Wang,
Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, linux-mm, linux-kernel, linux-trace-kernel
In-Reply-To: <D9E6B402-528B-46F2-B4B0-9CD9549A449F@nvidia.com>
On 12/16/2025 9:08 PM, Zi Yan wrote:
> On 16 Dec 2025, at 6:11, Shivank Garg wrote:
>
>
>> int collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>> bool install_pmd)
>> {
>
> And here.
Since this function is declared in khugepaged.h, I need to
move the enum definition to that header. I see two options for handling
the CONFIG_TRANSPARENT_HUGEPAGE check:
1. Define enum OUTSIDE the ifdef: This allows the static inline stub
to also return enum scan_result, keeping the API consistent.
2. Define enum INSIDE the ifdef: The enum is hidden when THP is disabled,
forcing the stub to return int 0 instead.
The only external caller (uprobes.c) of collapse_pte_mapped_thp currently
ignores the return value.
What is the more preferred way for enum definition in such scenario?
Thanks,
Shivank
> In addition, the return types of find_pmd_or_thp_or_none(),
> hugepage_vma_revalidate(), alloc_charge_folio(), and check_pmd_state() need to be changed to enum scan_result too.
>
> Best Regards,
> Yan, Zi
^ permalink raw reply
* Re: [PATCH v3 4/4] tracing: move tracing declarations from kernel.h to a dedicated header
From: Yury Norov @ 2025-12-18 3:59 UTC (permalink / raw)
To: Randy Dunlap
Cc: Andrew Morton, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Andy Shevchenko, Christophe Leroy, Ingo Molnar,
Jani Nikula, Joonas Lahtinen, David Laight, Petr Pavlu,
Andi Shyti, Rodrigo Vivi, Tvrtko Ursulin, Daniel Gomez,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
linux-kernel, intel-gfx, dri-devel, linux-modules,
linux-trace-kernel, Kees Cook
In-Reply-To: <55ceb7bf-0fe9-4edc-81c2-d51366847eec@infradead.org>
On Tue, Dec 16, 2025 at 09:24:55PM -0800, Randy Dunlap wrote:
> [adding Kees]
>
> On 12/16/25 4:13 PM, Andrew Morton wrote:
> > On Fri, 5 Dec 2025 12:52:35 -0500 "Yury Norov (NVIDIA)" <yury.norov@gmail.com> wrote:
> >
> >> Tracing is a half of the kernel.h in terms of LOCs, although it's
> >> a self-consistent part. It is intended for quick debugging purposes
> >> and isn't used by the normal tracing utilities.
> >>
> >> Move it to a separate header. If someone needs to just throw a
> >> trace_printk() in their driver, they will not have to pull all
> >> the heavy tracing machinery.
> >>
> >> This is a pure move, except for removing a few 'extern's.
> >>
>
> Hm, for a pure move, this shouldn't be necessary. Anyway, not using
> FORTIFY in purgatory.o fixes this build error.
> Or maybe there's a better answer.
>
> ---
> arch/x86/purgatory/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/arch/x86/purgatory/Makefile
> +++ b/arch/x86/purgatory/Makefile
> @@ -62,7 +62,7 @@ PURGATORY_CFLAGS_REMOVE += $(CC_FLAGS_C
> endif
>
> CFLAGS_REMOVE_purgatory.o += $(PURGATORY_CFLAGS_REMOVE)
> -CFLAGS_purgatory.o += $(PURGATORY_CFLAGS)
> +CFLAGS_purgatory.o += $(PURGATORY_CFLAGS) -D__NO_FORTIFY
>
> CFLAGS_REMOVE_sha256.o += $(PURGATORY_CFLAGS_REMOVE)
> CFLAGS_sha256.o += $(PURGATORY_CFLAGS)
That happened because the new trace_printk.h includes string.h for
strlen(), so all kernel.h users now indirectly include it, and it
causes, seemingly, a circular dependency if FORTIFY is enabled.
A fix would be dropping trace_printk.h from kernel.h, or switching the
only user of string.h, trace_puts(), to __builtin_strlen().
Notice, Andy has concerned about this on the previous round, and also
suggested __builtin_strlen():
https://lkml.org/lkml/2025/12/3/910
I deem to drop trace_printk.h from kernel.h - it is more aligned with
the idea of unloading the header. The original motivation to keep
trace_printk.h in kernel.h was just because a similar printk.h is living
there. But after all, this is a purely debugging header, so no need for
almost every C file to bear debugging stuff.
I can actually do both - switch to an intrinsic and drop the header.
Guys, please let me know what do you thing.
Thanks,
Yury
^ permalink raw reply
* Re: [PATCH 12/12] mm/damon/core: add trace point for damos stat per apply interval
From: SeongJae Park @ 2025-12-18 3:04 UTC (permalink / raw)
To: Andrew Morton
Cc: SeongJae Park, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, damon, linux-kernel, linux-mm,
linux-trace-kernel
In-Reply-To: <20251217182915.5284d66b58ac7b744b8dcf7f@linux-foundation.org>
On Wed, 17 Dec 2025 18:29:15 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 17 Dec 2025 15:52:18 -0800 SeongJae Park <sj@kernel.org> wrote:
>
> > On Wed, 17 Dec 2025 17:48:51 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > > On Tue, 16 Dec 2025 00:01:25 -0800
> > > SeongJae Park <sj@kernel.org> wrote:
> > >
> > > > + TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
> > > > + "nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
> > > > + "qt_exceeds=%lu nr_snapshots=%lu",
> > >
> > > Nit, but it's been stated that strings should not be broken up because of
> > > the column limit.
>
> screw the rules
>
> > --- a/include/trace/events/damon.h
> > +++ b/include/trace/events/damon.h
> > @@ -40,9 +40,7 @@ TRACE_EVENT(damos_stat_after_apply_interval,
> > __entry->nr_snapshots = stat->nr_snapshots;
> > ),
> >
> > - TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
> > - "nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
> > - "qt_exceeds=%lu nr_snapshots=%lu",
> > + TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu qt_exceeds=%lu nr_snapshots=%lu",
> > __entry->context_idx, __entry->scheme_idx,
> > __entry->nr_tried, __entry->sz_tried,
> > __entry->nr_applied, __entry->sz_applied,
>
> because that's just crazy. Let's use some judgment here!
I'm fine with either direction. So I understand you want to just keep the
original patch without this fixup, and therefore no action is needed from my
side? Let me know if I'm getting anything wrong.
Thanks,
SJ
[...]
^ permalink raw reply
* Re: [PATCH 12/12] mm/damon/core: add trace point for damos stat per apply interval
From: Andrew Morton @ 2025-12-18 2:29 UTC (permalink / raw)
To: SeongJae Park
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, damon,
linux-kernel, linux-mm, linux-trace-kernel
In-Reply-To: <20251217235219.2750-1-sj@kernel.org>
On Wed, 17 Dec 2025 15:52:18 -0800 SeongJae Park <sj@kernel.org> wrote:
> On Wed, 17 Dec 2025 17:48:51 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > On Tue, 16 Dec 2025 00:01:25 -0800
> > SeongJae Park <sj@kernel.org> wrote:
> >
> > > + TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
> > > + "nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
> > > + "qt_exceeds=%lu nr_snapshots=%lu",
> >
> > Nit, but it's been stated that strings should not be broken up because of
> > the column limit.
screw the rules
> --- a/include/trace/events/damon.h
> +++ b/include/trace/events/damon.h
> @@ -40,9 +40,7 @@ TRACE_EVENT(damos_stat_after_apply_interval,
> __entry->nr_snapshots = stat->nr_snapshots;
> ),
>
> - TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
> - "nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
> - "qt_exceeds=%lu nr_snapshots=%lu",
> + TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu qt_exceeds=%lu nr_snapshots=%lu",
> __entry->context_idx, __entry->scheme_idx,
> __entry->nr_tried, __entry->sz_tried,
> __entry->nr_applied, __entry->sz_applied,
because that's just crazy. Let's use some judgment here!
^ permalink raw reply
* Re: [PATCHv5 bpf-next 5/9] ftrace: Add update_ftrace_direct_del function
From: Steven Rostedt @ 2025-12-18 1:48 UTC (permalink / raw)
To: Jiri Olsa
Cc: Florent Revest, Mark Rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu
In-Reply-To: <20251215211402.353056-6-jolsa@kernel.org>
On Mon, 15 Dec 2025 22:13:58 +0100
Jiri Olsa <jolsa@kernel.org> wrote:
> +/**
> + * hash_sub - substracts @b from @a and returns the result
> + * @a: struct ftrace_hash object
> + * @b: struct ftrace_hash object
> + *
> + * Returns struct ftrace_hash object on success, NULL on error.
> + */
> +static struct ftrace_hash *hash_sub(struct ftrace_hash *a, struct ftrace_hash *b)
> +{
> + struct ftrace_func_entry *entry, *del;
> + struct ftrace_hash *sub;
> + int size, i;
> +
> + sub = alloc_and_copy_ftrace_hash(a->size_bits, a);
> + if (!sub)
> + goto error;
Again, this can be just return NULL;
> +
> + size = 1 << b->size_bits;
> + for (i = 0; i < size; i++) {
You can make this for (int i = 0; ...) too.
> + hlist_for_each_entry(entry, &b->buckets[i], hlist) {
> + del = __ftrace_lookup_ip(sub, entry->ip);
> + if (WARN_ON_ONCE(!del))
> + goto error;
And you can remove the error label here too:
if (WARN_ON_ONCE(!del)) {
free_ftrace_hash(sub);
return NULL;
}
> + remove_hash_entry(sub, del);
> + kfree(del);
> + }
> + }
> + return sub;
> +
> + error:
> + free_ftrace_hash(sub);
> + return NULL;
> +}
> +
> +int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
> +{
> + struct ftrace_hash *old_direct_functions = NULL, *new_direct_functions;
> + struct ftrace_hash *old_filter_hash, *new_filter_hash = NULL;
> + struct ftrace_func_entry *del, *entry;
One variable per line.
> + unsigned long size, i;
> + int err = -EINVAL;
> +
> + if (!hash_count(hash))
> + return -EINVAL;
> + if (check_direct_multi(ops))
> + return -EINVAL;
> + if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
> + return -EINVAL;
> + if (direct_functions == EMPTY_HASH)
> + return -EINVAL;
> +
> + mutex_lock(&direct_mutex);
> +
> + old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
> +
> + if (!hash_count(old_filter_hash))
> + goto out_unlock;
> +
> + /* Make sure requested entries are already registered. */
> + size = 1 << hash->size_bits;
> + for (i = 0; i < size; i++) {
for (int i = 0; ...)
-- Steve
> + hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
> + del = __ftrace_lookup_ip(direct_functions, entry->ip);
> + if (!del || del->direct != entry->direct)
> + goto out_unlock;
> + }
> + }
> +
> + err = -ENOMEM;
> + new_filter_hash = hash_sub(old_filter_hash, hash);
> + if (!new_filter_hash)
> + goto out_unlock;
> +
> + new_direct_functions = hash_sub(direct_functions, hash);
> + if (!new_direct_functions)
> + goto out_unlock;
> +
> + /* If there's nothing left, we need to unregister the ops. */
> + if (ftrace_hash_empty(new_filter_hash)) {
> + err = unregister_ftrace_function(ops);
> + if (!err) {
> + /* cleanup for possible another register call */
> + ops->func = NULL;
> + ops->trampoline = 0;
> + ftrace_free_filter(ops);
> + ops->func_hash->filter_hash = NULL;
> + }
> + } else {
> + err = ftrace_update_ops(ops, new_filter_hash, EMPTY_HASH);
> + /*
> + * new_filter_hash is dup-ed, so we need to release it anyway,
> + * old_filter_hash either stays on error or is already released
> + */
> + }
> +
> + if (err) {
> + /* free the new_direct_functions */
> + old_direct_functions = new_direct_functions;
> + } else {
> + rcu_assign_pointer(direct_functions, new_direct_functions);
> + }
> +
> + out_unlock:
> + mutex_unlock(&direct_mutex);
> +
> + if (old_direct_functions && old_direct_functions != EMPTY_HASH)
> + call_rcu_tasks(&old_direct_functions->rcu, register_ftrace_direct_cb);
> + free_ftrace_hash(new_filter_hash);
> +
> + return err;
> +}
> +
> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
>
> /**
^ 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