Linux Trace Kernel
 help / color / mirror / Atom feed
* Re: [PATCH v3 04/11] arm64/mm: Add set_memory_device() and set_memory_normal()
From: Will Deacon @ 2026-07-03 17:13 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Hunter,
	David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Sowjanya Komatineni, Luca Ceresoli,
	Mikko Perttunen, Yury Norov, Rasmus Villemoes, Russell King,
	Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, Sven Schnelle, Andrew Morton,
	David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
	Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
	Marek Szyprowski, Robin Murphy, Sumit Semwal, Benjamin Gaignard,
	Brian Starkey, John Stultz, T.J. Mercier, Christian König,
	Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	Catalin Marinas, Thierry Reding, devicetree, linux-tegra,
	linux-kernel, dri-devel, linux-media, linux-arm-kernel,
	linux-s390, linux-mm, iommu, linaro-mm-sig, linux-trace-kernel,
	Thierry Reding, Chun Ng
In-Reply-To: <akaSJ5D98w2cHqb6@orome>

On Thu, Jul 02, 2026 at 06:41:23PM +0200, Thierry Reding wrote:
> On Thu, Jul 02, 2026 at 03:46:44PM +0200, Thierry Reding wrote:
> > On Thu, Jul 02, 2026 at 10:18:47AM +0100, Will Deacon wrote:
> > > On Wed, Jul 01, 2026 at 06:08:15PM +0200, Thierry Reding wrote:
> > > > From: Chun Ng <chunn@nvidia.com>
> > > > 
> > > > Add helpers to swap PROT_NORMAL and PROT_DEVICE_nGnRnE protection bits
> > > > on a kernel-linear-map range.
> > > 
> > > That sounds like a really terrible idea. Why is this necessary and how
> > > does it interact with things like load_unaligned_zeropad()?
> > 
> > This is necessary because once the memory controller has walled off the
> > new memory region the CPU must not access it under any circumstances or
> > it'll cause the CPU to lock up (I think technically it'll hit an SError
> > but in practice that just means it'll freeze, as far as I can tell).
> > 
> > Probably doesn't interact well at all with load_unaligned_zeropad().
> > 
> > > I think you should unmap the memory from the linear map and memremap()
> > > it instead.
> > 
> > Given that the memory can never be accessed by the CPU after the memory
> > controller locks it down, I don't think we'll even need memremap(). The
> > only thing we really need is the sg_table we hand out via the DMA BUFs
> > so that they can be used by device drivers to program their DMA engines
> > internally.
> > 
> > Looking through some of the architecture code around this, shouldn't we
> > simply be using set_memory_encrypted() and set_memory_decrypted() for
> > this? While they might've been created for slightly other use-cases,
> > they seem to be doing exactly what we want (i.e. remove the page range
> > from the linear mapping and flushing it, or restoring the valid bit and
> > standard permissions, respectively).
> 
> Ah... I guess we can't do it because we're not in a realm world and so
> the early checks in __set_memory_enc_dec() would return early and turn
> it into a no-op.
> 
> How about if I extract a common helper and provide set_memory_p() and
> set_memory_np() in terms of those. Those are available on x86 and
> PowerPC as well, so fairly standard. I suppose at that point we're
> closer to set_memory_valid().

Why not just call set_direct_map_invalid_noflush() +
flush_tlb_kernel_range() for each page? We already have APIs for this.

The big challenge I see with any linear map manipulation, however, is
that it will rely on can_set_direct_map() which likely means you need to
give up some performance and/or security to make this work. Does memory
become inaccesible dynamically at runtime? If not, the best bet would
be to describe it as a carveout in the DT and mark it as "no-map" so
we avoid mapping it in the first place.

Will

^ permalink raw reply

* Re: [syzbot] [trace?] [usb?] general protection fault in mas_start (2)
From: Oleg Nesterov @ 2026-07-03 16:30 UTC (permalink / raw)
  To: syzbot
  Cc: bp, dave.hansen, hpa, linux-kernel, linux-trace-kernel, linux-usb,
	mhiramat, mingo, peterz, syzkaller-bugs, tglx, x86
In-Reply-To: <6a47dd16.6912059f.e0473.000e.GAE@google.com>

On 07/03, syzbot wrote:
>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit:    dc59e4fea9d8 Linux 7.2-rc1
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=12610e1c580000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=3c3d59be33cf7e9a
> dashboard link: https://syzkaller.appspot.com/bug?extid=0255c92c72505dcacba6
> compiler:       Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=143f8261580000
>
> Downloadable assets:
> disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-dc59e4fe.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/9ee1f0ea24f2/vmlinux-dc59e4fe.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/729e963a1370/bzImage-dc59e4fe.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+0255c92c72505dcacba6@syzkaller.appspotmail.com

...

> If the report is a duplicate of another one, reply with:
> #syz dup: exact-subject-of-another-report

#syz dup: [syzbot] [trace?] general protection fault in mtree_load

Oleg.


^ permalink raw reply

* Re: [PATCH] tracing: make tracepoint_printk static as not exported
From: Ben Dooks @ 2026-07-03 16:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel
In-Reply-To: <20260702184658.0e0ffb02@gandalf.local.home>

On 02/07/2026 23:46, Steven Rostedt wrote:
> On Wed, 17 Jun 2026 11:58:22 +0100
> Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> 
>> The tracepoint_printk symbol is not exported, so make it
>> static to remove the following sparse warning:
>>
>> kernel/trace/trace.c:90:5: warning: symbol 'tracepoint_printk' was not declared. Should it be static?
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> 
> Thanks, this became only used by this file via commit dd293df6395a2
> ("tracing: Move trace sysctls into trace.c")
> 
> I may add a Fixes tag to that commit.
> 
> -- Steve

ok, thanks.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

^ permalink raw reply

* [syzbot] [trace?] [usb?] general protection fault in mas_start (2)
From: syzbot @ 2026-07-03 16:02 UTC (permalink / raw)
  To: bp, dave.hansen, hpa, linux-kernel, linux-trace-kernel, linux-usb,
	mhiramat, mingo, oleg, peterz, syzkaller-bugs, tglx, x86

Hello,

syzbot found the following issue on:

HEAD commit:    dc59e4fea9d8 Linux 7.2-rc1
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12610e1c580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=3c3d59be33cf7e9a
dashboard link: https://syzkaller.appspot.com/bug?extid=0255c92c72505dcacba6
compiler:       Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=143f8261580000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-dc59e4fe.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/9ee1f0ea24f2/vmlinux-dc59e4fe.xz
kernel image: https://storage.googleapis.com/syzbot-assets/729e963a1370/bzImage-dc59e4fe.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+0255c92c72505dcacba6@syzkaller.appspotmail.com

Oops: general protection fault, probably for non-canonical address 0xdffffc0000000011: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000088-0x000000000000008f]
CPU: 0 UID: 0 PID: 5746 Comm: syz.3.20 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
RIP: 0010:mas_root lib/maple_tree.c:759 [inline]
RIP: 0010:mas_start+0xfe/0x550 lib/maple_tree.c:1179
Code: 4c 02 00 00 c6 43 4c 00 48 8b 44 24 28 42 80 3c 30 00 74 08 48 89 df e8 70 61 87 f6 4c 8b 23 49 83 c4 48 4c 89 e0 48 c1 e8 03 <42> 80 3c 30 00 74 08 4c 89 e7 e8 53 61 87 f6 4d 8b 24 24 e8 aa 9b
RSP: 0000:ffffc900034ff898 EFLAGS: 00010206
RAX: 0000000000000011 RBX: ffffc900034ff960 RCX: 1ffff9200069ff35
RDX: 1ffff9200069ff2c RSI: 0000000000000001 RDI: ffffc900034ff988
RBP: 0000000000000001 R08: ffffffff8baf8bfa R09: 0000000000000000
R10: 0000000000000000 R11: ffffffff8e959c20 R12: 0000000000000088
R13: ffffc900034ff960 R14: dffffc0000000000 R15: 1ffff9200069ff35
FS:  0000000000000000(0000) GS:ffff88808c815000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f53e2bc58d2 CR3: 000000000e746000 CR4: 0000000000352ef0
Call Trace:
 <TASK>
 mtree_load+0x1f5/0x780 lib/maple_tree.c:5657
 vma_lookup include/linux/mm.h:4238 [inline]
 __in_uprobe_trampoline arch/x86/kernel/uprobes.c:766 [inline]
 __is_optimized arch/x86/kernel/uprobes.c:1056 [inline]
 is_optimized arch/x86/kernel/uprobes.c:1067 [inline]
 set_orig_insn+0x226/0x310 arch/x86/kernel/uprobes.c:1098
 register_for_each_vma+0xa0e/0xb80 kernel/events/uprobes.c:1318
 uprobe_unregister_nosync+0x118/0x1b0 kernel/events/uprobes.c:1343
 bpf_uprobe_unregister kernel/trace/bpf_trace.c:2982 [inline]
 bpf_uprobe_multi_link_release+0x99/0x1a0 kernel/trace/bpf_trace.c:2993
 bpf_link_free+0xdd/0x3e0 kernel/bpf/syscall.c:3395
 bpf_link_put_direct kernel/bpf/syscall.c:3448 [inline]
 bpf_link_release+0x6b/0x80 kernel/bpf/syscall.c:3455
 __fput+0x418/0xa50 fs/file_table.c:512
 task_work_run+0x1d9/0x270 kernel/task_work.c:233
 exit_task_work include/linux/task_work.h:40 [inline]
 do_exit+0x73a/0x2360 kernel/exit.c:1004
 __do_sys_exit kernel/exit.c:1114 [inline]
 __se_sys_exit kernel/exit.c:1112 [inline]
 __x64_sys_exit+0x40/0x40 kernel/exit.c:1112
 x64_sys_call+0x2231/0x2240 arch/x86/include/generated/asm/syscalls_64.h:61
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fd88699ce59
Code: Unable to access opcode bytes at 0x7fd88699ce2f.
RSP: 002b:00007fffde6e40f8 EFLAGS: 00000246 ORIG_RAX: 000000000000003c
RAX: ffffffffffffffda RBX: 00007fd886c15fa0 RCX: 00007fd88699ce59
RDX: 0000555576db0808 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 00007fd886a32e6f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fd886c15fac R14: 00007fd886c15fa0 R15: 00007fd886c15fa0
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:mas_root lib/maple_tree.c:759 [inline]
RIP: 0010:mas_start+0xfe/0x550 lib/maple_tree.c:1179
Code: 4c 02 00 00 c6 43 4c 00 48 8b 44 24 28 42 80 3c 30 00 74 08 48 89 df e8 70 61 87 f6 4c 8b 23 49 83 c4 48 4c 89 e0 48 c1 e8 03 <42> 80 3c 30 00 74 08 4c 89 e7 e8 53 61 87 f6 4d 8b 24 24 e8 aa 9b
RSP: 0000:ffffc900034ff898 EFLAGS: 00010206
RAX: 0000000000000011 RBX: ffffc900034ff960 RCX: 1ffff9200069ff35
RDX: 1ffff9200069ff2c RSI: 0000000000000001 RDI: ffffc900034ff988
RBP: 0000000000000001 R08: ffffffff8baf8bfa R09: 0000000000000000
R10: 0000000000000000 R11: ffffffff8e959c20 R12: 0000000000000088
R13: ffffc900034ff960 R14: dffffc0000000000 R15: 1ffff9200069ff35
FS:  0000000000000000(0000) GS:ffff88808c815000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fa41db45000 CR3: 0000000012fed000 CR4: 0000000000352ef0
----------------
Code disassembly (best guess):
   0:	4c 02 00             	rex.WR add (%rax),%r8b
   3:	00 c6                	add    %al,%dh
   5:	43                   	rex.XB
   6:	4c 00 48 8b          	rex.WR add %r9b,-0x75(%rax)
   a:	44 24 28             	rex.R and $0x28,%al
   d:	42 80 3c 30 00       	cmpb   $0x0,(%rax,%r14,1)
  12:	74 08                	je     0x1c
  14:	48 89 df             	mov    %rbx,%rdi
  17:	e8 70 61 87 f6       	call   0xf687618c
  1c:	4c 8b 23             	mov    (%rbx),%r12
  1f:	49 83 c4 48          	add    $0x48,%r12
  23:	4c 89 e0             	mov    %r12,%rax
  26:	48 c1 e8 03          	shr    $0x3,%rax
* 2a:	42 80 3c 30 00       	cmpb   $0x0,(%rax,%r14,1) <-- trapping instruction
  2f:	74 08                	je     0x39
  31:	4c 89 e7             	mov    %r12,%rdi
  34:	e8 53 61 87 f6       	call   0xf687618c
  39:	4d 8b 24 24          	mov    (%r12),%r12
  3d:	e8                   	.byte 0xe8
  3e:	aa                   	stos   %al,%es:(%rdi)
  3f:	9b                   	fwait


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

^ permalink raw reply

* Re: [PATCH] tracing: Cleanup event_enable_trigger_parse() by using __free()
From: Steven Rostedt @ 2026-07-03 15:42 UTC (permalink / raw)
  To: Markus Elfring
  Cc: linux-trace-kernel, kernel-janitors, Masami Hiramatsu,
	Mathieu Desnoyers, LKML
In-Reply-To: <d8f33e0a-b378-4dad-92b1-9ce2098c04f3@web.de>

On Fri, 3 Jul 2026 07:00:10 +0200
Markus Elfring <Markus.Elfring@web.de> wrote:

> > No, it's fine as is.  
> Does this feedback mean that you disagree to an essential development requirement
> according to such a programming interface?
> https://elixir.bootlin.com/linux/v7.1.2/source/include/linux/cleanup.h#L142-L153

>  * Given that the "__free(...) = NULL" pattern for variables defined at
>  * the top of the function poses this potential interdependency problem
>  * the recommendation is to always define and assign variables in one
>  * statement and not group variable definitions at the top of the
>  * function when __free() is used.

This is really just a guideline.

>  *
>  * Lastly, given that the benefit of cleanup helpers is removal of
>  * "goto", and that the "goto" statement can jump between scopes, the
>  * expectation is that usage of "goto" and cleanup helpers is never
>  * mixed in the same function. I.e. for a given routine, convert all
>  * resources that need a "goto" cleanup to scope-based cleanup, or
>  * convert none of them.

