* [PATCH v3] kprobes: Use dedicated kthread for kprobe optimizer
From: Masami Hiramatsu (Google) @ 2026-01-21 1:08 UTC (permalink / raw)
To: Steven Rostedt, Naveen N Rao, David S . Miller, Masami Hiramatsu
Cc: linux-kernel, linux-trace-kernel
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Instead of using generic workqueue, use a dedicated kthread for optimizing
kprobes, because it can wait (sleep) for a long time inside the process
by synchronize_rcu_task(). This means other works can be stopped until it
finishes.
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v3:
- Wait events in interruptible state for avoiding hung_task.
Changes in v2:
- Make the kthread unfreezable same as workqueue.
- Add kthread_should_stop() check right before calling optimizer too.
- Initialize optimizer_state to OPTIMIZER_ST_IDLE instead of 0.
---
kernel/kprobes.c | 106 ++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 86 insertions(+), 20 deletions(-)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ab8f9fc1f0d1..71e8d6e81eee 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -32,6 +32,7 @@
#include <linux/debugfs.h>
#include <linux/sysctl.h>
#include <linux/kdebug.h>
+#include <linux/kthread.h>
#include <linux/memory.h>
#include <linux/ftrace.h>
#include <linux/cpu.h>
@@ -40,6 +41,7 @@
#include <linux/perf_event.h>
#include <linux/execmem.h>
#include <linux/cleanup.h>
+#include <linux/wait.h>
#include <asm/sections.h>
#include <asm/cacheflush.h>
@@ -514,8 +516,17 @@ static LIST_HEAD(optimizing_list);
static LIST_HEAD(unoptimizing_list);
static LIST_HEAD(freeing_list);
-static void kprobe_optimizer(struct work_struct *work);
-static DECLARE_DELAYED_WORK(optimizing_work, kprobe_optimizer);
+static struct task_struct *kprobe_optimizer_task;
+static wait_queue_head_t kprobe_optimizer_wait;
+static atomic_t optimizer_state;
+enum {
+ OPTIMIZER_ST_IDLE = 0,
+ OPTIMIZER_ST_KICKED = 1,
+ OPTIMIZER_ST_FLUSHING = 2,
+};
+
+static DECLARE_COMPLETION(optimizer_completion);
+
#define OPTIMIZE_DELAY 5
/*
@@ -597,14 +608,10 @@ static void do_free_cleaned_kprobes(void)
}
}
-/* Start optimizer after OPTIMIZE_DELAY passed */
-static void kick_kprobe_optimizer(void)
-{
- schedule_delayed_work(&optimizing_work, OPTIMIZE_DELAY);
-}
+static void kick_kprobe_optimizer(void);
/* Kprobe jump optimizer */
-static void kprobe_optimizer(struct work_struct *work)
+static void kprobe_optimizer(void)
{
guard(mutex)(&kprobe_mutex);
@@ -635,9 +642,53 @@ static void kprobe_optimizer(struct work_struct *work)
do_free_cleaned_kprobes();
}
- /* Step 5: Kick optimizer again if needed */
+ /* Step 5: Kick optimizer again if needed. But if there is a flush requested, */
+ if (completion_done(&optimizer_completion))
+ complete(&optimizer_completion);
+
if (!list_empty(&optimizing_list) || !list_empty(&unoptimizing_list))
- kick_kprobe_optimizer();
+ kick_kprobe_optimizer(); /*normal kick*/
+}
+
+static int kprobe_optimizer_thread(void *data)
+{
+ while (!kthread_should_stop()) {
+ /* To avoid hung_task, wait in interruptible state. */
+ wait_event_interruptible(kprobe_optimizer_wait,
+ atomic_read(&optimizer_state) != OPTIMIZER_ST_IDLE ||
+ kthread_should_stop());
+
+ if (kthread_should_stop())
+ break;
+
+ /*
+ * If it was a normal kick, wait for OPTIMIZE_DELAY.
+ * This wait can be interrupted by a flush request.
+ */
+ if (atomic_read(&optimizer_state) == 1)
+ wait_event_interruptible_timeout(
+ kprobe_optimizer_wait,
+ atomic_read(&optimizer_state) == OPTIMIZER_ST_FLUSHING ||
+ kthread_should_stop(),
+ OPTIMIZE_DELAY);
+
+ if (kthread_should_stop())
+ break;
+
+ atomic_set(&optimizer_state, OPTIMIZER_ST_IDLE);
+
+ kprobe_optimizer();
+ }
+ return 0;
+}
+
+/* Start optimizer after OPTIMIZE_DELAY passed */
+static void kick_kprobe_optimizer(void)
+{
+ lockdep_assert_held(&kprobe_mutex);
+ if (atomic_cmpxchg(&optimizer_state,
+ OPTIMIZER_ST_IDLE, OPTIMIZER_ST_KICKED) == OPTIMIZER_ST_IDLE)
+ wake_up(&kprobe_optimizer_wait);
}
static void wait_for_kprobe_optimizer_locked(void)
@@ -645,13 +696,17 @@ static void wait_for_kprobe_optimizer_locked(void)
lockdep_assert_held(&kprobe_mutex);
while (!list_empty(&optimizing_list) || !list_empty(&unoptimizing_list)) {
- mutex_unlock(&kprobe_mutex);
-
- /* This will also make 'optimizing_work' execute immmediately */
- flush_delayed_work(&optimizing_work);
- /* 'optimizing_work' might not have been queued yet, relax */
- cpu_relax();
+ init_completion(&optimizer_completion);
+ /*
+ * Set state to OPTIMIZER_ST_FLUSHING and wake up the thread if it's
+ * idle. If it's already kicked, it will see the state change.
+ */
+ if (atomic_xchg_acquire(&optimizer_state,
+ OPTIMIZER_ST_FLUSHING) != OPTIMIZER_ST_FLUSHING)
+ wake_up(&kprobe_optimizer_wait);
+ mutex_unlock(&kprobe_mutex);
+ wait_for_completion(&optimizer_completion);
mutex_lock(&kprobe_mutex);
}
}
@@ -1010,8 +1065,21 @@ static void __disarm_kprobe(struct kprobe *p, bool reopt)
*/
}
+static void __init init_optprobe(void)
+{
+#ifdef __ARCH_WANT_KPROBES_INSN_SLOT
+ /* Init 'kprobe_optinsn_slots' for allocation */
+ kprobe_optinsn_slots.insn_size = MAX_OPTINSN_SIZE;
+#endif
+
+ init_waitqueue_head(&kprobe_optimizer_wait);
+ atomic_set(&optimizer_state, OPTIMIZER_ST_IDLE);
+ kprobe_optimizer_task = kthread_run(kprobe_optimizer_thread, NULL,
+ "kprobe-optimizer");
+}
#else /* !CONFIG_OPTPROBES */
+#define init_optprobe() do {} while (0)
#define optimize_kprobe(p) do {} while (0)
#define unoptimize_kprobe(p, f) do {} while (0)
#define kill_optimized_kprobe(p) do {} while (0)
@@ -2694,10 +2762,8 @@ static int __init init_kprobes(void)
/* By default, kprobes are armed */
kprobes_all_disarmed = false;
-#if defined(CONFIG_OPTPROBES) && defined(__ARCH_WANT_KPROBES_INSN_SLOT)
- /* Init 'kprobe_optinsn_slots' for allocation */
- kprobe_optinsn_slots.insn_size = MAX_OPTINSN_SIZE;
-#endif
+ /* Initialize the optimization infrastructure */
+ init_optprobe();
err = arch_init_kprobes();
if (!err)
^ permalink raw reply related
* Re: [PATCH v2] kprobes: Use dedicated kthread for kprobe optimizer
From: Masami Hiramatsu @ 2026-01-21 1:07 UTC (permalink / raw)
To: kernel test robot
Cc: oe-lkp, lkp, Steven Rostedt, linux-kernel, linux-trace-kernel,
Naveen N Rao, David S . Miller
In-Reply-To: <202601191507.74fccd0c-lkp@intel.com>
On Mon, 19 Jan 2026 15:29:58 +0800
kernel test robot <oliver.sang@intel.com> wrote:
>
>
> Hello,
>
> kernel test robot noticed "INFO:task_blocked_for_more_than#seconds" on:
OOps, yes. It should be wait in interruptible.
Thanks,
>
> commit: 62f65c2531ad66e84c1a5ef91389322357ef6db4 ("[PATCH v2] kprobes: Use dedicated kthread for kprobe optimizer")
> url: https://github.com/intel-lab-lkp/linux/commits/Masami-Hiramatsu-Google/kprobes-Use-dedicated-kthread-for-kprobe-optimizer/20260113-094928
> base: https://git.kernel.org/cgit/linux/kernel/git/trace/linux-trace for-next
> patch link: https://lore.kernel.org/all/176826884613.429923.16578111751623731056.stgit@devnote2/
> patch subject: [PATCH v2] kprobes: Use dedicated kthread for kprobe optimizer
>
> in testcase: trinity
> version: trinity-x86_64-294c4652-1_20251011
> with following parameters:
>
> runtime: 300s
> group: group-03
> nr_groups: 5
>
>
>
> config: x86_64-kexec
> compiler: clang-20
> test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 32G
>
> (please refer to attached dmesg/kmsg for entire log/backtrace)
>
>
> +-----------------------------------------+------------+------------+
> | | 78a419b44e | 62f65c2531 |
> +-----------------------------------------+------------+------------+
> | INFO:task_blocked_for_more_than#seconds | 0 | 6 |
> +-----------------------------------------+------------+------------+
>
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202601191507.74fccd0c-lkp@intel.com
>
>
>
> [ 972.359932][ T32] INFO: task kprobe-optimize:18 blocked for more than 491 seconds.
> [ 972.361221][ T32] Not tainted 6.19.0-rc5-00040-g62f65c2531ad #1
> [ 972.362516][ T32] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 972.364322][ T32] task:kprobe-optimize state:D stack:0 pid:18 tgid:18 ppid:2 task_flags:0x208040 flags:0x00080000
> [ 972.366628][ T32] Call Trace:
> [ 972.367378][ T32] <TASK>
> [ 972.368066][ T32] __schedule (kernel/sched/core.c:5259)
> [ 972.370837][ T32] schedule (arch/x86/include/asm/preempt.h:85 kernel/sched/core.c:6946 kernel/sched/core.c:6960)
> [ 972.371679][ T32] kprobe_optimizer_thread (kernel/kprobes.c:656)
> [ 972.373219][ T32] ? __pfx_autoremove_wake_function (kernel/sched/wait.c:402)
> [ 972.374017][ T32] kthread (kernel/kthread.c:465)
> [ 972.374573][ T32] ? __pfx_kprobe_optimizer_thread (kernel/kprobes.c:654)
> [ 972.375364][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 972.376005][ T32] ret_from_fork (arch/x86/kernel/process.c:164)
> [ 972.376679][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 972.377305][ T32] ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
> [ 972.377958][ T32] </TASK>
> [ 1463.879952][ T32] INFO: task kprobe-optimize:18 blocked for more than 983 seconds.
> [ 1463.882239][ T32] Not tainted 6.19.0-rc5-00040-g62f65c2531ad #1
> [ 1463.884371][ T32] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 1463.887443][ T32] task:kprobe-optimize state:D stack:0 pid:18 tgid:18 ppid:2 task_flags:0x208040 flags:0x00080000
> [ 1463.891867][ T32] Call Trace:
> [ 1463.893396][ T32] <TASK>
> [ 1463.894468][ T32] __schedule (kernel/sched/core.c:5259)
> [ 1463.903984][ T32] schedule (arch/x86/include/asm/preempt.h:85 kernel/sched/core.c:6946 kernel/sched/core.c:6960)
> [ 1463.904926][ T32] kprobe_optimizer_thread (kernel/kprobes.c:656)
> [ 1463.905949][ T32] ? __pfx_autoremove_wake_function (kernel/sched/wait.c:402)
> [ 1463.907111][ T32] kthread (kernel/kthread.c:465)
> [ 1463.907954][ T32] ? __pfx_kprobe_optimizer_thread (kernel/kprobes.c:654)
> [ 1463.909157][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 1463.910045][ T32] ret_from_fork (arch/x86/kernel/process.c:164)
> [ 1463.910939][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 1463.911933][ T32] ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
> [ 1463.912947][ T32] </TASK>
> [ 1955.399991][ T32] INFO: task kprobe-optimize:18 blocked for more than 1474 seconds.
> [ 1955.404266][ T32] Not tainted 6.19.0-rc5-00040-g62f65c2531ad #1
> [ 1955.407452][ T32] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 1955.414557][ T32] task:kprobe-optimize state:D stack:0 pid:18 tgid:18 ppid:2 task_flags:0x208040 flags:0x00080000
> [ 1955.416961][ T32] Call Trace:
> [ 1955.417705][ T32] <TASK>
> [ 1955.418382][ T32] __schedule (kernel/sched/core.c:5259)
> [ 1955.419274][ T32] schedule (arch/x86/include/asm/preempt.h:85 kernel/sched/core.c:6946 kernel/sched/core.c:6960)
> [ 1955.419929][ T32] kprobe_optimizer_thread (kernel/kprobes.c:656)
> [ 1955.421071][ T32] ? __pfx_autoremove_wake_function (kernel/sched/wait.c:402)
> [ 1955.422303][ T32] kthread (kernel/kthread.c:465)
> [ 1955.423217][ T32] ? __pfx_kprobe_optimizer_thread (kernel/kprobes.c:654)
> [ 1955.424452][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 1955.425189][ T32] ret_from_fork (arch/x86/kernel/process.c:164)
> [ 1955.425913][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 1955.426645][ T32] ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
> [ 1955.427357][ T32] </TASK>
> [ 2446.919964][ T32] INFO: task kprobe-optimize:18 blocked for more than 1966 seconds.
> [ 2446.922105][ T32] Not tainted 6.19.0-rc5-00040-g62f65c2531ad #1
> [ 2446.925697][ T32] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 2446.926569][ T32] task:kprobe-optimize state:D stack:0 pid:18 tgid:18 ppid:2 task_flags:0x208040 flags:0x00080000
> [ 2446.927775][ T32] Call Trace:
> [ 2446.928242][ T32] <TASK>
> [ 2446.928609][ T32] __schedule (kernel/sched/core.c:5259)
> [ 2446.929827][ T32] schedule (arch/x86/include/asm/preempt.h:85 kernel/sched/core.c:6946 kernel/sched/core.c:6960)
> [ 2446.930256][ T32] kprobe_optimizer_thread (kernel/kprobes.c:656)
> [ 2446.930789][ T32] ? __pfx_autoremove_wake_function (kernel/sched/wait.c:402)
> [ 2446.931434][ T32] kthread (kernel/kthread.c:465)
> [ 2446.931926][ T32] ? __pfx_kprobe_optimizer_thread (kernel/kprobes.c:654)
> [ 2446.932624][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 2446.933079][ T32] ret_from_fork (arch/x86/kernel/process.c:164)
> [ 2446.933534][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 2446.933983][ T32] ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
> [ 2446.934450][ T32] </TASK>
> [ 2938.439916][ T32] INFO: task kprobe-optimize:18 blocked for more than 2457 seconds.
> [ 2938.441278][ T32] Not tainted 6.19.0-rc5-00040-g62f65c2531ad #1
> [ 2938.442281][ T32] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 2938.444599][ T32] task:kprobe-optimize state:D stack:0 pid:18 tgid:18 ppid:2 task_flags:0x208040 flags:0x00080000
> [ 2938.446316][ T32] Call Trace:
> [ 2938.446892][ T32] <TASK>
> [ 2938.447449][ T32] __schedule (kernel/sched/core.c:5259)
> [ 2938.449286][ T32] schedule (arch/x86/include/asm/preempt.h:85 kernel/sched/core.c:6946 kernel/sched/core.c:6960)
> [ 2938.450230][ T32] kprobe_optimizer_thread (kernel/kprobes.c:656)
> [ 2938.451347][ T32] ? __pfx_autoremove_wake_function (kernel/sched/wait.c:402)
> [ 2938.452611][ T32] kthread (kernel/kthread.c:465)
> [ 2938.453462][ T32] ? __pfx_kprobe_optimizer_thread (kernel/kprobes.c:654)
> [ 2938.454622][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 2938.455583][ T32] ret_from_fork (arch/x86/kernel/process.c:164)
> [ 2938.456604][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 2938.457614][ T32] ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
> [ 2938.458645][ T32] </TASK>
> [ 3429.960057][ T32] INFO: task kprobe-optimize:18 blocked for more than 2949 seconds.
> [ 3429.962853][ T32] Not tainted 6.19.0-rc5-00040-g62f65c2531ad #1
> [ 3429.965133][ T32] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 3429.967986][ T32] task:kprobe-optimize state:D stack:0 pid:18 tgid:18 ppid:2 task_flags:0x208040 flags:0x00080000
> [ 3429.971731][ T32] Call Trace:
> [ 3429.973021][ T32] <TASK>
> [ 3429.974169][ T32] __schedule (kernel/sched/core.c:5259)
> [ 3429.975713][ T32] schedule (arch/x86/include/asm/preempt.h:85 kernel/sched/core.c:6946 kernel/sched/core.c:6960)
> [ 3429.977206][ T32] kprobe_optimizer_thread (kernel/kprobes.c:656)
> [ 3429.978375][ T32] ? __pfx_autoremove_wake_function (kernel/sched/wait.c:402)
> [ 3429.979623][ T32] kthread (kernel/kthread.c:465)
> [ 3429.980602][ T32] ? __pfx_kprobe_optimizer_thread (kernel/kprobes.c:654)
> [ 3429.981810][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 3429.982797][ T32] ret_from_fork (arch/x86/kernel/process.c:164)
> [ 3429.983793][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 3429.984838][ T32] ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
> [ 3429.985864][ T32] </TASK>
> [ 3921.480064][ T32] INFO: task kprobe-optimize:18 blocked for more than 3440 seconds.
> [ 3921.482942][ T32] Not tainted 6.19.0-rc5-00040-g62f65c2531ad #1
> [ 3921.485260][ T32] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 3921.488269][ T32] task:kprobe-optimize state:D stack:0 pid:18 tgid:18 ppid:2 task_flags:0x208040 flags:0x00080000
> [ 3921.491811][ T32] Call Trace:
> [ 3921.493488][ T32] <TASK>
> [ 3921.494960][ T32] __schedule (kernel/sched/core.c:5259)
> [ 3921.508616][ T32] schedule (arch/x86/include/asm/preempt.h:85 kernel/sched/core.c:6946 kernel/sched/core.c:6960)
> [ 3921.509430][ T32] kprobe_optimizer_thread (kernel/kprobes.c:656)
> [ 3921.510413][ T32] ? __pfx_autoremove_wake_function (kernel/sched/wait.c:402)
> [ 3921.511467][ T32] kthread (kernel/kthread.c:465)
> [ 3921.512331][ T32] ? __pfx_kprobe_optimizer_thread (kernel/kprobes.c:654)
> [ 3921.513388][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 3921.514202][ T32] ret_from_fork (arch/x86/kernel/process.c:164)
> [ 3921.514976][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 3921.515773][ T32] ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
> [ 3921.516817][ T32] </TASK>
> [ 4413.000017][ T32] INFO: task kprobe-optimize:18 blocked for more than 3932 seconds.
> [ 4413.004653][ T32] Not tainted 6.19.0-rc5-00040-g62f65c2531ad #1
> [ 4413.007745][ T32] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 4413.010446][ T32] task:kprobe-optimize state:D stack:0 pid:18 tgid:18 ppid:2 task_flags:0x208040 flags:0x00080000
> [ 4413.013906][ T32] Call Trace:
> [ 4413.015027][ T32] <TASK>
> [ 4413.016134][ T32] __schedule (kernel/sched/core.c:5259)
> [ 4413.017465][ T32] schedule (arch/x86/include/asm/preempt.h:85 kernel/sched/core.c:6946 kernel/sched/core.c:6960)
> [ 4413.018294][ T32] kprobe_optimizer_thread (kernel/kprobes.c:656)
> [ 4413.019366][ T32] ? __pfx_autoremove_wake_function (kernel/sched/wait.c:402)
> [ 4413.020594][ T32] kthread (kernel/kthread.c:465)
> [ 4413.021350][ T32] ? __pfx_kprobe_optimizer_thread (kernel/kprobes.c:654)
> [ 4413.022331][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 4413.023124][ T32] ret_from_fork (arch/x86/kernel/process.c:164)
> [ 4413.023925][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 4413.025015][ T32] ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
> [ 4413.026074][ T32] </TASK>
> [ 4904.520028][ T32] INFO: task kprobe-optimize:18 blocked for more than 4423 seconds.
> [ 4904.521949][ T32] Not tainted 6.19.0-rc5-00040-g62f65c2531ad #1
> [ 4904.528612][ T32] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 4904.529924][ T32] task:kprobe-optimize state:D stack:0 pid:18 tgid:18 ppid:2 task_flags:0x208040 flags:0x00080000
> [ 4904.531461][ T32] Call Trace:
> [ 4904.531896][ T32] <TASK>
> [ 4904.532406][ T32] __schedule (kernel/sched/core.c:5259)
> [ 4904.532947][ T32] schedule (arch/x86/include/asm/preempt.h:85 kernel/sched/core.c:6946 kernel/sched/core.c:6960)
> [ 4904.533381][ T32] kprobe_optimizer_thread (kernel/kprobes.c:656)
> [ 4904.533944][ T32] ? __pfx_autoremove_wake_function (kernel/sched/wait.c:402)
> [ 4904.534547][ T32] kthread (kernel/kthread.c:465)
> [ 4904.534985][ T32] ? __pfx_kprobe_optimizer_thread (kernel/kprobes.c:654)
> [ 4904.535670][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 4904.536359][ T32] ret_from_fork (arch/x86/kernel/process.c:164)
> [ 4904.536831][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 4904.537310][ T32] ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
> [ 4904.537788][ T32] </TASK>
> [ 5396.039936][ T32] INFO: task kprobe-optimize:18 blocked for more than 4915 seconds.
> [ 5396.040939][ T32] Not tainted 6.19.0-rc5-00040-g62f65c2531ad #1
> [ 5396.041631][ T32] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 5396.042742][ T32] task:kprobe-optimize state:D stack:0 pid:18 tgid:18 ppid:2 task_flags:0x208040 flags:0x00080000
> [ 5396.045182][ T32] Call Trace:
> [ 5396.045723][ T32] <TASK>
> [ 5396.046266][ T32] __schedule (kernel/sched/core.c:5259)
> [ 5396.046928][ T32] schedule (arch/x86/include/asm/preempt.h:85 kernel/sched/core.c:6946 kernel/sched/core.c:6960)
> [ 5396.047542][ T32] kprobe_optimizer_thread (kernel/kprobes.c:656)
> [ 5396.048329][ T32] ? __pfx_autoremove_wake_function (kernel/sched/wait.c:402)
> [ 5396.049182][ T32] kthread (kernel/kthread.c:465)
> [ 5396.049803][ T32] ? __pfx_kprobe_optimizer_thread (kernel/kprobes.c:654)
> [ 5396.050654][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 5396.051353][ T32] ret_from_fork (arch/x86/kernel/process.c:164)
> [ 5396.052048][ T32] ? __pfx_kthread (kernel/kthread.c:412)
> [ 5396.052723][ T32] ret_from_fork_asm (arch/x86/entry/entry_64.S:256)
> [ 5396.053187][ T32] </TASK>
> [ 5396.053524][ T32] Future hung task reports are suppressed, see sysctl kernel.hung_task_warnings
> BUG: kernel hang in test stage
>
>
>
> The kernel config and materials to reproduce are available at:
> https://download.01.org/0day-ci/archive/20260119/202601191507.74fccd0c-lkp@intel.com
>
>
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH bpf-next v4 1/2] bpf: support bpf_get_func_arg() for BPF_TRACE_RAW_TP
From: Andrii Nakryiko @ 2026-01-21 0:38 UTC (permalink / raw)
To: Menglong Dong
Cc: ast, andrii, yonghong.song, daniel, john.fastabend, martin.lau,
eddyz87, song, kpsingh, sdf, haoluo, jolsa, mattbobrowski,
rostedt, mhiramat, mathieu.desnoyers, bpf, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260120073046.324342-2-dongml2@chinatelecom.cn>
On Mon, Jan 19, 2026 at 11:31 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> For now, bpf_get_func_arg() and bpf_get_func_arg_cnt() is not supported by
> the BPF_TRACE_RAW_TP, which is not convenient to get the argument of the
> tracepoint, especially for the case that the position of the arguments in
> a tracepoint can change.
>
> The target tracepoint BTF type id is specified during loading time,
> therefore we can get the function argument count from the function
> prototype instead of the stack.
>
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
> ---
> v4:
> - fix the error of using bpf_get_func_arg() for BPF_TRACE_ITER
>
> v3:
> - remove unnecessary NULL checking for prog->aux->attach_func_proto
>
> v2:
> - for nr_args, skip first 'void *__data' argument in btf_trace_##name
> typedef
> ---
> kernel/bpf/verifier.c | 32 ++++++++++++++++++++++++++++----
> kernel/trace/bpf_trace.c | 4 ++++
> 2 files changed, 32 insertions(+), 4 deletions(-)
>
other than stylistical choices, looks good to me
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 9de0ec0c3ed9..0b281b7c41eb 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -23323,8 +23323,20 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
> /* Implement bpf_get_func_arg inline. */
> if (prog_type == BPF_PROG_TYPE_TRACING &&
> insn->imm == BPF_FUNC_get_func_arg) {
> - /* Load nr_args from ctx - 8 */
> - insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
> + if (eatype == BPF_TRACE_RAW_TP) {
> + int nr_args = btf_type_vlen(prog->aux->attach_func_proto);
> +
> + /*
> + * skip first 'void *__data' argument in btf_trace_##name
> + * typedef
> + */
> + nr_args--;
> + /* Save nr_args to reg0 */
> + insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, nr_args);
> + } else {
> + /* Load nr_args from ctx - 8 */
> + insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
> + }
> insn_buf[1] = BPF_JMP32_REG(BPF_JGE, BPF_REG_2, BPF_REG_0, 6);
> insn_buf[2] = BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 3);
> insn_buf[3] = BPF_ALU64_REG(BPF_ADD, BPF_REG_2, BPF_REG_1);
> @@ -23376,8 +23388,20 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
> /* Implement get_func_arg_cnt inline. */
> if (prog_type == BPF_PROG_TYPE_TRACING &&
> insn->imm == BPF_FUNC_get_func_arg_cnt) {
> - /* Load nr_args from ctx - 8 */
> - insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
> + if (eatype == BPF_TRACE_RAW_TP) {
> + int nr_args = btf_type_vlen(prog->aux->attach_func_proto);
> +
> + /*
> + * skip first 'void *__data' argument in btf_trace_##name
> + * typedef
> + */
> + nr_args--;
> + /* Save nr_args to reg0 */
> + insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, nr_args);
nit: isn't this just a very verbose way of writing:
int nr_args = btf_type_vlen(prog->aux->attach_func_proto);
/* skip 'void *__data' in btf_trace_##name() and save to reg0 */
insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, nr_args - 1);
even if you want to preserve nr_args-- for clarity, at least make that
4-line comment into a single-line one, please
> + } else {
> + /* Load nr_args from ctx - 8 */
> + insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
> + }
>
> new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, 1);
> if (!new_prog)
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index f73e08c223b5..0efdad3adcce 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1734,10 +1734,14 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> case BPF_FUNC_d_path:
> return &bpf_d_path_proto;
> case BPF_FUNC_get_func_arg:
> + if (prog->expected_attach_type == BPF_TRACE_RAW_TP)
> + return &bpf_get_func_arg_proto;
> return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_proto : NULL;
> case BPF_FUNC_get_func_ret:
> return bpf_prog_has_trampoline(prog) ? &bpf_get_func_ret_proto : NULL;
> case BPF_FUNC_get_func_arg_cnt:
> + if (prog->expected_attach_type == BPF_TRACE_RAW_TP)
> + return &bpf_get_func_arg_cnt_proto;
> return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_cnt_proto : NULL;
hm, wouldn't "has trampoline or is raw_tp" a more logical grouping?
if (bpf_prog_has_trampoline(prog) || prog->expected_attach_type ==
BPF_TRACE_RAW_TP)
return &bpf_get_func_arg_cnt_proto;
return NULL;
maybe you'll need to wrap that condition, but still, at least no one
has to double check that we return exactly the same prototype in both
cases, no?
> case BPF_FUNC_get_attach_cookie:
> if (prog->type == BPF_PROG_TYPE_TRACING &&
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH net-next v4] page_pool: Add page_pool_release_stalled tracepoint
From: Jakub Kicinski @ 2026-01-20 23:29 UTC (permalink / raw)
To: Leon Hwang
Cc: netdev, Jesper Dangaard Brouer, Ilias Apalodimas, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, David S . Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, kerneljasonxing,
lance.yang, jiayuan.chen, linux-kernel, linux-trace-kernel,
Leon Huang Fu
In-Reply-To: <b676baa0-2044-4a74-900d-f471620f2896@linux.dev>
On Tue, 20 Jan 2026 11:16:20 +0800 Leon Hwang wrote:
> I encountered the 'pr_warn()' messages during Mellanox NIC flapping on a
> system using the 'mlx5_core' driver (kernel 6.6). The root cause turned
> out to be an application-level issue: the IBM/sarama “Client SeekBroker
> Connection Leak” [1].
The scenario you are describing matches the situations we run into
at Meta. With the upstream kernel you can find that the pages are
leaking based on stats, and if you care use drgn to locate them
(in the recv queue).
The 6.6 kernel did not have page pool stats. I feel quite odd about
adding more uAPI because someone is running a 2+ years old kernel
and doesn't have access to the already existing facilities.
^ permalink raw reply
* Re: [PATCH bpf-next v4 2/2] selftests/bpf: test bpf_get_func_arg() for tp_btf
From: Yonghong Song @ 2026-01-20 22:52 UTC (permalink / raw)
To: Menglong Dong, ast, andrii
Cc: daniel, john.fastabend, martin.lau, eddyz87, song, kpsingh, sdf,
haoluo, jolsa, mattbobrowski, rostedt, mhiramat,
mathieu.desnoyers, bpf, linux-kernel, linux-trace-kernel
In-Reply-To: <20260120073046.324342-3-dongml2@chinatelecom.cn>
On 1/19/26 11:30 PM, Menglong Dong wrote:
> Test bpf_get_func_arg() and bpf_get_func_arg_cnt() for tp_btf. The code
> is most copied from test1 and test2.
>
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
^ permalink raw reply
* Re: [PATCH bpf-next v4 1/2] bpf: support bpf_get_func_arg() for BPF_TRACE_RAW_TP
From: Yonghong Song @ 2026-01-20 22:49 UTC (permalink / raw)
To: Menglong Dong, ast, andrii
Cc: daniel, john.fastabend, martin.lau, eddyz87, song, kpsingh, sdf,
haoluo, jolsa, mattbobrowski, rostedt, mhiramat,
mathieu.desnoyers, bpf, linux-kernel, linux-trace-kernel
In-Reply-To: <20260120073046.324342-2-dongml2@chinatelecom.cn>
On 1/19/26 11:30 PM, Menglong Dong wrote:
> For now, bpf_get_func_arg() and bpf_get_func_arg_cnt() is not supported by
> the BPF_TRACE_RAW_TP, which is not convenient to get the argument of the
> tracepoint, especially for the case that the position of the arguments in
> a tracepoint can change.
>
> The target tracepoint BTF type id is specified during loading time,
> therefore we can get the function argument count from the function
> prototype instead of the stack.
>
> Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
^ permalink raw reply
* Re: [PATCH v5 v5 1/3] docs: tracing/fprobe: Document list filters and :entry/:exit
From: Steven Rostedt @ 2026-01-20 20:53 UTC (permalink / raw)
To: Seokwoo Chung (Ryan)
Cc: mhiramat, corbet, shuah, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest
In-Reply-To: <20260118011815.56516-2-seokwoo.chung130@gmail.com>
On Sat, 17 Jan 2026 20:18:13 -0500
"Seokwoo Chung (Ryan)" <seokwoo.chung130@gmail.com> wrote:
> Update fprobe event documentation to describe comma-separated symbol lists,
> exclusions, and explicit suffixes.
Usually, the documentation updates comes *after* the changes.
-- Steve
>
> Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
> ---
> Documentation/trace/fprobetrace.rst | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/trace/fprobetrace.rst b/Documentation/trace/fprobetrace.rst
> index b4c2ca3d02c1..bbcfd57f0005 100644
> --- a/Documentation/trace/fprobetrace.rst
> +++ b/Documentation/trace/fprobetrace.rst
> @@ -25,14 +25,18 @@ Synopsis of fprobe-events
> -------------------------
> ::
>
> - f[:[GRP1/][EVENT1]] SYM [FETCHARGS] : Probe on function entry
> - f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS] : Probe on function exit
> + f[:[GRP1/][EVENT1]] SYM[%return] [FETCHARGS] : Single function
> + f[:[GRP1/][EVENT1]] SYM[,[!]SYM[,...]][:entry|:exit] [FETCHARGS] :Multiple
> + function
> t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS] : Probe on tracepoint
>
> GRP1 : Group name for fprobe. If omitted, use "fprobes" for it.
> GRP2 : Group name for tprobe. If omitted, use "tracepoints" for it.
> EVENT1 : Event name for fprobe. If omitted, the event name is
> - "SYM__entry" or "SYM__exit".
> + - For a single literal symbol, the event name is
> + "SYM__entry" or "SYM__exit".
> + - For a *list or any wildcard*, an explicit [GRP1/][EVENT1] is
> + required; otherwise the parser rejects it.
> EVENT2 : Event name for tprobe. If omitted, the event name is
> the same as "TRACEPOINT", but if the "TRACEPOINT" starts
> with a digit character, "_TRACEPOINT" is used.
> @@ -40,6 +44,13 @@ Synopsis of fprobe-events
> can be probed simultaneously, or 0 for the default value
> as defined in Documentation/trace/fprobe.rst
>
> + SYM : Function name or comma-separated list of symbols.
> + - SYM prefixed with "!" are exclusions.
> + - ":entry" suffix means it probes entry of given symbols
> + (default)
> + - ":exit" suffix means it probes exit of given symbols.
> + - "%return" suffix means it probes exit of SYM (single
> + symbol).
> FETCHARGS : Arguments. Each probe can have up to 128 args.
> ARG : Fetch "ARG" function argument using BTF (only for function
> entry or tracepoint.) (\*1)
^ permalink raw reply
* Re: [PATCH v5 1/3] tracing/fprobe: Support comma-separated symbols and :entry/:exit
From: Steven Rostedt @ 2026-01-20 20:52 UTC (permalink / raw)
To: Seokwoo Chung (Ryan)
Cc: mhiramat, corbet, shuah, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest
In-Reply-To: <20260120155047.4ef3c378@gandalf.local.home>
On Tue, 20 Jan 2026 15:50:47 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 14 Jan 2026 17:13:38 -0500
> "Seokwoo Chung (Ryan)" <seokwoo.chung130@gmail.com> wrote:
>
> Missing change log. The subject is the "what" the patch is doing, the
> change log needs the "why".
Ah, just noticed you sent a revision. I'll reply there.
-- Steve
^ permalink raw reply
* Re: [PATCH v5 3/3] selftests/ftrace: Add accept cases for fprobe list syntax
From: Steven Rostedt @ 2026-01-20 20:51 UTC (permalink / raw)
To: Seokwoo Chung (Ryan)
Cc: mhiramat, corbet, shuah, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest
In-Reply-To: <20260114221341.128038-4-seokwoo.chung130@gmail.com>
On Wed, 14 Jan 2026 17:13:40 -0500
"Seokwoo Chung (Ryan)" <seokwoo.chung130@gmail.com> wrote:
-ENOCHANGELOG
:-(
-- Steve
> Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
> ---
> tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
> index 45e57c6f487d..79392e268929 100644
> --- a/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/fprobe_list.tc
> @@ -1,7 +1,7 @@
> #!/bin/sh
> # SPDX-License-Identifier: GPL-2.0
> # description: Fprobe event list syntax and :entry/:exit suffixes
> -# requires: dynamic_events "f[:[<group>/][<event>]] <func-name>[:entry|:exit] [<args>]":README
> +# requires: dynamic_events "f[:[<group>/][<event>]] <func-list>[:entry|:exit] [<args>]":README
>
> # Setup symbols to test. These are common kernel functions.
> PLACE=vfs_read
^ permalink raw reply
* Re: [PATCH v5 2/3] docs: tracing/fprobe: Document list filters and :entry/:exit
From: Steven Rostedt @ 2026-01-20 20:51 UTC (permalink / raw)
To: Seokwoo Chung (Ryan)
Cc: mhiramat, corbet, shuah, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest
In-Reply-To: <20260114221341.128038-3-seokwoo.chung130@gmail.com>
On Wed, 14 Jan 2026 17:13:39 -0500
"Seokwoo Chung (Ryan)" <seokwoo.chung130@gmail.com> wrote:
Again, no change log :-(
-- Steve
> Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
> ---
> Documentation/trace/fprobetrace.rst | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/trace/fprobetrace.rst b/Documentation/trace/fprobetrace.rst
> index b4c2ca3d02c1..5efd9c374365 100644
> --- a/Documentation/trace/fprobetrace.rst
> +++ b/Documentation/trace/fprobetrace.rst
> @@ -25,14 +25,19 @@ Synopsis of fprobe-events
> -------------------------
> ::
>
> - f[:[GRP1/][EVENT1]] SYM [FETCHARGS] : Probe on function entry
> - f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS] : Probe on function exit
> + f[:[GRP1/][EVENT1]] SYM [FETCHARGS] : Probe on function entry
> + f[MAXACTIVE][:[GRP1/][EVENT1]] SYM%return [FETCHARGS] : Probe on function exit
> + f[:[GRP1/][EVENT1] SYM[,[!]SYM[,...][:entry|:exit] [FETCHARGS] : Probe on
> + list/wildcard
> t[:[GRP2/][EVENT2]] TRACEPOINT [FETCHARGS] : Probe on tracepoint
>
> GRP1 : Group name for fprobe. If omitted, use "fprobes" for it.
> GRP2 : Group name for tprobe. If omitted, use "tracepoints" for it.
> EVENT1 : Event name for fprobe. If omitted, the event name is
> - "SYM__entry" or "SYM__exit".
> + - For a single literal symbol, the event name is
> + "SYM__entry" or "SYM__exit".
> + - For a *list or any wildcard*, an explicit [GRP1/][EVENT1] is
> + required.
> EVENT2 : Event name for tprobe. If omitted, the event name is
> the same as "TRACEPOINT", but if the "TRACEPOINT" starts
> with a digit character, "_TRACEPOINT" is used.
> @@ -40,6 +45,13 @@ Synopsis of fprobe-events
> can be probed simultaneously, or 0 for the default value
> as defined in Documentation/trace/fprobe.rst
>
> + SYM : Function name or comma-separated list of symbols.
> + - SYM prefixed with "!" are exclusions.
> + - ":entry" suffix means it probes entry of given symbols
> + (default)
> + - ":exit" suffix means it probes exit of given symbols.
> + - "%return" suffix means it probes exit of SYM (single
> + symbol).
> FETCHARGS : Arguments. Each probe can have up to 128 args.
> ARG : Fetch "ARG" function argument using BTF (only for function
> entry or tracepoint.) (\*1)
^ permalink raw reply
* Re: [PATCH v5 1/3] tracing/fprobe: Support comma-separated symbols and :entry/:exit
From: Steven Rostedt @ 2026-01-20 20:50 UTC (permalink / raw)
To: Seokwoo Chung (Ryan)
Cc: mhiramat, corbet, shuah, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest
In-Reply-To: <20260114221341.128038-2-seokwoo.chung130@gmail.com>
On Wed, 14 Jan 2026 17:13:38 -0500
"Seokwoo Chung (Ryan)" <seokwoo.chung130@gmail.com> wrote:
Missing change log. The subject is the "what" the patch is doing, the
change log needs the "why".
> Signed-off-by: Seokwoo Chung (Ryan) <seokwoo.chung130@gmail.com>
> ---
> kernel/trace/trace.c | 4 +--
> kernel/trace/trace_fprobe.c | 49 ++++++++++++++++++++-----------------
> kernel/trace/trace_probe.h | 2 ++
> 3 files changed, 31 insertions(+), 24 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 10cdcc7b194e..73b59d47dfe7 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -5578,8 +5578,8 @@ static const char readme_msg[] =
> "\t r[maxactive][:[<group>/][<event>]] <place> [<args>]\n"
> #endif
> #ifdef CONFIG_FPROBE_EVENTS
> - "\t f[:[<group>/][<event>]] <func-name>[:entry|:exit] [<args>]\n"
> - "\t (single symbols still accept %return)\n"
> + "\t f[:[<group>/][<event>]] <func-name>[%return] [<args>]\n"
> + "\t f[:[<group>/][<event>]] <func-list>[:entry|:exit] [<args>]\n"
> "\t t[:[<group>/][<event>]] <tracepoint> [<args>]\n"
> #endif
> #ifdef CONFIG_HIST_TRIGGERS
> diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
> index 5a2a41eea603..1663c341ddb4 100644
> --- a/kernel/trace/trace_fprobe.c
> +++ b/kernel/trace/trace_fprobe.c
> @@ -187,16 +187,23 @@ DEFINE_FREE(tuser_put, struct tracepoint_user *,
> */
> struct trace_fprobe {
> struct dyn_event devent;
> - char *filter;
> +
> struct fprobe fp;
> - bool list_mode;
> - char *nofilter;
> const char *symbol;
> - struct trace_probe tp;
> + char *filter;
> + char *nofilter;
> +
> bool tprobe;
> struct tracepoint_user *tuser;
> +
> + struct trace_probe tp;
Why is this being updated? Nothing in the change log says why?
> };
>
> +static inline bool trace_fprobe_has_list(const struct trace_fprobe *tf)
> +{
> + return tf->filter || tf->nofilter;
> +}
> +
> static bool is_trace_fprobe(struct dyn_event *ev)
> {
> return ev->ops == &trace_fprobe_ops;
> @@ -847,7 +854,7 @@ static int __register_trace_fprobe(struct trace_fprobe *tf)
> * - list_mode: pass filter/nofilter
> * - single: pass symbol only (legacy)
> */
> - if (tf->list_mode)
> + if (trace_fprobe_has_list(tf))
> return register_fprobe(&tf->fp, tf->filter, tf->nofilter);
> return register_fprobe(&tf->fp, tf->symbol, NULL);
> }
> @@ -1188,11 +1195,18 @@ static int parse_fprobe_spec(const char *in, bool is_tracepoint,
> *base = NULL; *filter = NULL; *nofilter = NULL;
> *is_return = false; *list_mode = false;
>
> - if (is_tracepoint) {
> + if (is_tracepoint)
> + {
> if (strchr(in, ',') || strchr(in, ':'))
> + {
> + trace_probe_log_err(0, BAD_TP_NAME);
> return -EINVAL;
> + }
> if (strstr(in, "%return"))
> + {
> + trace_probe_log_err(p - in, BAD_TP_NAME);
> return -EINVAL;
> + }
> for (p = in; *p; p++)
> if (!isalnum(*p) && *p != '_')
> return -EINVAL;
> @@ -1225,6 +1239,7 @@ static int parse_fprobe_spec(const char *in, bool is_tracepoint,
> } else if (!strcmp(p, ":entry")) {
> *(char *)p = '\0';
> } else {
> + trace_probe_log_err(p - work, BAD_LIST_SUFFIX);
> return -EINVAL;
> }
> }
> @@ -1233,6 +1248,7 @@ static int parse_fprobe_spec(const char *in, bool is_tracepoint,
> list = !!strchr(work, ',');
>
> if (list && legacy_ret) {
> + trace_probe_log_err(p - work, BAD_LEGACY_RET);
> return -EINVAL;
> }
>
> @@ -1245,12 +1261,9 @@ static int parse_fprobe_spec(const char *in, bool is_tracepoint,
>
> if (list) {
> char *tmp = b, *tok;
> - size_t fsz, nfsz;
>
> - fsz = nfsz = strlen(b) + 1;
> -
> - f = kzalloc(fsz, GFP_KERNEL);
> - nf = kzalloc(nfsz, GFP_KERNEL);
> + f = kzalloc(strlen(b) + 1, GFP_KERNEL);
> + nf = kzalloc(strlen(b) + 1, GFP_KERNEL);
> if (!f || !nf)
> return -ENOMEM;
>
> @@ -1261,6 +1274,7 @@ static int parse_fprobe_spec(const char *in, bool is_tracepoint,
> if (*tok == '\0') {
> trace_probe_log_err(tmp - b - 1, BAD_TP_NAME);
> return -EINVAL;
> + }
>
> if (neg)
> tok++;
> @@ -1455,17 +1469,8 @@ static int trace_fprobe_create_internal(int argc, const char *argv[],
>
> /* carry list parsing result into tf */
> if (!is_tracepoint) {
> - tf->list_mode = list_mode;
> - if (parsed_filter) {
> - tf->filter = kstrdup(parsed_filter, GFP_KERNEL);
> - if (!tf->filter)
> - return -ENOMEM;
> - }
> - if (parsed_nofilter) {
> - tf->nofilter = kstrdup(parsed_nofilter, GFP_KERNEL);
> - if (!tf->nofilter)
> - return -ENOMEM;
> - }
> + tf->filter = no_free_ptr(parsed_filter);
> + tf->nofilter = no_free_ptr(parsed_nofilter);
> }
>
> /* parse arguments */
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 9fc56c937130..b8b0544eb7cd 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -494,9 +494,11 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
> C(BAD_PROBE_ADDR, "Invalid probed address or symbol"), \
> C(NON_UNIQ_SYMBOL, "The symbol is not unique"), \
> C(BAD_RETPROBE, "Retprobe address must be an function entry"), \
> + C(BAD_LEGACY_RET, "Legacy %return not allowed with list"), \
> C(NO_TRACEPOINT, "Tracepoint is not found"), \
> C(BAD_TP_NAME, "Invalid character in tracepoint name"),\
> C(BAD_ADDR_SUFFIX, "Invalid probed address suffix"), \
> + C(BAD_LIST_SUFFIX, "Bad list suffix"), \
> C(NO_GROUP_NAME, "Group name is not specified"), \
> C(GROUP_TOO_LONG, "Group name is too long"), \
> C(BAD_GROUP_NAME, "Group name must follow the same rules as C identifiers"), \
This definitely needs discussion on why this is needed.
-- Steve
^ permalink raw reply
* Re: [PATCH v4 0/2] mm/vmscan: mitigate spurious kswapd_failures reset and add tracepoints
From: Andrew Morton @ 2026-01-20 20:30 UTC (permalink / raw)
To: Jiayuan Chen
Cc: linux-mm, David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Brendan Jackman,
Johannes Weiner, Zi Yan, Qi Zheng, Shakeel Butt, Jiayuan Chen,
linux-kernel, linux-trace-kernel
In-Reply-To: <20260120024402.387576-1-jiayuan.chen@linux.dev>
On Tue, 20 Jan 2026 10:43:47 +0800 Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> == Problem ==
>
> We observed an issue in production on a multi-NUMA system where kswapd
> runs endlessly, causing sustained heavy IO READ pressure across the
> entire system.
>
> The root cause is that direct reclaim triggered by cgroup memory.high
> keeps resetting kswapd_failures to 0, even when the node cannot be
> balanced. This prevents kswapd from ever stopping after reaching
> MAX_RECLAIM_RETRIES.
>
Updated, thanks.
> v3 -> v4: https://lore.kernel.org/linux-mm/20260114074049.229935-1-jiayuan.chen@linux.dev/
> - Add Acked-by tags
> - Some modifications suggested by Johannes Weiner
Here's how v4 altered mm.git:
include/linux/mmzone.h | 26 ++++++++-----
include/trace/events/vmscan.h | 24 ++++++------
mm/memory-tiers.c | 2 -
mm/page_alloc.c | 4 +-
mm/show_mem.c | 3 -
mm/vmscan.c | 60 +++++++++++++++++---------------
mm/vmstat.c | 2 -
7 files changed, 64 insertions(+), 57 deletions(-)
--- a/include/linux/mmzone.h~b
+++ a/include/linux/mmzone.h
@@ -1531,26 +1531,30 @@ static inline unsigned long pgdat_end_pf
return pgdat->node_start_pfn + pgdat->node_spanned_pages;
}
-enum reset_kswapd_failures_reason {
- RESET_KSWAPD_FAILURES_OTHER = 0,
- RESET_KSWAPD_FAILURES_KSWAPD,
- RESET_KSWAPD_FAILURES_DIRECT,
- RESET_KSWAPD_FAILURES_PCP,
-};
-
-void pgdat_reset_kswapd_failures(pg_data_t *pgdat, enum reset_kswapd_failures_reason reason);
-
#include <linux/memory_hotplug.h>
void build_all_zonelists(pg_data_t *pgdat);
-void wakeup_kswapd(struct zone *zone, gfp_t gfp_mask, int order,
- enum zone_type highest_zoneidx);
bool __zone_watermark_ok(struct zone *z, unsigned int order, unsigned long mark,
int highest_zoneidx, unsigned int alloc_flags,
long free_pages);
bool zone_watermark_ok(struct zone *z, unsigned int order,
unsigned long mark, int highest_zoneidx,
unsigned int alloc_flags);
+
+enum kswapd_clear_hopeless_reason {
+ KSWAPD_CLEAR_HOPELESS_OTHER = 0,
+ KSWAPD_CLEAR_HOPELESS_KSWAPD,
+ KSWAPD_CLEAR_HOPELESS_DIRECT,
+ KSWAPD_CLEAR_HOPELESS_PCP,
+};
+
+void wakeup_kswapd(struct zone *zone, gfp_t gfp_mask, int order,
+ enum zone_type highest_zoneidx);
+void kswapd_try_clear_hopeless(struct pglist_data *pgdat,
+ unsigned int order, int highest_zoneidx);
+void kswapd_clear_hopeless(pg_data_t *pgdat, enum kswapd_clear_hopeless_reason reason);
+bool kswapd_test_hopeless(pg_data_t *pgdat);
+
/*
* Memory initialization context, use to differentiate memory added by
* the platform statically or via memory hotplug interface.
--- a/include/trace/events/vmscan.h~b
+++ a/include/trace/events/vmscan.h
@@ -40,16 +40,16 @@
{_VMSCAN_THROTTLE_CONGESTED, "VMSCAN_THROTTLE_CONGESTED"} \
) : "VMSCAN_THROTTLE_NONE"
-TRACE_DEFINE_ENUM(RESET_KSWAPD_FAILURES_OTHER);
-TRACE_DEFINE_ENUM(RESET_KSWAPD_FAILURES_KSWAPD);
-TRACE_DEFINE_ENUM(RESET_KSWAPD_FAILURES_DIRECT);
-TRACE_DEFINE_ENUM(RESET_KSWAPD_FAILURES_PCP);
-
-#define reset_kswapd_src \
- {RESET_KSWAPD_FAILURES_KSWAPD, "KSWAPD"}, \
- {RESET_KSWAPD_FAILURES_DIRECT, "DIRECT"}, \
- {RESET_KSWAPD_FAILURES_PCP, "PCP"}, \
- {RESET_KSWAPD_FAILURES_OTHER, "OTHER"}
+TRACE_DEFINE_ENUM(KSWAPD_CLEAR_HOPELESS_OTHER);
+TRACE_DEFINE_ENUM(KSWAPD_CLEAR_HOPELESS_KSWAPD);
+TRACE_DEFINE_ENUM(KSWAPD_CLEAR_HOPELESS_DIRECT);
+TRACE_DEFINE_ENUM(KSWAPD_CLEAR_HOPELESS_PCP);
+
+#define kswapd_clear_hopeless_reason_ops \
+ {KSWAPD_CLEAR_HOPELESS_KSWAPD, "KSWAPD"}, \
+ {KSWAPD_CLEAR_HOPELESS_DIRECT, "DIRECT"}, \
+ {KSWAPD_CLEAR_HOPELESS_PCP, "PCP"}, \
+ {KSWAPD_CLEAR_HOPELESS_OTHER, "OTHER"}
#define trace_reclaim_flags(file) ( \
(file ? RECLAIM_WB_FILE : RECLAIM_WB_ANON) | \
@@ -566,7 +566,7 @@ TRACE_EVENT(mm_vmscan_kswapd_reclaim_fai
__entry->nid, __entry->failures)
);
-TRACE_EVENT(mm_vmscan_reset_kswapd_failures,
+TRACE_EVENT(mm_vmscan_kswapd_clear_hopeless,
TP_PROTO(int nid, int reason),
@@ -584,7 +584,7 @@ TRACE_EVENT(mm_vmscan_reset_kswapd_failu
TP_printk("nid=%d reason=%s",
__entry->nid,
- __print_symbolic(__entry->reason, reset_kswapd_src))
+ __print_symbolic(__entry->reason, kswapd_clear_hopeless_reason_ops))
);
#endif /* _TRACE_VMSCAN_H */
--- a/mm/memory-tiers.c~b
+++ a/mm/memory-tiers.c
@@ -955,7 +955,7 @@ static ssize_t demotion_enabled_store(st
struct pglist_data *pgdat;
for_each_online_pgdat(pgdat)
- pgdat_reset_kswapd_failures(pgdat, RESET_KSWAPD_FAILURES_OTHER);
+ kswapd_clear_hopeless(pgdat, KSWAPD_CLEAR_HOPELESS_OTHER);
}
return count;
--- a/mm/page_alloc.c~b
+++ a/mm/page_alloc.c
@@ -2945,9 +2945,9 @@ static bool free_frozen_page_commit(stru
* 'hopeless node' to stay in that state for a while. Let
* kswapd work again by resetting kswapd_failures.
*/
- if (atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES &&
+ if (kswapd_test_hopeless(pgdat) &&
next_memory_node(pgdat->node_id) < MAX_NUMNODES)
- pgdat_reset_kswapd_failures(pgdat, RESET_KSWAPD_FAILURES_PCP);
+ kswapd_clear_hopeless(pgdat, KSWAPD_CLEAR_HOPELESS_PCP);
}
return ret;
}
--- a/mm/show_mem.c~b
+++ a/mm/show_mem.c
@@ -278,8 +278,7 @@ static void show_free_areas(unsigned int
#endif
K(node_page_state(pgdat, NR_PAGETABLE)),
K(node_page_state(pgdat, NR_SECONDARY_PAGETABLE)),
- str_yes_no(atomic_read(&pgdat->kswapd_failures) >=
- MAX_RECLAIM_RETRIES),
+ str_yes_no(kswapd_test_hopeless(pgdat)),
K(node_page_state(pgdat, NR_BALLOON_PAGES)));
}
--- a/mm/vmscan.c~b
+++ a/mm/vmscan.c
@@ -506,7 +506,7 @@ static bool skip_throttle_noprogress(pg_
* If kswapd is disabled, reschedule if necessary but do not
* throttle as the system is likely near OOM.
*/
- if (atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES)
+ if (kswapd_test_hopeless(pgdat))
return true;
/*
@@ -2647,28 +2647,6 @@ static bool can_age_anon_pages(struct lr
lruvec_memcg(lruvec));
}
-void pgdat_reset_kswapd_failures(pg_data_t *pgdat, enum reset_kswapd_failures_reason reason)
-{
- /* Only trace actual resets, not redundant zero-to-zero */
- if (atomic_xchg(&pgdat->kswapd_failures, 0))
- trace_mm_vmscan_reset_kswapd_failures(pgdat->node_id, reason);
-}
-
-/*
- * Reset kswapd_failures only when the node is balanced. Without this
- * check, successful direct reclaim (e.g., from cgroup memory.high
- * throttling) can keep resetting kswapd_failures even when the node
- * cannot be balanced, causing kswapd to run endlessly.
- */
-static bool pgdat_balanced(pg_data_t *pgdat, int order, int highest_zoneidx);
-static inline void pgdat_try_reset_kswapd_failures(struct pglist_data *pgdat,
- struct scan_control *sc)
-{
- if (pgdat_balanced(pgdat, sc->order, sc->reclaim_idx))
- pgdat_reset_kswapd_failures(pgdat, current_is_kswapd() ?
- RESET_KSWAPD_FAILURES_KSWAPD : RESET_KSWAPD_FAILURES_DIRECT);
-}
-
#ifdef CONFIG_LRU_GEN
#ifdef CONFIG_LRU_GEN_ENABLED
@@ -5086,7 +5064,7 @@ static void lru_gen_shrink_node(struct p
blk_finish_plug(&plug);
done:
if (sc->nr_reclaimed > reclaimed)
- pgdat_try_reset_kswapd_failures(pgdat, sc);
+ kswapd_try_clear_hopeless(pgdat, sc->order, sc->reclaim_idx);
}
/******************************************************************************
@@ -6153,7 +6131,7 @@ again:
* successful direct reclaim run will revive a dormant kswapd.
*/
if (reclaimable)
- pgdat_try_reset_kswapd_failures(pgdat, sc);
+ kswapd_try_clear_hopeless(pgdat, sc->order, sc->reclaim_idx);
else if (sc->cache_trim_mode)
sc->cache_trim_mode_failed = 1;
}
@@ -6458,7 +6436,7 @@ static bool allow_direct_reclaim(pg_data
int i;
bool wmark_ok;
- if (atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES)
+ if (kswapd_test_hopeless(pgdat))
return true;
for_each_managed_zone_pgdat(zone, pgdat, i, ZONE_NORMAL) {
@@ -6867,7 +6845,7 @@ static bool prepare_kswapd_sleep(pg_data
wake_up_all(&pgdat->pfmemalloc_wait);
/* Hopeless node, leave it to direct reclaim */
- if (atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES)
+ if (kswapd_test_hopeless(pgdat))
return true;
if (pgdat_balanced(pgdat, order, highest_zoneidx)) {
@@ -7395,7 +7373,7 @@ void wakeup_kswapd(struct zone *zone, gf
return;
/* Hopeless node, leave it to direct reclaim if possible */
- if (atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES ||
+ if (kswapd_test_hopeless(pgdat) ||
(pgdat_balanced(pgdat, order, highest_zoneidx) &&
!pgdat_watermark_boosted(pgdat, highest_zoneidx))) {
/*
@@ -7415,6 +7393,32 @@ void wakeup_kswapd(struct zone *zone, gf
wake_up_interruptible(&pgdat->kswapd_wait);
}
+void kswapd_clear_hopeless(pg_data_t *pgdat, enum kswapd_clear_hopeless_reason reason)
+{
+ /* Only trace actual resets, not redundant zero-to-zero */
+ if (atomic_xchg(&pgdat->kswapd_failures, 0))
+ trace_mm_vmscan_kswapd_clear_hopeless(pgdat->node_id, reason);
+}
+
+/*
+ * Reset kswapd_failures only when the node is balanced. Without this
+ * check, successful direct reclaim (e.g., from cgroup memory.high
+ * throttling) can keep resetting kswapd_failures even when the node
+ * cannot be balanced, causing kswapd to run endlessly.
+ */
+void kswapd_try_clear_hopeless(struct pglist_data *pgdat,
+ unsigned int order, int highest_zoneidx)
+{
+ if (pgdat_balanced(pgdat, order, highest_zoneidx))
+ kswapd_clear_hopeless(pgdat, current_is_kswapd() ?
+ KSWAPD_CLEAR_HOPELESS_KSWAPD : KSWAPD_CLEAR_HOPELESS_DIRECT);
+}
+
+bool kswapd_test_hopeless(pg_data_t *pgdat)
+{
+ return atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES;
+}
+
#ifdef CONFIG_HIBERNATION
/*
* Try to free `nr_to_reclaim' of memory, system-wide, and return the number of
--- a/mm/vmstat.c~b
+++ a/mm/vmstat.c
@@ -1840,7 +1840,7 @@ static void zoneinfo_show_print(struct s
"\n start_pfn: %lu"
"\n reserved_highatomic: %lu"
"\n free_highatomic: %lu",
- atomic_read(&pgdat->kswapd_failures) >= MAX_RECLAIM_RETRIES,
+ kswapd_test_hopeless(pgdat),
zone->zone_start_pfn,
zone->nr_reserved_highatomic,
zone->nr_free_highatomic);
_
^ permalink raw reply
* Re: [PATCH 17/26] rv/rvgen: fix possibly unbound variable in ltl2k
From: Wander Lairson Costa @ 2026-01-20 19:38 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <3507c2f454ca66aa27dd3a24e33f5941fac96639.camel@redhat.com>
On Tue, Jan 20, 2026 at 01:30:35PM +0100, Gabriele Monaco wrote:
> On Tue, 2026-01-20 at 08:37 -0300, Wander Lairson Costa wrote:
> > On Tue, Jan 20, 2026 at 09:59:11AM +0100, Gabriele Monaco wrote:
> > > On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> > > > Initialize loop variable `i` before the for loop in abbreviate_atoms
> > > > function to fix pyright static type checker error. The previous code
> > > > left `i` potentially unbound in edge cases where the range could be
> > > > empty, though this would not occur in practice since the loop always
> > > > executes at least once with the given range parameters.
> > > >
> > > > The initialization to zero ensures that `i` has a defined value before
> > > > entering the loop scope, satisfying static analysis requirements
> > > > while preserving the existing logic. The for loop immediately assigns
> > > > i to the first value from the range, so the initialization value is
> > > > never actually used in normal execution paths.
> > > >
> > > > This change resolves the pyright reportPossiblyUnbound error without
> > > > altering the function's behavior or performance characteristics.
> > >
> > > So are we just pleasing the tool or is there a real implication of this?
> > >
> > > Apparently code like
> > >
> > > for i in range(len([]), -1, -1):
> > > pass
> > > print(i)
> > >
> > > works just fine since range() returns at least 0 (as you mentioned in the
> > > commit
> > > message) and i is not used before assignation in the loop, so I don't really
> > > see
> > > a problem.
> > >
> > > Apparently pyright devs don't want ([1]) to implement a logic to sort out
> > > the
> > > /possibly/ unbound error here.
> > >
> > > From what I understand, this code is already not pythonic, so rather than
> > > silence the warning to please this tool I'd just refactor the code not to
> > > use i
> > > after the loop (or leave it as it is, since it works fine).
> > >
> > > What do you think?
> >
> > You're right, I could have done:
> >
> > for atom in reversed(atoms): ...
> >
>
> I'm missing what you mean with this, the range is iterating over the string
> representation of atom (in reverse) not the array of atoms.
>
Sorry, I misinterpreted you previous comment and picked the wrong piece
of code.
Yes, the basic goal was to make pyright happy.
> You basically want i to be the length of the longest prefix common to at least
> another atom.
>
> You could assign i to some python trick doing the exact same thing the loop
> does, like:
>
> i = next((i for i in range(len(atom), -1, -1)
> if sum(a.startswith(atom[:i]) for a in atoms) > 1))
>
> next() is basically doing the break at the first occurrence from the generator,
> just now your i doesn't live (only) inside the loop.
>
> So now you save 2 lines and get any C developer scratch their head when they
> look at the code, but hey, pyright is happy!
>
Or just leave the assignment.
> If you do find the trick with next() readable or have any better idea, feel free
> to try though.
>
Definitely the next() trick is not worth to make pyright happy.
> Thanks,
> Gabriele
>
> > I will modify it in v2.
> >
> > >
> > > Thanks,
> > > Gabriele
> > >
> > > [1] - https://github.com/microsoft/pyright/issues/844
> > >
> > > >
> > > > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > > > ---
> > > > tools/verification/rvgen/rvgen/ltl2k.py | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/tools/verification/rvgen/rvgen/ltl2k.py
> > > > b/tools/verification/rvgen/rvgen/ltl2k.py
> > > > index fa9ea6d597095..94dc64af1716d 100644
> > > > --- a/tools/verification/rvgen/rvgen/ltl2k.py
> > > > +++ b/tools/verification/rvgen/rvgen/ltl2k.py
> > > > @@ -45,6 +45,7 @@ def abbreviate_atoms(atoms: list[str]) -> list[str]:
> > > >
> > > > abbrs = []
> > > > for atom in atoms:
> > > > + i = 0
> > > > for i in range(len(atom), -1, -1):
> > > > if sum(a.startswith(atom[:i]) for a in atoms) > 1:
> > > > break
> > >
>
^ permalink raw reply
* Re: [PATCH 26/26] rv/rvgen: extract node marker string to class constant
From: Wander Lairson Costa @ 2026-01-20 18:56 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <d15348f9f6d8ed71ba3e928ae1b5da15711894e3.camel@redhat.com>
On Tue, Jan 20, 2026 at 02:11:46PM +0100, Gabriele Monaco wrote:
> On Tue, 2026-01-20 at 13:36 +0100, Gabriele Monaco wrote:
> > On Tue, 2026-01-20 at 08:34 -0300, Wander Lairson Costa wrote:
> > > On Tue, Jan 20, 2026 at 10:03:20AM +0100, Gabriele Monaco wrote:
> > > > On Mon, 2026-01-19 at 17:46 -0300, Wander Lairson Costa wrote:
> > > > > Add a node_marker class constant to the Automata class to replace the
> > > > > hardcoded "{node" string literal used throughout the DOT file parsing
> > > > > logic. This follows the existing pattern established by the init_marker
> > > > > and invalid_state_str class constants in the same class.
> > > > >
> > > > > The "{node" string is used as a marker to identify node declaration
> > > > > lines in DOT files during state variable extraction and cursor
> > > > > positioning. Extracting it to a named constant improves code
> > > > > maintainability and makes the marker's purpose explicit.
> > > > >
> > > > > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > > >
> > > > Looks fine for me, thanks!
> > > >
> > > > I wonder if we could merge this patch with 15/26 that is introducing a
> > > > very
> > > > similar change on init_marker.
> > >
> > > The idea was to make each patch doing one thing to make the reviewer's
> > > life easier (I think I broke my own rule in a couple of patch). But if
> > > there is a strong feeling about the merge, I could merged them in a
> > > possible v2 patch series.
> > >
> >
> > Yeah I get the idea, I guess I could just pick the most obvious patches and
> > send
> > a PR to Steve before the merge window, so I can see directly if it makes sense
> > to squash them and you don't need to send them all in the v2.
>
> Screamed victory too fast.. I tried the patches on the latest version of the
> tree (which reached already linux-next) and they're have several conflicts.
>
> I don't have time to rebase them one by one right now, let's continue with this
> series as a whole.
> As a rule of thumb, considering these patches get first reviewed, then PR-ed to
> Steve and then to Linus, I'd favour to lower the number if possible, but feel
> free to choose where it makes sense for them to stay separate.
I created the patches on top of linux-trace/tools/for-next. Is this the
wrong branch?
>
> Thanks,
> Gabriele
>
^ permalink raw reply
* Re: [PATCH bpf-next v3 2/2] bpf: Require ARG_PTR_TO_MEM with memory flag
From: Eduard Zingerman @ 2026-01-20 18:06 UTC (permalink / raw)
To: Zesen Liu, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Matt Bobrowski,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Daniel Xu
Cc: bpf, linux-kernel, linux-trace-kernel, netdev, Shuran Liu,
Peili Gao, Haoran Ni
In-Reply-To: <20260120-helper_proto-v3-2-27b0180b4e77@gmail.com>
On Tue, 2026-01-20 at 16:28 +0800, Zesen Liu wrote:
> Add check to ensure that ARG_PTR_TO_MEM is used with either MEM_WRITE or
> MEM_RDONLY.
>
> Using ARG_PTR_TO_MEM alone without flags does not make sense because:
>
> - If the helper does not change the argument, missing MEM_RDONLY causes the
> verifier to incorrectly reject a read-only buffer.
> - If the helper does change the argument, missing MEM_WRITE causes the
> verifier to incorrectly assume the memory is unchanged, leading to errors
> in code optimization.
>
> Co-developed-by: Shuran Liu <electronlsr@gmail.com>
> Signed-off-by: Shuran Liu <electronlsr@gmail.com>
> Co-developed-by: Peili Gao <gplhust955@gmail.com>
> Signed-off-by: Peili Gao <gplhust955@gmail.com>
> Co-developed-by: Haoran Ni <haoran.ni.cs@gmail.com>
> Signed-off-by: Haoran Ni <haoran.ni.cs@gmail.com>
> Signed-off-by: Zesen Liu <ftyghome@gmail.com>
> ---
At the moment, for helper arguments processing I see that MEM_RDONLY
influences verifier only when argument type is ARG_PTR_TO_DYNPTR or
ARG_PTR_TO_BTF_ID. Hence, I think this change is safe to apply,
effects are limited to the check_mem_arg_rw_flag_ok() below.
Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>
> kernel/bpf/verifier.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 9de0ec0c3ed9..a89f5bc7eff7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -10351,10 +10351,27 @@ static bool check_btf_id_ok(const struct bpf_func_proto *fn)
> return true;
> }
>
> +static bool check_mem_arg_rw_flag_ok(const struct bpf_func_proto *fn)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(fn->arg_type); i++) {
> + enum bpf_arg_type arg_type = fn->arg_type[i];
> +
> + if (base_type(arg_type) != ARG_PTR_TO_MEM)
> + continue;
> + if (!(arg_type & (MEM_WRITE | MEM_RDONLY)))
> + return false;
> + }
> +
> + return true;
> +}
> +
> static int check_func_proto(const struct bpf_func_proto *fn)
> {
> return check_raw_mode_ok(fn) &&
> check_arg_pair_ok(fn) &&
> + check_mem_arg_rw_flag_ok(fn) &&
> check_btf_id_ok(fn) ? 0 : -EINVAL;
> }
>
^ permalink raw reply
* Re: [PATCH bpf-next v3 1/2] bpf: Fix memory access flags in helper prototypes
From: Eduard Zingerman @ 2026-01-20 17:56 UTC (permalink / raw)
To: Zesen Liu, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Matt Bobrowski,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Daniel Xu
Cc: bpf, linux-kernel, linux-trace-kernel, netdev, Shuran Liu,
Peili Gao, Haoran Ni
In-Reply-To: <20260120-helper_proto-v3-1-27b0180b4e77@gmail.com>
On Tue, 2026-01-20 at 16:28 +0800, Zesen Liu wrote:
> After commit 37cce22dbd51 ("bpf: verifier: Refactor helper access type tracking"),
> the verifier started relying on the access type flags in helper
> function prototypes to perform memory access optimizations.
>
> Currently, several helper functions utilizing ARG_PTR_TO_MEM lack the
> corresponding MEM_RDONLY or MEM_WRITE flags. This omission causes the
> verifier to incorrectly assume that the buffer contents are unchanged
> across the helper call. Consequently, the verifier may optimize away
> subsequent reads based on this wrong assumption, leading to correctness
> issues.
>
> For bpf_get_stack_proto_raw_tp, the original MEM_RDONLY was incorrect
> since the helper writes to the buffer. Change it to ARG_PTR_TO_UNINIT_MEM
> which correctly indicates write access to potentially uninitialized memory.
>
> Similar issues were recently addressed for specific helpers in commit
> ac44dcc788b9 ("bpf: Fix verifier assumptions of bpf_d_path's output buffer")
> and commit 2eb7648558a7 ("bpf: Specify access type of bpf_sysctl_get_name args").
>
> Fix these prototypes by adding the correct memory access flags.
>
> Fixes: 37cce22dbd51 ("bpf: verifier: Refactor helper access type tracking")
> Co-developed-by: Shuran Liu <electronlsr@gmail.com>
> Signed-off-by: Shuran Liu <electronlsr@gmail.com>
> Co-developed-by: Peili Gao <gplhust955@gmail.com>
> Signed-off-by: Peili Gao <gplhust955@gmail.com>
> Co-developed-by: Haoran Ni <haoran.ni.cs@gmail.com>
> Signed-off-by: Haoran Ni <haoran.ni.cs@gmail.com>
> Signed-off-by: Zesen Liu <ftyghome@gmail.com>
> ---
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
^ permalink raw reply
* Re: [PATCH bpf RESEND v2 1/2] bpf: Fix memory access flags in helper prototypes
From: Eduard Zingerman @ 2026-01-20 17:54 UTC (permalink / raw)
To: Zesen Liu
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Matt Bobrowski,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Daniel Xu, bpf, linux-kernel, linux-trace-kernel,
netdev, Shuran Liu, Peili Gao, Haoran Ni
In-Reply-To: <B91D5A7E-4967-416D-A2AC-CD3428F3C702@gmail.com>
On Tue, 2026-01-20 at 11:39 +0800, Zesen Liu wrote:
> [...]
>
> > On Jan 20, 2026, at 04:24, Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > Q: why ARG_PTR_TO_UNINIT_MEM here, but not for a previous function and
> > not for snprintf variants?
>
> For bpf_get_stack_proto_raw_tp, I chose ARG_PTR_TO_UNINIT_MEM to be consistent with its siblings:
>
> • bpf_get_stack_proto_tp (bpf_trace.c:1425)
> • bpf_get_stack_proto (stackmap.c:525)
> • bpf_get_stack_sleepable_proto (stackmap.c:541)
>
> All of these are wrappers around the same core function bpf_get_stack() / __bpf_get_stack(), passing the buffer with identical semantics, and all use ARG_PTR_TO_UNINIT_MEM.
Ack, makes sense, thank you for explaining.
^ permalink raw reply
* Re: [PATCH bpf-next 2/4] x86/fgraph,bpf: Switch kprobe_multi program stack unwind to hw_regs path
From: Jiri Olsa @ 2026-01-20 16:17 UTC (permalink / raw)
To: Jiri Olsa
Cc: Steven Rostedt, Masami Hiramatsu, Josh Poimboeuf, Mahe Tardy,
Peter Zijlstra, bpf, linux-trace-kernel, x86, Yonghong Song,
Song Liu, Andrii Nakryiko
In-Reply-To: <aWYv6864cdO2PWbb@krava>
On Tue, Jan 13, 2026 at 12:43:39PM +0100, Jiri Olsa wrote:
> On Mon, Jan 12, 2026 at 05:07:57PM -0500, Steven Rostedt wrote:
> > On Mon, 12 Jan 2026 22:49:38 +0100
> > Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > > To recreate same stack setup for return probe as we have for entry
> > > probe, we set the instruction pointer to the attached function address,
> > > which gets us the same unwind setup and same stack trace.
> > >
> > > With the fix, entry probe:
> > >
> > > # bpftrace -e 'kprobe:__x64_sys_newuname* { print(kstack)}'
> > > Attaching 1 probe...
> > >
> > > __x64_sys_newuname+9
> > > do_syscall_64+134
> > > entry_SYSCALL_64_after_hwframe+118
> > >
> > > return probe:
> > >
> > > # bpftrace -e 'kretprobe:__x64_sys_newuname* { print(kstack)}'
> > > Attaching 1 probe...
> > >
> > > __x64_sys_newuname+4
> > > do_syscall_64+134
> > > entry_SYSCALL_64_after_hwframe+118
> >
> > But is this really correct?
> >
> > The stack trace of the return from __x86_sys_newuname is from offset "+4".
> >
> > The stack trace from entry is offset "+9". Isn't it confusing that the
> > offset is likely not from the return portion of that function?
>
> right, makes sense.. so standard kprobe actualy skips attached function
> (__x86_sys_newuname) on return probe stacktrace.. perhaps we should do
> the same for kprobe_multi
>
> I managed to get that with the change below, but it's wrong wrt arch code,
> note the ftrace_regs_set_stack_pointer(fregs, stack + 8) .. will try to
> figure out better way when we agree on the solution
>
> thanks,
> jirka
>
>
> ---
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index c56e1e63b893..b0e8ce4934e7 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -71,6 +71,9 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)
> #define ftrace_regs_set_instruction_pointer(fregs, _ip) \
> do { arch_ftrace_regs(fregs)->regs.ip = (_ip); } while (0)
>
> +#define ftrace_regs_set_stack_pointer(fregs, _sp) \
> + do { arch_ftrace_regs(fregs)->regs.sp = (_sp); } while (0)
> +
>
> static __always_inline unsigned long
> ftrace_regs_get_return_address(struct ftrace_regs *fregs)
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 6279e0a753cf..b1510c412dcb 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -717,7 +717,8 @@ int function_graph_enter_regs(unsigned long ret, unsigned long func,
> /* Retrieve a function return address to the trace stack on thread info.*/
> static struct ftrace_ret_stack *
> ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
> - unsigned long frame_pointer, int *offset)
> + unsigned long *stack, unsigned long frame_pointer,
> + int *offset)
> {
> struct ftrace_ret_stack *ret_stack;
>
> @@ -762,6 +763,7 @@ ftrace_pop_return_trace(struct ftrace_graph_ret *trace, unsigned long *ret,
>
> *offset += FGRAPH_FRAME_OFFSET;
> *ret = ret_stack->ret;
> + *stack = (unsigned long) ret_stack->retp;
> trace->func = ret_stack->func;
> trace->overrun = atomic_read(¤t->trace_overrun);
> trace->depth = current->curr_ret_depth;
> @@ -810,12 +812,13 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
> struct ftrace_ret_stack *ret_stack;
> struct ftrace_graph_ret trace;
> unsigned long bitmap;
> + unsigned long stack;
> unsigned long ret;
> int offset;
> int bit;
> int i;
>
> - ret_stack = ftrace_pop_return_trace(&trace, &ret, frame_pointer, &offset);
> + ret_stack = ftrace_pop_return_trace(&trace, &ret, &stack, frame_pointer, &offset);
>
> if (unlikely(!ret_stack)) {
> ftrace_graph_stop();
> @@ -824,8 +827,11 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
> return (unsigned long)panic;
> }
>
> - if (fregs)
> - ftrace_regs_set_instruction_pointer(fregs, trace.func);
> + if (fregs) {
> + ftrace_regs_set_instruction_pointer(fregs, ret);
> + ftrace_regs_set_stack_pointer(fregs, stack + 8);
actually looks like this might be better solution.. storing the proper
rsp value directly to the regs in return_to_handler
jirka
---
diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
index a132608265f6..971883045b75 100644
--- a/arch/x86/kernel/ftrace_64.S
+++ b/arch/x86/kernel/ftrace_64.S
@@ -368,13 +368,16 @@ SYM_CODE_START(return_to_handler)
subq $8, %rsp
UNWIND_HINT_FUNC
+ movq %rsp, %rdi
+ addq $8, %rdi
+
/* Save ftrace_regs for function exit context */
subq $(FRAME_SIZE), %rsp
movq %rax, RAX(%rsp)
movq %rdx, RDX(%rsp)
movq %rbp, RBP(%rsp)
- movq %rsp, RSP(%rsp)
+ movq %rdi, RSP(%rsp)
movq %rsp, %rdi
call ftrace_return_to_handler
^ permalink raw reply related
* Re: [PATCH 01/26] rv/rvgen: introduce AutomataError exception class
From: Gabriele Monaco @ 2026-01-20 15:08 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Nam Cao, Steven Rostedt, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <aW9ySuaPMZVm-MrN@fedora>
On Tue, 2026-01-20 at 09:39 -0300, Wander Lairson Costa wrote:
> On Tue, Jan 20, 2026 at 08:33:10AM +0100, Gabriele Monaco wrote:
> > I agree catching all exceptions like this is quite detrimental while
> > debugging,
> > but I see the original intent.
> > When you run commands written in python, you normally don't expect them to
> > blurt
> > a stack trace when doing relatively normal things, like opening a wrong
> > file.
> > Sure that might be useful when debugging, but for a user-facing tool we want
> > to
> > write a meaningful error message and gracefully fail.
> >
>
> One option I thought was to keep it as it is but adding a --debug option
> which would reraise the exception and then print the stack trace.
> But as the users are developers themselves, leaving the exception
> unchaught would help them identify the error (although I am strongly
> against doing this in server side code). Another reason is the case
> when the code itself has a bug. That would facilitate bug reports.
That could be a good tradeoff. Users are developer but (although I'm not sure if
it really happened yet) are not the rvgen developers, they don't need to know
where exactly the code complained, unless it really broke.
All errors that are expected (OSError or wrong format) should have a meaningful
message for the user, I believe by doing that we'd have a pretty clear idea
where the error came from in the code too (e.g. event parsing, opening a file,
etc.).
If the code has a bug, then yes we should throw the exception as is, that's why
I think it's good not to catch Exception, but to catch only the few exceptions
we know can happen, all others would be bugs.
>
> Perhaps we could catch more specific exceptions that would indicate a
> problem with the dot files instead of Exception. Like
>
> try: ...
> except AutomataError as e:
> print(f"There was a problem processing {dot_file}: {str(e)}",
> file=sys.stderr)
> sys.exit(1)
>
> Which would be a common case. And leaving other types of exceptions
> unchaught.
Yeah pretty much
>
> > Other story is when the exception is something unexpected (that's why
> > leaving a
> > generic Exception here is bad).
> >
> > > print("Writing the monitor into the directory %s" % monitor.name)
> > > monitor.print_files()
> > > diff --git a/tools/verification/rvgen/rvgen/automata.py
> > > b/tools/verification/rvgen/rvgen/automata.py
> > > index d9a3fe2b74bf2..8d88c3b65d00d 100644
> > > --- a/tools/verification/rvgen/rvgen/automata.py
> > > +++ b/tools/verification/rvgen/rvgen/automata.py
> > > @@ -10,6 +10,13 @@
> > >
> > > import ntpath
> > >
> > > +class AutomataError(OSError):
> > > + """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.
> > > + """
> > > +
> >
> > I'm not quite familiar with modern python best practices (so again, take my
> > comments with a grain of salt ;) ), but what is the advantage of using this
> > custom exception instead of using pre-existing specific exception types?
> >
> > Although the difference is minimal, here you're throwing an OSError for
> > something that quite isn't (e.g. wrong format for the dot file).
> > A ValueError feels more appropriate to me in most of the instances here.
> >
> > All in all, I would do something like:
> > * throw a ValueError (or a custom one based on that) whenever we expect
> > wrong
> > data not dependent on OS features
> > * throw OSError whenever that was the exception, perhaps changing the
> > message to
> > something more meaningful to us (like you're already doing here)
> > * intercept only those errors in main.py and print the message without stack
> > trace (if the message is clear enough we shouldn't need it).
> >
> > Does it make sense to you?
> >
>
> The reasoning behind specific exception types is to allow the calling code
> process diferent exceptions in more specialized code paths, like the
> example above.
>
> AutomataError could derive from both OSError and ValueError.
>
> class AutomataError(OSError, ValueError): ...
>
> Which would address your (valid) point. This way, the calling code could
> either process specific automata related errors with AutomataError, or
> handle general file error, like that:
>
> try...
> except OSError as e:
> print(f"File error: {str(e)", file=sys.stderr)
> except AutomataError as e:
> print(f"Ill formed dot file: {str(e)", file=stderr)
>
Yes! That's exactly what I mean, those are exceptions we expect, so no splat,
all others can just propagate.
Thanks,
Gabriele
> > Thanks,
> > Gabriele
> >
> > > class Automata:
> > > """Automata class: Reads a dot file and part it as an automata.
> > >
> > > @@ -32,11 +39,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
> > >
> > > @@ -45,8 +52,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(f"Cannot open the file:
> > > {self.__dot_path}")
> > > from exc
> > >
> > > dot_lines = dot_file.read().splitlines()
> > > dot_file.close()
> > > @@ -55,7 +62,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 b9b6f14cc536a..1a1770e7f20c0 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 = ""
> > > @@ -93,7 +93,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/generator.py
> > > b/tools/verification/rvgen/rvgen/generator.py
> > > index 3441385c11770..a7bee6b1ea70c 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)
> >
^ permalink raw reply
* Re: [PATCH bpf-next 2/4] x86/fgraph,bpf: Switch kprobe_multi program stack unwind to hw_regs path
From: Steven Rostedt @ 2026-01-20 14:50 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Masami Hiramatsu, Josh Poimboeuf, Mahe Tardy,
Peter Zijlstra, bpf, linux-trace-kernel, x86, Yonghong Song,
Song Liu, Andrii Nakryiko
In-Reply-To: <CAEf4BzZyF2MsF5CkLEsrd0dumeCJ3-zzP+azCZ4TRoDkzjGLdg@mail.gmail.com>
On Fri, 16 Jan 2026 14:24:51 -0800
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> I don't insist, but I'm just saying that practically speaking this
> would make sense. Even conceptually, kretprobe is (logically) called
> from traced function right before exit. In reality it's not exactly
> like that and we don't know where ret happened, but having traced
> function in kretprobe's stack trace is more useful than confusing,
> IMO.
It's more useful than confusing for you because you understand it. For
anyone else, it will be very confusing, or worse, miscalculated, to see the
backtrace coming from the start of the function when the function has
already executed.
Sure, having the function is very useful, but the function is already
completed. Technically it shouldn't be in the stacktrace. Having the return
address in the trace should point out that the stacktrace came from the
function right before that address.
-- Steve
^ permalink raw reply
* Re: [PATCH 26/26] rv/rvgen: extract node marker string to class constant
From: Gabriele Monaco @ 2026-01-20 13:11 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <85c7d4762464e6a50b4318f4ea56a70202a4b287.camel@redhat.com>
On Tue, 2026-01-20 at 13:36 +0100, Gabriele Monaco wrote:
> On Tue, 2026-01-20 at 08:34 -0300, Wander Lairson Costa wrote:
> > On Tue, Jan 20, 2026 at 10:03:20AM +0100, Gabriele Monaco wrote:
> > > On Mon, 2026-01-19 at 17:46 -0300, Wander Lairson Costa wrote:
> > > > Add a node_marker class constant to the Automata class to replace the
> > > > hardcoded "{node" string literal used throughout the DOT file parsing
> > > > logic. This follows the existing pattern established by the init_marker
> > > > and invalid_state_str class constants in the same class.
> > > >
> > > > The "{node" string is used as a marker to identify node declaration
> > > > lines in DOT files during state variable extraction and cursor
> > > > positioning. Extracting it to a named constant improves code
> > > > maintainability and makes the marker's purpose explicit.
> > > >
> > > > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > >
> > > Looks fine for me, thanks!
> > >
> > > I wonder if we could merge this patch with 15/26 that is introducing a
> > > very
> > > similar change on init_marker.
> >
> > The idea was to make each patch doing one thing to make the reviewer's
> > life easier (I think I broke my own rule in a couple of patch). But if
> > there is a strong feeling about the merge, I could merged them in a
> > possible v2 patch series.
> >
>
> Yeah I get the idea, I guess I could just pick the most obvious patches and
> send
> a PR to Steve before the merge window, so I can see directly if it makes sense
> to squash them and you don't need to send them all in the v2.
Screamed victory too fast.. I tried the patches on the latest version of the
tree (which reached already linux-next) and they're have several conflicts.
I don't have time to rebase them one by one right now, let's continue with this
series as a whole.
As a rule of thumb, considering these patches get first reviewed, then PR-ed to
Steve and then to Linus, I'd favour to lower the number if possible, but feel
free to choose where it makes sense for them to stay separate.
Thanks,
Gabriele
>
> I'm leaving the longer or trickier ones for the next cycle so that I can test
> them a bit better and perhaps get some rvgen selftest help with that. Also for
> some it's better to wait for Nam's comments.
>
> Will let you know the ones I pick. Thanks again for the extensive work!
>
> Gabriele
>
> > >
> > > Anyway:
> > >
> > > Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> > >
> > > > ---
> > > > tools/verification/rvgen/rvgen/automata.py | 9 +++++----
> > > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/tools/verification/rvgen/rvgen/automata.py
> > > > b/tools/verification/rvgen/rvgen/automata.py
> > > > index a6889d0c26c3f..5f23db1855cd3 100644
> > > > --- a/tools/verification/rvgen/rvgen/automata.py
> > > > +++ b/tools/verification/rvgen/rvgen/automata.py
> > > > @@ -29,6 +29,7 @@ class Automata:
> > > >
> > > > invalid_state_str = "INVALID_STATE"
> > > > init_marker = "__init_"
> > > > + node_marker = "{node"
> > > >
> > > > def __init__(self, file_path, model_name=None):
> > > > self.__dot_path = file_path
> > > > @@ -76,7 +77,7 @@ class Automata:
> > > > for cursor, line in enumerate(self.__dot_lines):
> > > > split_line = line.split()
> > > >
> > > > - if len(split_line) and split_line[0] == "{node":
> > > > + if len(split_line) and split_line[0] == self.node_marker:
> > > > return cursor
> > > >
> > > > raise AutomataError("Could not find a beginning state")
> > > > @@ -91,9 +92,9 @@ class Automata:
> > > > continue
> > > >
> > > > if state == 0:
> > > > - if line[0] == "{node":
> > > > + if line[0] == self.node_marker:
> > > > state = 1
> > > > - elif line[0] != "{node":
> > > > + elif line[0] != self.node_marker:
> > > > break
> > > > else:
> > > > raise AutomataError("Could not find beginning event")
> > > > @@ -116,7 +117,7 @@ class Automata:
> > > > # process nodes
> > > > for line in islice(self.__dot_lines, cursor, None):
> > > > split_line = line.split()
> > > > - if not split_line or split_line[0] != "{node":
> > > > + if not split_line or split_line[0] != self.node_marker:
> > > > break
> > > >
> > > > raw_state = split_line[-1]
^ permalink raw reply
* Re: [PATCH 01/26] rv/rvgen: introduce AutomataError exception class
From: Wander Lairson Costa @ 2026-01-20 12:39 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Nam Cao, Steven Rostedt, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <24e308b019cf9272884091f85b6675fd05201a2b.camel@redhat.com>
On Tue, Jan 20, 2026 at 08:33:10AM +0100, Gabriele Monaco wrote:
> On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> > Replace generic Exception usage with a custom AutomataError class
> > that inherits from OSError throughout the rvgen tool. This change
> > 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 inherits from OSError rather than Exception
> > because most error conditions involve file I/O operations such as
> > reading DOT files or handling file access issues. This semantic
> > alignment makes exception handling more specific and appropriate.
> > The exception is raised when DOT file processing fails due to invalid
> > format, I/O errors, or malformed automaton definitions.
> >
> > Additionally, remove the broad try-except block from __main__.py that
> > was catching all exceptions. This allows Python's default exception
> > handling to provide complete stack traces, making debugging
> > significantly easier by showing exact error types and locations.
> >
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
>
> Thanks for the extensive series!
> See my comments below.
> Mind that I likely know python less than you do, so just call me out when I
> start babbling.
>
> > ---
> > tools/verification/rvgen/__main__.py | 25 +++++++++------------
> > tools/verification/rvgen/rvgen/automata.py | 17 +++++++++-----
> > tools/verification/rvgen/rvgen/dot2c.py | 4 ++--
> > tools/verification/rvgen/rvgen/generator.py | 7 ++----
> > 4 files changed, 26 insertions(+), 27 deletions(-)
> >
> > diff --git a/tools/verification/rvgen/__main__.py
> > b/tools/verification/rvgen/__main__.py
> > index fa6fc1f4de2f7..768b11a1e978b 100644
> > --- a/tools/verification/rvgen/__main__.py
> > +++ b/tools/verification/rvgen/__main__.py
> > @@ -39,22 +39,17 @@ if __name__ == '__main__':
> >
> > params = parser.parse_args()
> >
> > - try:
> > - if params.subcmd == "monitor":
> > - print("Opening and parsing the specification file %s" %
> > params.spec)
> > - if params.monitor_class == "da":
> > - monitor = dot2k(params.spec, params.monitor_type,
> > vars(params))
> > - elif params.monitor_class == "ltl":
> > - monitor = ltl2k(params.spec, params.monitor_type,
> > vars(params))
> > - else:
> > - print("Unknown monitor class:", params.monitor_class)
> > - sys.exit(1)
> > + if params.subcmd == "monitor":
> > + print("Opening and parsing the specification file %s" % params.spec)
> > + if params.monitor_class == "da":
> > + monitor = dot2k(params.spec, params.monitor_type, vars(params))
> > + elif params.monitor_class == "ltl":
> > + monitor = ltl2k(params.spec, params.monitor_type, vars(params))
> > else:
> > - monitor = Container(vars(params))
> > - except Exception as e:
> > - print('Error: '+ str(e))
> > - print("Sorry : :-(")
> > - sys.exit(1)
> > + print("Unknown monitor class:", params.monitor_class)
> > + sys.exit(1)
> > + else:
> > + monitor = Container(vars(params))
> >
>
> I agree catching all exceptions like this is quite detrimental while debugging,
> but I see the original intent.
> When you run commands written in python, you normally don't expect them to blurt
> a stack trace when doing relatively normal things, like opening a wrong file.
> Sure that might be useful when debugging, but for a user-facing tool we want to
> write a meaningful error message and gracefully fail.
>
One option I thought was to keep it as it is but adding a --debug option
which would reraise the exception and then print the stack trace.
But as the users are developers themselves, leaving the exception
unchaught would help them identify the error (although I am strongly
against doing this in server side code). Another reason is the case
when the code itself has a bug. That would facilitate bug reports.
Perhaps we could catch more specific exceptions that would indicate a
problem with the dot files instead of Exception. Like
try: ...
except AutomataError as e:
print(f"There was a problem processing {dot_file}: {str(e)}",
file=sys.stderr)
sys.exit(1)
Which would be a common case. And leaving other types of exceptions
unchaught.
> Other story is when the exception is something unexpected (that's why leaving a
> generic Exception here is bad).
>
> > print("Writing the monitor into the directory %s" % monitor.name)
> > monitor.print_files()
> > diff --git a/tools/verification/rvgen/rvgen/automata.py
> > b/tools/verification/rvgen/rvgen/automata.py
> > index d9a3fe2b74bf2..8d88c3b65d00d 100644
> > --- a/tools/verification/rvgen/rvgen/automata.py
> > +++ b/tools/verification/rvgen/rvgen/automata.py
> > @@ -10,6 +10,13 @@
> >
> > import ntpath
> >
> > +class AutomataError(OSError):
> > + """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.
> > + """
> > +
>
> I'm not quite familiar with modern python best practices (so again, take my
> comments with a grain of salt ;) ), but what is the advantage of using this
> custom exception instead of using pre-existing specific exception types?
>
> Although the difference is minimal, here you're throwing an OSError for
> something that quite isn't (e.g. wrong format for the dot file).
> A ValueError feels more appropriate to me in most of the instances here.
>
> All in all, I would do something like:
> * throw a ValueError (or a custom one based on that) whenever we expect wrong
> data not dependent on OS features
> * throw OSError whenever that was the exception, perhaps changing the message to
> something more meaningful to us (like you're already doing here)
> * intercept only those errors in main.py and print the message without stack
> trace (if the message is clear enough we shouldn't need it).
>
> Does it make sense to you?
>
The reasoning behind specific exception types is to allow the calling code
process diferent exceptions in more specialized code paths, like the
example above.
AutomataError could derive from both OSError and ValueError.
class AutomataError(OSError, ValueError): ...
Which would address your (valid) point. This way, the calling code could
either process specific automata related errors with AutomataError, or
handle general file error, like that:
try...
except OSError as e:
print(f"File error: {str(e)", file=sys.stderr)
except AutomataError as e:
print(f"Ill formed dot file: {str(e)", file=stderr)
> Thanks,
> Gabriele
>
> > class Automata:
> > """Automata class: Reads a dot file and part it as an automata.
> >
> > @@ -32,11 +39,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
> >
> > @@ -45,8 +52,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(f"Cannot open the file: {self.__dot_path}")
> > from exc
> >
> > dot_lines = dot_file.read().splitlines()
> > dot_file.close()
> > @@ -55,7 +62,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 b9b6f14cc536a..1a1770e7f20c0 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 = ""
> > @@ -93,7 +93,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/generator.py
> > b/tools/verification/rvgen/rvgen/generator.py
> > index 3441385c11770..a7bee6b1ea70c 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)
>
^ permalink raw reply
* Re: [PATCH 26/26] rv/rvgen: extract node marker string to class constant
From: Gabriele Monaco @ 2026-01-20 12:36 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <aW9nfu8r0YcGj0z5@fedora>
On Tue, 2026-01-20 at 08:34 -0300, Wander Lairson Costa wrote:
> On Tue, Jan 20, 2026 at 10:03:20AM +0100, Gabriele Monaco wrote:
> > On Mon, 2026-01-19 at 17:46 -0300, Wander Lairson Costa wrote:
> > > Add a node_marker class constant to the Automata class to replace the
> > > hardcoded "{node" string literal used throughout the DOT file parsing
> > > logic. This follows the existing pattern established by the init_marker
> > > and invalid_state_str class constants in the same class.
> > >
> > > The "{node" string is used as a marker to identify node declaration
> > > lines in DOT files during state variable extraction and cursor
> > > positioning. Extracting it to a named constant improves code
> > > maintainability and makes the marker's purpose explicit.
> > >
> > > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> >
> > Looks fine for me, thanks!
> >
> > I wonder if we could merge this patch with 15/26 that is introducing a very
> > similar change on init_marker.
>
> The idea was to make each patch doing one thing to make the reviewer's
> life easier (I think I broke my own rule in a couple of patch). But if
> there is a strong feeling about the merge, I could merged them in a
> possible v2 patch series.
>
Yeah I get the idea, I guess I could just pick the most obvious patches and send
a PR to Steve before the merge window, so I can see directly if it makes sense
to squash them and you don't need to send them all in the v2.
I'm leaving the longer or trickier ones for the next cycle so that I can test
them a bit better and perhaps get some rvgen selftest help with that. Also for
some it's better to wait for Nam's comments.
Will let you know the ones I pick. Thanks again for the extensive work!
Gabriele
> >
> > Anyway:
> >
> > Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> >
> > > ---
> > > tools/verification/rvgen/rvgen/automata.py | 9 +++++----
> > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/verification/rvgen/rvgen/automata.py
> > > b/tools/verification/rvgen/rvgen/automata.py
> > > index a6889d0c26c3f..5f23db1855cd3 100644
> > > --- a/tools/verification/rvgen/rvgen/automata.py
> > > +++ b/tools/verification/rvgen/rvgen/automata.py
> > > @@ -29,6 +29,7 @@ class Automata:
> > >
> > > invalid_state_str = "INVALID_STATE"
> > > init_marker = "__init_"
> > > + node_marker = "{node"
> > >
> > > def __init__(self, file_path, model_name=None):
> > > self.__dot_path = file_path
> > > @@ -76,7 +77,7 @@ class Automata:
> > > for cursor, line in enumerate(self.__dot_lines):
> > > split_line = line.split()
> > >
> > > - if len(split_line) and split_line[0] == "{node":
> > > + if len(split_line) and split_line[0] == self.node_marker:
> > > return cursor
> > >
> > > raise AutomataError("Could not find a beginning state")
> > > @@ -91,9 +92,9 @@ class Automata:
> > > continue
> > >
> > > if state == 0:
> > > - if line[0] == "{node":
> > > + if line[0] == self.node_marker:
> > > state = 1
> > > - elif line[0] != "{node":
> > > + elif line[0] != self.node_marker:
> > > break
> > > else:
> > > raise AutomataError("Could not find beginning event")
> > > @@ -116,7 +117,7 @@ class Automata:
> > > # process nodes
> > > for line in islice(self.__dot_lines, cursor, None):
> > > split_line = line.split()
> > > - if not split_line or split_line[0] != "{node":
> > > + if not split_line or split_line[0] != self.node_marker:
> > > break
> > >
> > > raw_state = split_line[-1]
^ permalink raw reply
* Re: [PATCH 17/26] rv/rvgen: fix possibly unbound variable in ltl2k
From: Gabriele Monaco @ 2026-01-20 12:30 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <aW9oj7-iagg2-VF_@fedora>
On Tue, 2026-01-20 at 08:37 -0300, Wander Lairson Costa wrote:
> On Tue, Jan 20, 2026 at 09:59:11AM +0100, Gabriele Monaco wrote:
> > On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> > > Initialize loop variable `i` before the for loop in abbreviate_atoms
> > > function to fix pyright static type checker error. The previous code
> > > left `i` potentially unbound in edge cases where the range could be
> > > empty, though this would not occur in practice since the loop always
> > > executes at least once with the given range parameters.
> > >
> > > The initialization to zero ensures that `i` has a defined value before
> > > entering the loop scope, satisfying static analysis requirements
> > > while preserving the existing logic. The for loop immediately assigns
> > > i to the first value from the range, so the initialization value is
> > > never actually used in normal execution paths.
> > >
> > > This change resolves the pyright reportPossiblyUnbound error without
> > > altering the function's behavior or performance characteristics.
> >
> > So are we just pleasing the tool or is there a real implication of this?
> >
> > Apparently code like
> >
> > for i in range(len([]), -1, -1):
> > pass
> > print(i)
> >
> > works just fine since range() returns at least 0 (as you mentioned in the
> > commit
> > message) and i is not used before assignation in the loop, so I don't really
> > see
> > a problem.
> >
> > Apparently pyright devs don't want ([1]) to implement a logic to sort out
> > the
> > /possibly/ unbound error here.
> >
> > From what I understand, this code is already not pythonic, so rather than
> > silence the warning to please this tool I'd just refactor the code not to
> > use i
> > after the loop (or leave it as it is, since it works fine).
> >
> > What do you think?
>
> You're right, I could have done:
>
> for atom in reversed(atoms): ...
>
I'm missing what you mean with this, the range is iterating over the string
representation of atom (in reverse) not the array of atoms.
You basically want i to be the length of the longest prefix common to at least
another atom.
You could assign i to some python trick doing the exact same thing the loop
does, like:
i = next((i for i in range(len(atom), -1, -1)
if sum(a.startswith(atom[:i]) for a in atoms) > 1))
next() is basically doing the break at the first occurrence from the generator,
just now your i doesn't live (only) inside the loop.
So now you save 2 lines and get any C developer scratch their head when they
look at the code, but hey, pyright is happy!
If you do find the trick with next() readable or have any better idea, feel free
to try though.
Thanks,
Gabriele
> I will modify it in v2.
>
> >
> > Thanks,
> > Gabriele
> >
> > [1] - https://github.com/microsoft/pyright/issues/844
> >
> > >
> > > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > > ---
> > > tools/verification/rvgen/rvgen/ltl2k.py | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/tools/verification/rvgen/rvgen/ltl2k.py
> > > b/tools/verification/rvgen/rvgen/ltl2k.py
> > > index fa9ea6d597095..94dc64af1716d 100644
> > > --- a/tools/verification/rvgen/rvgen/ltl2k.py
> > > +++ b/tools/verification/rvgen/rvgen/ltl2k.py
> > > @@ -45,6 +45,7 @@ def abbreviate_atoms(atoms: list[str]) -> list[str]:
> > >
> > > abbrs = []
> > > for atom in atoms:
> > > + i = 0
> > > for i in range(len(atom), -1, -1):
> > > if sum(a.startswith(atom[:i]) for a in atoms) > 1:
> > > break
> >
^ permalink raw reply
* Re: [PATCH 14/26] rv/rvgen: remove redundant initial_state removal
From: Wander Lairson Costa @ 2026-01-20 12:05 UTC (permalink / raw)
To: Gabriele Monaco
Cc: Steven Rostedt, Nam Cao, open list,
open list:RUNTIME VERIFICATION (RV)
In-Reply-To: <63e032341807f9ece73937d5e20eee9e3162fb61.camel@redhat.com>
On Tue, Jan 20, 2026 at 09:01:01AM +0100, Gabriele Monaco wrote:
> On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote:
> > Remove an unnecessary and incorrect list removal operation in the
> > automata state variable processing. The code attempted to remove
> > initial_state from the states list, but this element was never
> > added to the list in the first place. States with the __init_
> > prefix are explicitly excluded from the states list during the
> > parsing loop, with only the initial_state variable being set
> > from them.
>
> The initial state is not the state with __init_, but the state pointed to by
> that, the purpose of removing it after sorting and putting it back is for it to
> be the first (we may argue there are better ways to do that, but it works).
>
> After this change, the initial state is duplicated..
> I think we should just drop this.
Ah, now I understood the reasoning, thanks. I will drop the patch.
>
> Thanks,
> Gabriele
>
> >
> > Calling remove() on an element that does not exist in a list
> > raises a ValueError. This code would have failed during execution
> > when processing any DOT file containing an initial state marker.
> > The subsequent insert operation at index 0 correctly adds the
> > initial_state to the beginning of the states list, making the
> > removal operation both incorrect and redundant.
> >
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > ---
> > tools/verification/rvgen/rvgen/automata.py | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/tools/verification/rvgen/rvgen/automata.py
> > b/tools/verification/rvgen/rvgen/automata.py
> > index 7841a6e70bad2..b302af3e5133e 100644
> > --- a/tools/verification/rvgen/rvgen/automata.py
> > +++ b/tools/verification/rvgen/rvgen/automata.py
> > @@ -111,7 +111,6 @@ class Automata:
> > cursor += 1
> >
> > states = sorted(set(states))
> > - states.remove(initial_state)
> >
> > # Insert the initial state at the beginning of the states
> > states.insert(0, initial_state)
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox