* [PATCH] rtla/cli: Unify and improve range validation logic
From: Tomas Glozar @ 2026-07-10 13:15 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar
Cc: John Kacur, Luis Goncalves, Crystal Wood, Costa Shulyupin,
Wander Lairson Costa, LKML, linux-trace-kernel
Several RTLA options do range validation inside the CLI parser layer
(e.g. -p/--period). When RTLA migrated CLI parsing to libsubcmd, this
logic was moved unchanged inside opt_*() callbacks.
Unify range validation so that all options use two newly added
functions, check_llong_range() and check_int_range(), to validate the
range.
The new range validation returns -1 from opt_*() callbacks rather than
hard-exit with fatal(), allowing the help message for the specific
option to be automatically displayed by libsubcmd logic.
Many options no longer need a custom callback, as they use the unified
range validation of opt_llong_callback() and opt_int_callback().
Validation for several other options is improved:
- timerlat -p/--period: lower bound raised from 1 to 100 us to match
the kernel's timerlat_min_period in trace_osnoise.c.
- timerlat -A/--aligned: reject negative values.
- timerlat --deepest-idle-state: add range [-1, INT_MAX]; previously,
values <= -2 were read as "option not set".
- timerlat -p/--period, -A/--aligned, -b/--bucket-size: properly reject
negative values instead of passing them to the tracer.
Remove unit tests for removed callbacks and test the new range
validation functionality of opt_llong_callback() and opt_int_callback().
Update runtime tests for histogram options to account for the new error
messages and exit value.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
Dependencies:
- "rtla: Allow unsetting non-list custom-callback CLI options" patchset
Link: https://lore.kernel.org/linux-trace-kernel/20260629083654.1548925-1-tglozar@redhat.com/T/#u
tools/tracing/rtla/src/cli_p.h | 234 +++++-------
tools/tracing/rtla/tests/osnoise.t | 2 +-
tools/tracing/rtla/tests/timerlat.t | 2 +-
.../rtla/tests/unit/cli_opt_callback.c | 353 ++++++------------
4 files changed, 215 insertions(+), 376 deletions(-)
diff --git a/tools/tracing/rtla/src/cli_p.h b/tools/tracing/rtla/src/cli_p.h
index 3a93dba60215b..b7688559ca53d 100644
--- a/tools/tracing/rtla/src/cli_p.h
+++ b/tools/tracing/rtla/src/cli_p.h
@@ -5,6 +5,7 @@
#error "Private header file included outside of cli.c module"
#endif
+#include <limits.h>
#include <linux/kernel.h>
#include <subcmd/parse-options.h>
@@ -32,9 +33,73 @@ static const int default_bucket_size = 1;
static const int default_entries = 256;
static const enum stack_format default_stack_format = STACK_FORMAT_TRUNCATE;
+/*
+ * Range checking for long long and int option callbacks.
+ *
+ * Pass a pointer to a const struct as opt->data to enable range checking.
+ * If opt->data is NULL, no range check is performed.
+ */
+struct llong_range {
+ long long min;
+ long long max;
+};
+
+struct int_range {
+ int min;
+ int max;
+};
+
+#define LLONG_RANGE(lo, hi) \
+ (&(const struct llong_range){ .min = (lo), .max = (hi) })
+
+#define INT_RANGE(lo, hi) \
+ (&(const struct int_range){ .min = (lo), .max = (hi) })
+
+static int check_llong_range(const struct option *opt, long long value)
+{
+ const struct llong_range *range = opt->data;
+
+ if (!range)
+ return 0;
+ if (value < range->min || value > range->max) {
+ fprintf(stderr, " Error: --%s value %lld is out of range [%lld, %lld]\n",
+ opt->long_name, value, range->min, range->max);
+ return -1;
+ }
+ return 0;
+}
+
+static int check_int_range(const struct option *opt, int value)
+{
+ const struct int_range *range = opt->data;
+
+ if (!range)
+ return 0;
+ if (value < range->min || value > range->max) {
+ fprintf(stderr, " Error: --%s value %d is out of range [%d, %d]\n",
+ opt->long_name, value, range->min, range->max);
+ return -1;
+ }
+ return 0;
+}
+
+/*
+ * OPT_CALLBACK variant that populates .data (for range checking).
+ */
+#define RTLA_OPT_CALLBACK_DATA(s, l, v, a, h, f, d) \
+ { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), \
+ .value = (v), .argh = (a), .help = (h), .callback = (f), \
+ .data = (void *)(d) }
+
+#define RTLA_OPT_CALLBACK_DATA_DEFVAL(s, l, v, a, h, f, d, dv) \
+ { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), \
+ .value = (v), .argh = (a), .help = (h), .callback = (f), \
+ .data = (void *)(d), .defval = (intptr_t)(dv) }
+
/*
* Shorthand macros for integer/long long command line options using
- * opt_int_callback/opt_llong_callback, with variants that set defval.
+ * opt_int_callback/opt_llong_callback, with variants that set defval
+ * and/or data (for range checking).
*
* Note: defval's type is intptr_t. opt_int_callback interprets it directly as
* an int, opt_llong_callback interprets it as a pointer to a long long, as
@@ -47,6 +112,10 @@ static const enum stack_format default_stack_format = STACK_FORMAT_TRUNCATE;
.short_name = (s), .long_name = (l), .value = (v), .argh = (a), \
.help = (h), .callback = opt_llong_callback, .defval = (intptr_t)(d) }
+#define RTLA_OPT_LLONG_DATA(s, l, v, a, h, d) { .type = OPTION_CALLBACK, \
+ .short_name = (s), .long_name = (l), .value = (v), .argh = (a), \
+ .help = (h), .callback = opt_llong_callback, .data = (void *)(d) }
+
#define RTLA_OPT_INT(s, l, v, a, h) \
OPT_CALLBACK(s, l, v, a, h, opt_int_callback)
@@ -54,6 +123,11 @@ static const enum stack_format default_stack_format = STACK_FORMAT_TRUNCATE;
.short_name = (s), .long_name = (l), .value = (v), .argh = (a), \
.help = (h), .callback = opt_int_callback, .defval = (intptr_t)(d) }
+#define RTLA_OPT_INT_DATA_DEFVAL(s, l, v, a, h, d, dv) { .type = OPTION_CALLBACK, \
+ .short_name = (s), .long_name = (l), .value = (v), .argh = (a), \
+ .help = (h), .callback = opt_int_callback, \
+ .data = (void *)(d), .defval = (intptr_t)(dv) }
+
/*
* Macros for command line options common to all tools
*
@@ -182,6 +256,8 @@ static int opt_llong_callback(const struct option *opt, const char *arg, int uns
return -1;
*value = get_llong_from_str((char *)arg);
+ if (check_llong_range(opt, *value))
+ return -1;
return 0;
}
@@ -199,6 +275,8 @@ static int opt_int_callback(const struct option *opt, const char *arg, int unset
if (strtoi(arg, value))
return -1;
+ if (check_int_range(opt, *value))
+ return -1;
return 0;
}
@@ -359,13 +437,13 @@ static int opt_filter_cb(const struct option *opt, const char *arg, int unset)
/*
* Macros for command line options specific to osnoise
*/
-#define OSNOISE_OPT_PERIOD OPT_CALLBACK('p', "period", ¶ms->period, "us", \
+#define OSNOISE_OPT_PERIOD RTLA_OPT_LLONG_DATA('p', "period", ¶ms->period, "us", \
"osnoise period in us", \
- opt_osnoise_period_cb)
+ LLONG_RANGE(1, 10000000))
-#define OSNOISE_OPT_RUNTIME OPT_CALLBACK('r', "runtime", ¶ms->runtime, "us", \
+#define OSNOISE_OPT_RUNTIME RTLA_OPT_LLONG_DATA('r', "runtime", ¶ms->runtime, "us", \
"osnoise runtime in us", \
- opt_osnoise_runtime_cb)
+ LLONG_RANGE(100, LLONG_MAX))
#define OSNOISE_OPT_THRESHOLD RTLA_OPT_LLONG('T', "threshold", ¶ms->threshold, "us", \
"the minimum delta to be considered a noise")
@@ -400,44 +478,6 @@ static int opt_osnoise_auto_cb(const struct option *opt, const char *arg, int un
return 0;
}
-static int opt_osnoise_period_cb(const struct option *opt, const char *arg, int unset)
-{
- unsigned long long *period = opt->value;
-
- if (unset) {
- *period = 0;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- *period = get_llong_from_str((char *)arg);
- if (*period > 10000000)
- fatal("Period longer than 10 s");
-
- return 0;
-}
-
-static int opt_osnoise_runtime_cb(const struct option *opt, const char *arg, int unset)
-{
- unsigned long long *runtime = opt->value;
-
- if (unset) {
- *runtime = 0;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- *runtime = get_llong_from_str((char *)arg);
- if (*runtime < 100)
- fatal("Runtime shorter than 100 us");
-
- return 0;
-}
-
static int opt_osnoise_trace_output_cb(const struct option *opt, const char *arg, int unset)
{
const char **trace_output = opt->value;
@@ -492,9 +532,9 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int
/*
* Macros for command line options specific to timerlat
*/
-#define TIMERLAT_OPT_PERIOD OPT_CALLBACK('p', "period", ¶ms->timerlat_period_us, "us", \
+#define TIMERLAT_OPT_PERIOD RTLA_OPT_LLONG_DATA('p', "period", ¶ms->timerlat_period_us, "us", \
"timerlat period in us", \
- opt_timerlat_period_cb)
+ LLONG_RANGE(100, 1000000))
#define TIMERLAT_OPT_STACK RTLA_OPT_LLONG('s', "stack", ¶ms->print_stack, "us", \
"save the stack trace at the IRQ if a thread latency is higher than the argument in us")
@@ -503,14 +543,15 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int
"display data in nanoseconds", \
opt_nano_cb)
-#define TIMERLAT_OPT_DMA_LATENCY OPT_CALLBACK(0, "dma-latency", ¶ms->dma_latency, "us", \
+#define TIMERLAT_OPT_DMA_LATENCY RTLA_OPT_INT_DATA_DEFVAL(0, "dma-latency", \
+ ¶ms->dma_latency, "us", \
"set /dev/cpu_dma_latency latency <us> to reduce exit from idle latency", \
- opt_dma_latency_cb)
+ INT_RANGE(0, 10000), default_dma_latency)
-#define TIMERLAT_OPT_DEEPEST_IDLE_STATE RTLA_OPT_INT_DEFVAL(0, "deepest-idle-state", \
+#define TIMERLAT_OPT_DEEPEST_IDLE_STATE RTLA_OPT_INT_DATA_DEFVAL(0, "deepest-idle-state", \
¶ms->deepest_idle_state, "n", \
"only go down to idle state n on cpus used by timerlat to reduce exit from idle latency", \
- default_deepest_idle_state)
+ INT_RANGE(-1, INT_MAX), default_deepest_idle_state)
#define TIMERLAT_OPT_AA_ONLY OPT_CALLBACK(0, "aa-only", params, "us", \
"stop if <us> latency is hit, only printing the auto analysis (reduces CPU usage)", \
@@ -530,33 +571,14 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int
"set the stack format (truncate, skip, full)", \
opt_stack_format_cb)
-#define TIMERLAT_OPT_ALIGNED OPT_CALLBACK('A', "aligned", params, "us", \
+#define TIMERLAT_OPT_ALIGNED RTLA_OPT_CALLBACK_DATA('A', "aligned", params, "us", \
"align thread wakeups to a specific offset", \
- opt_timerlat_align_cb)
+ opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX))
/*
* Callback functions for command line options for timerlat tools
*/
-static int opt_timerlat_period_cb(const struct option *opt, const char *arg, int unset)
-{
- long long *period = opt->value;
-
- if (unset) {
- *period = 0;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- *period = get_llong_from_str((char *)arg);
- if (*period > 1000000)
- fatal("Period longer than 1 s");
-
- return 0;
-}
-
static int opt_timerlat_auto_cb(const struct option *opt, const char *arg, int unset)
{
struct timerlat_cb_data *cb_data = opt->value;
@@ -585,28 +607,6 @@ static int opt_timerlat_auto_cb(const struct option *opt, const char *arg, int u
return 0;
}
-static int opt_dma_latency_cb(const struct option *opt, const char *arg, int unset)
-{
- int *dma_latency = opt->value;
- int retval;
-
- if (unset) {
- *dma_latency = default_dma_latency;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- retval = strtoi((char *)arg, dma_latency);
- if (retval)
- fatal("Invalid -dma-latency %s", arg);
- if (*dma_latency < 0 || *dma_latency > 10000)
- fatal("--dma-latency needs to be >= 0 and <= 10000");
-
- return 0;
-}
-
static int opt_aa_only_cb(const struct option *opt, const char *arg, int unset)
{
struct timerlat_params *params = opt->value;
@@ -748,6 +748,8 @@ static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int
params->timerlat_align = true;
params->timerlat_align_us = get_llong_from_str((char *)arg);
+ if (check_llong_range(opt, params->timerlat_align_us))
+ return -1;
return 0;
}
@@ -756,14 +758,15 @@ static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int
* Macros for command line options specific to histogram-based tools
*/
-#define HIST_OPT_BUCKET_SIZE OPT_CALLBACK('b', "bucket-size", \
+#define HIST_OPT_BUCKET_SIZE RTLA_OPT_INT_DATA_DEFVAL('b', "bucket-size", \
¶ms->common.hist.bucket_size, "N", \
"set the histogram bucket size (default 1)", \
- opt_bucket_size_cb)
+ INT_RANGE(1, 999999), default_bucket_size)
-#define HIST_OPT_ENTRIES OPT_CALLBACK('E', "entries", ¶ms->common.hist.entries, "N", \
+#define HIST_OPT_ENTRIES RTLA_OPT_INT_DATA_DEFVAL('E', "entries", \
+ ¶ms->common.hist.entries, "N", \
"set the number of entries of the histogram (default 256)", \
- opt_entries_cb)
+ INT_RANGE(10, 9999999), default_entries)
#define HIST_OPT_NO_IRQ OPT_BOOLEAN_FLAG(0, "no-irq", ¶ms->common.hist.no_irq, \
"ignore IRQ latencies", PARSE_OPT_NOAUTONEG)
@@ -783,42 +786,3 @@ static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int
#define HIST_OPT_WITH_ZEROS OPT_BOOLEAN(0, "with-zeros", ¶ms->common.hist.with_zeros, \
"print zero only entries")
-/* Histogram-specific callbacks */
-
-static int opt_bucket_size_cb(const struct option *opt, const char *arg, int unset)
-{
- int *bucket_size = opt->value;
-
- if (unset) {
- *bucket_size = default_bucket_size;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- *bucket_size = get_llong_from_str((char *)arg);
- if (*bucket_size == 0 || *bucket_size >= 1000000)
- fatal("Bucket size needs to be > 0 and <= 1000000");
-
- return 0;
-}
-
-static int opt_entries_cb(const struct option *opt, const char *arg, int unset)
-{
- int *entries = opt->value;
-
- if (unset) {
- *entries = default_entries;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- *entries = get_llong_from_str((char *)arg);
- if (*entries < 10 || *entries > 9999999)
- fatal("Entries must be > 10 and < 10000000");
-
- return 0;
-}
diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t
index 346a14a860c82..cd9769bb5a9f7 100644
--- a/tools/tracing/rtla/tests/osnoise.t
+++ b/tools/tracing/rtla/tests/osnoise.t
@@ -32,7 +32,7 @@ check "hist with -b/--bucket-size" \
check "hist with -E/--entries" \
"osnoise hist -E 10 -d 1s"
check "hist with -E/--entries out of range" \
- "osnoise hist -E 1 -d 1s" 1 "^Entries must be > 10 and < 10000000$"
+ "osnoise hist -E 1 -d 1s" 129 "out of range \[10, 9999999\]"
check "hist with --no-header" \
"osnoise hist --no-header -d 1s" 0 "" "RTLA osnoise histogram"
check "hist with --with-zeros" \
diff --git a/tools/tracing/rtla/tests/timerlat.t b/tools/tracing/rtla/tests/timerlat.t
index 8193048e8c8ce..57bada5b20631 100644
--- a/tools/tracing/rtla/tests/timerlat.t
+++ b/tools/tracing/rtla/tests/timerlat.t
@@ -61,7 +61,7 @@ check "hist with -b/--bucket-size" \
check "hist with -E/--entries" \
"timerlat hist -E 10 -d 1s"
check "hist with -E/--entries out of range" \
- "timerlat hist -E 1 -d 1s" 1 "^Entries must be > 10 and < 10000000$"
+ "timerlat hist -E 1 -d 1s" 129 "out of range \[10, 9999999\]"
check "hist with --no-header" \
"timerlat hist --no-header -d 1s" 0 "" "RTLA timerlat histogram"
check "hist with --with-zeros" \
diff --git a/tools/tracing/rtla/tests/unit/cli_opt_callback.c b/tools/tracing/rtla/tests/unit/cli_opt_callback.c
index 413a04f898fbb..8439fb5c6f0b3 100644
--- a/tools/tracing/rtla/tests/unit/cli_opt_callback.c
+++ b/tools/tracing/rtla/tests/unit/cli_opt_callback.c
@@ -9,6 +9,12 @@
#include "cli_params_assert.h"
#define TEST_CALLBACK(value, cb) OPT_CALLBACK('t', "test", value, "test value", "test help", cb)
+#define TEST_LLONG_RANGE(value, lo, hi) \
+ RTLA_OPT_CALLBACK_DATA('t', "test", value, "test value", "test help", \
+ opt_llong_callback, LLONG_RANGE(lo, hi))
+#define TEST_INT_RANGE(value, lo, hi) \
+ RTLA_OPT_CALLBACK_DATA('t', "test", value, "test value", "test help", \
+ opt_int_callback, INT_RANGE(lo, hi))
START_TEST(test_opt_llong_callback_simple)
{
@@ -137,6 +143,90 @@ START_TEST(test_opt_int_callback_unset_defval)
}
END_TEST
+START_TEST(test_opt_llong_callback_range_in)
+{
+ long long test_value = 0;
+ const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100);
+
+ ck_assert_int_eq(opt_llong_callback(&opt, "50", 0), 0);
+ ck_assert_int_eq(test_value, 50);
+}
+END_TEST
+
+START_TEST(test_opt_llong_callback_range_below)
+{
+ long long test_value = 0;
+ const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100);
+
+ assert(freopen("/dev/null", "w", stderr));
+ ck_assert_int_eq(opt_llong_callback(&opt, "9", 0), -1);
+}
+END_TEST
+
+START_TEST(test_opt_llong_callback_range_above)
+{
+ long long test_value = 0;
+ const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100);
+
+ assert(freopen("/dev/null", "w", stderr));
+ ck_assert_int_eq(opt_llong_callback(&opt, "101", 0), -1);
+}
+END_TEST
+
+START_TEST(test_opt_llong_callback_range_boundary)
+{
+ long long test_value = 0;
+ const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100);
+
+ ck_assert_int_eq(opt_llong_callback(&opt, "10", 0), 0);
+ ck_assert_int_eq(test_value, 10);
+ ck_assert_int_eq(opt_llong_callback(&opt, "100", 0), 0);
+ ck_assert_int_eq(test_value, 100);
+}
+END_TEST
+
+START_TEST(test_opt_int_callback_range_in)
+{
+ int test_value = 0;
+ const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000);
+
+ ck_assert_int_eq(opt_int_callback(&opt, "5000", 0), 0);
+ ck_assert_int_eq(test_value, 5000);
+}
+END_TEST
+
+START_TEST(test_opt_int_callback_range_below)
+{
+ int test_value = 0;
+ const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000);
+
+ assert(freopen("/dev/null", "w", stderr));
+ ck_assert_int_eq(opt_int_callback(&opt, "-1", 0), -1);
+}
+END_TEST
+
+START_TEST(test_opt_int_callback_range_above)
+{
+ int test_value = 0;
+ const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000);
+
+ assert(freopen("/dev/null", "w", stderr));
+ ck_assert_int_eq(opt_int_callback(&opt, "10001", 0), -1);
+}
+END_TEST
+
+START_TEST(test_opt_int_callback_range_boundary)
+{
+ int test_value = 0;
+ const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000);
+
+ ck_assert_int_eq(opt_int_callback(&opt, "0", 0), 0);
+ ck_assert_int_eq(test_value, 0);
+ ck_assert_int_eq(opt_int_callback(&opt, "10000", 0), 0);
+ ck_assert_int_eq(test_value, 10000);
+}
+END_TEST
+
START_TEST(test_opt_cpus_cb)
{
struct common_params params = {0};
@@ -388,67 +478,6 @@ START_TEST(test_opt_osnoise_auto_cb_unset)
}
END_TEST
-START_TEST(test_opt_osnoise_period_cb)
-{
- unsigned long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_osnoise_period_cb);
-
- ck_assert_int_eq(opt_osnoise_period_cb(&opt, "1000000", 0), 0);
- ck_assert_int_eq(period, 1000000);
-}
-END_TEST
-
-START_TEST(test_opt_osnoise_period_cb_invalid)
-{
- unsigned long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_osnoise_period_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_osnoise_period_cb(&opt, "10000001", 0);
-}
-END_TEST
-
-START_TEST(test_opt_osnoise_period_cb_unset)
-{
- unsigned long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_osnoise_period_cb);
-
- ck_assert_int_eq(opt_osnoise_period_cb(&opt, "1000000", 0), 0);
- ck_assert_int_eq(opt_osnoise_period_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(period, 0);
-}
-END_TEST
-
-START_TEST(test_opt_osnoise_runtime_cb)
-{
- unsigned long long runtime = 0;
- const struct option opt = TEST_CALLBACK(&runtime, opt_osnoise_runtime_cb);
-
- ck_assert_int_eq(opt_osnoise_runtime_cb(&opt, "900000", 0), 0);
- ck_assert_int_eq(runtime, 900000);
-}
-END_TEST
-
-START_TEST(test_opt_osnoise_runtime_cb_invalid)
-{
- unsigned long long runtime = 0;
- const struct option opt = TEST_CALLBACK(&runtime, opt_osnoise_runtime_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_osnoise_runtime_cb(&opt, "99", 0);
-}
-END_TEST
-
-START_TEST(test_opt_osnoise_runtime_cb_unset)
-{
- unsigned long long runtime = 0;
- const struct option opt = TEST_CALLBACK(&runtime, opt_osnoise_runtime_cb);
-
- ck_assert_int_eq(opt_osnoise_runtime_cb(&opt, "900000", 0), 0);
- ck_assert_int_eq(opt_osnoise_runtime_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(runtime, 0);
-}
-END_TEST
START_TEST(test_opt_osnoise_trace_output_cb)
{
@@ -525,37 +554,6 @@ START_TEST(test_opt_osnoise_on_end_cb_invalid)
}
END_TEST
-START_TEST(test_opt_timerlat_period_cb)
-{
- long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_timerlat_period_cb);
-
- ck_assert_int_eq(opt_timerlat_period_cb(&opt, "1000", 0), 0);
- ck_assert_int_eq(period, 1000);
-}
-END_TEST
-
-START_TEST(test_opt_timerlat_period_cb_invalid)
-{
- long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_timerlat_period_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_timerlat_period_cb(&opt, "1000001", 0);
-}
-END_TEST
-
-START_TEST(test_opt_timerlat_period_cb_unset)
-{
- long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_timerlat_period_cb);
-
- ck_assert_int_eq(opt_timerlat_period_cb(&opt, "1000", 0), 0);
- ck_assert_int_eq(opt_timerlat_period_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(period, 0);
-}
-END_TEST
-
START_TEST(test_opt_timerlat_auto_cb)
{
struct timerlat_params params = {0};
@@ -585,46 +583,6 @@ START_TEST(test_opt_timerlat_auto_cb_unset)
}
END_TEST
-START_TEST(test_opt_dma_latency_cb)
-{
- int dma_latency = 0;
- const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb);
-
- ck_assert_int_eq(opt_dma_latency_cb(&opt, "1000", 0), 0);
- ck_assert_int_eq(dma_latency, 1000);
-}
-END_TEST
-
-START_TEST(test_opt_dma_latency_cb_min)
-{
- int dma_latency = 0;
- const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_dma_latency_cb(&opt, "-1", 0);
-}
-END_TEST
-
-START_TEST(test_opt_dma_latency_cb_max)
-{
- int dma_latency = 0;
- const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_dma_latency_cb(&opt, "10001", 0);
-}
-END_TEST
-
-START_TEST(test_opt_dma_latency_cb_unset)
-{
- int dma_latency = 0;
- const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb);
-
- ck_assert_int_eq(opt_dma_latency_cb(&opt, "1000", 0), 0);
- ck_assert_int_eq(opt_dma_latency_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(dma_latency, default_dma_latency);
-}
-END_TEST
START_TEST(test_opt_aa_only_cb)
{
@@ -775,7 +733,8 @@ END_TEST
START_TEST(test_opt_timerlat_align_cb)
{
struct timerlat_params params = {0};
- const struct option opt = TEST_CALLBACK(¶ms, opt_timerlat_align_cb);
+ const struct option opt = RTLA_OPT_CALLBACK_DATA('A', "aligned", ¶ms, "us",
+ "test", opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX));
ck_assert_int_eq(opt_timerlat_align_cb(&opt, "500", 0), 0);
ck_assert(params.timerlat_align);
@@ -783,10 +742,22 @@ START_TEST(test_opt_timerlat_align_cb)
}
END_TEST
+START_TEST(test_opt_timerlat_align_cb_invalid)
+{
+ struct timerlat_params params = {0};
+ const struct option opt = RTLA_OPT_CALLBACK_DATA('A', "aligned", ¶ms, "us",
+ "test", opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX));
+
+ assert(freopen("/dev/null", "w", stderr));
+ ck_assert_int_eq(opt_timerlat_align_cb(&opt, "-1", 0), -1);
+}
+END_TEST
+
START_TEST(test_opt_timerlat_align_cb_unset)
{
struct timerlat_params params = {0};
- const struct option opt = TEST_CALLBACK(¶ms, opt_timerlat_align_cb);
+ const struct option opt = RTLA_OPT_CALLBACK_DATA('A', "aligned", ¶ms, "us",
+ "test", opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX));
ck_assert_int_eq(opt_timerlat_align_cb(&opt, "500", 0), 0);
ck_assert_int_eq(opt_timerlat_align_cb(&opt, NULL, 1), 0);
@@ -826,87 +797,6 @@ START_TEST(test_opt_stack_format_cb_unset)
}
END_TEST
-START_TEST(test_opt_bucket_size_cb)
-{
- int bucket_size = 0;
- const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb);
-
- ck_assert_int_eq(opt_bucket_size_cb(&opt, "100", 0), 0);
- ck_assert_int_eq(bucket_size, 100);
-}
-END_TEST
-
-START_TEST(test_opt_bucket_size_min)
-{
- int bucket_size = 0;
- const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_bucket_size_cb(&opt, "0", 0);
-}
-END_TEST
-
-START_TEST(test_opt_bucket_size_max)
-{
- int bucket_size = 0;
- const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_bucket_size_cb(&opt, "1000001", 0);
-}
-END_TEST
-
-START_TEST(test_opt_bucket_size_cb_unset)
-{
- int bucket_size = 0;
- const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb);
-
- ck_assert_int_eq(opt_bucket_size_cb(&opt, "100", 0), 0);
- ck_assert_int_eq(opt_bucket_size_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(bucket_size, default_bucket_size);
-}
-END_TEST
-
-START_TEST(test_opt_entries_cb)
-{
- int entries = 0;
- const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb);
-
- ck_assert_int_eq(opt_entries_cb(&opt, "100", 0), 0);
- ck_assert_int_eq(entries, 100);
-}
-END_TEST
-
-START_TEST(test_opt_entries_min)
-{
- int entries = 0;
- const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_entries_cb(&opt, "9", 0);
-}
-END_TEST
-
-START_TEST(test_opt_entries_max)
-{
- int entries = 0;
- const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_entries_cb(&opt, "10000000", 0);
-}
-END_TEST
-
-START_TEST(test_opt_entries_cb_unset)
-{
- int entries = 0;
- const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb);
-
- ck_assert_int_eq(opt_entries_cb(&opt, "100", 0), 0);
- ck_assert_int_eq(opt_entries_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(entries, default_entries);
-}
-END_TEST
Suite *cli_opt_callback_suite(void)
{
@@ -919,6 +809,10 @@ Suite *cli_opt_callback_suite(void)
tcase_add_test(tc, test_opt_llong_callback_min);
tcase_add_test(tc, test_opt_llong_callback_unset);
tcase_add_test(tc, test_opt_llong_callback_unset_defval);
+ tcase_add_test(tc, test_opt_llong_callback_range_in);
+ tcase_add_test(tc, test_opt_llong_callback_range_below);
+ tcase_add_test(tc, test_opt_llong_callback_range_above);
+ tcase_add_test(tc, test_opt_llong_callback_range_boundary);
tcase_add_test(tc, test_opt_int_callback_simple);
tcase_add_test(tc, test_opt_int_callback_max);
tcase_add_test(tc, test_opt_int_callback_min);
@@ -926,6 +820,10 @@ Suite *cli_opt_callback_suite(void)
tcase_add_test(tc, test_opt_int_callback_non_numeric_suffix);
tcase_add_test(tc, test_opt_int_callback_unset);
tcase_add_test(tc, test_opt_int_callback_unset_defval);
+ tcase_add_test(tc, test_opt_int_callback_range_in);
+ tcase_add_test(tc, test_opt_int_callback_range_below);
+ tcase_add_test(tc, test_opt_int_callback_range_above);
+ tcase_add_test(tc, test_opt_int_callback_range_boundary);
tcase_add_test(tc, test_opt_cpus_cb);
tcase_add_exit_test(tc, test_opt_cpus_cb_invalid, EXIT_FAILURE);
tcase_add_test(tc, test_opt_cgroup_cb);
@@ -951,12 +849,6 @@ Suite *cli_opt_callback_suite(void)
tc = tcase_create("osnoise");
tcase_add_test(tc, test_opt_osnoise_auto_cb);
tcase_add_test(tc, test_opt_osnoise_auto_cb_unset);
- tcase_add_test(tc, test_opt_osnoise_period_cb);
- tcase_add_test(tc, test_opt_osnoise_period_cb_unset);
- tcase_add_exit_test(tc, test_opt_osnoise_period_cb_invalid, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_osnoise_runtime_cb);
- tcase_add_exit_test(tc, test_opt_osnoise_runtime_cb_invalid, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_osnoise_runtime_cb_unset);
tcase_add_test(tc, test_opt_osnoise_trace_output_cb);
tcase_add_test(tc, test_opt_osnoise_trace_output_cb_noarg);
tcase_add_test(tc, test_opt_osnoise_trace_output_cb_unset);
@@ -967,15 +859,8 @@ Suite *cli_opt_callback_suite(void)
suite_add_tcase(s, tc);
tc = tcase_create("timerlat");
- tcase_add_test(tc, test_opt_timerlat_period_cb);
- tcase_add_exit_test(tc, test_opt_timerlat_period_cb_invalid, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_timerlat_period_cb_unset);
tcase_add_test(tc, test_opt_timerlat_auto_cb);
tcase_add_test(tc, test_opt_timerlat_auto_cb_unset);
- tcase_add_test(tc, test_opt_dma_latency_cb);
- tcase_add_exit_test(tc, test_opt_dma_latency_cb_min, EXIT_FAILURE);
- tcase_add_exit_test(tc, test_opt_dma_latency_cb_max, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_dma_latency_cb_unset);
tcase_add_test(tc, test_opt_aa_only_cb);
tcase_add_test(tc, test_opt_aa_only_cb_unset);
tcase_add_test(tc, test_opt_timerlat_trace_output_cb);
@@ -993,19 +878,9 @@ Suite *cli_opt_callback_suite(void)
tcase_add_exit_test(tc, test_opt_stack_format_cb_invalid, EXIT_FAILURE);
tcase_add_test(tc, test_opt_stack_format_cb_unset);
tcase_add_test(tc, test_opt_timerlat_align_cb);
+ tcase_add_test(tc, test_opt_timerlat_align_cb_invalid);
tcase_add_test(tc, test_opt_timerlat_align_cb_unset);
suite_add_tcase(s, tc);
- tc = tcase_create("histogram");
- tcase_add_test(tc, test_opt_bucket_size_cb);
- tcase_add_exit_test(tc, test_opt_bucket_size_min, EXIT_FAILURE);
- tcase_add_exit_test(tc, test_opt_bucket_size_max, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_bucket_size_cb_unset);
- tcase_add_test(tc, test_opt_entries_cb);
- tcase_add_exit_test(tc, test_opt_entries_min, EXIT_FAILURE);
- tcase_add_exit_test(tc, test_opt_entries_max, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_entries_cb_unset);
- suite_add_tcase(s, tc);
-
return s;
}
--
2.55.0
^ permalink raw reply related
* Re: [PATCH v5 2/2] serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver
From: Greg Kroah-Hartman @ 2026-07-10 12:41 UTC (permalink / raw)
To: Praveen Talari
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Jiri Slaby,
konrad.dybcio, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, mukesh.savaliya, aniket.randive, chandana.chiluveru
In-Reply-To: <20260615-add-tracepoints-for-qcom-geni-serial-v5-2-2efa4c97e0e2@oss.qualcomm.com>
On Mon, Jun 15, 2026 at 07:46:53PM +0530, Praveen Talari wrote:
> Add tracing to the Qualcomm GENI serial driver to improve runtime
> observability.
>
> Trace hooks are added at key points including termios and clock
> configuration, manual control get/set, interrupt handling, and data
> TX/RX paths.
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
> v2->v3:
> - Updated commit text(removed example as it was available on cover
> letter).
> ---
> drivers/tty/serial/qcom_geni_serial.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
Does not apply to my tty-next branch :(
^ permalink raw reply
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
From: Steven Rostedt @ 2026-07-10 12:37 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team,
linux-kernel, Sashiko
In-Reply-To: <alDd8OrrcI8vcPDL@google.com>
On Fri, 10 Jul 2026 12:56:32 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:
> > Ha yes this other one is real too. However this is in arch/arm64/kvm/ so I'll
> > post it separately to kvmarm.
>
> Posted here: https://lore.kernel.org/all/20260710114819.2689386-3-vdonnefort@google.com/
>
> Let me know if I should fold [1] into this series.
If they are separate, I'll pull this series in now and start testing it.
I just wanted to know if these would cause you to rethink this series.
-- Steve
^ permalink raw reply
* Re: [PATCH v2 1/2] module/kallsyms: fix nextval for data symbol lookup
From: Petr Pavlu @ 2026-07-10 12:36 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: linux-modules, Sami Tolvanen, Luis Chamberlain, linux-kernel,
linux-trace-kernel, live-patching, Daniel Gomez, Aaron Tomlin,
Steven Rostedt, Masami Hiramatsu, Jordan Rome, Viktor Malik
In-Reply-To: <20260327110005.16499-1-stf_xl@wp.pl>
On 3/27/26 12:00 PM, Stanislaw Gruszka wrote:
> The symbol lookup code assumes the queried address resides in either
> MOD_TEXT or MOD_INIT_TEXT. This breaks for addresses in other module
> memory regions (e.g. rodata or data), resulting in incorrect upper
> bounds and wrong symbol size.
>
> Select the module memory region the address belongs to instead of
> hardcoding text sections. Also initialize the lower bound to the start
> of that region, as searching from address 0 is unnecessary.
>
> Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
I've queued this first patch on modules-fixes.
--
Thanks,
Petr
^ permalink raw reply
* Re: [BUG] tracing: Too many tries to read user space
From: Steven Rostedt @ 2026-07-10 12:33 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Jeongho Choi, linux-trace-kernel, linux-kernel, ji2yoon.jo,
minki.jang, hajun.sung
In-Reply-To: <20260710122231.9bc9fae3dcfc72215f4a2dcd@kernel.org>
On Fri, 10 Jul 2026 12:22:31 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> Hm, in my view, this warning indicates that the circuit breaker has
> triggered correctly, so that is not a bug. Under the heavy memory
> pressure and low-memory situation, the page can be reclaimed soon
> after it is copied.
So you are saying that every time the copy_from_user() is executed, the
page is reclaimed? And this causes a schedule?
Now, I did have a version that used sched_switch and only incremented the
counter when a non-kernel thread was scheduled in. Then the test would
check if the counter increased by 2 or more. As an increase by 1 meant that
only kernel threads scheduled in which would not corrupt the buffer. The 1
increment was the current task scheduling back.
This is based on that work (I'm glad I save old versions in my git tree :-)
Funny, the comments were from the original change I did back in August of
2025, which mentions kernel threads scheduling in to handle the fault.
I also kept this around in case it was needed. Looks like it may be needed.
-- Steve
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 18710c190c92..19354fe2fca1 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -53,6 +53,8 @@
#include <linux/io.h> /* vmap_page_range() */
#include <linux/fs_context.h>
+#include <trace/events/sched.h>
+
#include <asm/setup.h> /* COMMAND_LINE_SIZE */
#include "trace.h"
@@ -5984,6 +5986,32 @@ struct trace_user_buf {
static DEFINE_MUTEX(trace_user_buffer_mutex);
static struct trace_user_buf_info *trace_user_buffer;
+static DEFINE_PER_CPU(unsigned long, sched_switch_cnt);
+
+/*
+ * The per CPU buffer trace_user_buffer is written to optimstically.
+ * The counter sched_switch_cnt is taken, preemption is enabled,
+ * the copying of the user space memory is placed into the trace_user_buffer,
+ * Preeption is re-enabled and the count is read again. If the count is greater
+ * than one from its previous reading, it means that another user space
+ * task scheduled in and the buffer is unreliable for use.
+ */
+static void
+probe_sched_switch(void *ignore, bool preempt,
+ struct task_struct *prev, struct task_struct *next,
+ unsigned int prev_state)
+{
+ /*
+ * The buffer can only be corrupted by another user space task.
+ * Ignore kernel tasks that may be scheduled in order to process
+ * the faulting memory.
+ */
+ if (!is_user_task(next))
+ return;
+
+ this_cpu_inc(sched_switch_cnt);
+}
+
/**
* trace_user_fault_destroy - free up allocated memory of a trace user buffer
* @tinfo: The descriptor to free up
@@ -6003,6 +6031,8 @@ void trace_user_fault_destroy(struct trace_user_buf_info *tinfo)
kfree(buf);
}
free_percpu(tinfo->tbuf);
+
+ unregister_trace_sched_switch(probe_sched_switch, NULL);
}
static int user_fault_buffer_enable(struct trace_user_buf_info *tinfo, size_t size)
@@ -6053,11 +6083,17 @@ static int user_buffer_init(struct trace_user_buf_info **tinfo, size_t size)
lockdep_assert_held(&trace_user_buffer_mutex);
+ ret = register_trace_sched_switch(probe_sched_switch, NULL);
+ if (ret < 0)
+ return ret;
+
if (!*tinfo) {
alloc = true;
*tinfo = kzalloc_obj(**tinfo);
- if (!*tinfo)
+ if (!*tinfo) {
+ unregister_trace_sched_switch(probe_sched_switch, NULL);
return -ENOMEM;
+ }
}
ret = user_fault_buffer_enable(*tinfo, size);
@@ -6241,7 +6277,7 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
return NULL;
/* Read the current CPU context switch counter */
- cnt = nr_context_switches_cpu(cpu);
+ cnt = this_cpu_read(sched_switch_cnt);
/*
* Preemption is going to be enabled, but this task must
@@ -6272,12 +6308,19 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
return NULL;
/*
- * Preemption is disabled again, now check the per CPU context
- * switch counter. If it doesn't match, then another user space
- * process may have schedule in and corrupted our buffer. In that
- * case the copying must be retried.
+ * Preemption is disabled again, now check the sched_switch_cnt.
+ * If it increased by two or more, then another user space process
+ * may have schedule in and corrupted our buffer. In that case
+ * the copying must be retried.
+ *
+ * Note, if this task was scheduled out and only kernel threads
+ * were scheduled in (maybe to process the fault), then the
+ * counter would increment again when this task scheduled in.
+ * If this task scheduled out and another user task scheduled
+ * in, this task would still need to be scheduled back in and
+ * the counter would increment by at least two.
*/
- } while (nr_context_switches_cpu(cpu) != cnt);
+ } while (this_cpu_read(sched_switch_cnt) > cnt + 1);
return buffer;
}
^ permalink raw reply related
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
From: Vincent Donnefort @ 2026-07-10 11:56 UTC (permalink / raw)
To: Steven Rostedt
Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team,
linux-kernel, Sashiko
In-Reply-To: <alCm5fq3UC2JtzZV@google.com>
On Fri, Jul 10, 2026 at 09:01:41AM +0100, Vincent Donnefort wrote:
> On Fri, Jul 10, 2026 at 08:58:51AM +0100, Vincent Donnefort wrote:
> > On Thu, Jul 09, 2026 at 07:15:16PM -0400, Steven Rostedt wrote:
> > > On Thu, 9 Jul 2026 17:00:15 +0100
> > > Vincent Donnefort <vdonnefort@google.com> wrote:
> > >
> > > > If page allocation fails in trace_remote_alloc_buffer(), desc->nr_cpus
> > > > is not yet incremented for the current CPU. As a consequence, on error,
> > > > half-allocated rb_desc will not be freed in trace_remote_free_buffer().
> > > >
> > > > Increment desc->nr_cpus as soon as the first allocation for the current
> > > > CPU has succeeded.
> > > >
> > > > Fixes: 96e43537af54 ("tracing: Introduce trace remotes")
> > > > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > > > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> > >
> > > This patch makes Sashiko find other possible issues with the code :-p
> > >
> > > https://sashiko.dev/#/patchset/20260709160017.1729517-2-vdonnefort%40google.com
> >
> > So one of them is fixed as part of "tracing/remotes: Add printk, dump_on_panic
> > and boot parameters" [1]. I can pull those fixes into this series instead.
> >
> > I'll look at the rest.
>
> Ha yes this other one is real too. However this is in arch/arm64/kvm/ so I'll
> post it separately to kvmarm.
Posted here: https://lore.kernel.org/all/20260710114819.2689386-3-vdonnefort@google.com/
Let me know if I should fold [1] into this series.
>
> >
> > [1] https://lore.kernel.org/all/20260605163825.1762953-2-vdonnefort@google.com/
> >
> > >
> > > -- Steve
^ permalink raw reply
* Re: [BUG] tracing: Too many tries to read user space
From: Steven Rostedt @ 2026-07-10 11:46 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Jeongho Choi, linux-trace-kernel, linux-kernel, ji2yoon.jo,
minki.jang, hajun.sung
In-Reply-To: <20260710122231.9bc9fae3dcfc72215f4a2dcd@kernel.org>
On Fri, 10 Jul 2026 12:22:31 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> However, this seems a bit strange that we only checks the CPU-wide context
> switching in the loop. Instead, can we introduce a per-cpu sequence counter
> to per-cpu buffer, and check it?
I originally tried this but found a situation that it fails:
tbuf->sequence = 0;
Task 1 Task 2
------ ------
tbuf->sequence++;
seq = tbuf->sequence; (seq = 1)
preempt_enable();
[schedule] ---------------------->
tbuf->sequence++;
seq = tbuf->sequence; (seq = 2);
preempt_enable();
copy_from_user(buffer);
<--------------------[schedule]
copy_from_user(buffer);
*** BUFFER NOW CORRUPTED ***
[schedule] ---------------------->
preempt_disable();
} while (tubf->sequence != seq); // tbuf->sequence == seq !!!!
This is why we use a CPU wide counter.
-- Steve
^ permalink raw reply
* Re: [RFC v3 2/2] arm64: kprobes: Allow reentering kprobes while single-stepping
From: Masami Hiramatsu @ 2026-07-10 9:38 UTC (permalink / raw)
To: Pu Hu
Cc: ada.coupriediaz@arm.com, catalin.marinas@arm.com,
davem@davemloft.net, Hongyan Xia, Jiazi Li,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
naveen@kernel.org, will@kernel.org, yang@os.amperecomputing.com
In-Reply-To: <20260710063242.228714-3-hupu@transsion.com>
On Fri, 10 Jul 2026 06:32:55 +0000
Pu Hu <hupu@transsion.com> wrote:
> From: Pu Hu <hupu@transsion.com>
>
> A kprobe can be hit while another kprobe is in KPROBE_HIT_SS state. This
> can happen when tracing or perf code runs from the debug exception path
> while the first kprobe is preparing or executing its out-of-line
> single-step instruction.
>
> Currently arm64 treats a kprobe hit in KPROBE_HIT_SS as unrecoverable,
> the same as a hit in KPROBE_REENTER. This is too strict. A hit in
> KPROBE_HIT_SS is still a one-level reentry and can be handled by saving
> the current kprobe state and setting up single-step for the new probe,
> just like reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
>
> The truly unrecoverable case is hitting another kprobe while already in
> KPROBE_REENTER, because the reentry save area has already been consumed.
>
> Move KPROBE_HIT_SS to the recoverable reentry cases and leave
> KPROBE_REENTER as the unrecoverable nested reentry case.
>
> This change also requires saving saved_irqflag in struct prev_kprobe.
> When a nested kprobe calls kprobes_save_local_irqflag(), it overwrites
> kcb->saved_irqflag with the currently masked DAIF value, losing the
> outer kprobe's original DAIF state. Without this fix, when the outer
> kprobe's single-step finishes, kprobes_restore_local_irqflag() applies
> the wrong DAIF mask and leaves interrupts permanently disabled.
>
> Extend struct prev_kprobe with a saved_irqflag field and save/restore it
> alongside kp and status. This ensures the outer kprobe's original
> interrupt state is preserved across reentry.
>
> This mirrors the x86 fix in commit 6a5022a56ac3
> ("kprobes/x86: Allow to handle reentered kprobe on single-stepping").
>
OK, this looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
for this series.
Will, Catalin, can you pick this series?
Thanks!
> Signed-off-by: Pu Hu <hupu@transsion.com>
> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
> ---
> arch/arm64/include/asm/kprobes.h | 6 ++++++
> arch/arm64/kernel/probes/kprobes.c | 23 ++++++++++++++++++++++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
> index f2782560647b..35ce2c94040e 100644
> --- a/arch/arm64/include/asm/kprobes.h
> +++ b/arch/arm64/include/asm/kprobes.h
> @@ -26,6 +26,12 @@
> struct prev_kprobe {
> struct kprobe *kp;
> unsigned int status;
> +
> + /*
> + * The original DAIF state of the outer kprobe, saved here before
> + * a nested kprobe overwrites kcb->saved_irqflag during reentry.
> + */
> + unsigned long saved_irqflag;
> };
>
> /* per-cpu kprobe control block */
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 798e4b091d1a..4e0efad5caf2 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -174,12 +174,27 @@ static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
> {
> kcb->prev_kprobe.kp = kprobe_running();
> kcb->prev_kprobe.status = kcb->kprobe_status;
> +
> + /*
> + * Save the outer kprobe's original DAIF flags before the nested
> + * kprobe calls kprobes_save_local_irqflag() and overwrites
> + * kcb->saved_irqflag. Without this, the outer kprobe will restore
> + * the wrong DAIF state and leave interrupts permanently masked.
> + */
> + kcb->prev_kprobe.saved_irqflag = kcb->saved_irqflag;
> }
>
> static void __kprobes restore_previous_kprobe(struct kprobe_ctlblk *kcb)
> {
> __this_cpu_write(current_kprobe, kcb->prev_kprobe.kp);
> kcb->kprobe_status = kcb->prev_kprobe.status;
> +
> + /*
> + * Restore the outer kprobe's saved_irqflag so that when its
> + * single-step completes, kprobes_restore_local_irqflag() uses
> + * the correct original DAIF value.
> + */
> + kcb->saved_irqflag = kcb->prev_kprobe.saved_irqflag;
> }
>
> static void __kprobes set_current_kprobe(struct kprobe *p)
> @@ -240,10 +255,16 @@ static int __kprobes reenter_kprobe(struct kprobe *p,
> switch (kcb->kprobe_status) {
> case KPROBE_HIT_SSDONE:
> case KPROBE_HIT_ACTIVE:
> + case KPROBE_HIT_SS:
> + /*
> + * A probe can be hit while another kprobe is preparing or
> + * executing its XOL single-step instruction. This is still a
> + * recoverable one-level reentry, so handle it in the same way as
> + * reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
> + */
> kprobes_inc_nmissed_count(p);
> setup_singlestep(p, regs, kcb, 1);
> break;
> - case KPROBE_HIT_SS:
> case KPROBE_REENTER:
> pr_warn("Failed to recover from reentered kprobes.\n");
> dump_kprobe(p);
> --
> 2.43.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test
From: Gabriele Monaco @ 2026-07-10 8:41 UTC (permalink / raw)
To: Nam Cao, linux-trace-kernel, linux-kernel
Cc: Thomas Weissschuh, Tomas Glozar, John Kacur, Wen Yang,
Steven Rostedt, Shuah Khan, linux-kselftest
In-Reply-To: <87zezzxt9a.fsf@yellow.woof>
On Fri, 2026-07-10 at 08:18 +0200, Nam Cao wrote:
> Gabriele Monaco <gmonaco@redhat.com> writes:
> > Simplify the test and run the steps expecting no reaction before the one
> > expecting reactions.
>
> This requirement is not clear from the code. I fear a new developer who
> wants to introduce a new test case will easily trip over this, or even
> us a few months from now.
>
> Instead of this, how about each test case discards or ignores all
> previous reactions?
Fair points, the issue is that we are hammering wwnr, on some systems we may
still see printks for long after the monitor was disabled, I tried that grep -m1
trick to mitigate that.
To ignore all previous reactions reliably we need to wait for the buffers to
flush, something like waiting for a full second for nothing new on the
ringbuffer perhaps. I could think of that or at least make this more clear with
comments.
Thanks,
Gabriele
^ permalink raw reply
* Re: [PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test
From: Gabriele Monaco @ 2026-07-10 8:31 UTC (permalink / raw)
To: Wen Yang, linux-trace-kernel, linux-kernel
Cc: Nam Cao, Thomas Weissschuh, Tomas Glozar, John Kacur,
Steven Rostedt, Shuah Khan, linux-kselftest
In-Reply-To: <cd0a678b-1eec-42d8-b027-0d27b9c804a3@linux.dev>
On Fri, 2026-07-10 at 01:08 +0800, Wen Yang wrote:
> > load() { # returns true if there was a reaction
> > - local lines_before num
> > + local lines_before num load_pid ret
> > num=$((($(nproc) + 1) / 2))
> > lines_before=$(dmesg | wc -l)
> > - stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q
> > - dmesg | tail -n $((lines_before + 1)) | grep -q "rv: monitor wwnr
> > does not allow event"
> > + stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q &
> > + load_pid=$!
> > + timeout 5 dmesg -w | tail -n +$((lines_before + 1)) | grep -m 1 -q
> > "rv: monitor wwnr does not allow event"
>
>
> Minor nit: could we add a small delay (e.g., sleep 0.1) before dmesg?
I don't get what the benefit would be.
The intent of that line is to run a continuous dmesg (-w) for up to 5s (timeout)
but stopping if an occurrence is found (grep -m 1 would close the pipe).
Do you see a case in which this wouldn't happen?
>
> Reviewed-by: Wen Yang <wen.yang@linux.dev>
>
> > + ret=$?
> > + kill "$load_pid"
> > + wait "$load_pid"
> > + return $ret
> > }
> >
> > echo 1 > monitors/wwnr/enable
> > echo printk > monitors/wwnr/reactors
> >
> > -load
> > -
> > echo 0 > monitoring_on
> > ! load || false
> > echo 1 > monitoring_on
> >
> > -load
> > -
> > echo 0 > reacting_on
> > ! load || false
> > echo 1 > reacting_on
> >
> > +load
> > +
> > echo nop > monitors/wwnr/reactors
> > echo 0 > monitors/wwnr/enable
^ permalink raw reply
* Re: [PATCH 3/3] rv/reactors: add KUnit tests for reactor_panic
From: Gabriele Monaco @ 2026-07-10 8:27 UTC (permalink / raw)
To: Wen Yang; +Cc: Nam Cao, linux-trace-kernel, linux-kernel
In-Reply-To: <18dcc24d-329e-43f4-bc69-e368c1f1d066@linux.dev>
On Fri, 2026-07-10 at 00:46 +0800, Wen Yang wrote:
> Since rv_panic_reaction calls vpanic() which is __noreturn, direct
> testing is not feasible in KUnit.
>
> A preparatory v2 drops reactor_printk_kunit.c and reactor_panic_kunit.c
> entirely, replacing them with a single rv_reactors_kunit.c containing
> two suites:
>
> rv_reactor_registration: register/unregister lifecycle, duplicate
> rejection (-EINVAL), name-too-long rejection, and safe unregister
> of a never-registered reactor.
>
> rv_react_dispatch: null-callback guard, callback invocation check,
> and the mdelay lockdep stress test.
>
>
> v2 also adds EXPORT_SYMBOL_GPL for rv_react(), rv_register_reactor(),
> and rv_unregister_reactor(), and the Kconfig entry is tristate.
Alright, sounds good.
> Additional fix: rv_unregister_reactor()
>
> Testing exposed a real bug: rv_unregister_reactor() called list_del()
> unconditionally. On a never-registered reactor the list_head is
> zero-initialised, so list_del() dereferences NULL->prev and crashes.
> v2 adds a patch that iterates rv_reactors_list first and only calls
> list_del() when the reactor is found. test_unregister_nonexistent
> documents and guards this behaviour.
Mmh, besides unit testing, when can we even see a reactor unregistration before
it's ever registered? Do we really need to actively guard against this?
What is probably an issue is to call unregistration also if registration failed
in reactors (which would cause the /bug/ you see, but shouldn't happen on
current reactors anyway), we can fix that.
I wouldn't guard against an issue that can only be triggered by broken code.
What am I missing?
Thanks,
Gabriele
^ permalink raw reply
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
From: Vincent Donnefort @ 2026-07-10 8:01 UTC (permalink / raw)
To: Steven Rostedt
Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team,
linux-kernel, Sashiko
In-Reply-To: <alCmOw8l4e-DS44v@google.com>
On Fri, Jul 10, 2026 at 08:58:51AM +0100, Vincent Donnefort wrote:
> On Thu, Jul 09, 2026 at 07:15:16PM -0400, Steven Rostedt wrote:
> > On Thu, 9 Jul 2026 17:00:15 +0100
> > Vincent Donnefort <vdonnefort@google.com> wrote:
> >
> > > If page allocation fails in trace_remote_alloc_buffer(), desc->nr_cpus
> > > is not yet incremented for the current CPU. As a consequence, on error,
> > > half-allocated rb_desc will not be freed in trace_remote_free_buffer().
> > >
> > > Increment desc->nr_cpus as soon as the first allocation for the current
> > > CPU has succeeded.
> > >
> > > Fixes: 96e43537af54 ("tracing: Introduce trace remotes")
> > > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> >
> > This patch makes Sashiko find other possible issues with the code :-p
> >
> > https://sashiko.dev/#/patchset/20260709160017.1729517-2-vdonnefort%40google.com
>
> So one of them is fixed as part of "tracing/remotes: Add printk, dump_on_panic
> and boot parameters" [1]. I can pull those fixes into this series instead.
>
> I'll look at the rest.
Ha yes this other one is real too. However this is in arch/arm64/kvm/ so I'll
post it separately to kvmarm.
>
> [1] https://lore.kernel.org/all/20260605163825.1762953-2-vdonnefort@google.com/
>
> >
> > -- Steve
^ permalink raw reply
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
From: Vincent Donnefort @ 2026-07-10 7:58 UTC (permalink / raw)
To: Steven Rostedt
Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team,
linux-kernel, Sashiko
In-Reply-To: <20260709191516.7a7ad6f6@gandalf.local.home>
On Thu, Jul 09, 2026 at 07:15:16PM -0400, Steven Rostedt wrote:
> On Thu, 9 Jul 2026 17:00:15 +0100
> Vincent Donnefort <vdonnefort@google.com> wrote:
>
> > If page allocation fails in trace_remote_alloc_buffer(), desc->nr_cpus
> > is not yet incremented for the current CPU. As a consequence, on error,
> > half-allocated rb_desc will not be freed in trace_remote_free_buffer().
> >
> > Increment desc->nr_cpus as soon as the first allocation for the current
> > CPU has succeeded.
> >
> > Fixes: 96e43537af54 ("tracing: Introduce trace remotes")
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
>
> This patch makes Sashiko find other possible issues with the code :-p
>
> https://sashiko.dev/#/patchset/20260709160017.1729517-2-vdonnefort%40google.com
So one of them is fixed as part of "tracing/remotes: Add printk, dump_on_panic
and boot parameters" [1]. I can pull those fixes into this series instead.
I'll look at the rest.
[1] https://lore.kernel.org/all/20260605163825.1762953-2-vdonnefort@google.com/
>
> -- Steve
^ permalink raw reply
* [RFC v3 2/2] arm64: kprobes: Allow reentering kprobes while single-stepping
From: Pu Hu @ 2026-07-10 6:32 UTC (permalink / raw)
To: mhiramat@kernel.org
Cc: ada.coupriediaz@arm.com, catalin.marinas@arm.com,
davem@davemloft.net, Hongyan Xia, Pu Hu, Jiazi Li,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
naveen@kernel.org, will@kernel.org, yang@os.amperecomputing.com
In-Reply-To: <20260710063242.228714-1-hupu@transsion.com>
From: Pu Hu <hupu@transsion.com>
A kprobe can be hit while another kprobe is in KPROBE_HIT_SS state. This
can happen when tracing or perf code runs from the debug exception path
while the first kprobe is preparing or executing its out-of-line
single-step instruction.
Currently arm64 treats a kprobe hit in KPROBE_HIT_SS as unrecoverable,
the same as a hit in KPROBE_REENTER. This is too strict. A hit in
KPROBE_HIT_SS is still a one-level reentry and can be handled by saving
the current kprobe state and setting up single-step for the new probe,
just like reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
The truly unrecoverable case is hitting another kprobe while already in
KPROBE_REENTER, because the reentry save area has already been consumed.
Move KPROBE_HIT_SS to the recoverable reentry cases and leave
KPROBE_REENTER as the unrecoverable nested reentry case.
This change also requires saving saved_irqflag in struct prev_kprobe.
When a nested kprobe calls kprobes_save_local_irqflag(), it overwrites
kcb->saved_irqflag with the currently masked DAIF value, losing the
outer kprobe's original DAIF state. Without this fix, when the outer
kprobe's single-step finishes, kprobes_restore_local_irqflag() applies
the wrong DAIF mask and leaves interrupts permanently disabled.
Extend struct prev_kprobe with a saved_irqflag field and save/restore it
alongside kp and status. This ensures the outer kprobe's original
interrupt state is preserved across reentry.
This mirrors the x86 fix in commit 6a5022a56ac3
("kprobes/x86: Allow to handle reentered kprobe on single-stepping").
Signed-off-by: Pu Hu <hupu@transsion.com>
Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
arch/arm64/include/asm/kprobes.h | 6 ++++++
arch/arm64/kernel/probes/kprobes.c | 23 ++++++++++++++++++++++-
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
index f2782560647b..35ce2c94040e 100644
--- a/arch/arm64/include/asm/kprobes.h
+++ b/arch/arm64/include/asm/kprobes.h
@@ -26,6 +26,12 @@
struct prev_kprobe {
struct kprobe *kp;
unsigned int status;
+
+ /*
+ * The original DAIF state of the outer kprobe, saved here before
+ * a nested kprobe overwrites kcb->saved_irqflag during reentry.
+ */
+ unsigned long saved_irqflag;
};
/* per-cpu kprobe control block */
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 798e4b091d1a..4e0efad5caf2 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -174,12 +174,27 @@ static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
{
kcb->prev_kprobe.kp = kprobe_running();
kcb->prev_kprobe.status = kcb->kprobe_status;
+
+ /*
+ * Save the outer kprobe's original DAIF flags before the nested
+ * kprobe calls kprobes_save_local_irqflag() and overwrites
+ * kcb->saved_irqflag. Without this, the outer kprobe will restore
+ * the wrong DAIF state and leave interrupts permanently masked.
+ */
+ kcb->prev_kprobe.saved_irqflag = kcb->saved_irqflag;
}
static void __kprobes restore_previous_kprobe(struct kprobe_ctlblk *kcb)
{
__this_cpu_write(current_kprobe, kcb->prev_kprobe.kp);
kcb->kprobe_status = kcb->prev_kprobe.status;
+
+ /*
+ * Restore the outer kprobe's saved_irqflag so that when its
+ * single-step completes, kprobes_restore_local_irqflag() uses
+ * the correct original DAIF value.
+ */
+ kcb->saved_irqflag = kcb->prev_kprobe.saved_irqflag;
}
static void __kprobes set_current_kprobe(struct kprobe *p)
@@ -240,10 +255,16 @@ static int __kprobes reenter_kprobe(struct kprobe *p,
switch (kcb->kprobe_status) {
case KPROBE_HIT_SSDONE:
case KPROBE_HIT_ACTIVE:
+ case KPROBE_HIT_SS:
+ /*
+ * A probe can be hit while another kprobe is preparing or
+ * executing its XOL single-step instruction. This is still a
+ * recoverable one-level reentry, so handle it in the same way as
+ * reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
+ */
kprobes_inc_nmissed_count(p);
setup_singlestep(p, regs, kcb, 1);
break;
- case KPROBE_HIT_SS:
case KPROBE_REENTER:
pr_warn("Failed to recover from reentered kprobes.\n");
dump_kprobe(p);
--
2.43.0
^ permalink raw reply related
* [RFC v3 1/2] arm64: kprobes: Only handle faults originating from XOL slot
From: Pu Hu @ 2026-07-10 6:32 UTC (permalink / raw)
To: mhiramat@kernel.org
Cc: ada.coupriediaz@arm.com, catalin.marinas@arm.com,
davem@davemloft.net, Hongyan Xia, Pu Hu, Jiazi Li,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
naveen@kernel.org, will@kernel.org, yang@os.amperecomputing.com
In-Reply-To: <20260710063242.228714-1-hupu@transsion.com>
From: Pu Hu <hupu@transsion.com>
kprobe_fault_handler() currently treats any page fault taken while in
KPROBE_HIT_SS or KPROBE_REENTER state as a kprobe single-step fault. This
assumption does not hold: perf or tracing code may run from the debug
exception path during the single-step window and take its own page fault.
When the fault is handled as a kprobe fault, the PC is rewritten to the
probe address, corrupting the exception recovery context for the real
fault. A typical reproducer is running perf with preemptirq tracepoints
and dwarf callchains while a kprobe is installed on a frequently
executed function.
Fix this in two layers:
1. At function entry, bail out immediately for simulated kprobes
(ainsn.xol_insn == NULL), since they have no XOL slot and any fault
taken during their execution cannot be a single-step fault.
2. For kprobes with an XOL slot, only handle the fault when the
faulting PC matches the XOL instruction address. Faults from any
other PC are left to the normal page fault handler.
This follows the same principle as the x86 fix in commit 6381c24cd6d5
("kprobes/x86: Fix page-fault handling logic").
Signed-off-by: Pu Hu <hupu@transsion.com>
Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
---
arch/arm64/kernel/probes/kprobes.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
index 43a0361a8bf0..798e4b091d1a 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -282,9 +282,31 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
struct kprobe *cur = kprobe_running();
struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+ /*
+ * Simulated kprobes execute in the debug trap context and have no
+ * XOL slot. Any page fault taken while a simulated kprobe is in
+ * progress cannot have been caused by kprobe single-stepping and
+ * must be left alone for the normal page fault handler, including
+ * fixup_exception.
+ */
+ if (cur && !cur->ainsn.xol_insn)
+ return 0;
+
switch (kcb->kprobe_status) {
case KPROBE_HIT_SS:
case KPROBE_REENTER:
+ /*
+ * A page fault taken while in KPROBE_HIT_SS or
+ * KPROBE_REENTER state is only attributable to kprobe
+ * single-stepping if the faulting PC points to the
+ * current kprobe's XOL instruction. If the fault occurred
+ * elsewhere (e.g. in perf or tracing code invoked from the
+ * debug exception path), leave it for the normal page fault
+ * handler to process.
+ */
+ if (instruction_pointer(regs) != (unsigned long)cur->ainsn.xol_insn)
+ break;
+
/*
* We are here because the instruction being single
* stepped caused a page fault. We reset the current
--
2.43.0
^ permalink raw reply related
* [RFC v3 0/2] rm64: kprobes: Fix single-step fault and reentry handling
From: Pu Hu @ 2026-07-10 6:32 UTC (permalink / raw)
To: mhiramat@kernel.org
Cc: ada.coupriediaz@arm.com, catalin.marinas@arm.com,
davem@davemloft.net, Hongyan Xia, Pu Hu, Jiazi Li,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
naveen@kernel.org, will@kernel.org, yang@os.amperecomputing.com
From: Pu Hu <hupu@transsion.com>
This series fixes two arm64 kprobes issues observed when running
simpleperf with preemptirq tracepoints and dwarf callchains while a
kprobe is active on a frequently executed kernel function.
The crash happens in the kprobe debug exception path. While a kprobe is
preparing or executing its XOL single-step instruction, perf/trace code
can run in the same window. That code may either take a fault of its own
or hit another kprobe.
Patch 1 fixes kprobe_fault_handler() so that it only handles a fault
taken in KPROBE_HIT_SS or KPROBE_REENTER state when the faulting PC
points at the current kprobe's XOL instruction. Simulated kprobes,
which have no XOL slot at all, are filtered out at function entry so
that the normal page fault handler (including fixup_exception) can
process any fault without interference.
Patch 2 allows a kprobe hit in KPROBE_HIT_SS to be handled as a
recoverable one-level reentry, instead of treating it as unrecoverable.
This is safe because the reentry save area has not yet been consumed at
that point. Only a hit while already in KPROBE_REENTER remains
unrecoverable. The same patch also extends struct prev_kprobe with a
saved_irqflag field so that the outer kprobe's original DAIF state is
preserved across reentry. Without this, the nested kprobe would
overwrite kcb->saved_irqflag, and the outer kprobe would restore the
wrong DAIF mask on completion, potentially leaving interrupts
permanently disabled.
This follows the same logic as the existing x86 fixes:
6381c24cd6d5 ("kprobes/x86: Fix page-fault handling logic")
6a5022a56ac3 ("kprobes/x86: Allow to handle reentered kprobe on single-stepping")
v2 -> v3:
- Patch 1: unchanged.
- Folded Patch 3 into Patch 2 so the saved_irqflag fix lands in the
same commit as the reentry change, keeping the series bisectable.
v1 -> v2:
- Patch 1: moved simulated kprobe check to function entry (per
maintainer review); removed redundant xol_insn NULL check from
the inner branch.
- Patch 2: unchanged.
- Patch 3: new in v2; fixes the IRQ flag save/restore gap that
Patch 2 exposes.
- Removed the selftest patch (old Patch 3) from this series.
- Fixed Signed-off-by to use full name (Pu Hu) instead of username
(hupu).
- Updated comments and commit messages across all patches.
Reproducer:
simpleperf record -p <pid> -f 10000 \
-e preemptirq:preempt_disable \
-e preemptirq:preempt_enable \
--duration 9 --call-graph dwarf \
-o /data/local/tmp/perf.data
Before this series, the crash reproduced frequently. With both patches
applied, it was no longer reproduced in our testing.
Pu Hu (2):
arm64: kprobes: Only handle faults originating from XOL slot
arm64: kprobes: Allow reentering kprobes while single-stepping
arch/arm64/include/asm/kprobes.h | 6 ++++
arch/arm64/kernel/probes/kprobes.c | 45 +++++++++++++++++++++++++++++-
2 files changed, 50 insertions(+), 1 deletion(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test
From: Nam Cao @ 2026-07-10 6:18 UTC (permalink / raw)
To: Gabriele Monaco, linux-trace-kernel, linux-kernel, Steven Rostedt,
Gabriele Monaco, Shuah Khan, linux-kselftest
Cc: Thomas Weissschuh, Tomas Glozar, John Kacur, Wen Yang
In-Reply-To: <20260625121440.116317-17-gmonaco@redhat.com>
Gabriele Monaco <gmonaco@redhat.com> writes:
> Simplify the test and run the steps expecting no reaction before the one
> expecting reactions.
This requirement is not clear from the code. I fear a new developer who
wants to introduce a new test case will easily trip over this, or even
us a few months from now.
Instead of this, how about each test case discards or ignores all
previous reactions?
Nam
^ permalink raw reply
* Re: [RFC v2 2/3] arm64: kprobes: Allow reentering kprobes while single-stepping
From: Pu Hu @ 2026-07-10 6:13 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: ada.coupriediaz@arm.com, catalin.marinas@arm.com,
davem@davemloft.net, Hongyan Xia, Jiazi Li,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
naveen@kernel.org, will@kernel.org, yang@os.amperecomputing.com
In-Reply-To: <20260710140936.8788416564452e490c71634c@kernel.org>
On 7/10/2026 1:09 PM, Masami Hiramatsu wrote:
> On Thu, 9 Jul 2026 14:22:25 +0000
> Pu Hu <hupu@transsion.com> wrote:
>
>> From: Pu Hu <hupu@transsion.com>
>>
>> A kprobe can be hit while another kprobe is in KPROBE_HIT_SS state. This
>> can happen when tracing or perf code runs from the debug exception path
>> while the first kprobe is preparing or executing its out-of-line
>> single-step instruction.
>>
>> Currently arm64 treats a kprobe hit in KPROBE_HIT_SS as unrecoverable,
>> the same as a hit in KPROBE_REENTER. This is too strict. A hit in
>> KPROBE_HIT_SS is still a one-level reentry and can be handled by saving
>> the current kprobe state and setting up single-step for the new probe,
>> just like reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
>>
>> The truly unrecoverable case is hitting another kprobe while already in
>> KPROBE_REENTER, because the reentry save area has already been consumed.
>>
>> Move KPROBE_HIT_SS to the recoverable reentry cases and leave
>> KPROBE_REENTER as the unrecoverable nested reentry case.
>>
>> This mirrors the x86 fix in commit 6a5022a56ac3
>> ("kprobes/x86: Allow to handle reentered kprobe on single-stepping").
>>
>
> Hi, as Sashiko commented, we have to save the saved_irqflag to
> prev_kprobbe.
>
> https://sashiko.dev/#/patchset/20260709142215.226872-1-hupu%40transsion.com?part=2
>
> Thank you,
>
Hi Masami,
We already added the saved_irqflag support in Patch 3 of this series:
arm64: kprobes: Save and restore saved_irqflag in prev_kprobe
However, I see the issue. Patch 2 and Patch 3 are separate commits,
so Patch 2 alone is not bisectable. I'll prepare a v3 that folds them
together, so the reentry logic and the saved_irqflag preservation land
in a single commit.
Thanks for pointing this out.
Thanks,
Pu Hu
^ permalink raw reply
* [PATCH v2 5/5] MAINTAINERS: add entries for ref_trace_final_put
From: Eugene Mavick via B4 Relay @ 2026-07-10 5:30 UTC (permalink / raw)
To: Will Deacon, Peter Zijlstra, Boqun Feng, Mark Rutland, Gary Guo,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andrew Morton, Dennis Zhou, Tejun Heo, Christoph Lameter
Cc: linux-kernel, linux-trace-kernel, linux-mm, Eugene Mavick
In-Reply-To: <20260710-refcount-final-put-trace-v2-0-557cfce860a2@mavick.dev>
From: Eugene Mavick <m@mavick.dev>
Add new files added in the patch series to MAINTAINERS
Signed-off-by: Eugene Mavick <m@mavick.dev>
---
v2:
This patch was added in v2
---
MAINTAINERS | 3 +++
1 file changed, 3 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 10e8253181d3..2dbb7441906e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4210,7 +4210,10 @@ S: Maintained
F: Documentation/atomic_*.txt
F: arch/*/include/asm/atomic*.h
F: include/*/atomic*.h
+F: include/linux/ref_trace.h
F: include/linux/refcount.h
+F: lib/ref_trace.c
+F: lib/tests/ref_trace_kunit.c
F: scripts/atomic/
F: rust/kernel/sync/atomic.rs
F: rust/kernel/sync/atomic/
--
2.51.2
^ permalink raw reply related
* [PATCH v2 4/5] kunit: add test for ref_trace_final_put
From: Eugene Mavick via B4 Relay @ 2026-07-10 5:30 UTC (permalink / raw)
To: Will Deacon, Peter Zijlstra, Boqun Feng, Mark Rutland, Gary Guo,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andrew Morton, Dennis Zhou, Tejun Heo, Christoph Lameter
Cc: linux-kernel, linux-trace-kernel, linux-mm, Eugene Mavick
In-Reply-To: <20260710-refcount-final-put-trace-v2-0-557cfce860a2@mavick.dev>
From: Eugene Mavick <m@mavick.dev>
Add a KUnit test suite for the ref_trace_final_put tracepoint.
The test registers a probe function and triggers both refcount_t and
percpu_ref final put paths, verifying that the tracepoint fires
correctly and that the recorded fields match expected values.
Signed-off-by: Eugene Mavick <m@mavick.dev>
---
v2:
-fix v1 bug where test gave consistent false pass
-make probe minimal, and only check obj and store the values
-add test_init macro to register and reset count
-move value checking to new test_exit macro
-move MAINTAINERS file entries to new MAINTAINERS commit
-v1:https://lore.kernel.org/all/20260705-refcount-final-put-trace-v1-4-cdd0014626a9@mavick.dev/
---
lib/Kconfig | 10 ++++
lib/tests/Makefile | 1 +
lib/tests/ref_trace_kunit.c | 136 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 147 insertions(+)
diff --git a/lib/Kconfig b/lib/Kconfig
index 00a9509636c1..c29b2ccb3c31 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -52,6 +52,16 @@ config PACKING_KUNIT_TEST
When in doubt, say N.
+config REF_TRACE_KUNIT_TEST
+ bool "ref_trace kunit test" if !KUNIT_ALL_TESTS
+ depends on KUNIT && FTRACE
+ default KUNIT_ALL_TESTS
+ help
+ This option enables the KUnit test suite for the ref_trace_final_put
+ tracepoint.
+
+ If unsure, say N
+
config BITREVERSE
tristate
diff --git a/lib/tests/Makefile b/lib/tests/Makefile
index 7e9c2fa52e35..828a030ad8c7 100644
--- a/lib/tests/Makefile
+++ b/lib/tests/Makefile
@@ -57,5 +57,6 @@ obj-$(CONFIG_USERCOPY_KUNIT_TEST) += usercopy_kunit.o
obj-$(CONFIG_UTIL_MACROS_KUNIT) += util_macros_kunit.o
obj-$(CONFIG_RATELIMIT_KUNIT_TEST) += test_ratelimit.o
obj-$(CONFIG_UUID_KUNIT_TEST) += uuid_kunit.o
+obj-$(CONFIG_REF_TRACE_KUNIT_TEST) += ref_trace_kunit.o
obj-$(CONFIG_TEST_RUNTIME_MODULE) += module/
diff --git a/lib/tests/ref_trace_kunit.c b/lib/tests/ref_trace_kunit.c
new file mode 100644
index 000000000000..5c7e9d29a030
--- /dev/null
+++ b/lib/tests/ref_trace_kunit.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/kernel.h>
+#include <kunit/test.h>
+#include <linux/compiler_attributes.h>
+#include <linux/wait_bit.h>
+#include <linux/instruction_pointer.h>
+#include <linux/kallsyms.h>
+#include <linux/percpu-refcount.h>
+#include <linux/refcount.h>
+#include <linux/types.h>
+#include <linux/atomic.h>
+#include <trace/events/ref_trace.h>
+
+struct data {
+ unsigned long caller;
+ const char *fn;
+ const void *obj;
+ atomic_t count;
+};
+
+struct data capture;
+
+const void *chk_obj;
+
+#define test_init() \
+ do { \
+ KUNIT_EXPECT_FALSE( \
+ test, register_trace_ref_trace_final_put(probe, NULL)); \
+ \
+ atomic_set_release(&capture.count, 0); \
+ \
+ chk_obj = &obj; \
+ } while (0)
+
+
+#define test_exit(func_name) \
+ do { \
+ /* wait for probe completion */ \
+ wait_var_event( \
+ &capture.count, \
+ atomic_read_acquire(&capture.count) != 0); \
+ \
+ unregister_trace_ref_trace_final_put(probe, NULL); \
+ \
+ KUNIT_EXPECT_EQ(test, atomic_read_acquire(&capture.count), 1); \
+ /* \
+ * caller testing may be flaky \
+ * due to compile optimisations so omit \
+ */ \
+ KUNIT_EXPECT_STREQ(test, capture.fn, #func_name); \
+ KUNIT_EXPECT_PTR_EQ(test, capture.obj, &obj); \
+ } while (0)
+
+static void probe(
+ void *ignore,
+ unsigned long caller,
+ const char *fn,
+ const void *obj)
+{
+ //prevent non test func final_puts from changing captured values
+ if (chk_obj != obj)
+ return;
+
+ capture.caller = caller;
+ capture.fn = fn;
+ capture.obj = obj;
+
+ atomic_inc_return_release(&capture.count); //increase count
+}
+
+static noinline void test_refcount_sub_and_test(struct kunit *test)
+{
+ refcount_t obj;
+
+ test_init();
+ refcount_set(&obj, 2);
+
+ KUNIT_EXPECT_FALSE(test, refcount_dec_and_test(&obj));
+ KUNIT_EXPECT_TRUE(test, refcount_dec_and_test(&obj));
+
+ test_exit(__refcount_sub_and_test);
+}
+
+static __always_inline void test_refcount_dec_if_one(struct kunit *test)
+{
+ refcount_t obj;
+
+ test_init();
+ refcount_set(&obj, 2);
+
+ KUNIT_EXPECT_FALSE(test, refcount_dec_and_test(&obj));
+ KUNIT_EXPECT_TRUE(test, refcount_dec_if_one(&obj));
+
+ test_exit(refcount_dec_if_one);
+}
+static void dummy_release(struct percpu_ref *ref) {}
+
+static noinline void test_percpu_ref_put_many(struct kunit *test)
+{
+ struct percpu_ref obj;
+
+ test_init();
+
+ KUNIT_EXPECT_FALSE(test, percpu_ref_init(&obj, dummy_release, 0, GFP_KERNEL));
+
+ percpu_ref_get(&obj);
+ percpu_ref_get(&obj);
+
+ percpu_ref_put(&obj);
+ percpu_ref_put(&obj);
+
+ percpu_ref_switch_to_atomic_sync(&obj);
+
+ percpu_ref_put(&obj);
+
+ test_exit(percpu_ref_put_many);
+ percpu_ref_exit(&obj);
+}
+
+static struct kunit_case __refdata ref_trace_test_cases[] = {
+ KUNIT_CASE(test_refcount_sub_and_test),
+ KUNIT_CASE(test_refcount_dec_if_one),
+ KUNIT_CASE(test_percpu_ref_put_many),
+ {}
+};
+
+static struct kunit_suite ref_trace_test_suite = {
+ .name = "ref-trace",
+ .test_cases = ref_trace_test_cases
+};
+
+kunit_test_suites(&ref_trace_test_suite);
+
+MODULE_AUTHOR("Eugene Mavick <m@mavick.dev>");
+MODULE_DESCRIPTION("KUnit test for ref_trace");
+MODULE_LICENSE("GPL");
--
2.51.2
^ permalink raw reply related
* [PATCH v2 3/5] percpu-refcount: add ref_trace_final_put trace
From: Eugene Mavick via B4 Relay @ 2026-07-10 5:30 UTC (permalink / raw)
To: Will Deacon, Peter Zijlstra, Boqun Feng, Mark Rutland, Gary Guo,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andrew Morton, Dennis Zhou, Tejun Heo, Christoph Lameter
Cc: linux-kernel, linux-trace-kernel, linux-mm, Eugene Mavick
In-Reply-To: <20260710-refcount-final-put-trace-v2-0-557cfce860a2@mavick.dev>
From: Eugene Mavick <m@mavick.dev>
Add the ref_trace_final_put tracepoint to percpu_ref_put_many().
The tracepoint fires when the atomic counter reaches zero in the
atomic fallback path (after percpu_ref_kill() has been called).
Signed-off-by: Eugene Mavick <m@mavick.dev>
---
include/linux/percpu-refcount.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/percpu-refcount.h b/include/linux/percpu-refcount.h
index d73a1c08c3e3..f870ac0e8c06 100644
--- a/include/linux/percpu-refcount.h
+++ b/include/linux/percpu-refcount.h
@@ -55,6 +55,7 @@
#include <linux/rcupdate.h>
#include <linux/types.h>
#include <linux/gfp.h>
+#include <linux/ref_trace.h>
struct percpu_ref;
typedef void (percpu_ref_func_t)(struct percpu_ref *);
@@ -331,8 +332,10 @@ static inline void percpu_ref_put_many(struct percpu_ref *ref, unsigned long nr)
if (__ref_is_percpu(ref, &percpu_count))
this_cpu_sub(*percpu_count, nr);
- else if (unlikely(atomic_long_sub_and_test(nr, &ref->data->count)))
+ else if (unlikely(atomic_long_sub_and_test(nr, &ref->data->count))) {
+ do_trace_ref_final_put(ref);
ref->data->release(ref);
+ }
rcu_read_unlock();
}
--
2.51.2
^ permalink raw reply related
* [PATCH v2 2/5] refcount: add ref_trace_final_put tracepoint
From: Eugene Mavick via B4 Relay @ 2026-07-10 5:30 UTC (permalink / raw)
To: Will Deacon, Peter Zijlstra, Boqun Feng, Mark Rutland, Gary Guo,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andrew Morton, Dennis Zhou, Tejun Heo, Christoph Lameter
Cc: linux-kernel, linux-trace-kernel, linux-mm, Eugene Mavick
In-Reply-To: <20260710-refcount-final-put-trace-v2-0-557cfce860a2@mavick.dev>
From: Eugene Mavick <m@mavick.dev>
Add the ref_trace_final_put tracepoint to __refcount_sub_and_test() and
refcount_dec_if_one()
This tracepoint fires when a refcount_t reaches zero, capturing the caller
address, the function name, and the refcount_t address.
Signed-off-by: Eugene Mavick <m@mavick.dev>
---
v2:
-update commit message
-add tracepoint to refcount_dec_if_one(), suggested by Usuma Arif
-move MAINTAINERS file entries to new MAINTAINERS commit
-v1:https://lore.kernel.org/all/20260705-refcount-final-put-trace-v1-2-cdd0014626a9@mavick.dev/
---
include/linux/refcount.h | 2 ++
lib/refcount.c | 8 +++++++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/linux/refcount.h b/include/linux/refcount.h
index ba7657ced281..70d07a462da1 100644
--- a/include/linux/refcount.h
+++ b/include/linux/refcount.h
@@ -107,6 +107,7 @@
#include <linux/limits.h>
#include <linux/refcount_types.h>
#include <linux/spinlock_types.h>
+#include <linux/ref_trace.h>
struct mutex;
@@ -393,6 +394,7 @@ bool __refcount_sub_and_test(int i, refcount_t *r, int *oldp)
if (old > 0 && old == i) {
smp_acquire__after_ctrl_dep();
+ do_trace_ref_final_put(r);
return true;
}
diff --git a/lib/refcount.c b/lib/refcount.c
index a207a8f22b3c..cd7e32df3919 100644
--- a/lib/refcount.c
+++ b/lib/refcount.c
@@ -7,6 +7,7 @@
#include <linux/refcount.h>
#include <linux/spinlock.h>
#include <linux/bug.h>
+#include <linux/ref_trace.h>
#define REFCOUNT_WARN(str) WARN_ONCE(1, "refcount_t: " str ".\n")
@@ -56,7 +57,12 @@ bool refcount_dec_if_one(refcount_t *r)
{
int val = 1;
- return atomic_try_cmpxchg_release(&r->refs, &val, 0);
+ bool ret = atomic_try_cmpxchg_release(&r->refs, &val, 0);
+
+ if (ret)
+ do_trace_ref_final_put(r);
+
+ return ret;
}
EXPORT_SYMBOL(refcount_dec_if_one);
--
2.51.2
^ permalink raw reply related
* [PATCH v2 1/5] tracing: add ref_trace_final_put tracepoint
From: Eugene Mavick via B4 Relay @ 2026-07-10 5:30 UTC (permalink / raw)
To: Will Deacon, Peter Zijlstra, Boqun Feng, Mark Rutland, Gary Guo,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andrew Morton, Dennis Zhou, Tejun Heo, Christoph Lameter
Cc: linux-kernel, linux-trace-kernel, linux-mm, Eugene Mavick
In-Reply-To: <20260710-refcount-final-put-trace-v2-0-557cfce860a2@mavick.dev>
From: Eugene Mavick <m@mavick.dev>
Add ref_trace_final_put tracepoint and related core infrastructure
ref_trace_final_put fires when a reference
count reaches zero and the object enters its final release path.
The tracepoint records three fields:
- caller: function that called the refcounting
function(refcount_sub_and_test, percpu_ref_put_many)
- fn: refcounting function(eg refcount_sub_and_test)
- obj: refcount object(struct percpu_ref, refcount_t)
Signed-off-by: Eugene Mavick <m@mavick.dev>
---
v2:
-change trace_ref_final_put macro name to do_trace_ref_final_put to
avoid overlap with tracepoint names(trace_*)
-change, in above mentioned macro, from trace_ref_trace_final_put( to
trace_call__ref_trace_final_put( to avoid double check
-changes suggested by Steven Rostedt
-v1:https://lore.kernel.org/all/20260705-refcount-final-put-trace-v1-1-cdd0014626a9@mavick.dev/
---
include/linux/ref_trace.h | 26 +++++++++++++++++++++++
include/trace/events/ref_trace.h | 46 ++++++++++++++++++++++++++++++++++++++++
lib/Makefile | 2 ++
lib/ref_trace.c | 12 +++++++++++
4 files changed, 86 insertions(+)
diff --git a/include/linux/ref_trace.h b/include/linux/ref_trace.h
new file mode 100644
index 000000000000..6c6600938a47
--- /dev/null
+++ b/include/linux/ref_trace.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_REF_TRACE_H
+#define _LINUX_REF_TRACE_H
+
+#include <linux/tracepoint-defs.h>
+#include <linux/instruction_pointer.h>
+
+/* Declare the tracepoint so tracepoint_enabled() can be used */
+DECLARE_TRACEPOINT(ref_trace_final_put);
+
+#ifdef CONFIG_TRACEPOINTS
+/* Wrapper function implemented in lib/ref_trace.c */
+extern void do_ref_trace_final_put(unsigned long caller, const char *fn, const void *obj);
+
+#define do_trace_ref_final_put(obj) \
+ do { \
+ if (tracepoint_enabled(ref_trace_final_put)) \
+ do_ref_trace_final_put(_RET_IP_, __func__, obj); \
+ } while (0)
+
+#else /* !CONFIG_TRACEPOINTS */
+static inline void do_ref_trace_final_put(unsigned long caller, const char *fn, const void *obj) { }
+#define do_trace_ref_final_put(obj) do { } while (0)
+#endif
+
+#endif /* _LINUX_REF_TRACE_H */
diff --git a/include/trace/events/ref_trace.h b/include/trace/events/ref_trace.h
new file mode 100644
index 000000000000..e6037a325be2
--- /dev/null
+++ b/include/trace/events/ref_trace.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM ref_trace
+
+#if !defined(_TRACE_REF_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_REF_TRACE_H
+
+#include <linux/tracepoint.h>
+
+/**
+ * ref_trace_final_put - trace when a reference count reaches zero
+ * @caller: function that called the refcounting
+ * function(refcount_sub_and_test, percpu_ref_put_many)
+ * @fn: refcounting function(eg refcount_sub_and_test)
+ * @obj: refcount object(struct percpu_ref, refcount_t)
+ *
+ * Tracepoint instrumentation can be added using the ref_trace_final_put
+ * macro defined in include/linux/ref_trace.h
+ * which uses _RET_IP_ and __func__ for caller and fn arguments respectively,
+ * thus only requiring obj arg to be supplied
+ */
+TRACE_EVENT(ref_trace_final_put,
+
+ TP_PROTO(unsigned long caller, const char *fn, const void *obj),
+
+ TP_ARGS(caller, fn, obj),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, caller)
+ __string(fn, fn)
+ __field(const void *, obj)
+ ),
+
+ TP_fast_assign(
+ __entry->caller = caller;
+ __assign_str(fn);
+ __entry->obj = obj;
+ ),
+
+ TP_printk("caller=%pS fn=%s obj=%p", (void *)__entry->caller, __get_str(fn), __entry->obj)
+);
+
+#endif /* _TRACE_REF_TRACE_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/lib/Makefile b/lib/Makefile
index f33a24bf1c19..41737090a95d 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -335,3 +335,5 @@ CONTEXT_ANALYSIS_test_context-analysis.o := y
obj-$(CONFIG_CONTEXT_ANALYSIS_TEST) += test_context-analysis.o
subdir-$(CONFIG_FORTIFY_SOURCE) += test_fortify
+
+obj-$(CONFIG_TRACEPOINTS) += ref_trace.o
diff --git a/lib/ref_trace.c b/lib/ref_trace.c
new file mode 100644
index 000000000000..9102dc7117db
--- /dev/null
+++ b/lib/ref_trace.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+#define CREATE_TRACE_POINTS
+#include <trace/events/ref_trace.h>
+
+//Wrapper function for functions defined entirely in header files
+void do_ref_trace_final_put(unsigned long caller, const char *fn, const void *obj)
+{
+ trace_call__ref_trace_final_put(caller, fn, obj);
+}
+EXPORT_SYMBOL_GPL(do_ref_trace_final_put);
+
+EXPORT_TRACEPOINT_SYMBOL_GPL(ref_trace_final_put);
--
2.51.2
^ permalink raw reply related
* [PATCH v2 0/5] tracing: add ref_trace_final_put tracing
From: Eugene Mavick via B4 Relay @ 2026-07-10 5:30 UTC (permalink / raw)
To: Will Deacon, Peter Zijlstra, Boqun Feng, Mark Rutland, Gary Guo,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andrew Morton, Dennis Zhou, Tejun Heo, Christoph Lameter
Cc: linux-kernel, linux-trace-kernel, linux-mm, Eugene Mavick
When debugging use-after-free(UAF) bugs, knowing when the object reaches
0 references and enters final release can significantly aid the
debugging process.
There is currently no universal way to trace this information.
This patch series implements tracing of the final puts in the
most widely used refcounting implementations,
refcount_t(and thus kref which uses it), and percpu-ref.
The tracepoint records three fields:
- caller: function that called the refcounting
function(refcount_sub_and_test, percpu_ref_put_many)
- fn: refcounting function(eg refcount_sub_and_test)
- obj: refcount object(struct percpu_ref, refcount_t)
Signed-off-by: Eugene Mavick <m@mavick.dev>
---
Changes in v2:
-include/linux/ref_trace.h: change macro name, use direct tracepoint
call in macro to avoid double check
-add tracepoint to refcount_dec_if_one
-kunit: make significant improvements to design, fix critical bug, add test case for
refcount_dec_if_one()
-Link to v1: https://lore.kernel.org/r/20260705-refcount-final-put-trace-v1-0-0ae936edb750@mavick.dev
---
Eugene Mavick (5):
tracing: add ref_trace_final_put tracepoint
refcount: add ref_trace_final_put tracepoint
percpu-refcount: add ref_trace_final_put trace
kunit: add test for ref_trace_final_put
MAINTAINERS: add entries for ref_trace_final_put
MAINTAINERS | 3 +
include/linux/percpu-refcount.h | 5 +-
include/linux/ref_trace.h | 26 ++++++++
include/linux/refcount.h | 2 +
include/trace/events/ref_trace.h | 46 +++++++++++++
lib/Kconfig | 10 +++
lib/Makefile | 2 +
lib/ref_trace.c | 12 ++++
lib/refcount.c | 8 ++-
lib/tests/Makefile | 1 +
lib/tests/ref_trace_kunit.c | 136 +++++++++++++++++++++++++++++++++++++++
11 files changed, 249 insertions(+), 2 deletions(-)
---
base-commit: df685633c3dbc67441cc86f1c3fee58de4652ba2
change-id: 20260624-refcount-final-put-trace-49bd7c39bd5a
Best regards,
--
Eugene Mavick <m@mavick.dev>
^ permalink raw reply
* Re: [RFC v2 2/3] arm64: kprobes: Allow reentering kprobes while single-stepping
From: Masami Hiramatsu @ 2026-07-10 5:09 UTC (permalink / raw)
To: Pu Hu
Cc: ada.coupriediaz@arm.com, catalin.marinas@arm.com,
davem@davemloft.net, Hongyan Xia, Jiazi Li,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
naveen@kernel.org, will@kernel.org, yang@os.amperecomputing.com
In-Reply-To: <20260709142215.226872-3-hupu@transsion.com>
On Thu, 9 Jul 2026 14:22:25 +0000
Pu Hu <hupu@transsion.com> wrote:
> From: Pu Hu <hupu@transsion.com>
>
> A kprobe can be hit while another kprobe is in KPROBE_HIT_SS state. This
> can happen when tracing or perf code runs from the debug exception path
> while the first kprobe is preparing or executing its out-of-line
> single-step instruction.
>
> Currently arm64 treats a kprobe hit in KPROBE_HIT_SS as unrecoverable,
> the same as a hit in KPROBE_REENTER. This is too strict. A hit in
> KPROBE_HIT_SS is still a one-level reentry and can be handled by saving
> the current kprobe state and setting up single-step for the new probe,
> just like reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
>
> The truly unrecoverable case is hitting another kprobe while already in
> KPROBE_REENTER, because the reentry save area has already been consumed.
>
> Move KPROBE_HIT_SS to the recoverable reentry cases and leave
> KPROBE_REENTER as the unrecoverable nested reentry case.
>
> This mirrors the x86 fix in commit 6a5022a56ac3
> ("kprobes/x86: Allow to handle reentered kprobe on single-stepping").
>
Hi, as Sashiko commented, we have to save the saved_irqflag to
prev_kprobbe.
https://sashiko.dev/#/patchset/20260709142215.226872-1-hupu%40transsion.com?part=2
Thank you,
> Signed-off-by: Pu Hu <hupu@transsion.com>
> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
> ---
> arch/arm64/kernel/probes/kprobes.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 798e4b091d1a..2ca5916eca2f 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -240,10 +240,16 @@ static int __kprobes reenter_kprobe(struct kprobe *p,
> switch (kcb->kprobe_status) {
> case KPROBE_HIT_SSDONE:
> case KPROBE_HIT_ACTIVE:
> + case KPROBE_HIT_SS:
> + /*
> + * A probe can be hit while another kprobe is preparing or
> + * executing its XOL single-step instruction. This is still a
> + * recoverable one-level reentry, so handle it in the same way as
> + * reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
> + */
> kprobes_inc_nmissed_count(p);
> setup_singlestep(p, regs, kcb, 1);
> break;
> - case KPROBE_HIT_SS:
> case KPROBE_REENTER:
> pr_warn("Failed to recover from reentered kprobes.\n");
> dump_kprobe(p);
> --
> 2.43.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ 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