The above has more to do with guard()s and not so much with a __free() at
the top of the function. The reason is because things get broken if a goto
jumps over a guard().

In other words, if you don't know what you are doing, don't mix them.

-- Steve

^ permalink raw reply

* Re: [RFC PATCH 1/1] psi: Introduce in-kernel PSI auto monitor feature
From: Pintu Kumar Agarwal @ 2026-07-03 15:32 UTC (permalink / raw)
  To: K Prateek Nayak
  Cc: linux-kernel, linux-trace-kernel, hannes, surenb, rostedt,
	mhiramat, peterz, mathieu.desnoyers, mingo, juri.lelli,
	vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
	pintu.ping, nathan, ojeda, nsc, gary, tglx, thomas.weissschuh,
	aliceryhl, dianders, linux.amoon, rdunlap, akpm, shuah
In-Reply-To: <6c190305-4137-411f-ba25-e0a2d8484db7@amd.com>

Hi Prateek,

Thank you so much for your review feedback and comments.
Please find my response below.

On Fri, Jul 3, 2026 at 1:21 AM K Prateek Nayak <kprateek.nayak@amd.com> wrote:
>
> Hello Pintu,
>
> On 7/2/2026 10:46 PM, Pintu Kumar Agarwal wrote:
> > diff --git a/kernel/sched/build_utility.c b/kernel/sched/build_utility.c
> > index e2cf3b08d4e9..30e9800ce947 100644
> > --- a/kernel/sched/build_utility.c
> > +++ b/kernel/sched/build_utility.c
> > @@ -104,3 +104,7 @@
> >  #ifdef CONFIG_SCHED_AUTOGROUP
> >  # include "autogroup.c"
> >  #endif
> > +
> > +#ifdef CONFIG_PSI_AUTO_MONITOR
> > +# include "psi_monitor.c"
> > +#endif
>
> Isn't this a module? Why is this being included as a scheduler file?
> Based on a quick glance, nothing in this module needs scheduler internal
> APIs (and nor it should) so tools/sched/ would probabaly be a better
> place to put it in if there is interest for this feature.
>
The scheduler placement was chosen because the feature currently
operates on PSI internals
and was developed as an extension to kernel/sched/psi.c.
I am open to alternative placement if another location is more appropriate.

> > diff --git a/kernel/sched/psi_monitor.c b/kernel/sched/psi_monitor.c
> > new file mode 100644
> > index 000000000000..e929a0c05494
> > --- /dev/null
> > +++ b/kernel/sched/psi_monitor.c
> > @@ -0,0 +1,307 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * PSI Automatic Monitor with Weighted Task Ranking + Tracepoints
> > + *
> > + * Periodically samples system PSI (CPU, memory, IO) and, when any
> > + * configured threshold is exceeded, ranks tasks using a composite
> > + * score based on RSS, I/O activity and CPU time, then logs the
> > + * top-N tasks via printk and a tracepoint.
> > + *
> > + * Sysfs interface:
> > + *   /sys/kernel/psi_monitor/cpu_thresh                 (percentage)
> > + *   /sys/kernel/psi_monitor/mem_thresh                 (percentage)
> > + *   /sys/kernel/psi_monitor/io_thresh          (percentage)
> > + *   /sys/kernel/psi_monitor/monitor_interval_ms (milliseconds)
> > + *   /sys/kernel/psi_monitor/rss_weight
> > + *   /sys/kernel/psi_monitor/io_weight
> > + *   /sys/kernel/psi_monitor/cpu_weight
> > + *
> > + * Author: Pintu Kumar Agarwal
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/sched.h>
> > +#include <linux/sched/signal.h>
> > +#include <linux/sched/loadavg.h>
> > +#include <linux/mm.h>
> > +#include <linux/delay.h>
> > +#include <linux/workqueue.h>
> > +#include <linux/psi_types.h>
> > +#include <linux/kobject.h>
> > +#include <linux/sort.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/time64.h>
> > +#include <linux/sched/cputime.h>
> > +
> > +/* Create tracepoints defined in include/trace/events/psi_monitor.h */
> > +#define CREATE_TRACE_POINTS
> > +#include <linux/psi.h>
> > +#include <trace/events/psi_monitor.h>
> > +
> > +
> > +/* Sysfs tunables */
> > +static unsigned int cpu_thresh = 80;     /* in percent */
> > +static unsigned int mem_thresh = 80;     /* in percent */
> > +static unsigned int io_thresh  = 80;     /* in percent */
> > +static unsigned int monitor_interval_ms = 10000;
> > +
> > +/* scoring weights */
> > +static unsigned int rss_weight = 2;
> > +static unsigned int io_weight  = 1;
> > +static unsigned int cpu_weight = 5;
>
> Insanely configurable but what makes it easy for developers to know
> the right configurations under severe pressure as you put it?
>
This is one of the goal of RFC to decide what parameters should be
kept configurable
and to what default values. Currently it is at experimental stage and
gathering feedback.
As per my experiments (2 core, < 1GB RAM), 80% number is good enough as default.
This triggers only during extreme pressure, otherwise kept silent.
Based on the scenario and workload, users can configure it.
Run scenario => check workload => configure => rerun
Scoring weight values are optional and it depends on which load we
want to give priority.
These are open for discussions and only needed for sorting logic.

> > +
> > +static struct delayed_work psi_work;
> > +static struct kobject *psi_kobj;
> > +
> > +#define TOP_N 20
> > +
> > +struct task_info {
> > +       struct task_struct *task;
> > +       unsigned long rss;      /* pages */
> > +       unsigned long io_kb;    /* kB */
> > +       unsigned long cpu_ms;   /* ms */
>
> Isn't the suffix selfexplanatory? Do you really need the comments?
>
oh yes, this can be removed if not needed.

> > +       u64 score;
> > +};
> > +
> > +/*
> > + * psi_avg10_percent() - derive a rough integer percentage from avg10
> > + * for a given PSI state (e.g. PSI_CPU_SOME, PSI_MEM_SOME, PSI_IO_SOME).
> > + *
> > + * psi_group.avg[state][0] is the avg10 window in fixed-point notation.
> > + * The conversion here is approximate but monotonic, which is sufficient
> > + * for thresholding and ranking in this internal monitor.
> > + */
> > +static unsigned long psi_avg10_percent(int state)
> > +{
> > +       u64 avg10;
> > +
> > +       if (state < 0 || state >= NR_PSI_STATES)
> > +               return 0;
> > +
> > +       avg10 = READ_ONCE(psi_system.avg[state][0]);
> > +       if (!avg10)
> > +               return 0;
> > +
> > +       /* Convert back from loadavg-style fixed-point to an approximate % */
> > +       /* Just consider the integer value and ignore fraction */
>
> Why two single line comments?
>
ok I will merge it in the next version.

> > +       return LOAD_INT(avg10);
> > +}
> > +
> > +static int compare_score_desc(const void *a, const void *b)
> > +{
> > +       const struct task_info *ta = a;
> > +       const struct task_info *tb = b;
> > +
> > +       if (tb->score > ta->score)
> > +               return 1;
> > +       if (tb->score < ta->score)
> > +               return -1;
> > +       return 0;
> > +}
> > +
> > +static void log_top_tasks(void)
> > +{
> > +       struct task_info tasks[TOP_N];
> > +       struct task_struct *p, *t;
> > +       int count = 0;
> > +       int i;
> > +
> > +       rcu_read_lock();
> > +       for_each_process_thread(p, t) {
>
> Thats a ton of work every 10s.
>
This happens only when the threshold is breached and the system is
already under pressure.
Based on the feedback we can rate-limit this.

> > +               struct mm_struct *mm;
> > +               unsigned long rss = 0;
> > +               unsigned long io_kb = 0;
> > +               unsigned long cpu_ms = 0;
> > +               u64 score;
> > +
> > +               /* Ignore tasks that are not on run queue or idle */
> > +               if (!t->on_rq && !is_idle_task(t))
>
> Condition doesn't match the comment. Tasks off rq that aren't idle will
> still go through.
>
Oh yes, good catch. I will fix the comment in the next version.

> > +                       continue;
> > +
> > +               mm = get_task_mm(t);
> > +
> > +               /* mm could be NULL for kernel threads */
> > +               if (mm) {
> > +                       rss = mm ? get_mm_rss(mm) : 0;
> > +                       mmput_async(mm);
> > +               }
> > +
> > +               /*
> > +                * Approximate I/O activity: sum of read + write bytes.
> > +                * This uses the task_io_accounting fields in task_struct.
> > +                * Values are best-effort and need not be perfectly accurate
> > +                * for our ranking purpose.
> > +                */
> > +               io_kb = (t->ioac.read_bytes + t->ioac.write_bytes) >> 10;
> > +
> > +               /*
> > +                * Approximate CPU usage via task_sched_runtime(), converted
> > +                * to milliseconds. This is cumulative since task start, but
> > +                * is still useful for comparing hotspots at a given point.
> > +                */
> > +               cpu_ms = (unsigned long)(task_sched_runtime(t) / NSEC_PER_MSEC);
> > +
> > +               score = (u64)rss_weight * (u64)rss +
> > +                       (u64)io_weight  * (u64)io_kb +
> > +                       (u64)cpu_weight * (u64)cpu_ms;
> > +
> > +               if (count < TOP_N) {
> > +                       tasks[count].task   = t;
> > +                       tasks[count].rss    = rss;
> > +                       tasks[count].io_kb  = io_kb;
> > +                       tasks[count].cpu_ms = cpu_ms;
> > +                       tasks[count].score  = score;
> > +                       count++;
> > +               } else {
> > +                       /* Maintain a simple streaming top-N: replace smallest */
> > +                       int min_idx = 0;
> > +                       int j;
> > +
> > +                       for (j = 1; j < TOP_N; j++) {
> > +                               if (tasks[j].score < tasks[min_idx].score)
> > +                                       min_idx = j;
> > +                       }
>
> Can't you just cache the min_idx and re-compute it when it changes
> instead of taking a O(20) iteration for every task?
>
ok will think about it and come back.
IMO the constant value may not affect the order.
Currently at RFC stage I wanted to keep things as simple.

> > +
> > +                       if (score > tasks[min_idx].score) {
> > +                               tasks[min_idx].task   = t;
> > +                               tasks[min_idx].rss    = rss;
> > +                               tasks[min_idx].io_kb  = io_kb;
> > +                               tasks[min_idx].cpu_ms = cpu_ms;
> > +                               tasks[min_idx].score  = score;
> > +                       }
> > +               }
> > +       }
> > +       rcu_read_unlock();
> > +
> > +       sort(tasks, count, sizeof(struct task_info), compare_score_desc, NULL);
> > +
> > +       pr_info("psi_monitor: logging top %d tasks under pressure:\n", count);
> > +
> > +       for (i = 0; i < count; i++) {
> > +               struct task_struct *ts = tasks[i].task;
> > +               unsigned long rss_kb = tasks[i].rss << (PAGE_SHIFT - 10);
> > +               char name[128] = {0,};
> > +
> > +               if (ts->flags & PF_WQ_WORKER)
> > +                       wq_worker_comm(name, sizeof(name), ts);
> > +               else
> > +                       scnprintf(name, sizeof(name) - 1, ts->comm);
> > +
> > +               trace_psi_monitor_top_task(ts->pid, name,
> > +                               tasks[i].cpu_ms,
> > +                               rss_kb,
> > +                               tasks[i].io_kb,
> > +                               tasks[i].score);
> > +
> > +               pr_info("psi_monitor: pid=%d comm=%s psi_flag=%d oncpu=%d cputime(ms)=%lu rss(kB)=%lu io(kB)=%lu score=%llu\n",
> > +                       ts->pid, name, ts->psi_flags, task_cpu(ts),
> > +                       tasks[i].cpu_ms, rss_kb, tasks[i].io_kb,
> > +                       (unsigned long long)tasks[i].score);
>
> This will unnecessarily dump to dmesg even if you have tracevent
> enabled. Why?
>
This is also one point of discussion for RFC.
Currently  have kept both the options available and open for suggestion.
The idea is to dump it like OOM message and only during pressure and
threshold breach when we really need it.
Once the pressure releases this will automatically stop.
We can also make it as pr_debug or ratelimit or even put it under
another CONFIG.
The idea is to automatically get the information in logs instead of
user intervention.
But I am open for suggestions.

> > +               }
> > +}
> > +
> > +static void psi_monitor_fn(struct work_struct *work)
> > +{
> > +       unsigned long cpu_pct, mem_pct, io_pct;
> > +       bool trigger = false;
> > +
> > +       cpu_pct = psi_avg10_percent(PSI_CPU_SOME);
> > +       mem_pct = psi_avg10_percent(PSI_MEM_SOME);
> > +       io_pct  = psi_avg10_percent(PSI_IO_SOME);
> > +
> > +       if (cpu_pct >= cpu_thresh || mem_pct >= mem_thresh ||
> > +               io_pct >= io_thresh)
> > +               trigger = true;
> > +
> > +       if (trigger) {
> > +               pr_info("psi_monitor: pressure high: cpu=%lu%% mem=%lu%% io=%lu%% (thresh cpu=%u mem=%u io=%u)\n",
> > +                       cpu_pct, mem_pct, io_pct,
> > +                       cpu_thresh, mem_thresh, io_thresh);
> > +               log_top_tasks();
> > +       }
> > +
> > +       queue_delayed_work(system_wq, &psi_work,
> > +               msecs_to_jiffies(monitor_interval_ms));
>
> If I set monitor_interval_ms to 6 hours, and then change it back to 10s,
> it'll only take effect after this callback has fired 6 hours later.
>
Oh yes, good catch, I will fix this in the next version.
So, it should override with the new request.

> > +}
> > +
> > +/* Sysfs helpers */
> > +#define PSI_ATTR_RW(_name)                                             \
> > +static ssize_t _name##_show(struct kobject *kobj,                      \
> > +                       struct kobj_attribute *attr, char *buf)         \
> > +{                                                                      \
> > +       return sysfs_emit(buf, "%u\n", _name);                          \
> > +}                                                                      \
> > +static ssize_t _name##_store(struct kobject *kobj,                     \
> > +                           struct kobj_attribute *attr,                \
> > +                           const char *buf, size_t count)              \
> > +{                                                                      \
> > +       unsigned int val;                                               \
> > +       if (kstrtouint(buf, 10, &val))                                  \
> > +               return -EINVAL;                                         \
> > +       _name = val;                                                    \
> > +       return count;                                                   \
> > +}                                                                      \
> > +static struct kobj_attribute _name##_attr = __ATTR_RW(_name)
> > +
> > +PSI_ATTR_RW(cpu_thresh);
> > +PSI_ATTR_RW(mem_thresh);
> > +PSI_ATTR_RW(io_thresh);
> > +PSI_ATTR_RW(monitor_interval_ms);
> > +PSI_ATTR_RW(rss_weight);
> > +PSI_ATTR_RW(io_weight);
> > +PSI_ATTR_RW(cpu_weight);
> > +
> > +static struct attribute *psi_attrs[] = {
> > +       &cpu_thresh_attr.attr,
> > +       &mem_thresh_attr.attr,
> > +       &io_thresh_attr.attr,
> > +       &monitor_interval_ms_attr.attr,
> > +       &rss_weight_attr.attr,
> > +       &io_weight_attr.attr,
> > +       &cpu_weight_attr.attr,
> > +       NULL,
> > +};
> > +
> > +static const struct attribute_group psi_attr_group = {
> > +       .attrs = psi_attrs,
> > +};
> > +
> > +static int __init psi_monitor_init(void)
> > +{
> > +       int ret;
> > +
> > +       INIT_DELAYED_WORK(&psi_work, psi_monitor_fn);
> > +       queue_delayed_work(system_wq, &psi_work,
> > +                       msecs_to_jiffies(monitor_interval_ms));
> > +
> > +       psi_kobj = kobject_create_and_add("psi_monitor", kernel_kobj);
> > +       if (!psi_kobj)
> > +               return -ENOMEM;
> > +
> > +       ret = sysfs_create_group(psi_kobj, &psi_attr_group);
> > +       if (ret) {
> > +               kobject_put(psi_kobj);
> > +               cancel_delayed_work_sync(&psi_work);
> > +               return ret;
> > +       }
> > +
> > +       pr_info("psi_monitor: in-kernel PSI auto monitor (weighted + tracepoints) loaded\n");
> > +       return 0;
> > +}
> > +
> > +static void __exit psi_monitor_exit(void)
> > +{
> > +       cancel_delayed_work_sync(&psi_work);
> > +       if (psi_kobj)
> > +               kobject_put(psi_kobj);
> > +       pr_info("psi_monitor: unloaded\n");
> > +}
> > +
> > +module_init(psi_monitor_init);
> > +module_exit(psi_monitor_exit);
>
> There is nothing here that warrants putting this in kernel/sched.
The feature depends on sched/psi so I decided to keep it close.
But I am open for any location.

