Linux Trace Kernel
 help / color / mirror / Atom feed
* Re: [PATCH bpf-next 10/17] bpf: Add support for tracing_multi link session
From: Jiri Olsa @ 2026-02-22 14:35 UTC (permalink / raw)
  To: bot+bpf-ci
  Cc: ast, daniel, andrii, bpf, linux-trace-kernel, kafai, eddyz87,
	songliubraving, yhs, menglong8.dong, rostedt, martin.lau,
	yonghong.song, clm, ihor.solodrai
In-Reply-To: <0520af2c467a82b82aa3014b7e721f95bfde9e91324bb2e183b099e5a37bbc3e@mail.kernel.org>

On Fri, Feb 20, 2026 at 10:57:56AM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 5892dca20b7e..1cd6c1457bd3 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3611,6 +3611,7 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> >  		if (prog->expected_attach_type != BPF_TRACE_FENTRY &&
> >  		    prog->expected_attach_type != BPF_TRACE_FEXIT &&
> >  		    prog->expected_attach_type != BPF_TRACE_FSESSION &&
> > +		    prog->expected_attach_type != BPF_TRACE_FSESSION_MULTI &&
> >  		    prog->expected_attach_type != BPF_TRACE_FENTRY_MULTI &&
> >  		    prog->expected_attach_type != BPF_TRACE_FEXIT_MULTI &&
> >  		    prog->expected_attach_type != BPF_MODIFY_RETURN) {
> 
> BPF_TRACE_FSESSION_MULTI is now accepted here, which means
> a program with this type can enter bpf_tracing_prog_attach()
> via BPF_RAW_TRACEPOINT_OPEN:
> 
> bpf_raw_tracepoint_open()
>   bpf_raw_tp_link_attach()       /* name == NULL */
>     bpf_tracing_prog_attach()    /* BPF_TRACE_FSESSION_MULTI */
> 
> Further down in bpf_tracing_prog_attach(), the fexit node
> initialization only checks for BPF_TRACE_FSESSION:
> 
> kernel/bpf/syscall.c:bpf_tracing_prog_attach() {
>     ...
>     if (prog->expected_attach_type == BPF_TRACE_FSESSION) {
>         link->fexit.link = &link->link.link;
>         link->fexit.cookie = bpf_cookie;
>     }
>     ...
> }
> 
> So for BPF_TRACE_FSESSION_MULTI, link->fexit.link stays NULL
> (from kzalloc). When __bpf_trampoline_link_prog() later calls
> fsession_exit(), it returns &link->fexit with a NULL link
> field. This node gets added to the trampoline FEXIT list, and
> bpf_trampoline_get_progs() then dereferences it:
> 
> kernel/bpf/trampoline.c:bpf_trampoline_get_progs() {
>     ...
>     hlist_for_each_entry(node, &tr->progs_hlist[kind], tramp_hlist) {
>         *ip_arg |= node->link->prog->call_get_func_ip;
>                    ^^^^^^^^^^
>     ...
> }
> 
> Would it make sense to either add BPF_TRACE_FSESSION_MULTI to
> the fexit initialization, or reject this type in
> bpf_tracing_prog_attach() since it should only be used through
> bpf_tracing_multi_attach()?

yes, I think we need to reject or MULTI types in bpf_raw_tracepoint_open,
it's allowed to attach only through new multi_link attach api,

good catch, thnx

jirka

> 
> 
> ---
> 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/22220470181
> 
> AI-authorship-score: low
> AI-authorship-explanation: Code follows established kernel BPF patterns for adding new attach types, written by a well-known BPF maintainer with terse, practical commit message style.
> issues-found: 1
> issue-severity-score: high
> issue-severity-explanation: NULL pointer dereference reachable from BPF_RAW_TRACEPOINT_OPEN syscall when BPF_TRACE_FSESSION_MULTI enters bpf_tracing_prog_attach without fexit initialization, leading to kernel crash.


^ permalink raw reply

* Re: [PATCH bpf-next 11/17] libbpf: Add support to create tracing multi link
From: Jiri Olsa @ 2026-02-22 14:36 UTC (permalink / raw)
  To: bot+bpf-ci
  Cc: ast, daniel, andrii, bpf, linux-trace-kernel, kafai, eddyz87,
	songliubraving, yhs, menglong8.dong, rostedt, martin.lau,
	yonghong.song, clm, ihor.solodrai
In-Reply-To: <56ab264977d669e52dc1cbb2fa6224443cae58e723b4757ea59d21fd160f1b9f@mail.kernel.org>

On Fri, Feb 20, 2026 at 10:57:53AM +0000, bot+bpf-ci@kernel.org wrote:
> > The provided <pattern> is used as 'pattern' argument in
> > bpf_program__attach_kprobe_multi_opts function.
> 
> This isn't a bug, but the commit message references
> bpf_program__attach_kprobe_multi_opts when the code actually calls
> bpf_program__attach_tracing_multi. Looks like a leftover from
> the kprobe_multi code this was modeled after.

copy&paste error :-\ thnx

> 
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> 
> [ ... ]
> 
> > +struct bpf_link *
> > +bpf_program__attach_tracing_multi(const struct bpf_program *prog, const char *pattern,
> > +				  const struct bpf_tracing_multi_opts *opts)
> > +{
> > +	LIBBPF_OPTS(bpf_link_create_opts, lopts);
> > +	__u32 *ids, cnt, *free_ids = NULL;
> 
> cnt is declared as __u32 here, but collect_btf_func_ids_by_glob()
> returns int. When that function returns a negative error such as
> -ENOMEM, the value wraps to a large unsigned number.

yes, will fix

> 
> > +	__u64 *cookies;
> > +	int prog_fd, link_fd, err;
> > +	struct bpf_link *link;
> 
> [ ... ]
> 
> > +	if (pattern) {
> > +		err = bpf_object__load_vmlinux_btf(prog->obj, true);
> > +		if (err)
> > +			return libbpf_err_ptr(err);
> > +
> > +		cnt = collect_btf_func_ids_by_glob(prog->obj->btf_vmlinux, pattern, &ids);
> > +		if (cnt < 0)
> > +			return libbpf_err_ptr(cnt);
> 
> Since cnt is __u32, this comparison is always false. On an allocation
> failure inside collect_btf_func_ids_by_glob(), the error is silently
> ignored. That function also does free(*ids) before returning -ENOMEM,
> so ids becomes a dangling pointer. The code then continues with a
> freed ids pointer and a garbage cnt, and eventually double-frees ids
> through the error label via free_ids.
> 
> Should cnt be declared as int instead of __u32?

it should, will fix

> 
> > +		if (cnt == 0)
> > +			return libbpf_err_ptr(-EINVAL);
> > +		free_ids = ids;
> > +	}
> 
> [ ... ]
> 
> > +error:
> > +	free(link);
> > +	free(free_ids);
> > +	return libbpf_err_ptr(err);
> > +}
> > +
> > +static int attach_tracing_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> > +{
> > +	bool is_fexit, is_fsession;
> > +	const char *spec;
> > +	char *pattern;
> > +	int n;
> > +
> > +	/* Do not allow auto attach if there's no function pattern. */
> > +	if (strcmp(prog->sec_name, "fentry.multi") == 0 ||
> > +	    strcmp(prog->sec_name, "fexit.multi") == 0 ||
> > +	    strcmp(prog->sec_name, "fsession.multi") == 0 ||
> > +	    strcmp(prog->sec_name, "fentry.multi.s") == 0 ||
> > +	    strcmp(prog->sec_name, "fexit.multi.s") == 0 ||
> > +	    strcmp(prog->sec_name, "fsession.multi.s") == 0)
> > +		return 0;
> > +
> > +	is_fexit = str_has_pfx(prog->sec_name, "fexit.multi/");
> > +	is_fsession = str_has_pfx(prog->sec_name, "fsession.multi/");
> > +
> > +	if (is_fsession)
> > +		spec = prog->sec_name + sizeof("fsession.multi/") - 1;
> > +	else if (is_fexit)
> > +		spec = prog->sec_name + sizeof("fexit.multi/") - 1;
> > +	else
> > +		spec = prog->sec_name + sizeof("fentry.multi/") - 1;
> 
> The sleepable variants with patterns don't seem to be handled here.
> For a section name like "fentry.multi.s/my_func", none of the
> str_has_pfx checks match because "fexit.multi/" and "fsession.multi/"
> are not prefixes of "fentry.multi.s/..." (the character at position
> 11 or 14 is '.' not '/').
> 
> The else branch then computes spec using sizeof("fentry.multi/") - 1
> which gives an offset of 13, pointing into "s/my_func" instead of
> "my_func". The sscanf then extracts "s" as the pattern, and the
> program attaches to any function matching the glob "s" rather than
> "my_func".
> 
> Similarly for "fexit.multi.s/X" and "fsession.multi.s/X", the
> offsets are wrong and produce either error returns or incorrect
> patterns.
> 
> The SEC_DEF entries register all six sleepable variants, and
> sec_def_matches() correctly distinguishes "fentry.multi.s+" from
> "fentry.multi+" using the '/' separator check, so the matching
> works but the pattern extraction here does not.
> 
> Would it work to also check for the ".s/" variants, for example:
> 
>     is_fexit = str_has_pfx(prog->sec_name, "fexit.multi/") ||
>                str_has_pfx(prog->sec_name, "fexit.multi.s/");
> 
> with the corresponding sizeof adjustments for the spec offset?

yes, I mentioned in the cover letter that I did not add tests for
sleepable functions attachments.. I should have ;-) will fix

thanks,
jirka

^ permalink raw reply

* [PATCH 6.12.y] tracing: Fix to clear fprobe after unregister_fprobe() when module unloading
From: Masami Hiramatsu (Google) @ 2026-02-22 15:26 UTC (permalink / raw)
  To: stable, Steven Rostedt
  Cc: Mathieu Desnoyers, Masami Hiramatsu, linux-kernel,
	linux-trace-kernel

From: Masami Hiramatsu <mhiramat@kernel.org>

Clear fprobe after unregister_fprobe() for preventing double
unregistering fprobe.

Without this fix, test.d/dynevent/add_remove_tprobe_module.tc test
case of ftracetest caused a kernel panic as below on 6.12.y.

This is only happens on 6.12.y because this bug was introduced by
commit 5ba4f58ec2de ("tracing: tprobe-events: Fix to clean up tprobe
correctly when module unload"). This fix expects that the new fprobe
implementation based on fgraph, but on 6.12.y, fprobe is still using
ftrace.

 ------------[ cut here ]------------
 WARNING: CPU: 0 PID: 156 at kernel/trace/ftrace.c:378 __unregister_ftrace_function+0x154/0x170
 Modules linked in: [last unloaded: trace_events_sample]
 CPU: 0 UID: 0 PID: 156 Comm: ftracetest Not tainted 6.12.74 #1
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
 RIP: 0010:__unregister_ftrace_function+0x154/0x170
 Code: 85 30 ff ff ff c6 05 fd d5 85 01 01 48 c7 c7 eb 8e 14 82 be 39 01 00 00 48 c7 c2 dd bd 1c 82 e8 52 12 93 00 e9 0c ff ff ff 90 <0f> 0b 90 b8 f0 ff ff ff 5b e9 be 8b 95 00 cc 66 66 66 66 2e 0f 1f
 RSP: 0018:ffffc900005c3b48 EFLAGS: 00010246
 RAX: 0000000000000000 RBX: ffff8880054ba818 RCX: 7a7d3ccd1e752c00
 RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff8880054ba818
 RBP: 0000000000000000 R08: 000000000000017b R09: 0000000000000000
 R10: 0000000000000002 R11: 0000000000000000 R12: ffff8880048216d0
 R13: ffff8880052f7850 R14: ffff8880054ba818 R15: ffffffff8124e160
 FS:  000000002de743c0(0000) GS:ffff88807d800000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 000000002de8b898 CR3: 0000000005b3a000 CR4: 00000000000006b0
 Call Trace:
  <TASK>
  ftrace_shutdown+0x25/0x260
  ? __pfx_dyn_event_open+0x10/0x10
  unregister_ftrace_function+0x2a/0x140
  ? __pfx_dyn_event_open+0x10/0x10
  unregister_fprobe+0x57/0x90
  trace_fprobe_release+0x56/0x150
  dyn_event_open+0x99/0xe0
  do_dentry_open+0x14a/0x3e0
  vfs_open+0x2c/0xe0
  path_openat+0xca5/0xf10
  ? __lock_acquire+0xd38/0x2af0
  ? __create_object+0x36/0x100
  ? __create_object+0x36/0x100
  do_filp_open+0xb5/0x160
  do_sys_openat2+0x7f/0xd0
  __x64_sys_openat+0x81/0xa0
  do_syscall_64+0xec/0x1d0
  ? exc_page_fault+0x92/0x110
  entry_SYSCALL_64_after_hwframe+0x77/0x7f
 RIP: 0033:0x4aa9cb
 Code: 25 00 00 41 00 3d 00 00 41 00 74 4b 64 8b 04 25 18 00 00 00 85 c0 75 67 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00 f0 ff ff 0f 87 91 00 00 00 48 8b 54 24 28 64 48 2b 14 25
 RSP: 002b:00007ffce30daf50 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
 RAX: ffffffffffffffda RBX: 000000002de79bd0 RCX: 00000000004aa9cb
 RDX: 0000000000000241 RSI: 000000002deb08f0 RDI: 00000000ffffff9c
 RBP: 000000002deb08f0 R08: 0000000000000000 R09: 0000000000000000
 R10: 00000000000001b6 R11: 0000000000000246 R12: 0000000000000241
 R13: 000000002deb08f0 R14: 00007ffce30db3f8 R15: 0000000000000000
  </TASK>
 irq event stamp: 147361
 hardirqs last  enabled at (147373): [<ffffffff81128511>] __console_unlock+0x81/0xd0
 hardirqs last disabled at (147386): [<ffffffff811284f6>] __console_unlock+0x66/0xd0
 softirqs last  enabled at (146866): [<ffffffff8109a74f>] handle_softirqs+0x34f/0x3b0
 softirqs last disabled at (146861): [<ffffffff8109a956>] __irq_exit_rcu+0x66/0xd0
 ---[ end trace 0000000000000000 ]---
 BUG: kernel NULL pointer dereference, address: 000000000000002e
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 PGD 8000000005af0067 P4D 8000000005af0067 PUD 55d7067 PMD 0
 Oops: Oops: 0000 [#1] PREEMPT SMP PTI
 CPU: 0 UID: 0 PID: 156 Comm: ftracetest Tainted: G        W          6.12.74 #1
 Tainted: [W]=WARN
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
 RIP: 0010:trace_fprobe_release+0x78/0x150
 Code: 4c 89 f7 e8 ba e2 ff ff ba f0 01 00 00 4c 89 f7 31 f6 e8 9b 64 8d 00 48 8b bb 10 02 00 00 48 85 ff 74 21 4c 8d b3 10 02 00 00 <48> 8b 77 30 31 d2 e8 8d cf f8 ff 49 c7 06 00 00 00 00 49 c7 46 08
 RSP: 0018:ffffc900005c3be8 EFLAGS: 00010282
 RAX: ffff8880054ba818 RBX: ffff8880054ba800 RCX: 0000000000000000
 RDX: 0000000000000000 RSI: 0000000000000000 RDI: fffffffffffffffe
 RBP: 00000000fffffff0 R08: 000000000000017b R09: 0000000000000000
 R10: ffff8880054ba818 R11: 0000000000000000 R12: ffff8880048216d0
 R13: ffff8880052f7850 R14: ffff8880054baa10 R15: ffffffff8124e160
 FS:  000000002de743c0(0000) GS:ffff88807d800000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 000000000000002e CR3: 0000000005b3a000 CR4: 00000000000006b0
 Call Trace:
  <TASK>
  dyn_event_open+0x99/0xe0
  do_dentry_open+0x14a/0x3e0
  vfs_open+0x2c/0xe0
  path_openat+0xca5/0xf10
  ? __lock_acquire+0xd38/0x2af0
  ? __create_object+0x36/0x100
  ? __create_object+0x36/0x100
  do_filp_open+0xb5/0x160
  do_sys_openat2+0x7f/0xd0
  __x64_sys_openat+0x81/0xa0
  do_syscall_64+0xec/0x1d0
  ? exc_page_fault+0x92/0x110
  entry_SYSCALL_64_after_hwframe+0x77/0x7f
 RIP: 0033:0x4aa9cb
 Code: 25 00 00 41 00 3d 00 00 41 00 74 4b 64 8b 04 25 18 00 00 00 85 c0 75 67 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00 f0 ff ff 0f 87 91 00 00 00 48 8b 54 24 28 64 48 2b 14 25
 RSP: 002b:00007ffce30daf50 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
 RAX: ffffffffffffffda RBX: 000000002de79bd0 RCX: 00000000004aa9cb
 RDX: 0000000000000241 RSI: 000000002deb08f0 RDI: 00000000ffffff9c
 RBP: 000000002deb08f0 R08: 0000000000000000 R09: 0000000000000000
 R10: 00000000000001b6 R11: 0000000000000246 R12: 0000000000000241
 R13: 000000002deb08f0 R14: 00007ffce30db3f8 R15: 0000000000000000
  </TASK>
 Modules linked in: [last unloaded: trace_events_sample]
 CR2: 000000000000002e
 ---[ end trace 0000000000000000 ]---

Fixes: 5ba4f58ec2de ("tracing: tprobe-events: Fix to clean up tprobe correctly when module unload")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace_fprobe.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index 440dbfa6bbfd..2cf5036c825d 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -984,6 +984,7 @@ static int __tracepoint_probe_module_cb(struct notifier_block *self,
 			}
 		} else if (val == MODULE_STATE_GOING && tp_mod->mod == tf->mod) {
 			unregister_fprobe(&tf->fp);
+			memset(&tf->fp, 0, sizeof(tf->fp));
 			if (trace_fprobe_is_tracepoint(tf)) {
 				tracepoint_probe_unregister(tf->tpoint,
 					tf->tpoint->probestub, NULL);


^ permalink raw reply related

* Re: [PATCH v2 3/4] ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer
From: Masami Hiramatsu @ 2026-02-23  7:39 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel
In-Reply-To: <20260220145656.7dd289f5@gandalf.local.home>

On Fri, 20 Feb 2026 14:56:56 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 18 Feb 2026 19:14:35 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > Skip invalid sub-buffers when validating the persistent ring buffer
> > instead of invalidate all ring buffers.
> 
>   instead of discarding the entire ring buffer.
> 
> 
> > 
> > If the cache data in memory fails to be synchronized during a reboot,
> > the persistent ring buffer may become partially corrupted, but other
> > sub-buffers may still contain readable event data, allowing usersto
> > recover data from the corrupted ring buffer.
> 
>                   ... contain readable event data. Only discard the
>                   subbuffers that are found to be corrupted.
> 
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> >  kernel/trace/ring_buffer.c |   22 ++++++++++++----------
> >  1 file changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> > index d2b69221a94c..0ae2a5ad8c3e 100644
> > --- a/kernel/trace/ring_buffer.c
> > +++ b/kernel/trace/ring_buffer.c
> > @@ -2045,17 +2045,19 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
> >  		if (ret < 0) {
> >  			pr_info("Ring buffer meta [%d] invalid buffer page\n",
> >  				cpu_buffer->cpu);
> > -			goto invalid;
> > -		}
> > -
> > -		/* If the buffer has content, update pages_touched */
> > -		if (ret)
> > -			local_inc(&cpu_buffer->pages_touched);
> > -
> > -		entries += ret;
> > -		entry_bytes += local_read(&head_page->page->commit);
> > -		local_set(&cpu_buffer->head_page->entries, ret);
> > +			/* Instead of invalidate whole ring buffer, just clear this subbuffer. */
> > +			local_set(&head_page->entries, 0);
> > +			local_set(&head_page->page->commit, 0);
> > +			/* TODO: commit an event to mark this is broken. */
> 
> Here's how to fix the TODO:
> 
> 			local_set(&head_page->page->commit, RB_MISSED_EVENTS);

Ah, that's a nice flag!

Thanks!

> 
> -- Steve
> 
> 
> > +		} else {
> > +			/* If the buffer has content, update pages_touched */
> > +			if (ret)
> > +				local_inc(&cpu_buffer->pages_touched);
> >  
> > +			entries += ret;
> > +			entry_bytes += local_read(&head_page->page->commit);
> > +			local_set(&cpu_buffer->head_page->entries, ret);
> > +		}
> >  		if (head_page == cpu_buffer->commit_page)
> >  			break;
> >  	}
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply

* [syzbot] Monthly trace report (Feb 2026)
From: syzbot @ 2026-02-23  8:39 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel, syzkaller-bugs

Hello trace maintainers/developers,

This is a 31-day syzbot report for the trace subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/trace

During the period, 1 new issues were detected and 1 were fixed.
In total, 13 issues are still open and 63 have already been fixed.

Some of the still happening issues:

Ref Crashes Repro Title
<1> 111     Yes   WARNING in tracepoint_probe_unregister (3)
                  https://syzkaller.appspot.com/bug?extid=a1d25e53cd4a10f7f2d3
<2> 104     Yes   WARNING in blk_register_tracepoints
                  https://syzkaller.appspot.com/bug?extid=c54ded83396afee31eb1
<3> 73      Yes   INFO: task hung in blk_trace_ioctl (4)
                  https://syzkaller.appspot.com/bug?extid=ed812ed461471ab17a0c
<4> 41      Yes   INFO: task hung in blk_trace_startstop
                  https://syzkaller.appspot.com/bug?extid=774863666ef5b025c9d0
<5> 31      No    WARNING in ring_buffer_map_get_reader (2)
                  https://syzkaller.appspot.com/bug?extid=c7143161d8215214a993
<6> 16      Yes   KASAN: slab-use-after-free Read in bpf_trace_run2 (3)
                  https://syzkaller.appspot.com/bug?extid=59701a78e84b0bccfe1b
<7> 13      No    WARNING: refcount bug in call_timer_fn (4)
                  https://syzkaller.appspot.com/bug?extid=07dcf509f4c013e25dc5

---
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.

To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders

To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem

You may send multiple commands in a single email message.

^ permalink raw reply

* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: David Hildenbrand (Arm) @ 2026-02-23 13:07 UTC (permalink / raw)
  To: Gregory Price, lsf-pc
  Cc: linux-kernel, linux-cxl, cgroups, linux-mm, linux-trace-kernel,
	damon, kernel-team, gregkh, rafael, dakr, dave, jonathan.cameron,
	dave.jiang, alison.schofield, vishal.l.verma, ira.weiny,
	dan.j.williams, longman, akpm, lorenzo.stoakes, Liam.Howlett,
	vbabka, rppt, surenb, mhocko, osalvador, ziy, matthew.brost,
	joshua.hahnjy, rakie.kim, byungchul, ying.huang, apopple,
	axelrasmussen, yuanchu, weixugc, yury.norov, linux, mhiramat,
	mathieu.desnoyers, tj, hannes, mkoutny, jackmanb, sj, baolin.wang,
	npache, ryan.roberts, dev.jain, baohua, lance.yang, muchun.song,
	xu.xin16, chengming.zhou, jannh, linmiaohe, nao.horiguchi,
	pfalcato, rientjes, shakeel.butt, riel, harry.yoo, cl,
	roman.gushchin, chrisl, kasong, shikemeng, nphamcs, bhe,
	zhengqi.arch, terry.bowman
In-Reply-To: <20260222084842.1824063-1-gourry@gourry.net>

On 2/22/26 09:48, Gregory Price wrote:
> Topic type: MM
> 
> Presenter: Gregory Price <gourry@gourry.net>
> 
> This series introduces N_MEMORY_PRIVATE, a NUMA node state for memory
> managed by the buddy allocator but excluded from normal allocations.
> 
> I present it with an end-to-end Compressed RAM service (mm/cram.c)
> that would otherwise not be possible (or would be considerably more
> difficult, be device-specific, and add to the ZONE_DEVICE boondoggle).
> 
> 
> TL;DR
> ===
> 
> N_MEMORY_PRIVATE is all about isolating NUMA nodes and then punching
> explicit holes in that isolation to do useful things we couldn't do
> before without re-implementing entire portions of mm/ in a driver.
> 
> 
> /* This is my memory. There are many like it, but this one is mine. */
> rc = add_private_memory_driver_managed(nid, start, size, name, flags,
>                                         online_type, private_context);
> 
> page = alloc_pages_node(nid, __GFP_PRIVATE, 0);
> 
> /* Ok but I want to do something useful with it */
> static const struct node_private_ops ops = {
>          .migrate_to     = my_migrate_to,
>          .folio_migrate  = my_folio_migrate,
>          .flags = NP_OPS_MIGRATION | NP_OPS_MEMPOLICY,
> };
> node_private_set_ops(nid, &ops);
> 
> /* And now I can use mempolicy with my memory */
> buf = mmap(...);
> mbind(buf, len, mode, private_node, ...);
> buf[0] = 0xdeadbeef;  /* Faults onto private node */
> 
> /* And to be clear, no one else gets my memory */
> buf2 = malloc(4096);  /* Standard allocation */
> buf2[0] = 0xdeadbeef; /* Can never land on private node */
> 
> /* But i can choose to migrate it to the private node */
> move_pages(0, 1, &buf, &private_node, NULL, ...);
> 
> /* And more fun things like this */
> 
> 
> Patchwork
> ===
> A fully working branch based on cxl/next can be found here:
> https://github.com/gourryinverse/linux/tree/private_compression
> 
> A QEMU device which can inject high/low interrupts can be found here:
> https://github.com/gourryinverse/qemu/tree/compressed_cxl_clean
> 
> The additional patches on these branches are CXL and DAX driver
> housecleaning only tangentially relevant to this RFC, so i've
> omitted them for the sake of trying to keep it somewhat clean
> here.  Those patches should (hopefully) be going upstream anyway.
> 
> Patches 1-22: Core Private Node Infrastructure
> 
>    Patch  1:      Introduce N_MEMORY_PRIVATE scaffolding
>    Patch  2:      Introduce __GFP_PRIVATE
>    Patch  3:      Apply allocation isolation mechanisms
>    Patch  4:      Add N_MEMORY nodes to private fallback lists
>    Patches 5-9:   Filter operations not yet supported
>    Patch 10:      free_folio callback
>    Patch 11:      split_folio callback
>    Patches 12-20: mm/ service opt-ins:
>                     Migration, Mempolicy, Demotion, Write Protect,
>                     Reclaim, OOM, NUMA Balancing, Compaction,
>                     LongTerm Pinning
>    Patch 21:      memory_failure callback
>    Patch 22:      Memory hotplug plumbing for private nodes
> 
> Patch 23: mm/cram -- Compressed RAM Management
> 
> Patches 24-27: CXL Driver examples
>    Sysram Regions with Private node support
>    Basic Driver Example: (MIGRATION | MEMPOLICY)
>    Compression Driver Example (Generic)
> 
> 
> Background
> ===
> 
> Today, drivers that want mm-like services on non-general-purpose
> memory either use ZONE_DEVICE (self-managed memory) or hotplug into
> N_MEMORY and accept the risk of uncontrolled allocation.
> 
> Neither option provides what we really want - the ability to:
> 	1) selectively participate in mm/ subsystems, while
> 	2) isolating that memory from general purpose use.
> 
> Some device-attached memory cannot be managed as fully general-purpose
> system RAM.  CXL devices with inline compression, for example, may
> corrupt data or crash the machine if the compression ratio drops
> below a threshold -- we simply run out of physical memory.
> 
> This is a hard problem to solve: how does an operating system deal
> with a device that basically lies about how much capacity it has?
> 
> (We'll discuss that in the CRAM section)
> 
> 
> Core Proposal: N_MEMORY_PRIVATE
> ===
> 
> Introduce N_MEMORY_PRIVATE, a NUMA node state for memory managed by
> the buddy allocator, but excluded from normal allocation paths.
> 
> Private nodes:
> 
>    - Are filtered from zonelist fallback: all existing callers to
>      get_page_from_freelist cannot reach these nodes through any
>      normal fallback mechanism.
> 
>    - Filter allocation requests on __GFP_PRIVATE
>      	numa_zone_allowed() excludes them otherwise.
> 
>      Applies to systems with and without cpusets.
> 
>      GFP_PRIVATE is (__GFP_PRIVATE | __GFP_THISNODE).
> 
>      Services use it when they need to allocate specifically from
>      a private node (e.g., CRAM allocating a destination folio).
> 
>      No existing allocator path sets __GFP_PRIVATE, so private nodes
>      are unreachable by default.
> 
>    - Use standard struct page / folio.  No ZONE_DEVICE, no pgmap,
>      no struct page metadata limitations.
> 
>    - Use a node-scoped metadata structure to accomplish filtering
>      and callback support.
> 
>    - May participate in the buddy allocator, reclaim, compaction,
>      and LRU like normal memory, gated by an opt-in set of flags.
> 
> The key abstraction is node_private_ops: a per-node callback table
> registered by a driver or service.
> 
> Each callback is individually gated by an NP_OPS_* capability flag.
> 
> A driver opts in only to the mm/ operations it needs.
> 
> It is similar to ZONE_DEVICE's pgmap at a node granularity.
> 
> In fact...
> 
> 
> Re-use of ZONE_DEVICE Hooks
> ===
> 
> The callback insertion points deliberately mirror existing ZONE_DEVICE
> hooks to minimize the surface area of the mechanism.
> 
> I believe this could subsume most DEVICE_COHERENT users, and greatly
> simplify the device-managed memory development process (no more
> per-driver allocator and migration code).
> 
> (Also it's just "So Fresh, So Clean").
> 
> The base set of callbacks introduced include:
> 
>    free_folio           - mirrors ZONE_DEVICE's
>                           free_zone_device_page() hook in
>                           __folio_put() / folios_put_refs()
> 
>    folio_split          - mirrors ZONE_DEVICE's
>    			 called when a huge page is split up
> 
>    migrate_to           - demote_folio_list() custom demotion (same
>                           site as ZONE_DEVICE demotion rejection)
> 
>    folio_migrate        - called when private node folio is moved to
>                           another location (e.g. compaction)
> 
>    handle_fault         - mirrors the ZONE_DEVICE fault dispatch in
>                           handle_pte_fault() (do_wp_page path)
> 
>    reclaim_policy       - called by reclaim to let a driver own the
>                           boost lifecycle (driver can driver node reclaim)
> 
>    memory_failure       - parallels memory_failure_dev_pagemap(),
>                           but for online pages that enter the normal
>                           hwpoison path
> 
> At skip sites (mlock, madvise, KSM, user migration), a unified
> folio_is_private_managed() predicate covers both ZONE_DEVICE and
> N_MEMORY_PRIVATE folios, consolidating existing zone_device checks
> with private node checks rather than adding new ones.
> 
>    static inline bool folio_is_private_managed(struct folio *folio)
>    {
>            return folio_is_zone_device(folio) ||
>                   folio_is_private_node(folio);
>    }
> 
> Most integration points become a one-line swap:
> 
>    -     if (folio_is_zone_device(folio))
>    +     if (unlikely(folio_is_private_managed(folio)))
> 
> 
> Where a one-line integration is insufficient, the integration is
> kept as clean as possible with zone_device, rather than simply
> adding more call-sites on top of it:
> 
> static inline bool folio_managed_handle_fault(struct folio *folio,
>    struct vm_fault *vmf, vm_fault_t *ret)
> {
>    /* Zone device pages use swap entries; handled in do_swap_page */
>    if (folio_is_zone_device(folio))
>      return false;
> 
>    if (folio_is_private_node(folio)) {
>      const struct node_private_ops *ops = folio_node_private_ops(folio);
> 
>      if (ops && ops->handle_fault) {
>        *ret = ops->handle_fault(vmf);
>        return true;
>      }
>    }
>    return false;
> }
> 
> 
> 
> Flag-gated behavior (NP_OPS_*) controls:
> ===
> 
> We use OPS flags to denote what mm/ services we want to allow on our
> private node.   I've plumbed these through so far:
> 
>    NP_OPS_MIGRATION       - Node supports migration
>    NP_OPS_MEMPOLICY       - Node supports mempolicy actions
>    NP_OPS_DEMOTION        - Node appears in demotion target lists
>    NP_OPS_PROTECT_WRITE   - Node memory is read-only (wrprotect)
>    NP_OPS_RECLAIM         - Node supports reclaim
>    NP_OPS_NUMA_BALANCING  - Node supports numa balancing
>    NP_OPS_COMPACTION      - Node supports compaction
>    NP_OPS_LONGTERM_PIN    - Node supports longterm pinning
>    NP_OPS_OOM_ELIGIBLE	 - (MIGRATION | DEMOTION), node is reachable
>                             as normal system ram storage, so it should
> 			   be considered in OOM pressure calculations.
> 
> I wasn't quite sure how to classify ksm, khugepaged, madvise, and
> mlock - so i have omitted those for now.
> 
> Most hooks are straightforward.
> 
> Including a node as a demotion-eligible target was as simple as:
> 
> static void establish_demotion_targets(void)
> {
>    ..... snip .....
>    /*
>     * Include private nodes that have opted in to demotion
>     * via NP_OPS_DEMOTION.  A node might have custom migrate
>     */
>    all_memory = node_states[N_MEMORY];
>    for_each_node_state(node, N_MEMORY_PRIVATE) {
>        if (node_private_has_flag(node, NP_OPS_DEMOTION))
>        node_set(node, all_memory);
>    }
>    ..... snip .....
> }
> 
> The Migration and Mempolicy support are the two most complex pieces,
> and most useful things are built on top of Migration (meaning the
> remaining implementations are usually simple).
> 
> 
> Private Node Hotplug Lifecycle
> ===
> 
> Registration follows a strict order enforced by
> add_private_memory_driver_managed():
> 
>    1. Driver calls add_private_memory_driver_managed(nid, start,
>       size, resource_name, mhp_flags, online_type, &np).
> 
>    2. node_private_register(nid, &np) stores the driver's
>       node_private in pgdat and sets pgdat->private.  N_MEMORY and
>       N_MEMORY_PRIVATE are mutually exclusive -- registration fails
>       with -EBUSY if the node already has N_MEMORY set.
> 
>       Only one driver may register per private node.
> 
>    3. Memory is hotplugged via __add_memory_driver_managed().
> 
>       When online_pages() runs, it checks pgdat->private and sets
>       N_MEMORY_PRIVATE instead of N_MEMORY.
> 
>       Zonelist construction gives private nodes a self-only NOFALLBACK
>       list and an N_MEMORY fallback list (so kernel/slab allocations on
>       behalf of private node work can fall back to DRAM).
> 
>    4. kswapd and kcompactd are NOT started for private nodes.  The
>       owning service is responsible for driving reclaim if needed
>       (e.g., CRAM uses watermark_boost to wake kswapd on demand).
> 
> Teardown is the reverse:
> 
>    1. Driver calls offline_and_remove_private_memory(nid, start,
>       size).
> 
>    2. offline_pages() offlines the memory.  When the last block is
>       offlined, N_MEMORY_PRIVATE is cleared automatically.
> 
>    3. node_private_unregister() clears pgdat->node_private and
>       drops the refcount.  It refuses to unregister (-EBUSY) if
>       N_MEMORY_PRIVATE is still set (other memory ranges remain).
> 
> The driver is responsible for ensuring memory is hot-unpluggable
> before teardown.  The service must ensure all memory is cleaned
> up before hot-unplug - or the service must support migration (so
> memory_hotplug.c can evacuate the memory itself).
> 
> In the CRAM example, the service supports migration, so memory
> hot-unplug can remove memory without any special infrastructure.
> 
> 
> Application: Compressed RAM (mm/cram)
> ===
> 
> Compressed RAM has a serious design issue:  Its capacity a lie.
> 
> A compression device reports more capacity than it physically has.
> If workloads write faster than the OS can reclaim from the device,
> we run out of real backing store and corrupt data or crash.
> 
> I call this problem: "Trying to Out Run A Bear"
> 
> I.e. This is only stable as long as we stay ahead of the pressure.
> 
> We don't want to design a system where stability depends on outrunning
> a bear - I am slow and do not know where to acquire bear spray.
> 
>    Fun fact:   Grizzly bears have a top-speed of 56-64 km/h.
>    Unfun Fact: Humans typically top out at ~24 km/h.
> 
> This MVP takes a conservative position:
> 
>     all compressed memory is mapped read-only.
> 
>    - Folios reach the private node only via reclaim (demotion)
>    - migrate_to implements custom demotion with backpressure.
>    - fixup_migration_pte write-protects PTEs on arrival.
>    - wrprotect hooks prevent silent upgrades
>    - handle_fault promotes folios back to DRAM on write.
>    - free_folio scrubs stale data before buddy free.
> 
> Because pages are read-only, writes can never cause runaway
> compression ratio loss behind the allocator's back.  Every write
> goes through handle_fault, which promotes the folio to DRAM first.
> 
> The device only ever sees net compression (demotion in) and explicit
> decompression (promotion out via fault or reclaim), and has a much
> wider timeframe to respond to poor compression scenarios.
> 
> That means there's no bear to out run. The bears are safely asleep in
> their bear den, and even if they show up we have a bear-proof cage.
> 
> The backpressure system is our bear-proof cage: the driver reports real
> device utilization (generalized via watermark_boost on the private
> node's zone), and CRAM throttles demotion when capacity is tight.
> 
> If compression ratios are bad, we stop demoting pages and start
> evicting pages aggressively.
> 
> The service as designed is ~350 functional lines of code because it
> re-uses mm/ services:
> 
>    - Existing reclaim/vmscan code handles demotion.
>    - Existing migration code handles migration to/from.
>    - Existing page fault handling dispatches faults.
> 
> The driver contains all the CXL nastiness core developers don't want
> anything to do with - No vendor logic touches mm/ internals.
> 
> 
> 
> Future CRAM : Loosening the read-only constraint
> ===
> 
> The read-only model is safe but conservative.  For workloads where
> compressed pages are occasionally written, the promotion fault adds
> latency.  A future optimization could allow a tunable fraction of
> compressed pages to be mapped writable, accepting some risk of
> write-driven decompression in exchange for lower overhead.
> 
> The private node ops make this straightforward:
> 
>    - Adjust fixup_migration_pte to selectively skip
>      write-protection.
>    - Use the backpressure system to either revoke writable mappings,
>      deny additional demotions, or evict when device pressure rises.
> 
> This comes at a mild memory overhead: 32MB of DRAM per 1TB of CRAM.
> (1 bit per 4KB page).
> 
> This is not proposed here, but it should be somewhat trivial.
> 
> 
> Discussion Topics
> ===
> 0. Obviously I've included the set as an RFC, please rip it apart.
> 
> 1. Is N_MEMORY_PRIVATE the right isolation abstraction, or should
>     this extend ZONE_DEVICE?  Prior feedback pushed away from new
>     ZONE logic, but this will likely be debated further.
> 
>     My comments on this:
> 
>     ZONE_DEVICE requires re-implementing every service you want to
>     provide to your device memory, including basic allocation.
> 
>     Private nodes use real struct pages with no metadata
>     limitations, participate in the buddy allocator, and get NUMA
>     topology for free.
> 
> 2. Can this subsume ZONE_DEVICE COHERENT users?  The architecture
>     was designed with this in mind, but it is only a thought experiment.
> 
> 3. Is a dedicated mm/ service (cram) the right place for compressed
>     memory management, or should this be purely driver-side until
>     more devices exist?
> 
>     I wrote it this way because I forsee more "innovation" in the
>     compressed RAM space given current... uh... "Market Conditions".
> 
>     I don't see CRAM being CXL-specific, though the only solutions I've
>     seen have been CXL.  Nothing is stopping someone from soldering such
>     memory directly to a PCB.
> 
> 5. Where is your hardware-backed data that shows this works?
> 
>     I should have some by conference time.
> 
> Thanks for reading
> Gregory (Gourry)
> 
> 
> Gregory Price (27):
>    numa: introduce N_MEMORY_PRIVATE node state
>    mm,cpuset: gate allocations from N_MEMORY_PRIVATE behind __GFP_PRIVATE
>    mm/page_alloc: add numa_zone_allowed() and wire it up
>    mm/page_alloc: Add private node handling to build_zonelists
>    mm: introduce folio_is_private_managed() unified predicate
>    mm/mlock: skip mlock for managed-memory folios
>    mm/madvise: skip madvise for managed-memory folios
>    mm/ksm: skip KSM for managed-memory folios
>    mm/khugepaged: skip private node folios when trying to collapse.
>    mm/swap: add free_folio callback for folio release cleanup
>    mm/huge_memory.c: add private node folio split notification callback
>    mm/migrate: NP_OPS_MIGRATION - support private node user migration
>    mm/mempolicy: NP_OPS_MEMPOLICY - support private node mempolicy
>    mm/memory-tiers: NP_OPS_DEMOTION - support private node demotion
>    mm/mprotect: NP_OPS_PROTECT_WRITE - gate PTE/PMD write-upgrades

I'm concerned about adding more special-casing (similar to what we 
already added for ZONE_DEVICE) all over the place.

Like the whole folio_managed_() stuff in mprotect.c

Having that said, sounds like a reasonable topic to discuss.

-- 
Cheers,

David

^ permalink raw reply

* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Gregory Price @ 2026-02-23 14:54 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: lsf-pc, linux-kernel, linux-cxl, cgroups, linux-mm,
	linux-trace-kernel, damon, kernel-team, gregkh, rafael, dakr,
	dave, jonathan.cameron, dave.jiang, alison.schofield,
	vishal.l.verma, ira.weiny, dan.j.williams, longman, akpm,
	lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb, mhocko,
	osalvador, ziy, matthew.brost, joshua.hahnjy, rakie.kim,
	byungchul, ying.huang, apopple, axelrasmussen, yuanchu, weixugc,
	yury.norov, linux, mhiramat, mathieu.desnoyers, tj, hannes,
	mkoutny, jackmanb, sj, baolin.wang, npache, ryan.roberts,
	dev.jain, baohua, lance.yang, muchun.song, xu.xin16,
	chengming.zhou, jannh, linmiaohe, nao.horiguchi, pfalcato,
	rientjes, shakeel.butt, riel, harry.yoo, cl, roman.gushchin,
	chrisl, kasong, shikemeng, nphamcs, bhe, zhengqi.arch,
	terry.bowman
In-Reply-To: <c10400db-2259-4465-a07e-19d0691101a4@kernel.org>

On Mon, Feb 23, 2026 at 02:07:15PM +0100, David Hildenbrand (Arm) wrote:
> > 
> > Gregory Price (27):
> >    numa: introduce N_MEMORY_PRIVATE node state
> >    mm,cpuset: gate allocations from N_MEMORY_PRIVATE behind __GFP_PRIVATE
> >    mm/page_alloc: add numa_zone_allowed() and wire it up
> >    mm/page_alloc: Add private node handling to build_zonelists
> >    mm: introduce folio_is_private_managed() unified predicate
> >    mm/mlock: skip mlock for managed-memory folios
> >    mm/madvise: skip madvise for managed-memory folios
> >    mm/ksm: skip KSM for managed-memory folios
> >    mm/khugepaged: skip private node folios when trying to collapse.
> >    mm/swap: add free_folio callback for folio release cleanup
> >    mm/huge_memory.c: add private node folio split notification callback
> >    mm/migrate: NP_OPS_MIGRATION - support private node user migration
> >    mm/mempolicy: NP_OPS_MEMPOLICY - support private node mempolicy
> >    mm/memory-tiers: NP_OPS_DEMOTION - support private node demotion
> >    mm/mprotect: NP_OPS_PROTECT_WRITE - gate PTE/PMD write-upgrades
> 
> I'm concerned about adding more special-casing (similar to what we already
> added for ZONE_DEVICE) all over the place.
> 
> Like the whole folio_managed_() stuff in mprotect.c
> 
> Having that said, sounds like a reasonable topic to discuss.
> 

It's a valid concern - and is why I tried to re-use as many of the
zone_device hooks as possible.  It does not seem zone_device has quite
the same semantics for a case like this, so I had to make something new.

DEVICE_COHERENT injects a temporary swap entry to allow the device to do
a large atomic operation - then the page table is restored and the CPU
is free to change entries as it pleases.

Another option would be to add the hook to vma_wants_writenotify()
instead of the page table code - and mask MM_CP_TRY_CHANGE_WRITABLE.

This would require adding a vma flag - or maybe a count of protected /
device pages.

int mprotect_fixup() {
    ...
    if (vma_wants_manual_pte_write_upgrade(vma))
        mm_cp_flags |= MM_CP_TRY_CHANGE_WRITABLE;
}

bool vma_wants_writenotify(struct vm_area_struct *vma, pgprot_t vm_page_prot)
{
    if (vma->managed_wrprotect)
        return true;
}

That would localize the change in folio_managed_fixup_migration_pte() :

static inline pte_t folio_managed_fixup_migration_pte(struct page *new,
                                                      pte_t pte,
                                                      pte_t old_pte,
                                                      struct vm_area_struct *vma)
{
    ...
    } else if (folio_managed_wrprotect(page_folio(new))) {
        pte = pte_wrprotect(pte);
+       atomic_inc(&vma->managed_wrprotect);
    }
    return pte;
}

This would cover both the huge_memory.c and mprotect, and maybe that's
just generally cleaner? I can try that to see if it actually works.

~Gregory

^ permalink raw reply

* Re: [PATCH v4 10/12] riscv: Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS
From: Puranjay Mohan @ 2026-02-23 15:18 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Andy Chiu, linux-riscv, alexghiti, palmer, Björn Töpel,
	linux-kernel, linux-trace-kernel, Alexandre Ghiti, Mark Rutland,
	paul.walmsley, greentime.hu, nick.hu, nylon.chen, eric.lin,
	vicent.chen, zong.li, yongxuan.wang, samuel.holland, olivia.chu,
	c2232430, arnd, Sami Tolvanen, Kees Cook, Nathan Chancellor, llvm,
	pjw
In-Reply-To: <20260221-repeal-emphatic-ddc2e9b94208@spud>

On Sat, Feb 21, 2026 at 12:15 PM Conor Dooley <conor@kernel.org> wrote:
>
> Hey,
>
> On Tue, Apr 08, 2025 at 02:08:34AM +0800, Andy Chiu wrote:
> > From: Puranjay Mohan <puranjay12@gmail.com>
> >
> > This patch enables support for DYNAMIC_FTRACE_WITH_CALL_OPS on RISC-V.
> > This allows each ftrace callsite to provide an ftrace_ops to the common
> > ftrace trampoline, allowing each callsite to invoke distinct tracer
> > functions without the need to fall back to list processing or to
> > allocate custom trampolines for each callsite. This significantly speeds
> > up cases where multiple distinct trace functions are used and callsites
> > are mostly traced by a single tracer.
> >
> > The idea and most of the implementation is taken from the ARM64's
> > implementation of the same feature. The idea is to place a pointer to
> > the ftrace_ops as a literal at a fixed offset from the function entry
> > point, which can be recovered by the common ftrace trampoline.
> >
> > We use -fpatchable-function-entry to reserve 8 bytes above the function
> > entry by emitting 2 4 byte or 4 2 byte  nops depending on the presence of
> > CONFIG_RISCV_ISA_C. These 8 bytes are patched at runtime with a pointer
> > to the associated ftrace_ops for that callsite. Functions are aligned to
> > 8 bytes to make sure that the accesses to this literal are atomic.
> >
> > This approach allows for directly invoking ftrace_ops::func even for
> > ftrace_ops which are dynamically-allocated (or part of a module),
> > without going via ftrace_ops_list_func.
> >
> > We've benchamrked this with the ftrace_ops sample module on Spacemit K1
> > Jupiter:
> >
> > Without this patch:
> >
> > baseline (Linux rivos 6.14.0-09584-g7d06015d936c #3 SMP Sat Mar 29
> > +-----------------------+-----------------+----------------------------+
> > |  Number of tracers    | Total time (ns) | Per-call average time      |
> > |-----------------------+-----------------+----------------------------|
> > | Relevant | Irrelevant |    100000 calls | Total (ns) | Overhead (ns) |
> > |----------+------------+-----------------+------------+---------------|
> > |        0 |          0 |        1357958 |          13 |             - |
> > |        0 |          1 |        1302375 |          13 |             - |
> > |        0 |          2 |        1302375 |          13 |             - |
> > |        0 |         10 |        1379084 |          13 |             - |
> > |        0 |        100 |        1302458 |          13 |             - |
> > |        0 |        200 |        1302333 |          13 |             - |
> > |----------+------------+-----------------+------------+---------------|
> > |        1 |          0 |       13677833 |         136 |           123 |
> > |        1 |          1 |       18500916 |         185 |           172 |
> > |        1 |          2 |       22856459 |         228 |           215 |
> > |        1 |         10 |       58824709 |         588 |           575 |
> > |        1 |        100 |      505141584 |        5051 |          5038 |
> > |        1 |        200 |     1580473126 |       15804 |         15791 |
> > |----------+------------+-----------------+------------+---------------|
> > |        1 |          0 |       13561000 |         135 |           122 |
> > |        2 |          0 |       19707292 |         197 |           184 |
> > |       10 |          0 |       67774750 |         677 |           664 |
> > |      100 |          0 |      714123125 |        7141 |          7128 |
> > |      200 |          0 |     1918065668 |       19180 |         19167 |
> > +----------+------------+-----------------+------------+---------------+
> >
> > Note: per-call overhead is estimated relative to the baseline case with
> > 0 relevant tracers and 0 irrelevant tracers.
> >
> > With this patch:
> >
> > v4-rc4 (Linux rivos 6.14.0-09598-gd75747611c93 #4 SMP Sat Mar 29
> > +-----------------------+-----------------+----------------------------+
> > |  Number of tracers    | Total time (ns) | Per-call average time      |
> > |-----------------------+-----------------+----------------------------|
> > | Relevant | Irrelevant |    100000 calls | Total (ns) | Overhead (ns) |
> > |----------+------------+-----------------+------------+---------------|
> > |        0 |          0 |         1459917 |         14 |             - |
> > |        0 |          1 |         1408000 |         14 |             - |
> > |        0 |          2 |         1383792 |         13 |             - |
> > |        0 |         10 |         1430709 |         14 |             - |
> > |        0 |        100 |         1383791 |         13 |             - |
> > |        0 |        200 |         1383750 |         13 |             - |
> > |----------+------------+-----------------+------------+---------------|
> > |        1 |          0 |         5238041 |         52 |            38 |
> > |        1 |          1 |         5228542 |         52 |            38 |
> > |        1 |          2 |         5325917 |         53 |            40 |
> > |        1 |         10 |         5299667 |         52 |            38 |
> > |        1 |        100 |         5245250 |         52 |            39 |
> > |        1 |        200 |         5238459 |         52 |            39 |
> > |----------+------------+-----------------+------------+---------------|
> > |        1 |          0 |         5239083 |         52 |            38 |
> > |        2 |          0 |        19449417 |        194 |           181 |
> > |       10 |          0 |        67718584 |        677 |           663 |
> > |      100 |          0 |       709840708 |       7098 |          7085 |
> > |      200 |          0 |      2203580626 |      22035 |         22022 |
> > +----------+------------+-----------------+------------+---------------+
> >
> > Note: per-call overhead is estimated relative to the baseline case with
> > 0 relevant tracers and 0 irrelevant tracers.
> >
> > As can be seen from the above:
> >
> >  a) Whenever there is a single relevant tracer function associated with a
> >     tracee, the overhead of invoking the tracer is constant, and does not
> >     scale with the number of tracers which are *not* associated with that
> >     tracee.
> >
> >  b) The overhead for a single relevant tracer has dropped to ~1/3 of the
> >     overhead prior to this series (from 122ns to 38ns). This is largely
> >     due to permitting calls to dynamically-allocated ftrace_ops without
> >     going through ftrace_ops_list_func.
> >
> > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> >
> > [update kconfig, asm, refactor]
> >
> > Signed-off-by: Andy Chiu <andybnac@gmail.com>
> > Tested-by: Björn Töpel <bjorn@rivosinc.com>
>
> I bisected a boot failure to this commit [c217157bcd1df ("riscv:
> Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS")] yesterday, that appears
> to be affecting all LLVM versions that I currently have installed. From
> some initial testing of Kconfig options, it looks like the issue is
> CFI_CLANG related because when I disable CFI_CLANG things work once
> more. Since this option depends on !CFI_CLANG, but is def_bool y, I
> modified Kconfig to force disable it at all times and tested
> !DYNAMIC_FTRACE_WITH_CALL_OPS && !CFG_CLANG, which did boot.
>
> I dunno anything about what's going on in this patch, but so little in
> it relates to having DYNAMIC_FTRACE_WITH_CALL_OPS, that I was able to
> figure out that the problem is -fpatchable-function-entry=8,4
>

DYNAMIC_FTRACE_WITH_CALL_OPS can't work together with CFI_CLANG.

arm64 has:

select HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS \
if (DYNAMIC_FTRACE_WITH_ARGS && !CFI && \
   (CC_IS_CLANG || !CC_OPTIMIZE_FOR_SIZE))

would need something similar for riscv if not already done.

Thanks,
Puranjay

^ permalink raw reply

* Re: [PATCH v4 10/12] riscv: Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS
From: Conor Dooley @ 2026-02-23 15:27 UTC (permalink / raw)
  To: Puranjay Mohan
  Cc: Conor Dooley, Andy Chiu, linux-riscv, alexghiti, palmer,
	Björn Töpel, linux-kernel, linux-trace-kernel,
	Alexandre Ghiti, Mark Rutland, paul.walmsley, greentime.hu,
	nick.hu, nylon.chen, eric.lin, vicent.chen, zong.li,
	yongxuan.wang, samuel.holland, olivia.chu, c2232430, arnd,
	Sami Tolvanen, Kees Cook, Nathan Chancellor, llvm, pjw
In-Reply-To: <CANk7y0iTjHUBK66xx50b-eT=ssz9g0moFjOhCmipQC1SzkGs5g@mail.gmail.com>

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

On Mon, Feb 23, 2026 at 03:18:17PM +0000, Puranjay Mohan wrote:
> On Sat, Feb 21, 2026 at 12:15 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > Hey,
> >
> > On Tue, Apr 08, 2025 at 02:08:34AM +0800, Andy Chiu wrote:
> > > From: Puranjay Mohan <puranjay12@gmail.com>
> > >
> > > This patch enables support for DYNAMIC_FTRACE_WITH_CALL_OPS on RISC-V.
> > > This allows each ftrace callsite to provide an ftrace_ops to the common
> > > ftrace trampoline, allowing each callsite to invoke distinct tracer
> > > functions without the need to fall back to list processing or to
> > > allocate custom trampolines for each callsite. This significantly speeds
> > > up cases where multiple distinct trace functions are used and callsites
> > > are mostly traced by a single tracer.
> > >
> > > The idea and most of the implementation is taken from the ARM64's
> > > implementation of the same feature. The idea is to place a pointer to
> > > the ftrace_ops as a literal at a fixed offset from the function entry
> > > point, which can be recovered by the common ftrace trampoline.
> > >
> > > We use -fpatchable-function-entry to reserve 8 bytes above the function
> > > entry by emitting 2 4 byte or 4 2 byte  nops depending on the presence of
> > > CONFIG_RISCV_ISA_C. These 8 bytes are patched at runtime with a pointer
> > > to the associated ftrace_ops for that callsite. Functions are aligned to
> > > 8 bytes to make sure that the accesses to this literal are atomic.
> > >
> > > This approach allows for directly invoking ftrace_ops::func even for
> > > ftrace_ops which are dynamically-allocated (or part of a module),
> > > without going via ftrace_ops_list_func.
> > >
> > > We've benchamrked this with the ftrace_ops sample module on Spacemit K1
> > > Jupiter:
> > >
> > > Without this patch:
> > >
> > > baseline (Linux rivos 6.14.0-09584-g7d06015d936c #3 SMP Sat Mar 29
> > > +-----------------------+-----------------+----------------------------+
> > > |  Number of tracers    | Total time (ns) | Per-call average time      |
> > > |-----------------------+-----------------+----------------------------|
> > > | Relevant | Irrelevant |    100000 calls | Total (ns) | Overhead (ns) |
> > > |----------+------------+-----------------+------------+---------------|
> > > |        0 |          0 |        1357958 |          13 |             - |
> > > |        0 |          1 |        1302375 |          13 |             - |
> > > |        0 |          2 |        1302375 |          13 |             - |
> > > |        0 |         10 |        1379084 |          13 |             - |
> > > |        0 |        100 |        1302458 |          13 |             - |
> > > |        0 |        200 |        1302333 |          13 |             - |
> > > |----------+------------+-----------------+------------+---------------|
> > > |        1 |          0 |       13677833 |         136 |           123 |
> > > |        1 |          1 |       18500916 |         185 |           172 |
> > > |        1 |          2 |       22856459 |         228 |           215 |
> > > |        1 |         10 |       58824709 |         588 |           575 |
> > > |        1 |        100 |      505141584 |        5051 |          5038 |
> > > |        1 |        200 |     1580473126 |       15804 |         15791 |
> > > |----------+------------+-----------------+------------+---------------|
> > > |        1 |          0 |       13561000 |         135 |           122 |
> > > |        2 |          0 |       19707292 |         197 |           184 |
> > > |       10 |          0 |       67774750 |         677 |           664 |
> > > |      100 |          0 |      714123125 |        7141 |          7128 |
> > > |      200 |          0 |     1918065668 |       19180 |         19167 |
> > > +----------+------------+-----------------+------------+---------------+
> > >
> > > Note: per-call overhead is estimated relative to the baseline case with
> > > 0 relevant tracers and 0 irrelevant tracers.
> > >
> > > With this patch:
> > >
> > > v4-rc4 (Linux rivos 6.14.0-09598-gd75747611c93 #4 SMP Sat Mar 29
> > > +-----------------------+-----------------+----------------------------+
> > > |  Number of tracers    | Total time (ns) | Per-call average time      |
> > > |-----------------------+-----------------+----------------------------|
> > > | Relevant | Irrelevant |    100000 calls | Total (ns) | Overhead (ns) |
> > > |----------+------------+-----------------+------------+---------------|
> > > |        0 |          0 |         1459917 |         14 |             - |
> > > |        0 |          1 |         1408000 |         14 |             - |
> > > |        0 |          2 |         1383792 |         13 |             - |
> > > |        0 |         10 |         1430709 |         14 |             - |
> > > |        0 |        100 |         1383791 |         13 |             - |
> > > |        0 |        200 |         1383750 |         13 |             - |
> > > |----------+------------+-----------------+------------+---------------|
> > > |        1 |          0 |         5238041 |         52 |            38 |
> > > |        1 |          1 |         5228542 |         52 |            38 |
> > > |        1 |          2 |         5325917 |         53 |            40 |
> > > |        1 |         10 |         5299667 |         52 |            38 |
> > > |        1 |        100 |         5245250 |         52 |            39 |
> > > |        1 |        200 |         5238459 |         52 |            39 |
> > > |----------+------------+-----------------+------------+---------------|
> > > |        1 |          0 |         5239083 |         52 |            38 |
> > > |        2 |          0 |        19449417 |        194 |           181 |
> > > |       10 |          0 |        67718584 |        677 |           663 |
> > > |      100 |          0 |       709840708 |       7098 |          7085 |
> > > |      200 |          0 |      2203580626 |      22035 |         22022 |
> > > +----------+------------+-----------------+------------+---------------+
> > >
> > > Note: per-call overhead is estimated relative to the baseline case with
> > > 0 relevant tracers and 0 irrelevant tracers.
> > >
> > > As can be seen from the above:
> > >
> > >  a) Whenever there is a single relevant tracer function associated with a
> > >     tracee, the overhead of invoking the tracer is constant, and does not
> > >     scale with the number of tracers which are *not* associated with that
> > >     tracee.
> > >
> > >  b) The overhead for a single relevant tracer has dropped to ~1/3 of the
> > >     overhead prior to this series (from 122ns to 38ns). This is largely
> > >     due to permitting calls to dynamically-allocated ftrace_ops without
> > >     going through ftrace_ops_list_func.
> > >
> > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> > >
> > > [update kconfig, asm, refactor]
> > >
> > > Signed-off-by: Andy Chiu <andybnac@gmail.com>
> > > Tested-by: Björn Töpel <bjorn@rivosinc.com>
> >
> > I bisected a boot failure to this commit [c217157bcd1df ("riscv:
> > Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS")] yesterday, that appears
> > to be affecting all LLVM versions that I currently have installed. From
> > some initial testing of Kconfig options, it looks like the issue is
> > CFI_CLANG related because when I disable CFI_CLANG things work once
> > more. Since this option depends on !CFI_CLANG, but is def_bool y, I
> > modified Kconfig to force disable it at all times and tested
> > !DYNAMIC_FTRACE_WITH_CALL_OPS && !CFG_CLANG, which did boot.
> >
> > I dunno anything about what's going on in this patch, but so little in
> > it relates to having DYNAMIC_FTRACE_WITH_CALL_OPS, that I was able to
> > figure out that the problem is -fpatchable-function-entry=8,4
> >
> 
> DYNAMIC_FTRACE_WITH_CALL_OPS can't work together with CFI_CLANG.
> 
> arm64 has:
> 
> select HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS \
> if (DYNAMIC_FTRACE_WITH_ARGS && !CFI && \
>    (CC_IS_CLANG || !CC_OPTIMIZE_FOR_SIZE))
> 
> would need something similar for riscv if not already done.


I think you've misunderstood my email. We already have:

	select HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS if (DYNAMIC_FTRACE_WITH_ARGS && !CFI)

The problem is that the patch broke using CFI_CLANG, due to the
fpatchable-function-entry change.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH v4 10/12] riscv: Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS
From: Puranjay Mohan @ 2026-02-23 15:41 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Conor Dooley, Andy Chiu, linux-riscv, alexghiti, palmer,
	Björn Töpel, linux-kernel, linux-trace-kernel,
	Alexandre Ghiti, Mark Rutland, paul.walmsley, greentime.hu,
	nick.hu, nylon.chen, eric.lin, vicent.chen, zong.li,
	yongxuan.wang, samuel.holland, olivia.chu, c2232430, arnd,
	Sami Tolvanen, Kees Cook, Nathan Chancellor, llvm, pjw
In-Reply-To: <20260223-respect-chaste-60c3403b5bc9@wendy>

On Mon, Feb 23, 2026 at 3:28 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> On Mon, Feb 23, 2026 at 03:18:17PM +0000, Puranjay Mohan wrote:
> > On Sat, Feb 21, 2026 at 12:15 PM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > Hey,
> > >
> > > On Tue, Apr 08, 2025 at 02:08:34AM +0800, Andy Chiu wrote:
> > > > From: Puranjay Mohan <puranjay12@gmail.com>
> > > >
> > > > This patch enables support for DYNAMIC_FTRACE_WITH_CALL_OPS on RISC-V.
> > > > This allows each ftrace callsite to provide an ftrace_ops to the common
> > > > ftrace trampoline, allowing each callsite to invoke distinct tracer
> > > > functions without the need to fall back to list processing or to
> > > > allocate custom trampolines for each callsite. This significantly speeds
> > > > up cases where multiple distinct trace functions are used and callsites
> > > > are mostly traced by a single tracer.
> > > >
> > > > The idea and most of the implementation is taken from the ARM64's
> > > > implementation of the same feature. The idea is to place a pointer to
> > > > the ftrace_ops as a literal at a fixed offset from the function entry
> > > > point, which can be recovered by the common ftrace trampoline.
> > > >
> > > > We use -fpatchable-function-entry to reserve 8 bytes above the function
> > > > entry by emitting 2 4 byte or 4 2 byte  nops depending on the presence of
> > > > CONFIG_RISCV_ISA_C. These 8 bytes are patched at runtime with a pointer
> > > > to the associated ftrace_ops for that callsite. Functions are aligned to
> > > > 8 bytes to make sure that the accesses to this literal are atomic.
> > > >
> > > > This approach allows for directly invoking ftrace_ops::func even for
> > > > ftrace_ops which are dynamically-allocated (or part of a module),
> > > > without going via ftrace_ops_list_func.
> > > >
> > > > We've benchamrked this with the ftrace_ops sample module on Spacemit K1
> > > > Jupiter:
> > > >
> > > > Without this patch:
> > > >
> > > > baseline (Linux rivos 6.14.0-09584-g7d06015d936c #3 SMP Sat Mar 29
> > > > +-----------------------+-----------------+----------------------------+
> > > > |  Number of tracers    | Total time (ns) | Per-call average time      |
> > > > |-----------------------+-----------------+----------------------------|
> > > > | Relevant | Irrelevant |    100000 calls | Total (ns) | Overhead (ns) |
> > > > |----------+------------+-----------------+------------+---------------|
> > > > |        0 |          0 |        1357958 |          13 |             - |
> > > > |        0 |          1 |        1302375 |          13 |             - |
> > > > |        0 |          2 |        1302375 |          13 |             - |
> > > > |        0 |         10 |        1379084 |          13 |             - |
> > > > |        0 |        100 |        1302458 |          13 |             - |
> > > > |        0 |        200 |        1302333 |          13 |             - |
> > > > |----------+------------+-----------------+------------+---------------|
> > > > |        1 |          0 |       13677833 |         136 |           123 |
> > > > |        1 |          1 |       18500916 |         185 |           172 |
> > > > |        1 |          2 |       22856459 |         228 |           215 |
> > > > |        1 |         10 |       58824709 |         588 |           575 |
> > > > |        1 |        100 |      505141584 |        5051 |          5038 |
> > > > |        1 |        200 |     1580473126 |       15804 |         15791 |
> > > > |----------+------------+-----------------+------------+---------------|
> > > > |        1 |          0 |       13561000 |         135 |           122 |
> > > > |        2 |          0 |       19707292 |         197 |           184 |
> > > > |       10 |          0 |       67774750 |         677 |           664 |
> > > > |      100 |          0 |      714123125 |        7141 |          7128 |
> > > > |      200 |          0 |     1918065668 |       19180 |         19167 |
> > > > +----------+------------+-----------------+------------+---------------+
> > > >
> > > > Note: per-call overhead is estimated relative to the baseline case with
> > > > 0 relevant tracers and 0 irrelevant tracers.
> > > >
> > > > With this patch:
> > > >
> > > > v4-rc4 (Linux rivos 6.14.0-09598-gd75747611c93 #4 SMP Sat Mar 29
> > > > +-----------------------+-----------------+----------------------------+
> > > > |  Number of tracers    | Total time (ns) | Per-call average time      |
> > > > |-----------------------+-----------------+----------------------------|
> > > > | Relevant | Irrelevant |    100000 calls | Total (ns) | Overhead (ns) |
> > > > |----------+------------+-----------------+------------+---------------|
> > > > |        0 |          0 |         1459917 |         14 |             - |
> > > > |        0 |          1 |         1408000 |         14 |             - |
> > > > |        0 |          2 |         1383792 |         13 |             - |
> > > > |        0 |         10 |         1430709 |         14 |             - |
> > > > |        0 |        100 |         1383791 |         13 |             - |
> > > > |        0 |        200 |         1383750 |         13 |             - |
> > > > |----------+------------+-----------------+------------+---------------|
> > > > |        1 |          0 |         5238041 |         52 |            38 |
> > > > |        1 |          1 |         5228542 |         52 |            38 |
> > > > |        1 |          2 |         5325917 |         53 |            40 |
> > > > |        1 |         10 |         5299667 |         52 |            38 |
> > > > |        1 |        100 |         5245250 |         52 |            39 |
> > > > |        1 |        200 |         5238459 |         52 |            39 |
> > > > |----------+------------+-----------------+------------+---------------|
> > > > |        1 |          0 |         5239083 |         52 |            38 |
> > > > |        2 |          0 |        19449417 |        194 |           181 |
> > > > |       10 |          0 |        67718584 |        677 |           663 |
> > > > |      100 |          0 |       709840708 |       7098 |          7085 |
> > > > |      200 |          0 |      2203580626 |      22035 |         22022 |
> > > > +----------+------------+-----------------+------------+---------------+
> > > >
> > > > Note: per-call overhead is estimated relative to the baseline case with
> > > > 0 relevant tracers and 0 irrelevant tracers.
> > > >
> > > > As can be seen from the above:
> > > >
> > > >  a) Whenever there is a single relevant tracer function associated with a
> > > >     tracee, the overhead of invoking the tracer is constant, and does not
> > > >     scale with the number of tracers which are *not* associated with that
> > > >     tracee.
> > > >
> > > >  b) The overhead for a single relevant tracer has dropped to ~1/3 of the
> > > >     overhead prior to this series (from 122ns to 38ns). This is largely
> > > >     due to permitting calls to dynamically-allocated ftrace_ops without
> > > >     going through ftrace_ops_list_func.
> > > >
> > > > Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> > > >
> > > > [update kconfig, asm, refactor]
> > > >
> > > > Signed-off-by: Andy Chiu <andybnac@gmail.com>
> > > > Tested-by: Björn Töpel <bjorn@rivosinc.com>
> > >
> > > I bisected a boot failure to this commit [c217157bcd1df ("riscv:
> > > Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS")] yesterday, that appears
> > > to be affecting all LLVM versions that I currently have installed. From
> > > some initial testing of Kconfig options, it looks like the issue is
> > > CFI_CLANG related because when I disable CFI_CLANG things work once
> > > more. Since this option depends on !CFI_CLANG, but is def_bool y, I
> > > modified Kconfig to force disable it at all times and tested
> > > !DYNAMIC_FTRACE_WITH_CALL_OPS && !CFG_CLANG, which did boot.
> > >
> > > I dunno anything about what's going on in this patch, but so little in
> > > it relates to having DYNAMIC_FTRACE_WITH_CALL_OPS, that I was able to
> > > figure out that the problem is -fpatchable-function-entry=8,4
> > >
> >
> > DYNAMIC_FTRACE_WITH_CALL_OPS can't work together with CFI_CLANG.
> >
> > arm64 has:
> >
> > select HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS \
> > if (DYNAMIC_FTRACE_WITH_ARGS && !CFI && \
> >    (CC_IS_CLANG || !CC_OPTIMIZE_FOR_SIZE))
> >
> > would need something similar for riscv if not already done.
>
>
> I think you've misunderstood my email. We already have:
>
>         select HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS if (DYNAMIC_FTRACE_WITH_ARGS && !CFI)
>
> The problem is that the patch broke using CFI_CLANG, due to the
> fpatchable-function-entry change.


Yeah, sorry I did not see the patch,
the original one I sent had:

+ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS), y)
+ifeq ($(CONFIG_RISCV_ISA_C),y)
+ CC_FLAGS_FTRACE := -fpatchable-function-entry=8,4
+else
+ CC_FLAGS_FTRACE := -fpatchable-function-entry=4,2
+endif
+else


The basic Idea is that we can't put nops before the function entry
when using CFI_CLANG, because they both interfere with each other.

the fix should be something like:

-- >8 --

diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 371da75a47f9..94100810a6a4 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -14,11 +14,19 @@ endif
 ifeq ($(CONFIG_DYNAMIC_FTRACE),y)
        LDFLAGS_vmlinux += --no-relax
        KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
+ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS),y)
 ifeq ($(CONFIG_RISCV_ISA_C),y)
        CC_FLAGS_FTRACE := -fpatchable-function-entry=8,4
 else
        CC_FLAGS_FTRACE := -fpatchable-function-entry=4,2
 endif
+else
+ifeq ($(CONFIG_RISCV_ISA_C),y)
+       CC_FLAGS_FTRACE := -fpatchable-function-entry=4
+else
+       CC_FLAGS_FTRACE := -fpatchable-function-entry=2
+endif
+endif
 endif

 ifeq ($(CONFIG_CMODEL_MEDLOW),y)

-- 8< --


Thanks,
Puranjay

^ permalink raw reply related

* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Gregory Price @ 2026-02-23 16:08 UTC (permalink / raw)
  To: David Hildenbrand (Arm)
  Cc: linux-kernel, linux-cxl, cgroups, linux-mm, linux-trace-kernel,
	damon, kernel-team, gregkh, rafael, dakr, dave, jonathan.cameron,
	dave.jiang, alison.schofield, vishal.l.verma, ira.weiny,
	dan.j.williams, longman, akpm, lorenzo.stoakes, Liam.Howlett,
	vbabka, rppt, surenb, mhocko, osalvador, ziy, matthew.brost,
	joshua.hahnjy, rakie.kim, byungchul, ying.huang, apopple,
	axelrasmussen, yuanchu, weixugc, yury.norov, linux, mhiramat,
	mathieu.desnoyers, tj, hannes, mkoutny, jackmanb, sj, baolin.wang,
	npache, ryan.roberts, dev.jain, baohua, lance.yang, muchun.song,
	xu.xin16, chengming.zhou, jannh, linmiaohe, nao.horiguchi,
	pfalcato, rientjes, shakeel.butt, riel, harry.yoo, cl,
	roman.gushchin, chrisl, kasong, shikemeng, nphamcs, bhe,
	zhengqi.arch, terry.bowman
In-Reply-To: <aZxqP7J1kOClQUPQ@gourry-fedora-PF4VCD3F>

On Mon, Feb 23, 2026 at 09:54:55AM -0500, Gregory Price wrote:
> On Mon, Feb 23, 2026 at 02:07:15PM +0100, David Hildenbrand (Arm) wrote:
> > 
> > I'm concerned about adding more special-casing (similar to what we already
> > added for ZONE_DEVICE) all over the place.
> > 
> > Like the whole folio_managed_() stuff in mprotect.c
> > 
> > Having that said, sounds like a reasonable topic to discuss.
> > 
> 
> Another option would be to add the hook to vma_wants_writenotify()
> instead of the page table code - and mask MM_CP_TRY_CHANGE_WRITABLE.
> 

scratch all this - existing hooks exist for exactly this purpose:

	can_change_[pte|pmd]_writable()

Surprised I missed this.

I can clean this up to remove it from the page table walks.

Still valid to question whether we want this, but at least the hook
lives with other write-protect hooks now.

~Gregory

^ permalink raw reply

* [PATCH v3 0/3] ring-buffer: Making persistent ring buffer robust
From: Masami Hiramatsu (Google) @ 2026-02-23 16:15 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel

Hi,

Here is the 3rd version of improvement patches for making persistent
ring buffers robust to failures. This fixes some issues of persistent
ring buffer on real machines.
The previous version is here:

https://lore.kernel.org/all/177140965047.1537493.15501794841217306382.stgit@mhiramat.tok.corp.google.com/

In this version, I rebased the series on top of trace/fixes
branch (thus the first patch has been merged.)

I updated the description [1/3], used RB_MISSED_EVENTS flag to
indicate the corrupted page [3/3] and I found a new bug on
using rb_data_page::commit as index. So added a new fix [2/3].

Thank you,


---

Masami Hiramatsu (Google) (3):
      ring-buffer: Flush and stop persistent ring buffer on panic
      ring-buffer: Handle RB_MISSED_* flags on commit field correctly
      ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer


 kernel/trace/ring_buffer.c |   72 +++++++++++++++++++++++++++++---------------
 1 file changed, 47 insertions(+), 25 deletions(-)

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply

* [PATCH v3 1/3] ring-buffer: Flush and stop persistent ring buffer on panic
From: Masami Hiramatsu (Google) @ 2026-02-23 16:16 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel
In-Reply-To: <177186335195.133407.907308822749006594.stgit@devnote2>

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

On real hardware, panic and machine reboot may not flush hardware cache
to memory. This means the persistent ring buffer, which relies on a
coherent state of memory, may not have its events written to the buffer
and they may be lost. Moreover, there may be inconsistency with the
counters which are used for validation of the integrity of the
persistent ring buffer which may cause all data to be discarded.

To avoid this issue, stop recording of the ring buffer on panic and
flush the cache of the ring buffer's memory.

Fixes: e645535a954a ("tracing: Add option to use memmapped memory for trace boot instance")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/ring_buffer.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 1e7a34a31851..84a6459ed494 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -6,6 +6,7 @@
  */
 #include <linux/sched/isolation.h>
 #include <linux/trace_recursion.h>
+#include <linux/panic_notifier.h>
 #include <linux/trace_events.h>
 #include <linux/ring_buffer.h>
 #include <linux/trace_clock.h>
@@ -589,6 +590,7 @@ struct trace_buffer {
 
 	unsigned long			range_addr_start;
 	unsigned long			range_addr_end;
+	struct notifier_block		flush_nb;
 
 	struct ring_buffer_meta		*meta;
 
@@ -2471,6 +2473,16 @@ static void rb_free_cpu_buffer(struct ring_buffer_per_cpu *cpu_buffer)
 	kfree(cpu_buffer);
 }
 
+static int rb_flush_buffer_cb(struct notifier_block *nb, unsigned long event, void *data)
+{
+	struct trace_buffer *buffer = container_of(nb, struct trace_buffer, flush_nb);
+
+	ring_buffer_record_disable(buffer);
+	flush_kernel_vmap_range((void *)buffer->range_addr_start,
+				buffer->range_addr_end - buffer->range_addr_start);
+	return NOTIFY_DONE;
+}
+
 static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 					 int order, unsigned long start,
 					 unsigned long end,
@@ -2590,6 +2602,12 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 
 	mutex_init(&buffer->mutex);
 
+	/* Persistent ring buffer needs to flush cache before reboot. */
+	if (start & end) {
+		buffer->flush_nb.notifier_call = rb_flush_buffer_cb;
+		atomic_notifier_chain_register(&panic_notifier_list, &buffer->flush_nb);
+	}
+
 	return_ptr(buffer);
 
  fail_free_buffers:
@@ -2677,6 +2695,9 @@ ring_buffer_free(struct trace_buffer *buffer)
 {
 	int cpu;
 
+	if (buffer->range_addr_start && buffer->range_addr_end)
+		atomic_notifier_chain_unregister(&panic_notifier_list, &buffer->flush_nb);
+
 	cpuhp_state_remove_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
 
 	irq_work_sync(&buffer->irq_work.work);


^ permalink raw reply related

* [PATCH v3 2/3] ring-buffer: Handle RB_MISSED_* flags on commit field correctly
From: Masami Hiramatsu (Google) @ 2026-02-23 16:16 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel
In-Reply-To: <177186335195.133407.907308822749006594.stgit@devnote2>

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Since the MSBs of rb_data_page::commit are used for storing
RB_MISSED_EVENTS and RB_MISSED_STORED, we need to mask out those bits
when it is used for finding the size of data pages.

Fixes: 5f3b6e839f3c ("ring-buffer: Validate boot range memory events")
Fixes: 5b7be9c709e1 ("ring-buffer: Add test to validate the time stamp deltas")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Changes in v3:
  - Newly added.
---
 kernel/trace/ring_buffer.c |   32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 84a6459ed494..1ef718d21796 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -395,6 +395,18 @@ static __always_inline unsigned int rb_page_commit(struct buffer_page *bpage)
 	return local_read(&bpage->page->commit);
 }
 
+/* Size is determined by what has been committed */
+static __always_inline unsigned rb_page_size(struct buffer_page *bpage)
+{
+	return rb_page_commit(bpage) & ~RB_MISSED_MASK;
+}
+
+static __always_inline unsigned
+rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer)
+{
+	return rb_page_commit(cpu_buffer->commit_page);
+}
+
 static void free_buffer_page(struct buffer_page *bpage)
 {
 	/* Range pages are not to be freed */
@@ -1907,7 +1919,7 @@ static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu)
 	u64 delta;
 	int tail;
 