> Also this gets included by default when config is enabled and starts
> dumping a bunch of stats to dmesg without anyone asking. No?
>
This is included as a dependent feature of PSI.
If someone enables this CONFIG as part of PSI this indicates that they
are interested in getting auto-monitor stats.
Also, the dump will happen only if threshold is breached with high
default values.
However, for RFC stage I wanted to keep things simple.
Later, we can add an enable/disable flag in cmdline just like PSI.

> Afaict, almost all of the detail used here is also available from
> procfs and people can easily put together a userspace tool if they
> need it. Why do we need an in-kernel module?
>
This is the most fundamental aspect of this auto-monitor feature.
This point is already described in the cover letter.
Let me put it again:
- Get kernel stats early during boot_time before userspace comes up.
   -> Set slightly lower threshold and boot stats (helps in analysing boot time)
- No user intervention or continuous polling or daemons needed
  (Just enable config and start auto monitoring)
- userspace scheduling delays under high pressure
- risk of missing short-lived spikes
- capturing details as soon as pressure hits and at same timestamp
- useful for analysing real-time latency workload.
- useful for minimal environment like initramfs or busybox

The motivation is not to replace existing PSI interfaces or the ability
to build userspace monitoring tools.
The goal is attribution at the moment pressure thresholds are crossed.
A userspace implementation observes the system after being scheduled,
whereas the in-kernel implementation captures contributors at the point
where pressure is detected.
During LPC-2024 I have done significant changes to core psi module
to implement the similar logic.
But the feedback was not to disturb the core psi interface, instead
develop a separate interface and make it configurable.
So, I came up with this auto-monitor idea.

For more details please have a look at my OSS paper with data.
https://hosted-files.sched.co/ossindia2026/19/OSS-IND-26-PSI-Auto-Monitor.pdf
And also the reference data here:
https://github.com/pintuk/KERNEL/tree/master/PSI_WORK

I am also looking out for someone who can test this on a larger
workload and capture data.
This will help us to gather insights, how the feature behaves.

> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Pintu Kumar Agarwal");
> > +MODULE_DESCRIPTION("In-kernel PSI automatic monitor with sysfs, weighted scoring and tracepoints");
> > --
> > 2.34.1
> >
>
> --
> Thanks and Regards,
> Prateek
>

Thanks,
Pintu

^ permalink raw reply

* [PATCH v2 2/2] i2c: qcom-geni: Add trace events for Qualcomm GENI I2C driver
From: Praveen Talari @ 2026-07-03 15:08 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	Mukesh Kumar Savaliya, Viken Dadhaniya, Andi Shyti
  Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-i2c,
	aniket.randive, chandana.chiluveru, Praveen Talari, Konrad Dybcio
In-Reply-To: <20260703-add-tracepoints-for-qcom-geni-i2c-v2-0-e8bf8b178290@oss.qualcomm.com>

Add trace event definitions for the Qualcomm GENI (Generic Interface)
I2C driver. These trace events enable runtime debugging and performance
analysis of I2C operations.

The trace events capture I2C clock configuration, interrupt status and
error code and message.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 drivers/i2c/busses/i2c-qcom-geni.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 96dbf04138be..3227fab6d76e 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/qcom_geni_i2c.h>
+
 #include <linux/acpi.h>
 #include <linux/clk.h>
 #include <linux/dmaengine.h>
@@ -219,6 +222,9 @@ static int qcom_geni_i2c_conf(struct geni_se *se, unsigned long freq)
 	val |= itr->t_low_cnt << LOW_COUNTER_SHFT;
 	val |= itr->t_cycle_cnt;
 	writel_relaxed(val, gi2c->se.base + SE_I2C_SCL_COUNTERS);
+	trace_geni_i2c_bus_setup(gi2c->se.dev, gi2c->clk_freq_out,
+				 itr->clk_div, itr->t_high_cnt,
+				 itr->t_low_cnt, itr->t_cycle_cnt);
 	return 0;
 }
 
@@ -252,6 +258,8 @@ static void geni_i2c_err(struct geni_i2c_dev *gi2c, int err)
 		dev_dbg(gi2c->se.dev, "len:%d, slv-addr:0x%x, RD/WR:%d\n",
 			gi2c->cur->len, gi2c->cur->addr, gi2c->cur->flags);
 
+	trace_geni_i2c_err(gi2c->se.dev, gi2c_log[err].err, gi2c_log[err].msg);
+
 	switch (err) {
 	case GENI_ABORT_DONE:
 		gi2c->abort_done = true;
@@ -288,6 +296,8 @@ static irqreturn_t geni_i2c_irq(int irq, void *dev)
 	dma = readl_relaxed(base + SE_GENI_DMA_MODE_EN);
 	cur = gi2c->cur;
 
+	trace_geni_i2c_irq(gi2c->se.dev, m_stat, rx_st, dm_tx_st, dm_rx_st);
+
 	if (!cur ||
 	    m_stat & (M_CMD_FAILURE_EN | M_CMD_ABORT_EN) ||
 	    dm_rx_st & (DM_I2C_CB_ERR)) {
@@ -788,6 +798,10 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
 	peripheral.set_config = 1;
 	peripheral.multi_msg = false;
 
+	trace_geni_i2c_bus_setup(gi2c->se.dev, gi2c->clk_freq_out,
+				 itr->clk_div, itr->t_high_cnt,
+				 itr->t_low_cnt, itr->t_cycle_cnt);
+
 	gi2c->num_msgs = num;
 	gi2c->is_tx_multi_desc_xfer = false;
 
@@ -895,6 +909,7 @@ static int geni_i2c_fifo_xfer(struct geni_i2c_dev *gi2c,
 		m_param |= ((msgs[i].addr << SLV_ADDR_SHFT) & SLV_ADDR_MSK);
 
 		gi2c->cur = &msgs[i];
+
 		if (msgs[i].flags & I2C_M_RD)
 			ret = geni_i2c_rx_one_msg(gi2c, &msgs[i], m_param);
 		else

-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 1/2] i2c: qcom-geni: trace: Add trace events for Qualcomm GENI I2C
From: Praveen Talari @ 2026-07-03 15:08 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	Mukesh Kumar Savaliya, Viken Dadhaniya, Andi Shyti
  Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-i2c,
	aniket.randive, chandana.chiluveru, Praveen Talari
In-Reply-To: <20260703-add-tracepoints-for-qcom-geni-i2c-v2-0-e8bf8b178290@oss.qualcomm.com>

Add trace event support to the Qualcomm GENI I2C driver to enable
detailed runtime debugging and analysis.

The trace events capture I2C clock configuration, interrupt status
and error code and message.

Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 include/trace/events/qcom_geni_i2c.h | 82 ++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/include/trace/events/qcom_geni_i2c.h b/include/trace/events/qcom_geni_i2c.h
new file mode 100644
index 000000000000..c7e7984f3620
--- /dev/null
+++ b/include/trace/events/qcom_geni_i2c.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM qcom_geni_i2c
+
+#if !defined(_TRACE_QCOM_GENI_I2C_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_QCOM_GENI_I2C_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(geni_i2c_bus_setup,
+	    TP_PROTO(struct device *dev, u32 clk_freq, u8 clk_div,
+		     u8 t_high_cnt, u8 t_low_cnt, u8 t_cycle_cnt),
+	    TP_ARGS(dev, clk_freq, clk_div, t_high_cnt, t_low_cnt, t_cycle_cnt),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, clk_freq)
+			     __field(u8,  clk_div)
+			     __field(u8,  t_high_cnt)
+			     __field(u8,  t_low_cnt)
+			     __field(u8,  t_cycle_cnt)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->clk_freq   = clk_freq;
+			   __entry->clk_div    = clk_div;
+			   __entry->t_high_cnt = t_high_cnt;
+			   __entry->t_low_cnt  = t_low_cnt;
+			   __entry->t_cycle_cnt = t_cycle_cnt;
+	    ),
+
+	    TP_printk("%s: clk_freq=%u clk_div=%u t_high=%u t_low=%u t_cycle=%u",
+		      __get_str(name), __entry->clk_freq, __entry->clk_div,
+		      __entry->t_high_cnt, __entry->t_low_cnt,
+		      __entry->t_cycle_cnt)
+);
+
+TRACE_EVENT(geni_i2c_irq,
+	    TP_PROTO(struct device *dev, u32 m_stat, u32 rx_st,
+		     u32 dm_tx_st, u32 dm_rx_st),
+	    TP_ARGS(dev, m_stat, rx_st, dm_tx_st, dm_rx_st),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, m_stat)
+			     __field(u32, rx_st)
+			     __field(u32, dm_tx_st)
+			     __field(u32, dm_rx_st)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->m_stat = m_stat;
+			   __entry->rx_st = rx_st;
+			   __entry->dm_tx_st = dm_tx_st;
+			   __entry->dm_rx_st = dm_rx_st;
+	    ),
+
+	    TP_printk("%s: m_stat=0x%08x rx_st=0x%08x dm_tx=0x%08x dm_rx=0x%08x",
+		      __get_str(name), __entry->m_stat, __entry->rx_st,
+		      __entry->dm_tx_st, __entry->dm_rx_st)
+);
+
+TRACE_EVENT(geni_i2c_err,
+	    TP_PROTO(struct device *dev, int err, const char *msg),
+	    TP_ARGS(dev, err, msg),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(int, err)
+			     __string(msg, msg)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->err = err;
+			   __assign_str(msg);
+	    ),
+
+	    TP_printk("%s: err=%d msg=%s",
+		      __get_str(name), __entry->err, __get_str(msg))
+);
+
+#endif /* _TRACE_QCOM_GENI_I2C_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>

-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 0/2] Add trace events for Qualcomm GENI I2C drivers
From: Praveen Talari @ 2026-07-03 15:08 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
	Mukesh Kumar Savaliya, Viken Dadhaniya, Andi Shyti
  Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-i2c,
	aniket.randive, chandana.chiluveru, Praveen Talari, Konrad Dybcio

Add a new trace event header providing tracepoints for the Qualcomm
GENI I2C controller.

The trace events cover controller bus setup, interrupt status and 
error reporting. These events enable structured debugging and performance
analysis using ftrace.

Usage examples:

Enable all I2C traces:
echo 1 > /sys/kernel/tracing/events/i2c/enable
echo 1 > /sys/kernel/tracing/events/qcom_geni_i2c/enable

cat /sys/kernel/debug/tracing/trace_pipe

Example trace output:
79.737075: i2c_write: i2c-11 #0 a=057 f=0200 l=3 [00-00-3f]
79.737075: geni_i2c_bus_setup: a90000.i2c: clk_freq=400000 clk_div=2
   t_high=5 t_low=11 t_cycle=22
79.737084: geni_i2c_irq: a90000.i2c: m_stat=0x40000000 rx_st=0x00000000
   dm_tx=0x00000000 dm_rx=0x00000000
79.737201: geni_i2c_irq: a90000.i2c: m_stat=0x00000001 rx_st=0x00000000
   dm_tx=0x00000000 dm_rx=0x00000000
79.737211: i2c_result: i2c-11 n=1 ret=1

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
Changes in v2:
- Updated Reviewed-by tags
- Link to v1: https://patch.msgid.link/20260630-add-tracepoints-for-qcom-geni-i2c-v1-0-474cd6cdbe27@oss.qualcomm.com

---
Praveen Talari (2):
      i2c: qcom-geni: trace: Add trace events for Qualcomm GENI I2C
      i2c: qcom-geni: Add trace events for Qualcomm GENI I2C driver

 drivers/i2c/busses/i2c-qcom-geni.c   | 15 +++++++
 include/trace/events/qcom_geni_i2c.h | 82 ++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)
---
base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
change-id: 20260629-add-tracepoints-for-qcom-geni-i2c-33d14584228b

Best regards,
--  
Praveen Talari <praveen.talari@oss.qualcomm.com>


^ permalink raw reply

* Re: [PATCH 04/30] mm: introduce and use vma_end_pgoff()
From: Vlastimil Babka @ 2026-07-03 14:38 UTC (permalink / raw)
  To: Lorenzo Stoakes, Andrew Morton
  Cc: Russell King, Dinh Nguyen, Simon Schuster,
	James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
	Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
	Thierry Reding, Mikko Perttunen, Jonathan Hunter,
	Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
	Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
	Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
	Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
	Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
	Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
	Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
	linux-kernel, linux-parisc, linux-sgx, etnaviv, dri-devel,
	linux-arm-msm, freedreno, linux-tegra, kvm, linux-fsdevel, nvdimm,
	linux-mm, iommu, linux-perf-users, linux-trace-kernel, kasan-dev,
	damon, Pedro Falcato, Rik van Riel, Harry Yoo, Jann Horn
In-Reply-To: <e379a1cb6a897126ad96e3a263fdb91d6c11f6cb.1782735110.git.ljs@kernel.org>

On 6/29/26 14:23, Lorenzo Stoakes wrote:
> We already have vma_last_pgoff() which retrieves the last page offset
> within a VMA.
> 
> However, code often wishes to span a page offset range, which requires the
> exclusive end of this range.
> 
> So provide this in vma_end_pgoff() and update vma_last_pgoff() to use this
> function.
> 
> No functional change intended.
> 
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>


^ permalink raw reply

* Re: [PATCH 03/30] tools/testing/vma: use vma_start_pgoff() in merge tests
From: Vlastimil Babka @ 2026-07-03 14:34 UTC (permalink / raw)
  To: Lorenzo Stoakes, Andrew Morton
  Cc: Russell King, Dinh Nguyen, Simon Schuster,
	James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
	Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
	Thierry Reding, Mikko Perttunen, Jonathan Hunter,
	Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
	Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
	Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
	Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
	Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
	Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
	Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
	linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
	dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
	linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
	linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
	Harry Yoo, Jann Horn
In-Reply-To: <b501eca378b9d9734e83838102aadc9276590fba.1782735110.git.ljs@kernel.org>

On 6/29/26 14:23, Lorenzo Stoakes wrote:
> Now we have the vma_start_pgoff() helper, update the merge tests to make
> use of it for consistency.
> 
> No functional change intended.
> 
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>


^ permalink raw reply

* Re: [PATCH 02/30] mm: add kdoc comments for vma_start/last_pgoff()
From: Vlastimil Babka @ 2026-07-03 14:19 UTC (permalink / raw)
  To: Lorenzo Stoakes, Andrew Morton
  Cc: Russell King, Dinh Nguyen, Simon Schuster,
	James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
	Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
	Thierry Reding, Mikko Perttunen, Jonathan Hunter,
	Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
	Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
	Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
	Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
	Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
	Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
	Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
	linux-kernel, linux-parisc, linux-sgx, etnaviv, dri-devel,
	freedreno, linux-tegra, kvm, linux-fsdevel, nvdimm, linux-mm,
	iommu, linux-perf-users, linux-trace-kernel, kasan-dev, damon,
	Pedro Falcato, Rik van Riel, Harry Yoo, Jann Horn
In-Reply-To: <8c618dfd7de419e3b797b8bd1cd921d4c5b8878b.1782735110.git.ljs@kernel.org>

On 6/29/26 14:23, Lorenzo Stoakes wrote:
> Describe what vma_start_pgoff() and vma_last_pgoff() actually provide in
> detail.
> 
> This is in order that we can differentiate this between functions that will
> be added in a subsequent patch which provide a different page offset.
> 
> No functional change intended.
> 
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  include/linux/mm.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 059144435729..2f00c75e66bd 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4278,11 +4278,37 @@ static inline unsigned long vma_pages(const struct vm_area_struct *vma)
>  	return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
>  }
>  
> +/**
> + * vma_start_pgoff() - Get the page offset of the start of @vma
> + * @vma: The VMA whose page offset is required.
> + *
> + * If the VMA is file-backed, this is the page offset into the file.
> + *
> + * If the VMA is anonymous, this is the virtual page offset of the start of the
> + * VMA - if unfaulted, then vma->vm_start >> PAGE_SHIFT, if faulted then the
> + * virtual page offset at the time of first fault.
> + *
> + * Note that if @vma is a MAP_PRIVATE file-backed mapping, then this returns the
> + * file offset.
> + *
> + * Returns: The page offset of the start of @vma.
> + */
>  static inline pgoff_t vma_start_pgoff(const struct vm_area_struct *vma)
>  {
>  	return vma->vm_pgoff;
>  }
>  
> +/**
> + * vma_last_pgoff() - Get the page offset of the last page in @vma
> + * @vma: The VMA whose last page offset is required.
> + *
> + * This returns the last page offset contained within @vma.
> + *
> + * See the description of vma_start_pgoff() for a description of VMA page
> + * offsets.
> + *
> + * Returns: The last page offset of @vma.
> + */
>  static inline pgoff_t vma_last_pgoff(const struct vm_area_struct *vma)
>  {
>  	return vma_start_pgoff(vma) + vma_pages(vma) - 1;


^ permalink raw reply

* Re: [PATCHv6 03/13] uprobes/x86: Do not leak trampoline vma mapping on optimization failure
From: Jiri Olsa @ 2026-07-03 14:11 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Oleg Nesterov, Ingo Molnar, Masami Hiramatsu, Andrii Nakryiko,
	bpf, linux-trace-kernel
In-Reply-To: <20260703124503.GP751831@noisy.programming.kicks-ass.net>

On Fri, Jul 03, 2026 at 02:45:03PM +0200, Peter Zijlstra wrote:
> On Fri, Jul 03, 2026 at 01:49:07PM +0200, Jiri Olsa wrote:
> > In case the optimization fails, we leak new-ly created trampoline
> > vma mapping (in case we just created it), let's unmap it.
> > 
> > Fixes: ba2bfc97b462 ("uprobes/x86: Add support to optimize uprobes")
> > Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> 
> What is the intention here? That is, Patch 1 has a Fixes and is already
> in tip/perf/urgent (from v5), patch 2 does not have Fixes, this patch
> (3) has Fixes and appears to rely on patch 2.
> 
> So should I stick both 2,3 into perf/urgent or what?

patch 3 fixes the vma mapping leak, and it is done on top
of bigger change in patch 2

I think we are ok without patch 3 in perf/urgent, but if we
want to have it there, it needs patch 2

jirka

^ permalink raw reply

* Re: [PATCH] rtla: Simplify osnoise tracer option setting code
From: Tomas Glozar @ 2026-07-03 14:09 UTC (permalink / raw)
  To: Crystal Wood
  Cc: Steven Rostedt, John Kacur, Luis Goncalves, Costa Shulyupin,
	Wander Lairson Costa, LKML, linux-trace-kernel
In-Reply-To: <CAP4=nvRMAZ6AJ_WQPVM2sm9jH++4RwN8DhodU7oL6-AcXNg8OA@mail.gmail.com>

pá 3. 7. 2026 v 14:11 odesílatel Tomas Glozar <tglozar@redhat.com> napsal:
>
> pá 12. 6. 2026 v 19:55 Crystal Wood <crwood@redhat.com> napsala:
>
> > > OSNOISE_LL_OPTIONS takes three options: name - struct osnoise_context
> > > field name (written "<opt>" above), path - filename inside
> > > /sys/kernel/tracing/osnoise passed to libtracefs, and init_val - initial
> > > value of struct fields, corresponding to an otherwise invalid option
> > > (some options use OSNOISE_OPTION_INIT_VAL = -1, some use
> > > OSNOISE_TIME_INIT_VAL = 0).
> >
> > Can we simplify by always using -1?  Especially since that's already
> > treated as the universal "invalid" by osnoise_read_ll_config().
> >
> > FWIW using "init val" to mean "invalid" rather than "default" is a bit
> > unintuitive.
> >
>
> The idea behind *_INIT_VAL is to re-use a value that is invalid on the
> osnoise tracer side to mean "not (read from tracer and) set yet (on
> the RTLA side)". OSNOISE_TIME_INIT_VAL is used for values where 0 is
> invalid (e.g. period, runtime), OSNOISE_OPTION_INIT_VAL is used for
> those where 0 is valid. I believe the distinction comes from the fact
> that some of the osnoise options are unsigned on the kernel side, and
> -1 (= 18446744073709551615) is actually a valid value there. E.g.:
>
> [root@cs9 osnoise]# cat period_us
> 1000000
> [root@cs9 osnoise]# echo 18446744073709551615 > period_us
> [root@cs9 osnoise]# cat period_us
> 18446744073709551615
>
> There are values where both 0 and -1 are valid (stop_tracing_us,
> stop_tracing_total_us, timerlat_align_us), and they use
> OSNOISE_OPTION_INIT_VAL. Those cannot use OSNOISE_TIME_INIT_VAL as 0
> is a meaningful and common value for them (disabled for
> stop_us/stop_total_us, zero alignment for timerlat_align_us). Those
> will break if pre-set to 18446744073709551615.
>
> Merging OSNOISE_OPTION_INIT_VAL and OSNOISE_TIME_INIT_VAL would
> introduce this breakage to options that are using
> OSNOISE_TIME_INIT_VAL now. IMHO the cleanest solution is dropping the
> entire complex logic around checking if the value was read from the
> kernel already, as there is no user: RTLA always saves the value once,
> then restores it at the end.
>

Ah sorry, I missed that we use get_llong_from_str() for everything,
which already treats all values as (signed) long long, despite period
and runtime ("OSNOISE_OPTION_INIT_VAL" options) being declared
unsigned. We can just drop the INIT_VALs then and just use -1.

Tomas


^ permalink raw reply

* Re: [PATCH 2/2] i2c: qcom-geni: Add trace events for Qualcomm GENI I2C driver
From: Konrad Dybcio @ 2026-07-03 13:41 UTC (permalink / raw)
  To: Praveen Talari, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Mukesh Kumar Savaliya, Viken Dadhaniya,
	Andi Shyti
  Cc: linux-kernel, linux-trace-kernel, linux-arm-msm, linux-i2c,
	aniket.randive, chandana.chiluveru
In-Reply-To: <20260630-add-tracepoints-for-qcom-geni-i2c-v1-2-474cd6cdbe27@oss.qualcomm.com>

On 6/30/26 8:02 AM, Praveen Talari wrote:
> Add trace event definitions for the Qualcomm GENI (Generic Interface)
> I2C driver. These trace events enable runtime debugging and performance
> analysis of I2C operations.
> 
> The trace events capture I2C clock configuration, interrupt status and
> error code and message.
> 
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

^ permalink raw reply

* Re: [PATCH 01/30] mm: move vma_start_pgoff() into mm.h and clean up
From: Vlastimil Babka @ 2026-07-03 13:36 UTC (permalink / raw)
  To: Lorenzo Stoakes, Andrew Morton
  Cc: Russell King, Dinh Nguyen, Simon Schuster,
	James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
	Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
	Thierry Reding, Mikko Perttunen, Jonathan Hunter,
	Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
	Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
	Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
	Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
	Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
	Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
	Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
	linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
	dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
	linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
	linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
	Harry Yoo, Jann Horn
In-Reply-To: <b28b698df4c009e85c4728446ca5863d8e633164.1782735110.git.ljs@kernel.org>

On 6/29/26 14:23, Lorenzo Stoakes wrote:
> vma_last_pgoff() already lives there, so it's a bit odd to keep
> vma_start_pgoff() in mm/interval_tree.c. Move them together.
> 
> These each return unsigned long, which pgoff_t is typedef'd to. Make this
> consistent and have these functions return pgoff_t instead.
> 
> Additionally, express vma_last_pgoff() in terms of vma_start_pgoff(), since
> we wrap the vma->vm_pgoff access, we may as well use it here.
> 
> Also while we're here, const-ify the VMA and cleanup a bit.
> 
> No functional change intended.
> 
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>


^ permalink raw reply

* Re: [PATCHv6 03/13] uprobes/x86: Do not leak trampoline vma mapping on optimization failure
From: Peter Zijlstra @ 2026-07-03 12:45 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Oleg Nesterov, Ingo Molnar, Masami Hiramatsu, Andrii Nakryiko,
	bpf, linux-trace-kernel
In-Reply-To: <20260703114917.238144-4-jolsa@kernel.org>

On Fri, Jul 03, 2026 at 01:49:07PM +0200, Jiri Olsa wrote:
> In case the optimization fails, we leak new-ly created trampoline
> vma mapping (in case we just created it), let's unmap it.
> 
> Fixes: ba2bfc97b462 ("uprobes/x86: Add support to optimize uprobes")
> Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

What is the intention here? That is, Patch 1 has a Fixes and is already
in tip/perf/urgent (from v5), patch 2 does not have Fixes, this patch
(3) has Fixes and appears to rely on patch 2.

So should I stick both 2,3 into perf/urgent or what?

^ permalink raw reply

* Re: [PATCHv6 03/13] uprobes/x86: Do not leak trampoline vma mapping on optimization failure
From: bot+bpf-ci @ 2026-07-03 12:32 UTC (permalink / raw)
  To: jolsa, oleg, peterz, mingo, mhiramat, andrii
  Cc: bpf, linux-trace-kernel, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260703114917.238144-4-jolsa@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]

> uprobes/x86: Do not leak trampoline vma mapping on optimization failure
>
> In case the optimization fails, we leak new-ly created trampoline
> vma mapping (in case we just created it), let's unmap it.
>
> Fixes: ba2bfc97b462 ("uprobes/x86: Add support to optimize uprobes")