-	tail = local_read(&dpage->commit);
+	tail = local_read(&dpage->commit) & ~RB_MISSED_MASK;
 	return rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
 }
 
@@ -1934,7 +1946,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
 		goto invalid;
 	}
 	entries += ret;
-	entry_bytes += local_read(&cpu_buffer->reader_page->page->commit);
+	entry_bytes += rb_page_size(cpu_buffer->reader_page);
 	local_set(&cpu_buffer->reader_page->entries, ret);
 
 	ts = head_page->page->time_stamp;
@@ -2054,7 +2066,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
 			local_inc(&cpu_buffer->pages_touched);
 
 		entries += ret;
-		entry_bytes += local_read(&head_page->page->commit);
+		entry_bytes += rb_page_size(head_page);
 		local_set(&cpu_buffer->head_page->entries, ret);
 
 		if (head_page == cpu_buffer->commit_page)
@@ -3257,18 +3269,6 @@ rb_iter_head_event(struct ring_buffer_iter *iter)
 	return NULL;
 }
 
-/* Size is determined by what has been committed */
-static __always_inline unsigned rb_page_size(struct buffer_page *bpage)
-{
-	return rb_page_commit(bpage) & ~RB_MISSED_MASK;
-}
-
-static __always_inline unsigned
-rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer)
-{
-	return rb_page_commit(cpu_buffer->commit_page);
-}
-
 static __always_inline unsigned
 rb_event_index(struct ring_buffer_per_cpu *cpu_buffer, struct ring_buffer_event *event)
 {
@@ -4433,7 +4433,7 @@ static void check_buffer(struct ring_buffer_per_cpu *cpu_buffer,
 
 	if (tail == CHECK_FULL_PAGE) {
 		full = true;
-		tail = local_read(&bpage->commit);
+		tail = local_read(&bpage->commit) & ~RB_MISSED_MASK;
 	} else if (info->add_timestamp &
 		   (RB_ADD_STAMP_FORCE | RB_ADD_STAMP_ABSOLUTE)) {
 		/* Ignore events with absolute time stamps */


^ permalink raw reply related

* [PATCH v3 3/3] ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer
From: Masami Hiramatsu (Google) @ 2026-02-23 16:16 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel
In-Reply-To: <177186335195.133407.907308822749006594.stgit@devnote2>

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Skip invalid sub-buffers when validating the persistent ring buffer
instead of discarding the entire ring buffer. Also, mark there are
missed events on the discarded buffer.

If the cache data in memory fails to be synchronized during a reboot,
the persistent ring buffer may become partially corrupted, but other
sub-buffers may still contain readable event data. Only discard the
subbuffersa that ar found to be corrupted.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Changes in v3:
  - Record missed data event on commit.
---
 kernel/trace/ring_buffer.c |   21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 1ef718d21796..18b0b33e1dc8 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2058,17 +2058,18 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
 		if (ret < 0) {
 			pr_info("Ring buffer meta [%d] invalid buffer page\n",
 				cpu_buffer->cpu);
-			goto invalid;
-		}
-
-		/* If the buffer has content, update pages_touched */
-		if (ret)
-			local_inc(&cpu_buffer->pages_touched);
-
-		entries += ret;
-		entry_bytes += rb_page_size(head_page);
-		local_set(&cpu_buffer->head_page->entries, ret);
+			/* Instead of discard whole ring buffer, discard only this sub-buffer. */
+			local_set(&head_page->entries, 0);
+			local_set(&head_page->page->commit, RB_MISSED_EVENTS);
+		} else {
+			/* If the buffer has content, update pages_touched */
+			if (ret)
+				local_inc(&cpu_buffer->pages_touched);
 
+			entries += ret;
+			entry_bytes += rb_page_size(head_page);
+			local_set(&cpu_buffer->head_page->entries, ret);
+		}
 		if (head_page == cpu_buffer->commit_page)
 			break;
 	}


^ permalink raw reply related

* [PATCH v3 00/19] rv/rvgen: Robustness, modernization, and fixes
From: Wander Lairson Costa @ 2026-02-23 16:17 UTC (permalink / raw)
  To: Steven Rostedt, Gabriele Monaco, Nam Cao, Wander Lairson Costa,
	open list:RUNTIME VERIFICATION (RV), open list

This patch series introduces fixes and improvements to the RV Generator
(rvgen) tool in tools/verification. The primary goal is to enhance the
tool's robustness, maintainability, and correctness by addressing
several latent bugs, modernizing the Python codebase, and improving its
overall structure and error handling.

The changes include fixing logic errors in DOT file validation,
resolving unbound variable errors that could lead to runtime crashes,
and improving exception handling by removing dangerous bare except
clauses. The codebase is updated to use contemporary Python idioms such
as f-strings and context managers. Additionally, type annotations are
added to resolve static analysis errors.

Changes in v3:
- Dropped patch "add missing return type annotations" as per Gabriele
  Monaco's feedback that annotations on stub methods are not needed at
  this stage.
- Expanded AutomataError exception class patch to also replace generic
  ValueError exceptions in HA and LTL modules, as suggested by Gabriele
  Monaco.
- Updated typo fix patch to also correct singular/plural inconsistencies
  ("automata" to "automaton"), as noted by Gabriele Monaco.
- Included additional f-string conversion in dot2k.py found by Gabriele
  Monaco after rebase.

Changes in v2:
- Dropped patches related to the @not_implemented decorator and abstract
  method stubs (v1 patches 9, 18, 19) to address class hierarchy
  improvements in a separate series.
- Dropped trivial cleanup patches (boolean simplification, __contains__)
  as they were already fixed after rebasing from
  linux-trace/latency/for-next.
- Merged typo fix patches into a single patch.
- Changed the fix for the unbound loop variable warning in abbreviate_atoms
  pyright report.
- Reworked initial state validation to strictly enforce the presence of
  an initial state.

Wander Lairson Costa (19):
  rv/rvgen: introduce AutomataError exception class
  rv/rvgen: remove bare except clauses in generator
  rv/rvgen: replace % string formatting with f-strings
  rv/rvgen: replace __len__() calls with len()
  rv/rvgen: remove unnecessary semicolons
  rv/rvgen: use context managers for file operations
  rv/rvgen: fix typos in automata and generator docstring and comments
  rv/rvgen: fix PEP 8 whitespace violations
  rv/rvgen: fix DOT file validation logic error
  rv/rvgen: use class constant for init marker
  rv/rvgen: refactor automata.py to use iterator-based parsing
  rv/rvgen: remove unused sys import from dot2c
  rv/rvgen: remove unused __get_main_name method
  rv/rvgen: make monitor arguments required in rvgen
  rv/rvgen: fix isinstance check in Variable.expand()
  rv/rvgen: extract node marker string to class constant
  rv/rvgen: enforce presence of initial state
  rv/rvgen: fix unbound loop variable warning
  rv/rvgen: fix _fill_states() return type annotation

 tools/verification/rvgen/__main__.py        |  19 +--
 tools/verification/rvgen/dot2c              |   1 -
 tools/verification/rvgen/rvgen/automata.py  | 155 ++++++++++++--------
 tools/verification/rvgen/rvgen/dot2c.py     |  58 ++++----
 tools/verification/rvgen/rvgen/dot2k.py     |  61 ++++----
 tools/verification/rvgen/rvgen/generator.py |  91 +++++-------
 tools/verification/rvgen/rvgen/ltl2ba.py    |  11 +-
 tools/verification/rvgen/rvgen/ltl2k.py     |  54 ++++---
 8 files changed, 237 insertions(+), 213 deletions(-)

-- 
2.53.0


^ permalink raw reply

* [PATCH v3 01/19] rv/rvgen: introduce AutomataError exception class
From: Wander Lairson Costa @ 2026-02-23 16:17 UTC (permalink / raw)
  To: Steven Rostedt, Gabriele Monaco, Nam Cao, Wander Lairson Costa,
	open list:RUNTIME VERIFICATION (RV), open list
In-Reply-To: <20260223162407.147003-1-wander@redhat.com>

Replace the generic except Exception block with a custom AutomataError
class that inherits from Exception. This provides more precise exception
handling for automata parsing and validation errors while avoiding
overly broad exception catches that could mask programming errors like
SyntaxError or TypeError.

The AutomataError class is raised when DOT file processing fails due to
invalid format, I/O errors, or malformed automaton definitions. The
main entry point catches this specific exception and provides a
user-friendly error message to stderr before exiting.

Also, replace generic exceptions raising in HA and LTL with
AutomataError.

Co-authored-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/verification/rvgen/__main__.py        |  6 ++---
 tools/verification/rvgen/rvgen/automata.py  | 17 ++++++++++----
 tools/verification/rvgen/rvgen/dot2c.py     |  4 ++--
 tools/verification/rvgen/rvgen/dot2k.py     | 26 ++++++++++-----------
 tools/verification/rvgen/rvgen/generator.py |  7 ++----
 tools/verification/rvgen/rvgen/ltl2ba.py    |  9 +++----
 tools/verification/rvgen/rvgen/ltl2k.py     |  8 +++++--
 7 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/tools/verification/rvgen/__main__.py b/tools/verification/rvgen/__main__.py
index 9a5a9f08eae21..5a3f090ac3316 100644
--- a/tools/verification/rvgen/__main__.py
+++ b/tools/verification/rvgen/__main__.py
@@ -13,6 +13,7 @@ if __name__ == '__main__':
     from rvgen.generator import Monitor
     from rvgen.container import Container
     from rvgen.ltl2k import ltl2k
+    from rvgen.automata import AutomataError
     import argparse
     import sys
 
@@ -55,9 +56,8 @@ if __name__ == '__main__':
                 sys.exit(1)
         else:
             monitor = Container(vars(params))
-    except Exception as e:
-        print('Error: '+ str(e))
-        print("Sorry : :-(")
+    except AutomataError as e:
+        print(f"There was an error processing {params.spec}: {e}", file=sys.stderr)
         sys.exit(1)
 
     print("Writing the monitor into the directory %s" % monitor.name)
diff --git a/tools/verification/rvgen/rvgen/automata.py b/tools/verification/rvgen/rvgen/automata.py
index 5c1c5597d839f..9cc452305a2aa 100644
--- a/tools/verification/rvgen/rvgen/automata.py
+++ b/tools/verification/rvgen/rvgen/automata.py
@@ -25,6 +25,13 @@ class _EventConstraintKey(_ConstraintKey, tuple):
     def __new__(cls, state_id: int, event_id: int):
         return super().__new__(cls, (state_id, event_id))
 
+class AutomataError(Exception):
+    """Exception raised for errors in automata parsing and validation.
+
+    Raised when DOT file processing fails due to invalid format, I/O errors,
+    or malformed automaton definitions.
+    """
+
 class Automata:
     """Automata class: Reads a dot file and part it as an automata.
 
@@ -72,11 +79,11 @@ class Automata:
         basename = ntpath.basename(self.__dot_path)
         if not basename.endswith(".dot") and not basename.endswith(".gv"):
             print("not a dot file")
-            raise Exception("not a dot file: %s" % self.__dot_path)
+            raise AutomataError("not a dot file: %s" % self.__dot_path)
 
         model_name = ntpath.splitext(basename)[0]
         if model_name.__len__() == 0:
-            raise Exception("not a dot file: %s" % self.__dot_path)
+            raise AutomataError("not a dot file: %s" % self.__dot_path)
 
         return model_name
 
@@ -85,8 +92,8 @@ class Automata:
         dot_lines = []
         try:
             dot_file = open(self.__dot_path)
-        except:
-            raise Exception("Cannot open the file: %s" % self.__dot_path)
+        except OSError as exc:
+            raise AutomataError(exc.strerror) from exc
 
         dot_lines = dot_file.read().splitlines()
         dot_file.close()
@@ -95,7 +102,7 @@ class Automata:
         line = dot_lines[cursor].split()
 
         if (line[0] != "digraph") and (line[1] != "state_automaton"):
-            raise Exception("Not a valid .dot format: %s" % self.__dot_path)
+            raise AutomataError("Not a valid .dot format: %s" % self.__dot_path)
         else:
             cursor += 1
         return dot_lines
diff --git a/tools/verification/rvgen/rvgen/dot2c.py b/tools/verification/rvgen/rvgen/dot2c.py
index f779d9528af3f..6878cc79e6f70 100644
--- a/tools/verification/rvgen/rvgen/dot2c.py
+++ b/tools/verification/rvgen/rvgen/dot2c.py
@@ -13,7 +13,7 @@
 # For further information, see:
 #   Documentation/trace/rv/deterministic_automata.rst
 
-from .automata import Automata
+from .automata import Automata, AutomataError
 
 class Dot2c(Automata):
     enum_suffix = ""
@@ -103,7 +103,7 @@ class Dot2c(Automata):
             min_type = "unsigned int"
 
         if self.states.__len__() > 1000000:
-            raise Exception("Too many states: %d" % self.states.__len__())
+            raise AutomataError("Too many states: %d" % self.states.__len__())
 
         return min_type
 
diff --git a/tools/verification/rvgen/rvgen/dot2k.py b/tools/verification/rvgen/rvgen/dot2k.py
index e7ba68a54c1f8..55222e38323f5 100644
--- a/tools/verification/rvgen/rvgen/dot2k.py
+++ b/tools/verification/rvgen/rvgen/dot2k.py
@@ -11,7 +11,7 @@
 from collections import deque
 from .dot2c import Dot2c
 from .generator import Monitor
-from .automata import _EventConstraintKey, _StateConstraintKey
+from .automata import _EventConstraintKey, _StateConstraintKey, AutomataError
 
 
 class dot2k(Monitor, Dot2c):
@@ -166,14 +166,14 @@ class da2k(dot2k):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         if self.is_hybrid_automata():
-            raise ValueError("Detected hybrid automata, use the 'ha' class")
+            raise AutomataError("Detected hybrid automata, use the 'ha' class")
 
 class ha2k(dot2k):
     """Hybrid automata only"""
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         if not self.is_hybrid_automata():
-            raise ValueError("Detected deterministic automata, use the 'da' class")
+            raise AutomataError("Detected deterministic automata, use the 'da' class")
         self.trace_h = self._read_template_file("trace_hybrid.h")
         self.__parse_constraints()
 
@@ -266,22 +266,22 @@ class ha2k(dot2k):
         # state constraints are only used for expirations (e.g. clk<N)
         if self.is_event_constraint(key):
             if not rule and not reset:
-                raise ValueError("Unrecognised event constraint "
-                                 f"({self.states[key[0]]}/{self.events[key[1]]}: {constr})")
+                raise AutomataError("Unrecognised event constraint "
+                                    f"({self.states[key[0]]}/{self.events[key[1]]}: {constr})")
             if rule and (rule["env"] in self.env_types and
                          rule["env"] not in self.env_stored):
-                raise ValueError("Clocks in hybrid automata always require a storage"
-                                 f" ({rule["env"]})")
+                raise AutomataError("Clocks in hybrid automata always require a storage"
+                                    f" ({rule["env"]})")
         else:
             if not rule:
-                raise ValueError("Unrecognised state constraint "
-                                 f"({self.states[key]}: {constr})")
+                raise AutomataError("Unrecognised state constraint "
+                                    f"({self.states[key]}: {constr})")
             if rule["env"] not in self.env_stored:
-                raise ValueError("State constraints always require a storage "
-                                 f"({rule["env"]})")
+                raise AutomataError("State constraints always require a storage "
+                                    f"({rule["env"]})")
             if rule["op"] not in ["<", "<="]:
-                raise ValueError("State constraints must be clock expirations like"
-                                 f" clk<N ({rule.string})")
+                raise AutomataError("State constraints must be clock expirations like"
+                                    f" clk<N ({rule.string})")
 
     def __parse_constraints(self) -> None:
         self.guards: dict[_EventConstraintKey, str] = {}
diff --git a/tools/verification/rvgen/rvgen/generator.py b/tools/verification/rvgen/rvgen/generator.py
index 5eac12e110dce..571093a92bdc8 100644
--- a/tools/verification/rvgen/rvgen/generator.py
+++ b/tools/verification/rvgen/rvgen/generator.py
@@ -51,10 +51,7 @@ class RVGenerator:
         raise FileNotFoundError("Could not find the rv directory, do you have the kernel source installed?")
 
     def _read_file(self, path):
-        try:
-            fd = open(path, 'r')
-        except OSError:
-            raise Exception("Cannot open the file: %s" % path)
+        fd = open(path, 'r')
 
         content = fd.read()
 
@@ -65,7 +62,7 @@ class RVGenerator:
         try:
             path = os.path.join(self.abs_template_dir, file)
             return self._read_file(path)
-        except Exception:
+        except OSError:
             # Specific template file not found. Try the generic template file in the template/
             # directory, which is one level up
             path = os.path.join(self.abs_template_dir, "..", file)
diff --git a/tools/verification/rvgen/rvgen/ltl2ba.py b/tools/verification/rvgen/rvgen/ltl2ba.py
index f14e6760ac3db..f9855dfa3bc1c 100644
--- a/tools/verification/rvgen/rvgen/ltl2ba.py
+++ b/tools/verification/rvgen/rvgen/ltl2ba.py
@@ -9,6 +9,7 @@
 
 from ply.lex import lex
 from ply.yacc import yacc
+from .automata import AutomataError
 
 # Grammar:
 # 	ltl ::= opd | ( ltl ) | ltl binop ltl | unop ltl
@@ -62,7 +63,7 @@ t_ignore_COMMENT = r'\#.*'
 t_ignore = ' \t\n'
 
 def t_error(t):
-    raise ValueError(f"Illegal character '{t.value[0]}'")
+    raise AutomataError(f"Illegal character '{t.value[0]}'")
 
 lexer = lex()
 
@@ -487,7 +488,7 @@ def p_unop(p):
     elif p[1] == "not":
         op = NotOp(p[2])
     else:
-        raise ValueError(f"Invalid unary operator {p[1]}")
+        raise AutomataError(f"Invalid unary operator {p[1]}")
 
     p[0] = ASTNode(op)
 
@@ -507,7 +508,7 @@ def p_binop(p):
     elif p[2] == "imply":
         op = ImplyOp(p[1], p[3])
     else:
-        raise ValueError(f"Invalid binary operator {p[2]}")
+        raise AutomataError(f"Invalid binary operator {p[2]}")
 
     p[0] = ASTNode(op)
 
@@ -526,7 +527,7 @@ def parse_ltl(s: str) -> ASTNode:
             subexpr[assign[0]] = assign[1]
 
     if rule is None:
-        raise ValueError("Please define your specification in the \"RULE = <LTL spec>\" format")
+        raise AutomataError("Please define your specification in the \"RULE = <LTL spec>\" format")
 
     for node in rule:
         if not isinstance(node.op, Variable):
diff --git a/tools/verification/rvgen/rvgen/ltl2k.py b/tools/verification/rvgen/rvgen/ltl2k.py
index b075f98d50c47..08ad245462e7d 100644
--- a/tools/verification/rvgen/rvgen/ltl2k.py
+++ b/tools/verification/rvgen/rvgen/ltl2k.py
@@ -4,6 +4,7 @@
 from pathlib import Path
 from . import generator
 from . import ltl2ba
+from .automata import AutomataError
 
 COLUMN_LIMIT = 100
 
@@ -60,8 +61,11 @@ class ltl2k(generator.Monitor):
         if MonitorType != "per_task":
             raise NotImplementedError("Only per_task monitor is supported for LTL")
         super().__init__(extra_params)
-        with open(file_path) as f:
-            self.atoms, self.ba, self.ltl = ltl2ba.create_graph(f.read())
+        try:
+            with open(file_path) as f:
+                self.atoms, self.ba, self.ltl = ltl2ba.create_graph(f.read())
+        except OSError as exc:
+            raise AutomataError(exc.strerror) from exc
         self.atoms_abbr = abbreviate_atoms(self.atoms)
         self.name = extra_params.get("model_name")
         if not self.name:
-- 
2.53.0


^ permalink raw reply related

* [PATCH v3 02/19] rv/rvgen: remove bare except clauses in generator
From: Wander Lairson Costa @ 2026-02-23 16:17 UTC (permalink / raw)
  To: Steven Rostedt, Gabriele Monaco, Nam Cao, Wander Lairson Costa,
	open list:RUNTIME VERIFICATION (RV), open list
In-Reply-To: <20260223162407.147003-1-wander@redhat.com>

Remove bare except clauses from the generator module that were
catching all exceptions including KeyboardInterrupt and SystemExit.
This follows the same exception handling improvements made in the
previous AutomataError commit and addresses PEP 8 violations.

The bare except clause in __create_directory was silently catching
and ignoring all errors after printing a message, which could mask
serious issues. For __write_file, the bare except created a critical
bug where the file variable could remain undefined if open() failed,
causing a NameError when attempting to write to or close the file.

These methods now let OSError propagate naturally, allowing callers
to handle file system errors appropriately. This provides clearer
error reporting and allows Python's exception handling to show
complete stack traces with proper error types and locations.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
---
 tools/verification/rvgen/rvgen/generator.py | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/tools/verification/rvgen/rvgen/generator.py b/tools/verification/rvgen/rvgen/generator.py
index 571093a92bdc8..a2391a4c21ed6 100644
--- a/tools/verification/rvgen/rvgen/generator.py
+++ b/tools/verification/rvgen/rvgen/generator.py
@@ -198,17 +198,10 @@ obj-$(CONFIG_RV_MON_%s) += monitors/%s/%s.o
             os.mkdir(path)
         except FileExistsError:
             return
-        except:
-            print("Fail creating the output dir: %s" % self.name)
 
     def __write_file(self, file_name, content):
-        try:
-            file = open(file_name, 'w')
-        except:
-            print("Fail writing to file: %s" % file_name)
-
+        file = open(file_name, 'w')
         file.write(content)
-
         file.close()
 
     def _create_file(self, file_name, content):
-- 
2.53.0


^ permalink raw reply related

* [PATCH v3 03/19] rv/rvgen: replace % string formatting with f-strings
From: Wander Lairson Costa @ 2026-02-23 16:17 UTC (permalink / raw)
  To: Steven Rostedt, Gabriele Monaco, Nam Cao, Wander Lairson Costa,
	open list:RUNTIME VERIFICATION (RV), open list
In-Reply-To: <20260223162407.147003-1-wander@redhat.com>

Replace all instances of percent-style string formatting with
f-strings across the rvgen codebase. This modernizes the string
formatting to use Python 3.6+ features, providing clearer and more
maintainable code while improving runtime performance.

The conversion handles all formatting cases including simple variable
substitution, multi-variable formatting, and complex format specifiers.
Dynamic width formatting is converted from "%*s" to "{var:>{width}}"
using proper alignment syntax. Template strings for generated C code
properly escape braces using double-brace syntax to produce literal
braces in the output.

F-strings provide approximately 2x performance improvement over percent
formatting and are the recommended approach in modern Python.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
---
 tools/verification/rvgen/__main__.py        |  6 +--
 tools/verification/rvgen/rvgen/automata.py  |  6 +--
 tools/verification/rvgen/rvgen/dot2c.py     | 38 ++++++-------
 tools/verification/rvgen/rvgen/dot2k.py     | 29 +++++-----
 tools/verification/rvgen/rvgen/generator.py | 59 ++++++++++-----------
 tools/verification/rvgen/rvgen/ltl2k.py     | 30 +++++------
 6 files changed, 83 insertions(+), 85 deletions(-)

diff --git a/tools/verification/rvgen/__main__.py b/tools/verification/rvgen/__main__.py
index 5a3f090ac3316..2e5b868535470 100644
--- a/tools/verification/rvgen/__main__.py
+++ b/tools/verification/rvgen/__main__.py
@@ -44,7 +44,7 @@ if __name__ == '__main__':
 
     try:
         if params.subcmd == "monitor":
-            print("Opening and parsing the specification file %s" % params.spec)
+            print(f"Opening and parsing the specification file {params.spec}")
             if params.monitor_class == "da":
                 monitor = da2k(params.spec, params.monitor_type, vars(params))
             elif params.monitor_class == "ha":
@@ -60,11 +60,11 @@ if __name__ == '__main__':
         print(f"There was an error processing {params.spec}: {e}", file=sys.stderr)
         sys.exit(1)
 
-    print("Writing the monitor into the directory %s" % monitor.name)
+    print(f"Writing the monitor into the directory {monitor.name}")
     monitor.print_files()
     print("Almost done, checklist")
     if params.subcmd == "monitor":
-        print("  - Edit the %s/%s.c to add the instrumentation" % (monitor.name, monitor.name))
+        print(f"  - Edit the {monitor.name}/{monitor.name}.c to add the instrumentation")
         print(monitor.fill_tracepoint_tooltip())
     print(monitor.fill_makefile_tooltip())
     print(monitor.fill_kconfig_tooltip())
diff --git a/tools/verification/rvgen/rvgen/automata.py b/tools/verification/rvgen/rvgen/automata.py
index 9cc452305a2aa..4fed58cfa3c6e 100644
--- a/tools/verification/rvgen/rvgen/automata.py
+++ b/tools/verification/rvgen/rvgen/automata.py
@@ -79,11 +79,11 @@ class Automata:
         basename = ntpath.basename(self.__dot_path)
         if not basename.endswith(".dot") and not basename.endswith(".gv"):
             print("not a dot file")
-            raise AutomataError("not a dot file: %s" % self.__dot_path)
+            raise AutomataError(f"not a dot file: {self.__dot_path}")
 
         model_name = ntpath.splitext(basename)[0]
         if model_name.__len__() == 0:
-            raise AutomataError("not a dot file: %s" % self.__dot_path)
+            raise AutomataError(f"not a dot file: {self.__dot_path}")
 
         return model_name
 
@@ -102,7 +102,7 @@ class Automata:
         line = dot_lines[cursor].split()
 
         if (line[0] != "digraph") and (line[1] != "state_automaton"):
-            raise AutomataError("Not a valid .dot format: %s" % self.__dot_path)
+            raise AutomataError(f"Not a valid .dot format: {self.__dot_path}")
         else:
             cursor += 1
         return dot_lines
diff --git a/tools/verification/rvgen/rvgen/dot2c.py b/tools/verification/rvgen/rvgen/dot2c.py
index 6878cc79e6f70..e6a440e1588cd 100644
--- a/tools/verification/rvgen/rvgen/dot2c.py
+++ b/tools/verification/rvgen/rvgen/dot2c.py
@@ -29,17 +29,17 @@ class Dot2c(Automata):
 
     def __get_enum_states_content(self) -> list[str]:
         buff = []
-        buff.append("\t%s%s," % (self.initial_state, self.enum_suffix))
+        buff.append(f"\t{self.initial_state}{self.enum_suffix},")
         for state in self.states:
             if state != self.initial_state:
-                buff.append("\t%s%s," % (state, self.enum_suffix))
-        buff.append("\tstate_max%s," % (self.enum_suffix))
+                buff.append(f"\t{state}{self.enum_suffix},")
+        buff.append(f"\tstate_max{self.enum_suffix},")
 
         return buff
 
     def format_states_enum(self) -> list[str]:
         buff = []
-        buff.append("enum %s {" % self.enum_states_def)
+        buff.append(f"enum {self.enum_states_def} {{")
         buff += self.__get_enum_states_content()
         buff.append("};\n")
 
@@ -48,15 +48,15 @@ class Dot2c(Automata):
     def __get_enum_events_content(self) -> list[str]:
         buff = []
         for event in self.events:
-            buff.append("\t%s%s," % (event, self.enum_suffix))
+            buff.append(f"\t{event}{self.enum_suffix},")
 
-        buff.append("\tevent_max%s," % self.enum_suffix)
+        buff.append(f"\tevent_max{self.enum_suffix},")
 
         return buff
 
     def format_events_enum(self) -> list[str]:
         buff = []
-        buff.append("enum %s {" % self.enum_events_def)
+        buff.append(f"enum {self.enum_events_def} {{")
         buff += self.__get_enum_events_content()
         buff.append("};\n")
 
@@ -103,27 +103,27 @@ class Dot2c(Automata):
             min_type = "unsigned int"
 
         if self.states.__len__() > 1000000:
-            raise AutomataError("Too many states: %d" % self.states.__len__())
+            raise AutomataError(f"Too many states: {self.states.__len__()}")
 
         return min_type
 
     def format_automaton_definition(self) -> list[str]:
         min_type = self.get_minimun_type()
         buff = []
-        buff.append("struct %s {" % self.struct_automaton_def)
-        buff.append("\tchar *state_names[state_max%s];" % (self.enum_suffix))
-        buff.append("\tchar *event_names[event_max%s];" % (self.enum_suffix))
+        buff.append(f"struct {self.struct_automaton_def} {{")
+        buff.append(f"\tchar *state_names[state_max{self.enum_suffix}];")
+        buff.append(f"\tchar *event_names[event_max{self.enum_suffix}];")
         if self.is_hybrid_automata():
             buff.append(f"\tchar *env_names[env_max{self.enum_suffix}];")
-        buff.append("\t%s function[state_max%s][event_max%s];" % (min_type, self.enum_suffix, self.enum_suffix))
-        buff.append("\t%s initial_state;" % min_type)
-        buff.append("\tbool final_states[state_max%s];" % (self.enum_suffix))
+        buff.append(f"\t{min_type} function[state_max{self.enum_suffix}][event_max{self.enum_suffix}];")
+        buff.append(f"\t{min_type} initial_state;")
+        buff.append(f"\tbool final_states[state_max{self.enum_suffix}];")
         buff.append("};\n")
         return buff
 
     def format_aut_init_header(self) -> list[str]:
         buff = []
-        buff.append("static const struct %s %s = {" % (self.struct_automaton_def, self.var_automaton_def))
+        buff.append(f"static const struct {self.struct_automaton_def} {self.var_automaton_def} = {{")
         return buff
 
     def __get_string_vector_per_line_content(self, entries: list[str]) -> str:
@@ -179,9 +179,9 @@ class Dot2c(Automata):
                     next_state = self.function[x][y] + self.enum_suffix
 
                 if linetoolong:
-                    line += "\t\t\t%s" % next_state
+                    line += f"\t\t\t{next_state}"
                 else:
-                    line += "%*s" % (maxlen, next_state)
+                    line += f"{next_state:>{maxlen}}"
                 if y != nr_events-1:
                     line += ",\n" if linetoolong else ", "
                 else:
@@ -225,7 +225,7 @@ class Dot2c(Automata):
 
     def format_aut_init_final_states(self) -> list[str]:
        buff = []
-       buff.append("\t.final_states = { %s }," % self.get_aut_init_final_states())
+       buff.append(f"\t.final_states = {{ {self.get_aut_init_final_states()} }},")
 
        return buff
 
@@ -241,7 +241,7 @@ class Dot2c(Automata):
 
     def format_invalid_state(self) -> list[str]:
         buff = []
-        buff.append("#define %s state_max%s\n" % (self.invalid_state_str, self.enum_suffix))
+        buff.append(f"#define {self.invalid_state_str} state_max{self.enum_suffix}\n")
 
         return buff
 
diff --git a/tools/verification/rvgen/rvgen/dot2k.py b/tools/verification/rvgen/rvgen/dot2k.py
index 55222e38323f5..e26f2b47390ab 100644
--- a/tools/verification/rvgen/rvgen/dot2k.py
+++ b/tools/verification/rvgen/rvgen/dot2k.py
@@ -21,7 +21,8 @@ class dot2k(Monitor, Dot2c):
         self.monitor_type = MonitorType
         Monitor.__init__(self, extra_params)
         Dot2c.__init__(self, file_path, extra_params.get("model_name"))
-        self.enum_suffix = "_%s" % self.name
+        self.enum_suffix = f"_{self.name}"
+        self.enum_suffix = f"_{self.name}"
         self.monitor_class = extra_params["monitor_class"]
 
     def fill_monitor_type(self) -> str:
@@ -35,7 +36,7 @@ class dot2k(Monitor, Dot2c):
         buff = []
         buff += self._fill_hybrid_definitions()
         for event in self.events:
-            buff.append("static void handle_%s(void *data, /* XXX: fill header */)" % event)
+            buff.append(f"static void handle_{event}(void *data, /* XXX: fill header */)")
             buff.append("{")
             handle = "handle_event"
             if self.is_start_event(event):
@@ -46,13 +47,13 @@ class dot2k(Monitor, Dot2c):
                 handle = "handle_start_run_event"
             if self.monitor_type == "per_task":
                 buff.append("\tstruct task_struct *p = /* XXX: how do I get p? */;");
-                buff.append("\tda_%s(p, %s%s);" % (handle, event, self.enum_suffix));
+                buff.append(f"\tda_{handle}(p, {event}{self.enum_suffix});");
             elif self.monitor_type == "per_obj":
                 buff.append("\tint id = /* XXX: how do I get the id? */;")
                 buff.append("\tmonitor_target t = /* XXX: how do I get t? */;")
                 buff.append(f"\tda_{handle}(id, t, {event}{self.enum_suffix});")
             else:
-                buff.append("\tda_%s(%s%s);" % (handle, event, self.enum_suffix));
+                buff.append(f"\tda_{handle}({event}{self.enum_suffix});");
             buff.append("}")
             buff.append("")
         return '\n'.join(buff)
@@ -60,25 +61,25 @@ class dot2k(Monitor, Dot2c):
     def fill_tracepoint_attach_probe(self) -> str:
         buff = []
         for event in self.events:
-            buff.append("\trv_attach_trace_probe(\"%s\", /* XXX: tracepoint */, handle_%s);" % (self.name, event))
+            buff.append(f"\trv_attach_trace_probe(\"{self.name}\", /* XXX: tracepoint */, handle_{event});")
         return '\n'.join(buff)
 
     def fill_tracepoint_detach_helper(self) -> str:
         buff = []
         for event in self.events:
-            buff.append("\trv_detach_trace_probe(\"%s\", /* XXX: tracepoint */, handle_%s);" % (self.name, event))
+            buff.append(f"\trv_detach_trace_probe(\"{self.name}\", /* XXX: tracepoint */, handle_{event});")
         return '\n'.join(buff)
 
     def fill_model_h_header(self) -> list[str]:
         buff = []
         buff.append("/* SPDX-License-Identifier: GPL-2.0 */")
         buff.append("/*")
-        buff.append(" * Automatically generated C representation of %s automaton" % (self.name))
+        buff.append(f" * Automatically generated C representation of {self.name} automaton")
         buff.append(" * For further information about this format, see kernel documentation:")
         buff.append(" *   Documentation/trace/rv/deterministic_automata.rst")
         buff.append(" */")
         buff.append("")
-        buff.append("#define MONITOR_NAME %s" % (self.name))
+        buff.append(f"#define MONITOR_NAME {self.name}")
         buff.append("")
 
         return buff
@@ -87,11 +88,11 @@ class dot2k(Monitor, Dot2c):
         #
         # Adjust the definition names
         #
-        self.enum_states_def = "states_%s" % self.name
-        self.enum_events_def = "events_%s" % self.name
+        self.enum_states_def = f"states_{self.name}"
+        self.enum_events_def = f"events_{self.name}"
         self.enum_envs_def = f"envs_{self.name}"
-        self.struct_automaton_def = "automaton_%s" % self.name
-        self.var_automaton_def = "automaton_%s" % self.name
+        self.struct_automaton_def = f"automaton_{self.name}"
+        self.var_automaton_def = f"automaton_{self.name}"
 
         buff = self.fill_model_h_header()
         buff += self.format_model()
@@ -135,8 +136,8 @@ class dot2k(Monitor, Dot2c):
             tp_args.insert(0, tp_args_id)
         tp_proto_c = ", ".join([a+b for a,b in tp_args])
         tp_args_c = ", ".join([b for a,b in tp_args])
-        buff.append("	     TP_PROTO(%s)," % tp_proto_c)
-        buff.append("	     TP_ARGS(%s)" % tp_args_c)
+        buff.append(f"	     TP_PROTO({tp_proto_c}),")
+        buff.append(f"	     TP_ARGS({tp_args_c})")
         return '\n'.join(buff)
 
     def _fill_hybrid_definitions(self) -> list:
diff --git a/tools/verification/rvgen/rvgen/generator.py b/tools/verification/rvgen/rvgen/generator.py
index a2391a4c21ed6..61174b139123b 100644
--- a/tools/verification/rvgen/rvgen/generator.py
+++ b/tools/verification/rvgen/rvgen/generator.py
@@ -40,7 +40,7 @@ class RVGenerator:
         if platform.system() != "Linux":
             raise OSError("I can only run on Linux.")
 
-        kernel_path = os.path.join("/lib/modules/%s/build" % platform.release(), self.rv_dir)
+        kernel_path = os.path.join(f"/lib/modules/{platform.release()}/build", self.rv_dir)
 
         # if the current kernel is from a distro this may not be a full kernel tree
         # verify that one of the files we are going to modify is available
@@ -69,11 +69,11 @@ class RVGenerator:
             return self._read_file(path)
 
     def fill_parent(self):
-        return "&rv_%s" % self.parent if self.parent else "NULL"
+        return f"&rv_{self.parent}" if self.parent else "NULL"
 
     def fill_include_parent(self):
         if self.parent:
-            return "#include <monitors/%s/%s.h>\n" % (self.parent, self.parent)
+            return f"#include <monitors/{self.parent}/{self.parent}.h>\n"
         return ""
 
     def fill_tracepoint_handlers_skel(self):
@@ -119,7 +119,7 @@ class RVGenerator:
         buff = []
         buff.append("	# XXX: add dependencies if there")
         if self.parent:
-            buff.append("	depends on RV_MON_%s" % self.parent.upper())
+            buff.append(f"	depends on RV_MON_{self.parent.upper()}")
             buff.append("	default y")
         return '\n'.join(buff)
 
@@ -145,31 +145,30 @@ class RVGenerator:
         monitor_class_type = self.fill_monitor_class_type()
         if self.auto_patch:
             self._patch_file("rv_trace.h",
-                            "// Add new monitors based on CONFIG_%s here" % monitor_class_type,
-                            "#include <monitors/%s/%s_trace.h>" % (self.name, self.name))
-            return "  - Patching %s/rv_trace.h, double check the result" % self.rv_dir
+                            f"// Add new monitors based on CONFIG_{monitor_class_type} here",
+                            f"#include <monitors/{self.name}/{self.name}_trace.h>")
+            return f"  - Patching {self.rv_dir}/rv_trace.h, double check the result"
 
-        return """  - Edit %s/rv_trace.h:
-Add this line where other tracepoints are included and %s is defined:
-#include <monitors/%s/%s_trace.h>
-""" % (self.rv_dir, monitor_class_type, self.name, self.name)
+        return f"""  - Edit {self.rv_dir}/rv_trace.h:
+Add this line where other tracepoints are included and {monitor_class_type} is defined:
+#include <monitors/{self.name}/{self.name}_trace.h>
+"""
 
     def _kconfig_marker(self, container=None) -> str:
-        return "# Add new %smonitors here" % (container + " "
-                                              if container else "")
+        return f"# Add new {container + ' ' if container else ''}monitors here"
 
     def fill_kconfig_tooltip(self):
         if self.auto_patch:
             # monitors with a container should stay together in the Kconfig
             self._patch_file("Kconfig",
                              self._kconfig_marker(self.parent),
-                            "source \"kernel/trace/rv/monitors/%s/Kconfig\"" % (self.name))
-            return "  - Patching %s/Kconfig, double check the result" % self.rv_dir
+                            f"source \"kernel/trace/rv/monitors/{self.name}/Kconfig\"")
+            return f"  - Patching {self.rv_dir}/Kconfig, double check the result"
 
-        return """  - Edit %s/Kconfig:
+        return f"""  - Edit {self.rv_dir}/Kconfig:
 Add this line where other monitors are included:
-source \"kernel/trace/rv/monitors/%s/Kconfig\"
-""" % (self.rv_dir, self.name)
+source \"kernel/trace/rv/monitors/{self.name}/Kconfig\"
+"""
 
     def fill_makefile_tooltip(self):
         name = self.name
@@ -177,18 +176,18 @@ source \"kernel/trace/rv/monitors/%s/Kconfig\"
         if self.auto_patch:
             self._patch_file("Makefile",
                             "# Add new monitors here",
-                            "obj-$(CONFIG_RV_MON_%s) += monitors/%s/%s.o" % (name_up, name, name))
-            return "  - Patching %s/Makefile, double check the result" % self.rv_dir
+                            f"obj-$(CONFIG_RV_MON_{name_up}) += monitors/{name}/{name}.o")
+            return f"  - Patching {self.rv_dir}/Makefile, double check the result"
 
-        return """  - Edit %s/Makefile:
+        return f"""  - Edit {self.rv_dir}/Makefile:
 Add this line where other monitors are included:
-obj-$(CONFIG_RV_MON_%s) += monitors/%s/%s.o
-""" % (self.rv_dir, name_up, name, name)
+obj-$(CONFIG_RV_MON_{name_up}) += monitors/{name}/{name}.o
+"""
 
     def fill_monitor_tooltip(self):
         if self.auto_patch:
-            return "  - Monitor created in %s/monitors/%s" % (self.rv_dir, self. name)
-        return "  - Move %s/ to the kernel's monitor directory (%s/monitors)" % (self.name, self.rv_dir)
+            return f"  - Monitor created in {self.rv_dir}/monitors/{self.name}"
+        return f"  - Move {self.name}/ to the kernel's monitor directory ({self.rv_dir}/monitors)"
 
     def __create_directory(self):
         path = self.name
@@ -205,13 +204,13 @@ obj-$(CONFIG_RV_MON_%s) += monitors/%s/%s.o
         file.close()
 
     def _create_file(self, file_name, content):
-        path = "%s/%s" % (self.name, file_name)
+        path = f"{self.name}/{file_name}"
         if self.auto_patch:
             path = os.path.join(self.rv_dir, "monitors", path)
         self.__write_file(path, content)
 
     def __get_main_name(self):
-        path = "%s/%s" % (self.name, "main.c")
+        path = f"{self.name}/main.c"
         if not os.path.exists(path):
             return "main.c"
         return "__main.c"
@@ -221,11 +220,11 @@ obj-$(CONFIG_RV_MON_%s) += monitors/%s/%s.o
 
         self.__create_directory()
 
-        path = "%s.c" % self.name
+        path = f"{self.name}.c"
         self._create_file(path, main_c)
 
         model_h = self.fill_model_h()
-        path = "%s.h" % self.name
+        path = f"{self.name}.h"
         self._create_file(path, model_h)
 
         kconfig = self.fill_kconfig()
@@ -258,5 +257,5 @@ class Monitor(RVGenerator):
     def print_files(self):
         super().print_files()
         trace_h = self.fill_trace_h()
-        path = "%s_trace.h" % self.name
+        path = f"{self.name}_trace.h"
         self._create_file(path, trace_h)
diff --git a/tools/verification/rvgen/rvgen/ltl2k.py b/tools/verification/rvgen/rvgen/ltl2k.py
index 08ad245462e7d..b6300c38154dc 100644
--- a/tools/verification/rvgen/rvgen/ltl2k.py
+++ b/tools/verification/rvgen/rvgen/ltl2k.py
@@ -77,7 +77,7 @@ class ltl2k(generator.Monitor):
         ]
 
         for node in self.ba:
-            buf.append("\tS%i," % node.id)
+            buf.append(f"\tS{node.id},")
         buf.append("\tRV_NUM_BA_STATES")
         buf.append("};")
         buf.append("static_assert(RV_NUM_BA_STATES <= RV_MAX_BA_STATES);")
@@ -86,7 +86,7 @@ class ltl2k(generator.Monitor):
     def _fill_atoms(self):
         buf = ["enum ltl_atom {"]
         for a in sorted(self.atoms):
-            buf.append("\tLTL_%s," % a)
+            buf.append(f"\tLTL_{a},")
         buf.append("\tLTL_NUM_ATOM")
         buf.append("};")
         buf.append("static_assert(LTL_NUM_ATOM <= RV_MAX_LTL_ATOM);")
@@ -100,7 +100,7 @@ class ltl2k(generator.Monitor):
         ]
 
         for name in self.atoms_abbr:
-            buf.append("\t\t\"%s\"," % name)
+            buf.append(f"\t\t\"{name}\",")
 
         buf.extend([
             "\t};",
@@ -117,19 +117,19 @@ class ltl2k(generator.Monitor):
                 continue
 
             if isinstance(node.op, ltl2ba.AndOp):
-                buf.append("\tbool %s = %s && %s;" % (node, node.op.left, node.op.right))
+                buf.append(f"\tbool {node} = {node.op.left} && {node.op.right};")
                 required_values |= {str(node.op.left), str(node.op.right)}
             elif isinstance(node.op, ltl2ba.OrOp):
-                buf.append("\tbool %s = %s || %s;" % (node, node.op.left, node.op.right))
+                buf.append(f"\tbool {node} = {node.op.left} || {node.op.right};")
                 required_values |= {str(node.op.left), str(node.op.right)}
             elif isinstance(node.op, ltl2ba.NotOp):
-                buf.append("\tbool %s = !%s;" % (node, node.op.child))
+                buf.append(f"\tbool {node} = !{node.op.child};")
                 required_values.add(str(node.op.child))
 
         for atom in self.atoms:
             if atom.lower() not in required_values:
                 continue
-            buf.append("\tbool %s = test_bit(LTL_%s, mon->atoms);" % (atom.lower(), atom))
+            buf.append(f"\tbool {atom.lower()} = test_bit(LTL_{atom}, mon->atoms);")
 
         buf.reverse()
 
@@ -157,7 +157,7 @@ class ltl2k(generator.Monitor):
         ])
 
         for node in self.ba:
-            buf.append("\tcase S%i:" % node.id)
+            buf.append(f"\tcase S{node.id}:")
 
             for o in sorted(node.outgoing):
                 line   = "\t\tif "
@@ -167,7 +167,7 @@ class ltl2k(generator.Monitor):
                 lines = break_long_line(line, indent)
                 buf.extend(lines)
 
-                buf.append("\t\t\t__set_bit(S%i, next);" % o.id)
+                buf.append(f"\t\t\t__set_bit(S{o.id}, next);")
             buf.append("\t\tbreak;")
         buf.extend([
             "\t}",
@@ -201,7 +201,7 @@ class ltl2k(generator.Monitor):
             lines = break_long_line(line, indent)
             buf.extend(lines)
 
-            buf.append("\t\t__set_bit(S%i, mon->states);" % node.id)
+            buf.append(f"\t\t__set_bit(S{node.id}, mon->states);")
         buf.append("}")
         return buf
 
@@ -209,23 +209,21 @@ class ltl2k(generator.Monitor):
         buff = []
         buff.append("static void handle_example_event(void *data, /* XXX: fill header */)")
         buff.append("{")
-        buff.append("\tltl_atom_update(task, LTL_%s, true/false);" % self.atoms[0])
+        buff.append(f"\tltl_atom_update(task, LTL_{self.atoms[0]}, true/false);")
         buff.append("}")
         buff.append("")
         return '\n'.join(buff)
 
     def fill_tracepoint_attach_probe(self):
-        return "\trv_attach_trace_probe(\"%s\", /* XXX: tracepoint */, handle_example_event);" \
-                % self.name
+        return f"\trv_attach_trace_probe(\"{self.name}\", /* XXX: tracepoint */, handle_example_event);"
 
     def fill_tracepoint_detach_helper(self):
-        return "\trv_detach_trace_probe(\"%s\", /* XXX: tracepoint */, handle_sample_event);" \
-                % self.name
+        return f"\trv_detach_trace_probe(\"{self.name}\", /* XXX: tracepoint */, handle_sample_event);"
 
     def fill_atoms_init(self):
         buff = []
         for a in self.atoms:
-            buff.append("\tltl_atom_set(mon, LTL_%s, true/false);" % a)
+            buff.append(f"\tltl_atom_set(mon, LTL_{a}, true/false);")
         return '\n'.join(buff)
 
     def fill_model_h(self):
-- 
2.53.0


^ permalink raw reply related

* [PATCH v3 04/19] rv/rvgen: replace __len__() calls with len()
From: Wander Lairson Costa @ 2026-02-23 16:17 UTC (permalink / raw)
  To: Steven Rostedt, Gabriele Monaco, Nam Cao, Wander Lairson Costa,
	open list:RUNTIME VERIFICATION (RV), open list
In-Reply-To: <20260223162407.147003-1-wander@redhat.com>

Replace all direct calls to the __len__() dunder method with the
idiomatic len() built-in function across the rvgen codebase. This
change eliminates a Python anti-pattern where dunder methods are
called directly instead of using their corresponding built-in
functions.

The changes affect nine instances across two files. In automata.py,
the empty string check is further improved by using truthiness
testing instead of explicit length comparison. In dot2c.py, all
length checks in the get_minimun_type, __get_max_strlen_of_states,
and get_aut_init_function methods now use the standard len()
function. Additionally, spacing around keyword arguments has been
corrected to follow PEP 8 guidelines.

Direct calls to dunder methods like __len__() are discouraged in
Python because they bypass the language's abstraction layer and
reduce code readability. Using len() provides the same functionality
while adhering to Python community standards and making the code more
familiar to Python developers.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
---
 tools/verification/rvgen/rvgen/automata.py |  2 +-
 tools/verification/rvgen/rvgen/dot2c.py    | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/verification/rvgen/rvgen/automata.py b/tools/verification/rvgen/rvgen/automata.py
index 4fed58cfa3c6e..4f5681265ee24 100644
--- a/tools/verification/rvgen/rvgen/automata.py
+++ b/tools/verification/rvgen/rvgen/automata.py
@@ -82,7 +82,7 @@ class Automata:
             raise AutomataError(f"not a dot file: {self.__dot_path}")
 
         model_name = ntpath.splitext(basename)[0]
-        if model_name.__len__() == 0:
+        if not model_name:
             raise AutomataError(f"not a dot file: {self.__dot_path}")
 
         return model_name
diff --git a/tools/verification/rvgen/rvgen/dot2c.py b/tools/verification/rvgen/rvgen/dot2c.py
index e6a440e1588cd..fa44795adef46 100644
--- a/tools/verification/rvgen/rvgen/dot2c.py
+++ b/tools/verification/rvgen/rvgen/dot2c.py
@@ -96,14 +96,14 @@ class Dot2c(Automata):
     def get_minimun_type(self) -> str:
         min_type = "unsigned char"
 
-        if self.states.__len__() > 255:
+        if len(self.states) > 255:
             min_type = "unsigned short"
 
-        if self.states.__len__() > 65535:
+        if len(self.states) > 65535:
             min_type = "unsigned int"
 
-        if self.states.__len__() > 1000000:
-            raise AutomataError(f"Too many states: {self.states.__len__()}")
+        if len(self.states) > 1000000:
+            raise AutomataError(f"Too many states: {len(self.states)}")
 
         return min_type
 
@@ -159,12 +159,12 @@ class Dot2c(Automata):
         return buff
 
     def __get_max_strlen_of_states(self) -> int:
-        max_state_name = max(self.states, key = len).__len__()
-        return max(max_state_name, self.invalid_state_str.__len__())
+        max_state_name = len(max(self.states, key=len))
+        return max(max_state_name, len(self.invalid_state_str))
 
     def get_aut_init_function(self) -> str:
-        nr_states = self.states.__len__()
-        nr_events = self.events.__len__()
+        nr_states = len(self.states)
+        nr_events = len(self.events)
         buff = []
 
         maxlen = self.__get_max_strlen_of_states() + len(self.enum_suffix)
-- 
2.53.0


^ permalink raw reply related

* [PATCH v3 05/19] rv/rvgen: remove unnecessary semicolons
From: Wander Lairson Costa @ 2026-02-23 16:17 UTC (permalink / raw)
  To: Steven Rostedt, Gabriele Monaco, Nam Cao, Wander Lairson Costa,
	open list:RUNTIME VERIFICATION (RV), open list
In-Reply-To: <20260223162407.147003-1-wander@redhat.com>

Remove unnecessary semicolons from Python code in the rvgen tool.
Python does not require semicolons to terminate statements, and
their presence goes against PEP 8 style guidelines. These semicolons
were likely added out of habit from C-style languages.

This cleanup improves consistency with Python coding standards and
aligns with the recent improvements to remove other Python
anti-patterns from the codebase.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
---
 tools/verification/rvgen/rvgen/dot2k.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/verification/rvgen/rvgen/dot2k.py b/tools/verification/rvgen/rvgen/dot2k.py
index e26f2b47390ab..47af9f104a829 100644
--- a/tools/verification/rvgen/rvgen/dot2k.py
+++ b/tools/verification/rvgen/rvgen/dot2k.py
@@ -46,14 +46,14 @@ class dot2k(Monitor, Dot2c):
                 buff.append("\t/* XXX: validate that this event is only valid in the initial state */")
                 handle = "handle_start_run_event"
             if self.monitor_type == "per_task":
-                buff.append("\tstruct task_struct *p = /* XXX: how do I get p? */;");
-                buff.append(f"\tda_{handle}(p, {event}{self.enum_suffix});");
+                buff.append("\tstruct task_struct *p = /* XXX: how do I get p? */;")
+                buff.append(f"\tda_{handle}(p, {event}{self.enum_suffix});")
             elif self.monitor_type == "per_obj":
                 buff.append("\tint id = /* XXX: how do I get the id? */;")
                 buff.append("\tmonitor_target t = /* XXX: how do I get t? */;")
                 buff.append(f"\tda_{handle}(id, t, {event}{self.enum_suffix});")
             else:
-                buff.append(f"\tda_{handle}({event}{self.enum_suffix});");
+                buff.append(f"\tda_{handle}({event}{self.enum_suffix});")
             buff.append("}")
             buff.append("")
         return '\n'.join(buff)
-- 
2.53.0


^ permalink raw reply related

* [PATCH v3 06/19] rv/rvgen: use context managers for file operations
From: Wander Lairson Costa @ 2026-02-23 16:17 UTC (permalink / raw)
  To: Steven Rostedt, Gabriele Monaco, Nam Cao, Wander Lairson Costa,
	open list:RUNTIME VERIFICATION (RV), open list
In-Reply-To: <20260223162407.147003-1-wander@redhat.com>

Replace manual file open and close operations with context managers
throughout the rvgen codebase. The previous implementation used
explicit open() and close() calls, which could lead to resource leaks
if exceptions occurred between opening and closing the file handles.

This change affects three file operations: reading DOT specification
files in the automata parser, reading template files in the generator
base class, and writing generated monitor files. All now use the with
statement to ensure proper resource cleanup even in error conditions.

Context managers provide automatic cleanup through the with statement,
which guarantees that file handles are closed when the with block
exits regardless of whether an exception occurred. This follows PEP
343 recommendations and is the standard Python idiom for resource
management. The change also reduces code verbosity while improving
safety and maintainability.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
---
 tools/verification/rvgen/rvgen/automata.py  |  6 ++----
 tools/verification/rvgen/rvgen/generator.py | 12 ++++--------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/tools/verification/rvgen/rvgen/automata.py b/tools/verification/rvgen/rvgen/automata.py
index 4f5681265ee24..10146b6061ed2 100644
--- a/tools/verification/rvgen/rvgen/automata.py
+++ b/tools/verification/rvgen/rvgen/automata.py
@@ -91,13 +91,11 @@ class Automata:
         cursor = 0
         dot_lines = []
         try:
-            dot_file = open(self.__dot_path)
+            with open(self.__dot_path) as dot_file:
+                dot_lines = dot_file.read().splitlines()
         except OSError as exc:
             raise AutomataError(exc.strerror) from exc
 
-        dot_lines = dot_file.read().splitlines()
-        dot_file.close()
-
         # checking the first line:
         line = dot_lines[cursor].split()
 
diff --git a/tools/verification/rvgen/rvgen/generator.py b/tools/verification/rvgen/rvgen/generator.py
index 61174b139123b..d932e96dd66d3 100644
--- a/tools/verification/rvgen/rvgen/generator.py
+++ b/tools/verification/rvgen/rvgen/generator.py
@@ -51,11 +51,8 @@ class RVGenerator:
         raise FileNotFoundError("Could not find the rv directory, do you have the kernel source installed?")
 
     def _read_file(self, path):
-        fd = open(path, 'r')
-
-        content = fd.read()
-
-        fd.close()
+        with open(path, 'r') as fd:
+            content = fd.read()
         return content
 
     def _read_template_file(self, file):
@@ -199,9 +196,8 @@ obj-$(CONFIG_RV_MON_{name_up}) += monitors/{name}/{name}.o
             return
 
     def __write_file(self, file_name, content):
-        file = open(file_name, 'w')
-        file.write(content)
-        file.close()
+        with open(file_name, 'w') as file:
+            file.write(content)
 
     def _create_file(self, file_name, content):
         path = f"{self.name}/{file_name}"
-- 
2.53.0


^ permalink raw reply related

* [PATCH v3 07/19] rv/rvgen: fix typos in automata and generator docstring and comments
From: Wander Lairson Costa @ 2026-02-23 16:17 UTC (permalink / raw)
  To: Steven Rostedt, Gabriele Monaco, Nam Cao, Wander Lairson Costa,
	open list:RUNTIME VERIFICATION (RV), open list
In-Reply-To: <20260223162407.147003-1-wander@redhat.com>

Fix two typos in the Automata class documentation that have been
present since the initial implementation. Fix the class
docstring: "part it" instead of "parses it". Additionally, a
comment describing transition labels contained the misspelling
"lables" instead of "labels".

Fix a typo in the comment describing the insertion of the initial
state into the states list: "bein og" should be "beginning of".

Fix typo in the module docstring: "Abtract" should be "Abstract".

Fix several occurrences of "automata" where it should be the singular
form "automaton".

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
---
 tools/verification/rvgen/rvgen/automata.py  | 8 ++++----
 tools/verification/rvgen/rvgen/dot2c.py     | 2 +-
 tools/verification/rvgen/rvgen/dot2k.py     | 4 ++--
 tools/verification/rvgen/rvgen/generator.py | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/verification/rvgen/rvgen/automata.py b/tools/verification/rvgen/rvgen/automata.py
index 10146b6061ed2..b25378e92b16d 100644
--- a/tools/verification/rvgen/rvgen/automata.py
+++ b/tools/verification/rvgen/rvgen/automata.py
@@ -3,7 +3,7 @@
 #
 # Copyright (C) 2019-2022 Red Hat, Inc. Daniel Bristot de Oliveira <bristot@kernel.org>
 #
-# Automata object: parse an automata in dot file digraph format into a python object
+# Automata class: parse an automaton in dot file digraph format into a python object
 #
 # For further information, see:
 #   Documentation/trace/rv/deterministic_automata.rst
@@ -33,7 +33,7 @@ class AutomataError(Exception):
     """
 
 class Automata:
-    """Automata class: Reads a dot file and part it as an automata.
+    """Automata class: Reads a dot file and parses it as an automaton.
 
     It supports both deterministic and hybrid automata.
 
@@ -153,7 +153,7 @@ class Automata:
         states = sorted(set(states))
         states.remove(initial_state)
 
-        # Insert the initial state at the bein og the states
+        # Insert the initial state at the beginning of the states
         states.insert(0, initial_state)
 
         if not has_final_states:
@@ -175,7 +175,7 @@ class Automata:
                 line = self.__dot_lines[cursor].split()
                 event = "".join(line[line.index("label") + 2:-1]).replace('"', '')
 
-                # when a transition has more than one lables, they are like this
+                # when a transition has more than one labels, they are like this
                 # "local_irq_enable\nhw_local_irq_enable_n"
                 # so split them.
 
diff --git a/tools/verification/rvgen/rvgen/dot2c.py b/tools/verification/rvgen/rvgen/dot2c.py
index fa44795adef46..9255cc2153a31 100644
--- a/tools/verification/rvgen/rvgen/dot2c.py
+++ b/tools/verification/rvgen/rvgen/dot2c.py
@@ -3,7 +3,7 @@
 #
 # Copyright (C) 2019-2022 Red Hat, Inc. Daniel Bristot de Oliveira <bristot@kernel.org>
 #
-# dot2c: parse an automata in dot file digraph format into a C
+# dot2c: parse an automaton in dot file digraph format into a C
 #
 # This program was written in the development of this paper:
 #  de Oliveira, D. B. and Cucinotta, T. and de Oliveira, R. S.
diff --git a/tools/verification/rvgen/rvgen/dot2k.py b/tools/verification/rvgen/rvgen/dot2k.py
index 47af9f104a829..aedc2a7799b32 100644
--- a/tools/verification/rvgen/rvgen/dot2k.py
+++ b/tools/verification/rvgen/rvgen/dot2k.py
@@ -167,14 +167,14 @@ class da2k(dot2k):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         if self.is_hybrid_automata():
-            raise AutomataError("Detected hybrid automata, use the 'ha' class")
+            raise AutomataError("Detected hybrid automaton, use the 'ha' class")
 
 class ha2k(dot2k):
     """Hybrid automata only"""
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         if not self.is_hybrid_automata():
-            raise AutomataError("Detected deterministic automata, use the 'da' class")
+            raise AutomataError("Detected deterministic automaton, use the 'da' class")
         self.trace_h = self._read_template_file("trace_hybrid.h")
         self.__parse_constraints()
 
diff --git a/tools/verification/rvgen/rvgen/generator.py b/tools/verification/rvgen/rvgen/generator.py
index d932e96dd66d3..988ccdc27fa37 100644
--- a/tools/verification/rvgen/rvgen/generator.py
+++ b/tools/verification/rvgen/rvgen/generator.py
@@ -3,7 +3,7 @@
 #
 # Copyright (C) 2019-2022 Red Hat, Inc. Daniel Bristot de Oliveira <bristot@kernel.org>
 #
-# Abtract class for generating kernel runtime verification monitors from specification file
+# Abstract class for generating kernel runtime verification monitors from specification file
 
 import platform
 import os
-- 
2.53.0


^ permalink raw reply related

* [PATCH v3 08/19] rv/rvgen: fix PEP 8 whitespace violations
From: Wander Lairson Costa @ 2026-02-23 16:17 UTC (permalink / raw)
  To: Steven Rostedt, Gabriele Monaco, Nam Cao, Wander Lairson Costa,
	open list:RUNTIME VERIFICATION (RV), open list
In-Reply-To: <20260223162407.147003-1-wander@redhat.com>

Fix whitespace violations throughout the rvgen codebase to comply
with PEP 8 style guidelines. The changes address missing whitespace
after commas, around operators, and in collection literals that
were flagged by pycodestyle.

The fixes include adding whitespace after commas in string replace
chains and function arguments, adding whitespace around arithmetic
operators, removing extra whitespace in list comprehensions, and
fixing dictionary literal spacing. These changes improve code
readability and consistency with Python coding standards.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
---
 tools/verification/rvgen/rvgen/automata.py  | 8 ++++----
 tools/verification/rvgen/rvgen/dot2c.py     | 2 +-
 tools/verification/rvgen/rvgen/dot2k.py     | 4 ++--
 tools/verification/rvgen/rvgen/generator.py | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/verification/rvgen/rvgen/automata.py b/tools/verification/rvgen/rvgen/automata.py
index b25378e92b16d..e54486c69a191 100644
--- a/tools/verification/rvgen/rvgen/automata.py
+++ b/tools/verification/rvgen/rvgen/automata.py
@@ -135,7 +135,7 @@ class Automata:
             raw_state = line[-1]
 
             #  "enabled_fired"}; -> enabled_fired
-            state = raw_state.replace('"', '').replace('};', '').replace(',','_')
+            state = raw_state.replace('"', '').replace('};', '').replace(',', '_')
             if state[0:7] == "__init_":
                 initial_state = state[7:]
             else:
@@ -264,7 +264,7 @@ class Automata:
             nr_state += 1
 
         # declare the matrix....
-        matrix = [[ self.invalid_state_str for x in range(nr_event)] for y in range(nr_state)]
+        matrix = [[self.invalid_state_str for x in range(nr_event)] for y in range(nr_state)]
         constraints: dict[_ConstraintKey, list[str]] = {}
 
         # and we are back! Let's fill the matrix
@@ -273,8 +273,8 @@ class Automata:
         while self.__dot_lines[cursor].lstrip()[0] == '"':
             if self.__dot_lines[cursor].split()[1] == "->":
                 line = self.__dot_lines[cursor].split()
-                origin_state = line[0].replace('"','').replace(',','_')
-                dest_state = line[2].replace('"','').replace(',','_')
+                origin_state = line[0].replace('"', '').replace(',', '_')
+                dest_state = line[2].replace('"', '').replace(',', '_')
                 possible_events = "".join(line[line.index("label") + 2:-1]).replace('"', '')
                 for event in possible_events.split("\\n"):
                     event, *constr = event.split(";")
diff --git a/tools/verification/rvgen/rvgen/dot2c.py b/tools/verification/rvgen/rvgen/dot2c.py
index 9255cc2153a31..fc85ba1f649e7 100644
--- a/tools/verification/rvgen/rvgen/dot2c.py
+++ b/tools/verification/rvgen/rvgen/dot2c.py
@@ -182,7 +182,7 @@ class Dot2c(Automata):
                     line += f"\t\t\t{next_state}"
                 else:
                     line += f"{next_state:>{maxlen}}"
-                if y != nr_events-1:
+                if y != nr_events - 1:
                     line += ",\n" if linetoolong else ", "
                 else:
                     line += ",\n\t\t}," if linetoolong else " },"
diff --git a/tools/verification/rvgen/rvgen/dot2k.py b/tools/verification/rvgen/rvgen/dot2k.py
index aedc2a7799b32..e6f476b903b0a 100644
--- a/tools/verification/rvgen/rvgen/dot2k.py
+++ b/tools/verification/rvgen/rvgen/dot2k.py
@@ -134,8 +134,8 @@ class dot2k(Monitor, Dot2c):
         tp_args = tp_args_dict[tp_type]
         if self._is_id_monitor():
             tp_args.insert(0, tp_args_id)
-        tp_proto_c = ", ".join([a+b for a,b in tp_args])
-        tp_args_c = ", ".join([b for a,b in tp_args])
+        tp_proto_c = ", ".join([a + b for a, b in tp_args])
+        tp_args_c = ", ".join([b for a, b in tp_args])
         buff.append(f"	     TP_PROTO({tp_proto_c}),")
         buff.append(f"	     TP_ARGS({tp_args_c})")
         return '\n'.join(buff)
diff --git a/tools/verification/rvgen/rvgen/generator.py b/tools/verification/rvgen/rvgen/generator.py
index 988ccdc27fa37..40d82afb018f5 100644
--- a/tools/verification/rvgen/rvgen/generator.py
+++ b/tools/verification/rvgen/rvgen/generator.py
@@ -228,7 +228,7 @@ obj-$(CONFIG_RV_MON_{name_up}) += monitors/{name}/{name}.o
 
 
 class Monitor(RVGenerator):
-    monitor_types = { "global" : 1, "per_cpu" : 2, "per_task" : 3, "per_obj" : 4 }
+    monitor_types = {"global": 1, "per_cpu": 2, "per_task": 3, "per_obj": 4}
 
     def __init__(self, extra_params={}):
         super().__init__(extra_params)
-- 
2.53.0


^ permalink raw reply related

* [PATCH v3 09/19] rv/rvgen: fix DOT file validation logic error
From: Wander Lairson Costa @ 2026-02-23 16:17 UTC (permalink / raw)
  To: Steven Rostedt, Gabriele Monaco, Nam Cao, Wander Lairson Costa,
	open list:RUNTIME VERIFICATION (RV), open list
In-Reply-To: <20260223162407.147003-1-wander@redhat.com>

Fix incorrect boolean logic in automata DOT file format validation
that allowed malformed files to pass undetected. The previous
implementation used a logical AND operator where OR was required,
causing the validation to only reject files when both the first
token was not "digraph" AND the second token was not
"state_automaton". This meant a file starting with "digraph" but
having an incorrect second token would incorrectly pass validation.

The corrected logic properly rejects DOT files where either the
first token is not "digraph" or the second token is not
"state_automaton", ensuring that only properly formatted automaton
definition files are accepted for processing. Without this fix,
invalid DOT files could cause downstream parsing failures or
generate incorrect C code for runtime verification monitors.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
---
 tools/verification/rvgen/rvgen/automata.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/verification/rvgen/rvgen/automata.py b/tools/verification/rvgen/rvgen/automata.py
index e54486c69a191..e4c0335cd0fba 100644
--- a/tools/verification/rvgen/rvgen/automata.py
+++ b/tools/verification/rvgen/rvgen/automata.py
@@ -99,7 +99,7 @@ class Automata:
         # checking the first line:
         line = dot_lines[cursor].split()
 
-        if (line[0] != "digraph") and (line[1] != "state_automaton"):
+        if (line[0] != "digraph") or (line[1] != "state_automaton"):
             raise AutomataError(f"Not a valid .dot format: {self.__dot_path}")
         else:
             cursor += 1
-- 
2.53.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