Should the Fixes: tag point to f4216a203c5d ("uprobes/x86: Remove struct
uprobe_trampoline object") instead?

The commit message of f4216a203c5d explicitly states: "Note the original
code called destroy_uprobe_trampoline if the optimiation failed, but it
only freed the struct uprobe_trampoline object, not the vma. The new vma
leak is fixed in following change."

This commit (ba6320f8c716) appears to be that following change fixing the
new vma leak introduced by f4216a203c5d.

> Reviewed-by: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>


---
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/28659696456

^ permalink raw reply

* Re: [PATCH v3 07/11] dma-buf: heaps: Add debugfs support
From: Maxime Ripard @ 2026-07-03 12:14 UTC (permalink / raw)
  To: Thierry Reding
  Cc: devicetree, dri-devel, iommu, linaro-mm-sig, linux-arm-kernel,
	linux-kernel, linux-media, linux-mm, linux-s390, linux-tegra,
	linux-trace-kernel, Alexander Gordeev, Andrew Morton,
	Benjamin Gaignard, Brian Starkey, Catalin Marinas,
	Christian Borntraeger, Christian König, Conor Dooley,
	David Airlie, David Hildenbrand, Gerald Schaefer, Heiko Carstens,
	John Stultz, Jonathan Hunter, Krzysztof Kozlowski,
	Liam R. Howlett, Lorenzo Stoakes, Luca Ceresoli,
	Maarten Lankhorst, Marek Szyprowski, Masami Hiramatsu,
	Mathieu Desnoyers, Maxime Ripard, Michal Hocko, Mike Rapoport,
	Mikko Perttunen, Rasmus Villemoes, Rob Herring, Robin Murphy,
	Russell King, Simona Vetter, Sowjanya Komatineni, Steven Rostedt,
	Sumit Semwal, Suren Baghdasaryan, Sven Schnelle, T.J. Mercier,
	Thierry Reding, Thierry Reding, Thomas Zimmermann, Vasily Gorbik,
	Vlastimil Babka, Will Deacon, Yury Norov
In-Reply-To: <20260701-tegra-vpr-v3-7-d80f7b871bb4@nvidia.com>

On Wed, 1 Jul 2026 18:08:18 +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Add a callback to struct dma_heap_ops that heap providers can implement
> to show information about the state of the heap in debugfs. A top-level
> directory named "dma_heap" is created in debugfs and individual files
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

^ permalink raw reply

* Re: [PATCH] rtla: Simplify osnoise tracer option setting code
From: Tomas Glozar @ 2026-07-03 12:11 UTC (permalink / raw)
  To: Crystal Wood
  Cc: Steven Rostedt, John Kacur, Luis Goncalves, Costa Shulyupin,
	Wander Lairson Costa, LKML, linux-trace-kernel
In-Reply-To: <725da8a4ca6a46ea8ba41778971602b49d650e60.camel@redhat.com>

pá 12. 6. 2026 v 19:55 Crystal Wood <crwood@redhat.com> napsala:

> > OSNOISE_LL_OPTIONS takes three options: name - struct osnoise_context
> > field name (written "<opt>" above), path - filename inside
> > /sys/kernel/tracing/osnoise passed to libtracefs, and init_val - initial
> > value of struct fields, corresponding to an otherwise invalid option
> > (some options use OSNOISE_OPTION_INIT_VAL = -1, some use
> > OSNOISE_TIME_INIT_VAL = 0).
>
> Can we simplify by always using -1?  Especially since that's already
> treated as the universal "invalid" by osnoise_read_ll_config().
>
> FWIW using "init val" to mean "invalid" rather than "default" is a bit
> unintuitive.
>

The idea behind *_INIT_VAL is to re-use a value that is invalid on the
osnoise tracer side to mean "not (read from tracer and) set yet (on
the RTLA side)". OSNOISE_TIME_INIT_VAL is used for values where 0 is
invalid (e.g. period, runtime), OSNOISE_OPTION_INIT_VAL is used for
those where 0 is valid. I believe the distinction comes from the fact
that some of the osnoise options are unsigned on the kernel side, and
-1 (= 18446744073709551615) is actually a valid value there. E.g.:

[root@cs9 osnoise]# cat period_us
1000000
[root@cs9 osnoise]# echo 18446744073709551615 > period_us
[root@cs9 osnoise]# cat period_us
18446744073709551615

There are values where both 0 and -1 are valid (stop_tracing_us,
stop_tracing_total_us, timerlat_align_us), and they use
OSNOISE_OPTION_INIT_VAL. Those cannot use OSNOISE_TIME_INIT_VAL as 0
is a meaningful and common value for them (disabled for
stop_us/stop_total_us, zero alignment for timerlat_align_us). Those
will break if pre-set to 18446744073709551615.

Merging OSNOISE_OPTION_INIT_VAL and OSNOISE_TIME_INIT_VAL would
introduce this breakage to options that are using
OSNOISE_TIME_INIT_VAL now. IMHO the cleanest solution is dropping the
entire complex logic around checking if the value was read from the
kernel already, as there is no user: RTLA always saves the value once,
then restores it at the end.

> > OSNOISE_FLAG_OPTION is similar, but instead of path, it takes the option
> > string inside /sys/kernel/tracing/osnoise/options (opt_string), and no
> > init_val, as it is purely boolean (0 or 1).
> >
> > Previously, for options timerlat_align and osnoise_workload, the return
> > value of osnoise_set_<opt>() distinguished between -2 (option cannot be
> > set) and -1 (option not present). This distinction is expanded for all
> > options for consistency; for most options, it is currently not used,
> > only osnoise_workload is implemented to avoid error on -1 on older RTLA
> > versions.
>
> "on -1 on"?

On older RTLA versions, osnoise_workload distinguishes between -1 and
-2 to avoid error when the kernel doesn't support the feature.

>
> > The change overall has two main benefits: it makes it much simpler to
> > add a new option, as well as to change existing logic consistently for
> > all of them. It also makes the code shorter by a bit over 500 lines.
> >
> > There is no intentional user-visible change coming from the refactoring.
> > osnoise_restore_<opt>() for flag options now sets <opt> instead of
> > orig_<opt>. As the latter is also set by osnoise_put_<opt>(), plus long
> > long options set <opt> in both the old and new implementation, the old
> > behavior was likely a mistake, and should not matter for now, as the
> > options are only restored once at the end of tracing and neither <opt>
> > nor orig_<opt> field is read again.
> >
> > Assisted-by: Claude:claude-opus-4-6
> > Signed-off-by: Tomas Glozar <tglozar@redhat.com>
> > ---
> >  tools/tracing/rtla/src/common.h  |  79 +--
> >  tools/tracing/rtla/src/osnoise.c | 836 ++++++-------------------------
> >  tools/tracing/rtla/src/osnoise.h |  22 -
> >  3 files changed, 188 insertions(+), 749 deletions(-)
>
> While we're at it, can we move this code to common.c, and drop
> "osnoise" from the names, to move closer to using that only for the
> actual osnoise mode?
>
> Or if we really want to namespace things that are specific to the
> osnoise subsystem (i.e. everything implemented in trace_osnoise.c) but
> not specific with respect to the osnoise/timerlat split, I'd suggest
> something different like "osn_".
>

They are called "osnoise options" in the interface (although they are
shared with the timerlat tracer), which cannot be changed. I don't
like using an esoteric prefix like "osn".

> > + * Long long option get/set/restore/put functions, generated from OSNOISE_LL_OPTIONS.
> > + */
> > +#define OSNOISE_LL_OPTION(name, path, init_val)                                              \
> > +static long long                                                                     \
> > +osnoise_get_##name(struct osnoise_context *context)                                  \
> > +{                                                                                    \
> > +     long long name;                                                                 \
> > +                                                                                     \
> > +     if (context->name != (init_val))                                                \
> > +             return context->name;                                                   \
> > +                                                                                     \
> > +     if (context->orig_##name != (init_val))                                         \
> > +             return context->orig_##name;                                            \
> > +                                                                                     \
> > +     name = osnoise_read_ll_config(path);                                            \
> > +     if (name < 0)                                                                   \
> > +             return (init_val);                                                      \
> > +                                                                                     \
> > +     context->orig_##name = name;                                                    \
> > +     return name;                                                                    \
> > +}                                                                                    \
> > +                                                                                     \
> > +int osnoise_set_##name(struct osnoise_context *context, long long name)                      \
> > +{                                                                                    \
> > +     long long curr = osnoise_get_##name(context);                                   \
> > +     int retval;                                                                     \
> > +                                                                                     \
> > +     if (curr == (init_val))                                                         \
> > +             return -1;                                                              \
> > +                                                                                     \
> > +     retval = osnoise_write_ll_config(path, name);                                   \
> > +     if (retval < 0)                                                                 \
> > +             return -2;                                                              \
> > +                                                                                     \
> > +     context->name = name;                                                           \
> > +     return 0;                                                                       \
> > +}                                                                                    \
>
> Using "name" for the value is confusing... "val" would be better.
>

But it's the *name* of the option/field/argument here, not the value.
If you use "value" you'll get:

#define OSNOISE_LL_OPTION(value, path, init_val)

which is incorrect. Would making the macro options in capital letters
(i.e. NAME) make it more clear?

> > +                                                                                     \
> > +void osnoise_restore_##name(struct osnoise_context *context)                         \
> > +{                                                                                    \
> > +     int retval;                                                                     \
> > +                                                                                     \
> > +     if (context->orig_##name == (init_val))                                         \
> > +             return;                                                                 \
> > +                                                                                     \
> > +     if (context->orig_##name == context->name)                                      \
> > +             goto out_done_##name;                                                   \
> > +                                                                                     \
> > +     retval = osnoise_write_ll_config(path, context->orig_##name);                   \
> > +     if (retval < 0)                                                                 \
> > +             err_msg("Could not restore original " #name "\n");                      \
> > +                                                                                     \
> > +out_done_##name:                                                                     \
> > +     context->name = (init_val);                                                     \
> > +}                                                                                    \
>
> Why does the label need to have ##name in it?
>

It doesn't. Code assistants are just not smart enough to distinguish
labels from global symbols yet, it seems...

> > +                                                                                     \
> > +static void osnoise_put_##name(struct osnoise_context *context)                              \
> > +{                                                                                    \
> > +     osnoise_restore_##name(context);                                                \
> > +                                                                                     \
> > +     if (context->orig_##name == (init_val))                                         \
> > +             return;                                                                 \
> > +                                                                                     \
> > +     context->orig_##name = (init_val);                                              \
> > +}
> [snip]
> > +/*
> > + * Flag option get/set/restore/put functions, generated from OSNOISE_FLAG_OPTIONS.
> > + */
> > +#define OSNOISE_FLAG_OPTION(name, option_str)                                                \
> > +static int osnoise_get_##name(struct osnoise_context *context)                               \
> > +{                                                                                    \
> > +     if (context->opt_##name != OSNOISE_OPTION_INIT_VAL)                             \
> > +             return context->opt_##name;                                             \
> > +                                                                                     \
> > +     if (context->orig_opt_##name != OSNOISE_OPTION_INIT_VAL)                        \
> > +             return context->orig_opt_##name;                                        \
> > +                                                                                     \
> > +     context->orig_opt_##name = osnoise_options_get_option(option_str);              \
> > +     return context->orig_opt_##name;                                                \
> > +}                                                                                    \
> > +                                                                                     \
> > +int osnoise_set_##name(struct osnoise_context *context, bool onoff)                  \
> > +{                                                                                    \
> > +     int val = osnoise_get_##name(context);                                          \
> > +     int retval;                                                                     \
> > +                                                                                     \
> > +     if (val == OSNOISE_OPTION_INIT_VAL)                                             \
> > +             return -1;                                                              \
> > +                                                                                     \
> > +     if (val == onoff)                                                               \
> > +             return 0;                                                               \
> > +                                                                                     \
> > +     retval = osnoise_options_set_option(option_str, onoff);                         \
> > +     if (retval < 0)                                                                 \
> > +             return -2;                                                              \
> > +                                                                                     \
> > +     context->opt_##name = onoff;                                                    \
> > +     return 0;                                                                       \
> > +}                                                                                    \
> > +                                                                                     \
> > +void osnoise_restore_##name(struct osnoise_context *context)                         \
> > +{                                                                                    \
> > +     int retval;                                                                     \
> > +                                                                                     \
> > +     if (context->orig_opt_##name == OSNOISE_OPTION_INIT_VAL)                        \
> > +             return;                                                                 \
> > +                                                                                     \
> > +     if (context->orig_opt_##name == context->opt_##name)                            \
> > +             goto out_done_##name;                                                   \
> > +                                                                                     \
> > +     retval = osnoise_options_set_option(option_str, context->orig_opt_##name);      \
> > +     if (retval < 0)                                                                 \
> > +             err_msg("Could not restore original " option_str " option\n");          \
> > +                                                                                     \
> > +out_done_##name:                                                                     \
> > +     context->opt_##name = OSNOISE_OPTION_INIT_VAL;                                  \
> > +}                                                                                    \
> > +                                                                                     \
> > +static void osnoise_put_##name(struct osnoise_context *context)                              \
> > +{                                                                                    \
> > +     osnoise_restore_##name(context);                                                \
> > +                                                                                     \
> > +     if (context->orig_opt_##name == OSNOISE_OPTION_INIT_VAL)                        \
> > +             return;                                                                 \
> > +                                                                                     \
> > +     context->orig_opt_##name = OSNOISE_OPTION_INIT_VAL;                             \
> > +}
>
> Can we reduce the amount of code we put in macros by moving some of the
> logic to osnoise_read/write_ll_config() and osnoise_get/set_optino()?
> Or a non-macro wrapper around them if there are other callers that need
> the current behavior.
>
> Something like (assuming universal -1 invalid):
>
> static int osn_read_ll_config(const char *rel_path, long long *val, long long *orig)
> static int osn_write_ll_config(const char *rel_path, long long *val, long long *orig)
> static int osn_get_option(const char *name, int *val, int *orig)
> static int osn_set_option(const char *name, int *val, int *orig)
>

Yeah I think that could work. The get/set functions call each other,
but they can just pass the pointers instead.

> -Crystal (who wishes we were using a modern language that didn't require all
> this macro stuff)
>

Tomas


^ permalink raw reply

* [PATCHv6 13/13] selftests/bpf: Add tests for forked/cloned optimized uprobes
From: Jiri Olsa @ 2026-07-03 11:49 UTC (permalink / raw)
  To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
	Andrii Nakryiko
  Cc: Jakub Sitnicki, bpf, linux-trace-kernel
In-Reply-To: <20260703114917.238144-1-jolsa@kernel.org>

Adding tests for forked/cloned optimized uprobes and make
sure the child can properly execute optimized probe for
both fork (dups mm) and clone with CLONE_VM.

Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../selftests/bpf/prog_tests/uprobe_syscall.c | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index ff07e5df9a65..e193206fc5d2 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -4,6 +4,8 @@
 
 #ifdef __x86_64__
 
+#define _GNU_SOURCE
+#include <sched.h>
 #include <unistd.h>
 #include <asm/ptrace.h>
 #include <linux/compiler.h>
@@ -13,6 +15,7 @@
 #include <sys/syscall.h>
 #include <sys/prctl.h>
 #include <asm/prctl.h>
+#include <stdnoreturn.h>
 #include "uprobe_syscall.skel.h"
 #include "uprobe_syscall_executed.skel.h"
 #include "bpf/libbpf_internal.h"
@@ -954,6 +957,87 @@ static void test_uprobe_error(void)
 	ASSERT_EQ(errno, EPROTO, "errno");
 }
 
+__attribute__((aligned(16)))
+__nocf_check __weak __naked void uprobe_fork_test(void)
+{
+	asm volatile (
+		".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00\n" /* nop10 */
+		"ret\n"
+	);
+}
+
+static noreturn int child_func(void *arg)
+{
+	struct uprobe_syscall_executed *skel = arg;
+
+	/* Make sure the child's probe is still there and optimized.. */
+	if (memcmp(uprobe_fork_test, lea_rsp, sizeof(lea_rsp)))
+		_exit(1);
+
+	skel->bss->pid = getpid();
+
+	/* .. and it executes properly. */
+	uprobe_fork_test();
+
+	if (skel->bss->executed != 3)
+		_exit(2);
+
+	_exit(0);
+}
+
+static void test_uprobe_fork_optimized(bool clone_vm)
+{
+	struct uprobe_syscall_executed *skel = NULL;
+	unsigned long offset;
+	int pid, status, err;
+	char stack[65535];
+
+	offset = get_uprobe_offset(&uprobe_fork_test);
+	if (!ASSERT_GE(offset, 0, "get_uprobe_offset"))
+		return;
+
+	skel = uprobe_syscall_executed__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		goto cleanup;
+
+	skel->links.test_uprobe = bpf_program__attach_uprobe_opts(skel->progs.test_uprobe,
+					-1, "/proc/self/exe", offset, NULL);
+	if (!ASSERT_OK_PTR(skel->links.test_uprobe, "attach_uprobe"))
+		goto cleanup;
+
+	skel->bss->pid = getpid();
+
+	/* Trigger optimization of uprobe in uprobe_fork_test.  */
+	uprobe_fork_test();
+	uprobe_fork_test();
+
+	/* Make sure it got optimied. */
+	if (!ASSERT_OK(memcmp(uprobe_fork_test, lea_rsp, sizeof(lea_rsp)), "optimized"))
+		goto cleanup;
+
+	if (clone_vm) {
+		pid = clone(child_func, stack + sizeof(stack), CLONE_VM|SIGCHLD, skel);
+		if (!ASSERT_GT(pid, 0, "clone"))
+			goto cleanup;
+	} else {
+		pid = fork();
+		if (!ASSERT_GE(pid, 0, "fork"))
+			goto cleanup;
+		if (pid == 0)
+			child_func(skel);
+	}
+
+	/* Wait for the child and verify it exited properly with 0. */
+	err = waitpid(pid, &status, 0);
+	if (ASSERT_EQ(err, pid, "waitpid")) {
+		ASSERT_EQ(WIFEXITED(status), 1, "child_exited");
+		ASSERT_EQ(WEXITSTATUS(status), 0, "child_exit_code");
+	}
+
+cleanup:
+	uprobe_syscall_executed__destroy(skel);
+}
+
 static void __test_uprobe_syscall(void)
 {
 	if (test__start_subtest("uretprobe_regs_equal"))
@@ -974,6 +1058,10 @@ static void __test_uprobe_syscall(void)
 		test_uprobe_race();
 	if (test__start_subtest("uprobe_red_zone"))
 		test_uprobe_red_zone();
+	if (test__start_subtest("uprobe_optimized_fork"))
+		test_uprobe_fork_optimized(false);
+	if (test__start_subtest("uprobe_optimized_clone_vm"))
+		test_uprobe_fork_optimized(true);
 	if (test__start_subtest("uprobe_error"))
 		test_uprobe_error();
 	if (test__start_subtest("uprobe_regs_equal"))
-- 
2.54.0


^ permalink raw reply related

* [PATCHv6 12/13] selftests/bpf: Add tests for uprobe nop10 red zone clobbering
From: Jiri Olsa @ 2026-07-03 11:49 UTC (permalink / raw)
  To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
	Andrii Nakryiko
  Cc: Jakub Sitnicki, bpf, linux-trace-kernel
In-Reply-To: <20260703114917.238144-1-jolsa@kernel.org>

From: Andrii Nakryiko <andrii@kernel.org>

The uprobe nop5 optimization used to replace a 5-byte NOP with a 5-byte
CALL to a trampoline. The CALL pushes a return address onto the stack at
[rsp-8], clobbering whatever was stored there.

On x86-64, the red zone is the 128 bytes below rsp that user code may use
for temporary storage without adjusting rsp. Compilers can place USDT
argument operands there, generating specs like "8@-8(%rbp)" when rbp ==
rsp. With the CALL-based optimization, the return address overwrites that
argument before the BPF-side USDT argument fetch runs.

Add two tests for this case. The uprobe_syscall subtest stores known values
at -8(%rsp), -16(%rsp), and -24(%rsp), executes an optimized nop10 uprobe,
and verifies the red-zone data is still intact. The USDT subtest triggers a
probe in a function where the compiler places three USDT operands in the
red zone and verifies that all 10 optimized invocations deliver the expected
argument values to BPF.

On an unfixed kernel, the first hit goes through the INT3 path and later
hits use the optimized CALL path, so the red-zone checks fail after
optimization.

Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
[ updates to use nop10 ]
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../selftests/bpf/prog_tests/uprobe_syscall.c | 77 +++++++++++++++++++
 tools/testing/selftests/bpf/prog_tests/usdt.c | 49 ++++++++++++
 tools/testing/selftests/bpf/progs/test_usdt.c | 25 ++++++
 tools/testing/selftests/bpf/usdt_2.c          | 13 ++++
 4 files changed, 164 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index 7711018f8acd..ff07e5df9a65 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -357,6 +357,50 @@ __nocf_check __weak void usdt_test(void)
 	USDT(optimized_uprobe, usdt);
 }
 
+/*
+ * Assembly-level red zone clobbering test. Stores known values in the
+ * red zone (below RSP), executes a nop10 (uprobe site), and checks that
+ * the values survived. Returns 0 if intact, 1 if clobbered.
+ *
+ * The nop5 optimization used CALL (which pushes a return address to
+ * [rsp-8]), the value at -8(%rsp) was overwritten. The nop10 optimization
+ * should escape that by moving stackpointer below the redzone before
+ * doing the CALL.
+ *
+ * Align the code at 64 bytes, to make sure nop10 is not on page boundary.
+ */
+__attribute__((aligned(64)))
+__nocf_check __weak __naked unsigned long uprobe_red_zone_test(void)
+{
+	asm volatile (
+		"movabs $0x1111111111111111, %%rax\n"
+		"movq   %%rax, -8(%%rsp)\n"
+		"movabs $0x2222222222222222, %%rax\n"
+		"movq   %%rax, -16(%%rsp)\n"
+		"movabs $0x3333333333333333, %%rax\n"
+		"movq   %%rax, -24(%%rsp)\n"
+
+		".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00\n" /* nop10: uprobe site */
+
+		"movabs $0x1111111111111111, %%rax\n"
+		"cmpq   %%rax, -8(%%rsp)\n"
+		"jne    1f\n"
+		"movabs $0x2222222222222222, %%rax\n"
+		"cmpq   %%rax, -16(%%rsp)\n"
+		"jne    1f\n"
+		"movabs $0x3333333333333333, %%rax\n"
+		"cmpq   %%rax, -24(%%rsp)\n"
+		"jne    1f\n"
+
+		"xorl   %%eax, %%eax\n"
+		"retq\n"
+		"1:\n"
+		"movl   $1, %%eax\n"
+		"retq\n"
+		::: "rax", "memory"
+	);
+}
+
 static int find_uprobes_trampoline(void *tramp_addr)
 {
 	void *start, *end;
@@ -871,6 +915,37 @@ static void test_uprobe_race(void)
 #define __NR_uprobe 336
 #endif
 
+static void test_uprobe_red_zone(void)
+{
+	struct uprobe_syscall_executed *skel;
+	struct bpf_link *link;
+	void *nop10_addr;
+	size_t offset;
+	int i;
+
+	nop10_addr = find_nop10(uprobe_red_zone_test);
+	if (!ASSERT_NEQ(nop10_addr, NULL, "find_nop10"))
+		return;
+
+	skel = uprobe_syscall_executed__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+
+	offset = get_uprobe_offset(nop10_addr);
+	link = bpf_program__attach_uprobe_opts(skel->progs.test_uprobe,
+			0, "/proc/self/exe", offset, NULL);
+	if (!ASSERT_OK_PTR(link, "attach_uprobe"))
+		goto cleanup;
+
+	for (i = 0; i < 10; i++)
+		ASSERT_EQ(uprobe_red_zone_test(), 0, "red_zone_intact");
+
+	bpf_link__destroy(link);
+
+cleanup:
+	uprobe_syscall_executed__destroy(skel);
+}
+
 static void test_uprobe_error(void)
 {
 	long err = syscall(__NR_uprobe);
@@ -897,6 +972,8 @@ static void __test_uprobe_syscall(void)
 		test_uprobe_usdt();
 	if (test__start_subtest("uprobe_race"))
 		test_uprobe_race();
+	if (test__start_subtest("uprobe_red_zone"))
+		test_uprobe_red_zone();
 	if (test__start_subtest("uprobe_error"))
 		test_uprobe_error();
 	if (test__start_subtest("uprobe_regs_equal"))
diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
index fda3a298ccfc..8004c9568ffa 100644
--- a/tools/testing/selftests/bpf/prog_tests/usdt.c
+++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
@@ -250,6 +250,7 @@ static void subtest_basic_usdt(bool optimized)
 #ifdef __x86_64__
 extern void usdt_1(void);
 extern void usdt_2(void);
+extern void usdt_red_zone_trigger(void);
 
 static unsigned char nop1[1] = { 0x90 };
 static unsigned char nop1_nop10_combo[11] = { 0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 };
@@ -340,6 +341,52 @@ static void subtest_optimized_attach(void)
 cleanup:
 	test_usdt__destroy(skel);
 }
+
+/*
+ * Test that USDT arguments survive nop10 optimization in a function where
+ * the compiler places operands in the red zone.
+ *
+ * Signal handlers are prone to having the compiler place USDT argument
+ * operands in the red zone (below rsp).
+ *
+ * The nop5 optimization used CALL (which pushes a return address to
+ * [rsp-8]), the value at -8(%rsp) was overwritten. The nop10 optimization
+ * should escape that by moving stackpointer below the redzone before
+ * doing the CALL.
+ */
+static void subtest_optimized_red_zone(void)
+{
+	struct test_usdt *skel;
+	int i;
+
+	skel = test_usdt__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+
+	skel->bss->expected_arg[0] = 0xDEADBEEF;
+	skel->bss->expected_arg[1] = 0xCAFEBABE;
+	skel->bss->expected_arg[2] = 0xFEEDFACE;
+	skel->bss->expected_pid = getpid();
+
+	skel->links.usdt_check_arg = bpf_program__attach_usdt(
+		skel->progs.usdt_check_arg, 0, "/proc/self/exe",
+		"optimized_attach", "usdt_red_zone", NULL);
+	if (!ASSERT_OK_PTR(skel->links.usdt_check_arg, "attach_usdt_red_zone"))
+		goto cleanup;
+
+	for (i = 0; i < 10; i++)
+		usdt_red_zone_trigger();
+
+	ASSERT_EQ(skel->bss->arg_total, 10, "arg_total");
+	ASSERT_EQ(skel->bss->arg_bad, 0, "arg_bad");
+	ASSERT_EQ(skel->bss->arg_last[0], 0xDEADBEEF, "arg_last_1");
+	ASSERT_EQ(skel->bss->arg_last[1], 0xCAFEBABE, "arg_last_2");
+	ASSERT_EQ(skel->bss->arg_last[2], 0xFEEDFACE, "arg_last_3");
+
+cleanup:
+	test_usdt__destroy(skel);
+}
+
 #endif
 
 unsigned short test_usdt_100_semaphore SEC(".probes");
@@ -613,6 +660,8 @@ void test_usdt(void)
 		subtest_basic_usdt(true);
 	if (test__start_subtest("optimized_attach"))
 		subtest_optimized_attach();
+	if (test__start_subtest("optimized_red_zone"))
+		subtest_optimized_red_zone();
 #endif
 	if (test__start_subtest("multispec"))
 		subtest_multispec_usdt();
diff --git a/tools/testing/selftests/bpf/progs/test_usdt.c b/tools/testing/selftests/bpf/progs/test_usdt.c
index f00cb52874e0..0ee78fb050a1 100644
--- a/tools/testing/selftests/bpf/progs/test_usdt.c
+++ b/tools/testing/selftests/bpf/progs/test_usdt.c
@@ -149,5 +149,30 @@ int usdt_executed(struct pt_regs *ctx)
 		executed++;
 	return 0;
 }
+
+int arg_total;
+int arg_bad;
+long arg_last[3];
+long expected_arg[3];
+int expected_pid;
+
+SEC("usdt")
+int BPF_USDT(usdt_check_arg, long arg1, long arg2, long arg3)
+{
+	if (expected_pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
+
+	__sync_fetch_and_add(&arg_total, 1);
+	arg_last[0] = arg1;
+	arg_last[1] = arg2;
+	arg_last[2] = arg3;
+
+	if (arg1 != expected_arg[0] ||
+	    arg2 != expected_arg[1] ||
+	    arg3 != expected_arg[2])
+		__sync_fetch_and_add(&arg_bad, 1);
+
+	return 0;
+}
 #endif
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/usdt_2.c b/tools/testing/selftests/bpf/usdt_2.c
index b359b389f6c0..5e38f8605b02 100644
--- a/tools/testing/selftests/bpf/usdt_2.c
+++ b/tools/testing/selftests/bpf/usdt_2.c
@@ -13,4 +13,17 @@ void usdt_2(void)
 	USDT(optimized_attach, usdt_2);
 }
 
+static volatile unsigned long usdt_red_zone_arg1 = 0xDEADBEEF;
+static volatile unsigned long usdt_red_zone_arg2 = 0xCAFEBABE;
+static volatile unsigned long usdt_red_zone_arg3 = 0xFEEDFACE;
+
+void __attribute__((noinline)) usdt_red_zone_trigger(void)
+{
+	unsigned long a1 = usdt_red_zone_arg1;
+	unsigned long a2 = usdt_red_zone_arg2;
+	unsigned long a3 = usdt_red_zone_arg3;
+
+	USDT(optimized_attach, usdt_red_zone, a1, a2, a3);
+}
+
 #endif
-- 
2.54.0


^ permalink raw reply related

* [PATCHv6 11/13] selftests/bpf: Add reattach tests for uprobe syscall
From: Jiri Olsa @ 2026-07-03 11:49 UTC (permalink / raw)
  To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
	Andrii Nakryiko
  Cc: bpf, linux-trace-kernel
In-Reply-To: <20260703114917.238144-1-jolsa@kernel.org>

Adding reattach tests for uprobe syscall tests to make sure
we can re-attach and optimize same uprobe multiple times.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../selftests/bpf/prog_tests/uprobe_syscall.c | 130 ++++++++++++++++--
 1 file changed, 120 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index ba50071ace40..7711018f8acd 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -404,6 +404,16 @@ static void *find_nop10(void *fn)
 
 typedef void (__attribute__((nocf_check)) *trigger_t)(void);
 
+static void check_attach_notrigger(struct uprobe_syscall_executed *skel,
+				   void *addr, int executed)
+{
+	unsigned char *op = addr;
+
+	/* Make sure bpf program was not executed. */
+	ASSERT_EQ(skel->bss->executed, executed, "executed");
+	ASSERT_EQ(*op, 0xcc, "int3");
+}
+
 static void *check_attach(struct uprobe_syscall_executed *skel, trigger_t trigger,
 			  void *addr, int executed)
 {
@@ -430,23 +440,26 @@ static void *check_attach(struct uprobe_syscall_executed *skel, trigger_t trigge
 	return tramp;
 }
 
-static void check_detach(void *addr, void *tramp)
+static bool check_detach(void *addr, void *tramp)
 {
 	static const unsigned char nop10_prefix[] = { 0x66, 0x2e, 0x0f, 0x1f, 0x84 };
+	bool ok = true;
 
 	/* [uprobes_trampoline] stays after detach */
-	ASSERT_OK(find_uprobes_trampoline(tramp), "uprobes_trampoline");
-	ASSERT_OK(memcmp(addr, nop10_prefix, 5), "nop10_prefix");
+	ok &= ASSERT_OK(find_uprobes_trampoline(tramp), "uprobes_trampoline");
+	ok &= ASSERT_OK(memcmp(addr, nop10_prefix, 5), "nop10_prefix");
+	return ok;
 }
 
-static void check(struct uprobe_syscall_executed *skel, struct bpf_link *link,
-		  trigger_t trigger, void *addr, int executed)
+static void *check(struct uprobe_syscall_executed *skel, struct bpf_link *link,
+		   trigger_t trigger, void *addr, int executed)
 {
 	void *tramp;
 
 	tramp = check_attach(skel, trigger, addr, executed);
 	bpf_link__destroy(link);
 	check_detach(addr, tramp);
+	return tramp;
 }
 
 static void test_uprobe_legacy(void)
@@ -457,6 +470,7 @@ static void test_uprobe_legacy(void)
 	);
 	struct bpf_link *link;
 	unsigned long offset;
+	void *tramp;
 
 	offset = get_uprobe_offset(&uprobe_test);
 	if (!ASSERT_GE(offset, 0, "get_uprobe_offset"))
@@ -474,7 +488,30 @@ static void test_uprobe_legacy(void)
 	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_opts"))
 		goto cleanup;
 
-	check(skel, link, uprobe_test, uprobe_test, 2);
+	tramp = check(skel, link, uprobe_test, uprobe_test, 2);
+
+	/* reattach and detach without triggering optimization */
+	link = bpf_program__attach_uprobe_opts(skel->progs.test_uprobe,
+					       0, "/proc/self/exe", offset, NULL);
+	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_opts"))
+		goto cleanup;
+
+	check_attach_notrigger(skel, uprobe_test, 2);
+
+	bpf_link__destroy(link);
+	if (!check_detach(uprobe_test, tramp))
+		goto cleanup;
+
+	uprobe_test();
+	ASSERT_EQ(skel->bss->executed, 2, "executed_no_probe");
+
+	/* reattach with triggering optimization */
+	link = bpf_program__attach_uprobe_opts(skel->progs.test_uprobe,
+				0, "/proc/self/exe", offset, NULL);
+	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_opts"))
+		goto cleanup;
+
+	check(skel, link, uprobe_test, uprobe_test, 4);
 
 	/* uretprobe */
 	skel->bss->executed = 0;
@@ -496,6 +533,7 @@ static void test_uprobe_multi(void)
 	LIBBPF_OPTS(bpf_uprobe_multi_opts, opts);
 	struct bpf_link *link;
 	unsigned long offset;
+	void *tramp;
 
 	offset = get_uprobe_offset(&uprobe_test);
 	if (!ASSERT_GE(offset, 0, "get_uprobe_offset"))
@@ -516,7 +554,30 @@ static void test_uprobe_multi(void)
 	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
 		goto cleanup;
 
-	check(skel, link, uprobe_test, uprobe_test, 2);
+	tramp = check(skel, link, uprobe_test, uprobe_test, 2);
+
+	/* reattach and detach without triggering optimization */
+	link = bpf_program__attach_uprobe_multi(skel->progs.test_uprobe_multi,
+				0, "/proc/self/exe", NULL, &opts);
+	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
+		goto cleanup;
+
+	check_attach_notrigger(skel, uprobe_test, 2);
+
+	bpf_link__destroy(link);
+	if (!check_detach(uprobe_test, tramp))
+		goto cleanup;
+
+	uprobe_test();
+	ASSERT_EQ(skel->bss->executed, 2, "executed_no_probe");
+
+	/* reattach with triggering optimization */
+	link = bpf_program__attach_uprobe_multi(skel->progs.test_uprobe_multi,
+				0, "/proc/self/exe", NULL, &opts);
+	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
+		goto cleanup;
+
+	check(skel, link, uprobe_test, uprobe_test, 4);
 
 	/* uretprobe.multi */
 	skel->bss->executed = 0;
@@ -540,6 +601,7 @@ static void test_uprobe_session(void)
 	);
 	struct bpf_link *link;
 	unsigned long offset;
+	void *tramp;
 
 	offset = get_uprobe_offset(&uprobe_test);
 	if (!ASSERT_GE(offset, 0, "get_uprobe_offset"))
@@ -559,7 +621,30 @@ static void test_uprobe_session(void)
 	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
 		goto cleanup;
 
-	check(skel, link, uprobe_test, uprobe_test, 4);
+	tramp = check(skel, link, uprobe_test, uprobe_test, 4);
+
+	/* reattach and detach without triggering optimization */
+	link = bpf_program__attach_uprobe_multi(skel->progs.test_uprobe_session,
+				0, "/proc/self/exe", NULL, &opts);
+	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
+		goto cleanup;
+
+	check_attach_notrigger(skel, uprobe_test, 4);
+
+	bpf_link__destroy(link);
+	if (!check_detach(uprobe_test, tramp))
+		goto cleanup;
+
+	uprobe_test();
+	ASSERT_EQ(skel->bss->executed, 4, "executed_no_probe");
+
+	/* reattach with triggering optimization */
+	link = bpf_program__attach_uprobe_multi(skel->progs.test_uprobe_session,
+				0, "/proc/self/exe", NULL, &opts);
+	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
+		goto cleanup;
+
+	check(skel, link, uprobe_test, uprobe_test, 8);
 
 cleanup:
 	uprobe_syscall_executed__destroy(skel);
@@ -569,7 +654,7 @@ static void test_uprobe_usdt(void)
 {
 	struct uprobe_syscall_executed *skel;
 	struct bpf_link *link;
-	void *addr;
+	void *addr, *tramp;
 
 	errno = 0;
 	addr = find_nop10(usdt_test);
@@ -588,7 +673,32 @@ static void test_uprobe_usdt(void)
 	if (!ASSERT_OK_PTR(link, "bpf_program__attach_usdt"))
 		goto cleanup;
 
-	check(skel, link, usdt_test, addr, 2);
+	tramp = check(skel, link, usdt_test, addr, 2);
+
+	/* reattach and detach without triggering optimization */
+	link = bpf_program__attach_usdt(skel->progs.test_usdt,
+				-1 /* all PIDs */, "/proc/self/exe",
+				"optimized_uprobe", "usdt", NULL);
+	if (!ASSERT_OK_PTR(link, "bpf_program__attach_usdt"))
+		goto cleanup;
+
+	check_attach_notrigger(skel, addr, 2);
+
+	bpf_link__destroy(link);
+	if (!check_detach(addr, tramp))
+		goto cleanup;
+
+	usdt_test();
+	ASSERT_EQ(skel->bss->executed, 2, "executed_no_probe");
+
+	/* reattach with triggering optimization */
+	link = bpf_program__attach_usdt(skel->progs.test_usdt,
+				-1 /* all PIDs */, "/proc/self/exe",
+				"optimized_uprobe", "usdt", NULL);
+	if (!ASSERT_OK_PTR(link, "bpf_program__attach_usdt"))
+		goto cleanup;
+
+	check(skel, link, usdt_test, addr, 4);
 
 cleanup:
 	uprobe_syscall_executed__destroy(skel);
-- 
2.54.0


^ permalink raw reply related

* [PATCHv6 10/13] selftests/bpf: Change uprobe/usdt trigger bench code to use nop10
From: Jiri Olsa @ 2026-07-03 11:49 UTC (permalink / raw)
  To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
	Andrii Nakryiko
  Cc: Jakub Sitnicki, bpf, linux-trace-kernel
In-Reply-To: <20260703114917.238144-1-jolsa@kernel.org>

Changing uprobe/usdt trigger bench code to use nop10 instead
of nop5. Also changing run_bench_uprobes.sh to use nop10 triggers.

Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/testing/selftests/bpf/bench.c           | 20 +++++------
 .../selftests/bpf/benchs/bench_trigger.c      | 36 +++++++++----------
 .../selftests/bpf/benchs/run_bench_uprobes.sh |  2 +-
 3 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c
index 3d9d2cd7764b..c4a3a6b3eb83 100644
--- a/tools/testing/selftests/bpf/bench.c
+++ b/tools/testing/selftests/bpf/bench.c
@@ -539,12 +539,12 @@ extern const struct bench bench_trig_uretprobe_multi_push;
 extern const struct bench bench_trig_uprobe_multi_ret;
 extern const struct bench bench_trig_uretprobe_multi_ret;
 #ifdef __x86_64__
-extern const struct bench bench_trig_uprobe_nop5;
-extern const struct bench bench_trig_uretprobe_nop5;
-extern const struct bench bench_trig_uprobe_multi_nop5;
-extern const struct bench bench_trig_uretprobe_multi_nop5;
+extern const struct bench bench_trig_uprobe_nop10;
+extern const struct bench bench_trig_uretprobe_nop10;
+extern const struct bench bench_trig_uprobe_multi_nop10;
+extern const struct bench bench_trig_uretprobe_multi_nop10;
 extern const struct bench bench_trig_usdt_nop;
-extern const struct bench bench_trig_usdt_nop5;
+extern const struct bench bench_trig_usdt_nop10;
 #endif
 
 extern const struct bench bench_rb_libbpf;
@@ -622,12 +622,12 @@ static const struct bench *benchs[] = {
 	&bench_trig_uprobe_multi_ret,
 	&bench_trig_uretprobe_multi_ret,
 #ifdef __x86_64__
-	&bench_trig_uprobe_nop5,
-	&bench_trig_uretprobe_nop5,
-	&bench_trig_uprobe_multi_nop5,
-	&bench_trig_uretprobe_multi_nop5,
+	&bench_trig_uprobe_nop10,
+	&bench_trig_uretprobe_nop10,
+	&bench_trig_uprobe_multi_nop10,
+	&bench_trig_uretprobe_multi_nop10,
 	&bench_trig_usdt_nop,
-	&bench_trig_usdt_nop5,
+	&bench_trig_usdt_nop10,
 #endif
 	/* ringbuf/perfbuf benchmarks */
 	&bench_rb_libbpf,
diff --git a/tools/testing/selftests/bpf/benchs/bench_trigger.c b/tools/testing/selftests/bpf/benchs/bench_trigger.c
index a60b8173cdc4..61513efc167a 100644
--- a/tools/testing/selftests/bpf/benchs/bench_trigger.c
+++ b/tools/testing/selftests/bpf/benchs/bench_trigger.c
@@ -396,15 +396,15 @@ static void *uprobe_producer_ret(void *input)
 }
 
 #ifdef __x86_64__
-__nocf_check __weak void uprobe_target_nop5(void)
+__nocf_check __weak void uprobe_target_nop10(void)
 {
 	asm volatile (".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00");
 }
 
-static void *uprobe_producer_nop5(void *input)
+static void *uprobe_producer_nop10(void *input)
 {
 	while (true)
-		uprobe_target_nop5();
+		uprobe_target_nop10();
 	return NULL;
 }
 
@@ -418,7 +418,7 @@ static void *uprobe_producer_usdt_nop(void *input)
 	return NULL;
 }
 
-static void *uprobe_producer_usdt_nop5(void *input)
+static void *uprobe_producer_usdt_nop10(void *input)
 {
 	while (true)
 		usdt_2();
@@ -542,24 +542,24 @@ static void uretprobe_multi_ret_setup(void)
 }
 
 #ifdef __x86_64__
-static void uprobe_nop5_setup(void)
+static void uprobe_nop10_setup(void)
 {
-	usetup(false, false /* !use_multi */, &uprobe_target_nop5);
+	usetup(false, false /* !use_multi */, &uprobe_target_nop10);
 }
 
-static void uretprobe_nop5_setup(void)
+static void uretprobe_nop10_setup(void)
 {
-	usetup(true, false /* !use_multi */, &uprobe_target_nop5);
+	usetup(true, false /* !use_multi */, &uprobe_target_nop10);
 }
 
-static void uprobe_multi_nop5_setup(void)
+static void uprobe_multi_nop10_setup(void)
 {
-	usetup(false, true /* use_multi */, &uprobe_target_nop5);
+	usetup(false, true /* use_multi */, &uprobe_target_nop10);
 }
 
-static void uretprobe_multi_nop5_setup(void)
+static void uretprobe_multi_nop10_setup(void)
 {
-	usetup(true, true /* use_multi */, &uprobe_target_nop5);
+	usetup(true, true /* use_multi */, &uprobe_target_nop10);
 }
 
 static void usdt_setup(const char *name)
@@ -598,7 +598,7 @@ static void usdt_nop_setup(void)
 	usdt_setup("usdt_1");
 }
 
-static void usdt_nop5_setup(void)
+static void usdt_nop10_setup(void)
 {
 	usdt_setup("usdt_2");
 }
@@ -665,10 +665,10 @@ BENCH_TRIG_USERMODE(uretprobe_multi_nop, nop, "uretprobe-multi-nop");
 BENCH_TRIG_USERMODE(uretprobe_multi_push, push, "uretprobe-multi-push");
 BENCH_TRIG_USERMODE(uretprobe_multi_ret, ret, "uretprobe-multi-ret");
 #ifdef __x86_64__
-BENCH_TRIG_USERMODE(uprobe_nop5, nop5, "uprobe-nop5");
-BENCH_TRIG_USERMODE(uretprobe_nop5, nop5, "uretprobe-nop5");
-BENCH_TRIG_USERMODE(uprobe_multi_nop5, nop5, "uprobe-multi-nop5");
-BENCH_TRIG_USERMODE(uretprobe_multi_nop5, nop5, "uretprobe-multi-nop5");
+BENCH_TRIG_USERMODE(uprobe_nop10, nop10, "uprobe-nop10");
+BENCH_TRIG_USERMODE(uretprobe_nop10, nop10, "uretprobe-nop10");
+BENCH_TRIG_USERMODE(uprobe_multi_nop10, nop10, "uprobe-multi-nop10");
+BENCH_TRIG_USERMODE(uretprobe_multi_nop10, nop10, "uretprobe-multi-nop10");
 BENCH_TRIG_USERMODE(usdt_nop, usdt_nop, "usdt-nop");
-BENCH_TRIG_USERMODE(usdt_nop5, usdt_nop5, "usdt-nop5");
+BENCH_TRIG_USERMODE(usdt_nop10, usdt_nop10, "usdt-nop10");
 #endif
diff --git a/tools/testing/selftests/bpf/benchs/run_bench_uprobes.sh b/tools/testing/selftests/bpf/benchs/run_bench_uprobes.sh
index 9ec59423b949..e490b337e960 100755
--- a/tools/testing/selftests/bpf/benchs/run_bench_uprobes.sh
+++ b/tools/testing/selftests/bpf/benchs/run_bench_uprobes.sh
@@ -2,7 +2,7 @@
 
 set -eufo pipefail
 
-for i in usermode-count syscall-count {uprobe,uretprobe}-{nop,push,ret,nop5} usdt-nop usdt-nop5
+for i in usermode-count syscall-count {uprobe,uretprobe}-{nop,push,ret,nop10} usdt-nop usdt-nop10
 do
 	summary=$(sudo ./bench -w2 -d5 -a trig-$i | tail -n1 | cut -d'(' -f1 | cut -d' ' -f3-)
 	printf "%-15s: %s\n" $i "$summary"
-- 
2.54.0


^ permalink raw reply related

* [PATCHv6 09/13] selftests/bpf: Change uprobe syscall tests to use nop10
From: Jiri Olsa @ 2026-07-03 11:49 UTC (permalink / raw)
  To: Oleg Nesterov, Peter Zijlstra, Ingo Molnar, Masami Hiramatsu,
	Andrii Nakryiko
  Cc: Jakub Sitnicki, bpf, linux-trace-kernel
In-Reply-To: <20260703114917.238144-1-jolsa@kernel.org>

Optimized uprobes are now on top of 10-bytes nop instructions,
reflect that in existing tests.

Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 .../selftests/bpf/benchs/bench_trigger.c      |  2 +-
 .../selftests/bpf/prog_tests/uprobe_syscall.c | 30 +++++++++++--------
 tools/testing/selftests/bpf/prog_tests/usdt.c | 25 +++++++++-------
 tools/testing/selftests/bpf/usdt_2.c          |  2 +-
 4 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/tools/testing/selftests/bpf/benchs/bench_trigger.c b/tools/testing/selftests/bpf/benchs/bench_trigger.c
index 2f22ec61667b..a60b8173cdc4 100644
--- a/tools/testing/selftests/bpf/benchs/bench_trigger.c
+++ b/tools/testing/selftests/bpf/benchs/bench_trigger.c
@@ -398,7 +398,7 @@ static void *uprobe_producer_ret(void *input)
 #ifdef __x86_64__
 __nocf_check __weak void uprobe_target_nop5(void)
 {
-	asm volatile (".byte 0x0f, 0x1f, 0x44, 0x00, 0x00");
+	asm volatile (".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00");
 }
 
 static void *uprobe_producer_nop5(void *input)
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index c944136252c6..ba50071ace40 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -17,7 +17,7 @@
 #include "uprobe_syscall_executed.skel.h"
 #include "bpf/libbpf_internal.h"
 
-#define USDT_NOP .byte 0x0f, 0x1f, 0x44, 0x00, 0x00
+#define USDT_NOP .byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00
 #include "usdt.h"
 
 #pragma GCC diagnostic ignored "-Wattributes"
@@ -26,7 +26,7 @@ __attribute__((aligned(16)))
 __nocf_check __weak __naked unsigned long uprobe_regs_trigger(void)
 {
 	asm volatile (
-		".byte 0x0f, 0x1f, 0x44, 0x00, 0x00\n" /* nop5 */
+		".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00\n" /* nop10 */
 		"movq $0xdeadbeef, %rax\n"
 		"ret\n"
 	);
@@ -345,9 +345,9 @@ static void test_uretprobe_syscall_call(void)
 __attribute__((aligned(16)))
 __nocf_check __weak __naked void uprobe_test(void)
 {
-	asm volatile ("					\n"
-		".byte 0x0f, 0x1f, 0x44, 0x00, 0x00	\n"
-		"ret					\n"
+	asm volatile (
+		".byte 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00\n" /* nop10 */
+		"ret\n"
 	);
 }
 
@@ -388,14 +388,15 @@ static int find_uprobes_trampoline(void *tramp_addr)
 	return ret;
 }
 
-static unsigned char nop5[5] = { 0x0f, 0x1f, 0x44, 0x00, 0x00 };
+static unsigned char nop10[10]  = { 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 };
+static unsigned char lea_rsp[5] = { 0x48, 0x8d, 0x64, 0x24, 0x80 };
 
-static void *find_nop5(void *fn)
+static void *find_nop10(void *fn)
 {
 	int i;
 
-	for (i = 0; i < 10; i++) {
-		if (!memcmp(nop5, fn + i, 5))
+	for (i = 0; i < 128; i++) {
+		if (!memcmp(nop10, fn + i, 10))
 			return fn + i;
 	}
 	return NULL;
@@ -420,7 +421,8 @@ static void *check_attach(struct uprobe_syscall_executed *skel, trigger_t trigge
 	ASSERT_EQ(skel->bss->executed, executed, "executed");
 
 	/* .. and check the trampoline is as expected. */
-	call = (struct __arch_relative_insn *) addr;
+	ASSERT_OK(memcmp(addr, lea_rsp, 5), "lea_rsp");
+	call = (struct __arch_relative_insn *)(addr + 5);
 	tramp = (void *) (call + 1) + call->raddr;
 	ASSERT_EQ(call->op, 0xe8, "call");
 	ASSERT_OK(find_uprobes_trampoline(tramp), "uprobes_trampoline");
@@ -430,9 +432,11 @@ static void *check_attach(struct uprobe_syscall_executed *skel, trigger_t trigge
 
 static void check_detach(void *addr, void *tramp)
 {
+	static const unsigned char nop10_prefix[] = { 0x66, 0x2e, 0x0f, 0x1f, 0x84 };
+
 	/* [uprobes_trampoline] stays after detach */
 	ASSERT_OK(find_uprobes_trampoline(tramp), "uprobes_trampoline");
-	ASSERT_OK(memcmp(addr, nop5, 5), "nop5");
+	ASSERT_OK(memcmp(addr, nop10_prefix, 5), "nop10_prefix");
 }
 
 static void check(struct uprobe_syscall_executed *skel, struct bpf_link *link,
@@ -568,8 +572,8 @@ static void test_uprobe_usdt(void)
 	void *addr;
 
 	errno = 0;
-	addr = find_nop5(usdt_test);
-	if (!ASSERT_OK_PTR(addr, "find_nop5"))
+	addr = find_nop10(usdt_test);
+	if (!ASSERT_OK_PTR(addr, "find_nop10"))
 		return;
 
 	skel = uprobe_syscall_executed__open_and_load();
diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
index 69759b27794d..fda3a298ccfc 100644
--- a/tools/testing/selftests/bpf/prog_tests/usdt.c
+++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
@@ -252,7 +252,7 @@ extern void usdt_1(void);
 extern void usdt_2(void);
 
 static unsigned char nop1[1] = { 0x90 };
-static unsigned char nop1_nop5_combo[6] = { 0x90, 0x0f, 0x1f, 0x44, 0x00, 0x00 };
+static unsigned char nop1_nop10_combo[11] = { 0x90, 0x66, 0x2e, 0x0f, 0x1f, 0x84, 0x00, 0x00, 0x00, 0x00, 0x00 };
 
 static void *find_instr(void *fn, unsigned char *instr, size_t cnt)
 {
@@ -271,17 +271,17 @@ static void subtest_optimized_attach(void)
 	__u8 *addr_1, *addr_2;
 
 	/* usdt_1 USDT probe has single nop instruction */
-	addr_1 = find_instr(usdt_1, nop1_nop5_combo, 6);
-	if (!ASSERT_NULL(addr_1, "usdt_1_find_nop1_nop5_combo"))
+	addr_1 = find_instr(usdt_1, nop1_nop10_combo, 11);
+	if (!ASSERT_NULL(addr_1, "usdt_1_find_nop1_nop10_combo"))
 		return;
 
 	addr_1 = find_instr(usdt_1, nop1, 1);
 	if (!ASSERT_OK_PTR(addr_1, "usdt_1_find_nop1"))
 		return;
 
-	/* usdt_2 USDT probe has nop,nop5 instructions combo */
-	addr_2 = find_instr(usdt_2, nop1_nop5_combo, 6);
-	if (!ASSERT_OK_PTR(addr_2, "usdt_2_find_nop1_nop5_combo"))
+	/* usdt_2 USDT probe has nop,nop10 instructions combo */
+	addr_2 = find_instr(usdt_2, nop1_nop10_combo, 11);
+	if (!ASSERT_OK_PTR(addr_2, "usdt_2_find_nop1_nop10_combo"))
 		return;
 
 	skel = test_usdt__open_and_load();
@@ -309,12 +309,12 @@ static void subtest_optimized_attach(void)
 
 	bpf_link__destroy(skel->links.usdt_executed);
 
-	/* we expect the nop5 ip */
+	/* we expect the nop10 ip */
 	skel->bss->expected_ip = (unsigned long) addr_2 + 1;
 
 	/*
 	 * Attach program on top of usdt_2 which is probe defined on top
-	 * of nop1,nop5 combo, so the probe gets optimized on top of nop5.
+	 * of nop1,nop10 combo, so the probe gets optimized on top of nop10.
 	 */
 	skel->links.usdt_executed = bpf_program__attach_usdt(skel->progs.usdt_executed,
 						     0 /*self*/, "/proc/self/exe",
@@ -328,8 +328,13 @@ static void subtest_optimized_attach(void)
 	/* nop stays on addr_2 address */
 	ASSERT_EQ(*addr_2, 0x90, "nop");
 
-	/* call is on addr_2 + 1 address */
-	ASSERT_EQ(*(addr_2 + 1), 0xe8, "call");
+	/*
+	 * lea -0x80(%rsp), %rsp
+	 * call ...
+	 */
+	static unsigned char expected[] = { 0x48, 0x8d, 0x64, 0x24, 0x80, 0xe8 };
+
+	ASSERT_MEMEQ(addr_2 + 1, expected, sizeof(expected), "lea_and_call");
 	ASSERT_EQ(skel->bss->executed, 4, "executed");
 
 cleanup:
diff --git a/tools/testing/selftests/bpf/usdt_2.c b/tools/testing/selftests/bpf/usdt_2.c
index 789883aaca4c..b359b389f6c0 100644
--- a/tools/testing/selftests/bpf/usdt_2.c
+++ b/tools/testing/selftests/bpf/usdt_2.c
@@ -3,7 +3,7 @@
 #if defined(__x86_64__)
 
 /*
- * Include usdt.h with default nop,nop5 instructions combo.
+ * Include usdt.h with default nop,nop10 instructions combo.
  */
 #include "usdt.h"
 
-- 
2.54.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox