* [PATCH 2/2] selftests/user_events: wait for deferred event teardown after unregister
From: Michael Bommarito @ 2026-07-07 16:59 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: Beau Belgrave, XIAO WU, linux-trace-kernel, linux-kernel, stable
In-Reply-To: <20260707165912.2560537-1-michael.bommarito@gmail.com>
Unregistering a user event now defers the drop of the enabler's event
reference (and the freeing of the enabler) past an RCU grace period. As a
result DIAG_IOCSDEL can transiently fail with -EBUSY while that last
reference is still being dropped, where it previously succeeded
immediately.
Two tests assumed the delete takes effect the instant the unregister
returns:
- abi_test "flags" deletes the event right after disabling it.
- perf_test's fixture teardown clear() deletes __test_event before the
next test registers the same name; a stale event makes the following
registration fail with -EADDRINUSE.
Retry the delete until it succeeds (or the event is already gone) with a
bounded wait, matching the existing wait_for_delete() idiom in the same
suite, so the tests are robust to the deferred teardown.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
.../testing/selftests/user_events/abi_test.c | 24 ++++++++++++++++-
.../testing/selftests/user_events/perf_test.c | 26 ++++++++++++++++---
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/user_events/abi_test.c b/tools/testing/selftests/user_events/abi_test.c
index 85892b3b719cc..9e2f84d281afc 100644
--- a/tools/testing/selftests/user_events/abi_test.c
+++ b/tools/testing/selftests/user_events/abi_test.c
@@ -132,6 +132,28 @@ static int event_delete(void)
return ret;
}
+/*
+ * Deleting an event drops its last reference, but an unregister may defer
+ * that put (and the freeing of the associated enabler) past an RCU grace
+ * period. The delete can therefore transiently fail with -EBUSY while the
+ * previous reference is still being dropped. Retry for up to ~10 seconds.
+ */
+static int wait_for_event_delete(void)
+{
+ int i, ret;
+
+ for (i = 0; i < 10000; ++i) {
+ ret = event_delete();
+
+ if (ret == 0)
+ return 0;
+
+ usleep(1000);
+ }
+
+ return ret;
+}
+
static int reg_enable_multi(void *enable, int size, int bit, int flags,
char *args)
{
@@ -262,7 +284,7 @@ TEST_F(user, flags) {
ASSERT_TRUE(event_exists());
/* Ensure we can delete it */
- ASSERT_EQ(0, event_delete());
+ ASSERT_EQ(0, wait_for_event_delete());
/* USER_EVENT_REG_MAX or above is not allowed */
ASSERT_EQ(-1, reg_enable_flags(&self->check, sizeof(int), 0,
diff --git a/tools/testing/selftests/user_events/perf_test.c b/tools/testing/selftests/user_events/perf_test.c
index cafec0e52eb31..5727cb5b914cf 100644
--- a/tools/testing/selftests/user_events/perf_test.c
+++ b/tools/testing/selftests/user_events/perf_test.c
@@ -85,6 +85,7 @@ static int get_offset(void)
static int clear(int *check)
{
struct user_unreg unreg = {0};
+ int i, ret;
unreg.size = sizeof(unreg);
unreg.disable_bit = 31;
@@ -99,13 +100,32 @@ static int clear(int *check)
if (errno != ENOENT)
return -1;
- if (ioctl(fd, DIAG_IOCSDEL, "__test_event") == -1)
- if (errno != ENOENT)
+ /*
+ * Deleting the event drops its last reference, but the unregister
+ * above defers that put (and the freeing of the enabler) past an RCU
+ * grace period. The delete can therefore transiently fail with -EBUSY
+ * until that reference is dropped. Retry for up to ~10 seconds so the
+ * event is actually gone before the next test registers the same name.
+ */
+ for (i = 0; i < 10000; ++i) {
+ ret = ioctl(fd, DIAG_IOCSDEL, "__test_event");
+
+ if (ret == 0 || errno == ENOENT) {
+ ret = 0;
+ break;
+ }
+
+ if (errno != EBUSY) {
+ close(fd);
return -1;
+ }
+
+ usleep(1000);
+ }
close(fd);
- return 0;
+ return ret;
}
FIXTURE(user) {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH 2/2] selftests/user_events: wait for deferred event teardown after unregister
From: Steven Rostedt @ 2026-07-07 17:41 UTC (permalink / raw)
To: Michael Bommarito
Cc: Masami Hiramatsu, Mathieu Desnoyers, Beau Belgrave, XIAO WU,
linux-trace-kernel, linux-kernel, stable
In-Reply-To: <20260707165912.2560537-3-michael.bommarito@gmail.com>
On Tue, 7 Jul 2026 12:59:12 -0400
Michael Bommarito <michael.bommarito@gmail.com> wrote:
>
> +/*
> + * Deleting an event drops its last reference, but an unregister may defer
> + * that put (and the freeing of the associated enabler) past an RCU grace
> + * period. The delete can therefore transiently fail with -EBUSY while the
> + * previous reference is still being dropped. Retry for up to ~10 seconds.
> + */
> +static int wait_for_event_delete(void)
> +{
> + int i, ret;
> +
> + for (i = 0; i < 10000; ++i) {
> + ret = event_delete();
> +
> + if (ret == 0)
> + return 0;
> +
> + usleep(1000);
> + }
> +
> + return ret;
> +}
> +
Care to address Sashiko's comment: https://sashiko.dev/#/patchset/20260707165912.2560537-2-michael.bommarito%40gmail.com
I'll pull in patch 1 and start testing it as this one is just the tools
change, it doesn't need my testing (my tests only tests kernel changes)
-- Steve
^ permalink raw reply
* Re: [PATCH] rtla: Also link in ctype.c
From: Bastian Blank @ 2026-07-07 17:43 UTC (permalink / raw)
To: Tomas Glozar; +Cc: Steven Rostedt, linux-trace-kernel
In-Reply-To: <CAP4=nvTH5HhUk19w06aH6-xqWGOUQKesGbKCfvgpwmefLom0VQ@mail.gmail.com>
On Tue, Jul 07, 2026 at 12:22:37PM +0200, Tomas Glozar wrote:
> Thank you. It appears that GCC LTO drops the symbol use of "_ctype",
> so I didn't see it earlier. With removed -flto=auto from
> Makefile.rtla, I can reproduce it:
Ah, yes, we override LTO to disabled.
Bastian
--
There are certain things men must do to remain men.
-- Kirk, "The Ultimate Computer", stardate 4929.4
^ permalink raw reply
* Re: [PATCH v2] tracing/synthetic: Free type string on error path
From: Steven Rostedt @ 2026-07-07 17:54 UTC (permalink / raw)
To: Yu Peng; +Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, linux-kernel
In-Reply-To: <20260707132417.2193412-1-pengyu@kylinos.cn>
On Tue, 7 Jul 2026 21:24:17 +0800
Yu Peng <pengyu@kylinos.cn> wrote:
> parse_synth_field() builds a "__data_loc ..." type string before
> assigning it to field->type. If the seq_buf check fails, the temporary
> string is not owned by field and is leaked. Free it before leaving.
>
> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Yu Peng <pengyu@kylinos.cn>
> ---
> Changes in v2:
> - Use __free(kfree) and no_free_ptr() as suggested by Steven.
>
> kernel/trace/trace_events_synth.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
> index e6871230bde96..ad2e70258291b 100644
> --- a/kernel/trace/trace_events_synth.c
> +++ b/kernel/trace/trace_events_synth.c
> @@ -828,7 +828,7 @@ static struct synth_field *parse_synth_field(int argc, char **argv,
> } else if (size == 0) {
> if (synth_field_is_string(field->type) ||
> synth_field_is_stack(field->type)) {
> - char *type;
> + char *type __free(kfree) = NULL;
Ah, we can't do this here.
(Sashiko pointed this out: https://sashiko.dev/#/patchset/20260707132417.2193412-1-pengyu%40kylinos.cn )
>
> len = sizeof("__data_loc ") + strlen(field->type) + 1;
> type = kzalloc(len, GFP_KERNEL);
Because after this code we have:
type = kzalloc(len, GFP_KERNEL);
if (!type)
goto free;
Which jumps out of the if block, and that will break the cleanup.
I'll take you original version for now.
-- Steve
> @@ -844,7 +844,7 @@ static struct synth_field *parse_synth_field(int argc, char **argv,
> s.buffer[s.len] = '\0';
>
> kfree(field->type);
> - field->type = type;
> + field->type = no_free_ptr(type);
>
> field->is_dynamic = true;
> size = sizeof(u64);
^ permalink raw reply
* [PATCH v3] selftests/user_events: wait for deferred event teardown after unregister
From: Michael Bommarito @ 2026-07-07 18:02 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
Cc: Beau Belgrave, XIAO WU, Shuah Khan, linux-trace-kernel,
linux-kselftest, linux-kernel
Unregistering a user event now defers the drop of the enabler's event
reference (and the freeing of the enabler) past an RCU grace period. As a
result DIAG_IOCSDEL can transiently fail with -EBUSY while that last
reference is still being dropped, where it previously succeeded
immediately.
Two tests assumed the delete takes effect the instant the unregister
returns:
- abi_test "flags" deletes the event right after disabling it.
- perf_test's fixture teardown clear() deletes __test_event before the
next test registers the same name; a stale event makes the following
registration fail with -EADDRINUSE.
Retry the delete until it succeeds (or the event is already gone) with a
bounded wait, matching the existing wait_for_delete() idiom in the same
suite, so the tests are robust to the deferred teardown.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
This resends only the selftest patch; the tracing/user_events fix that
was patch 1/2 of the v2 series is unchanged and being applied separately.
v2 -> v3:
- abi_test wait_for_event_delete(): only retry the delete on -EBUSY,
treat an already-deleted event (-ENOENT) as success, and return any
other error immediately instead of spinning for the full 10s timeout.
perf_test's clear() already discriminated on errno this way; the two
now match. Reported by Sashiko automated review.
.../testing/selftests/user_events/abi_test.c | 29 ++++++++++++++++++-
.../testing/selftests/user_events/perf_test.c | 26 +++++++++++++++--
2 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/user_events/abi_test.c b/tools/testing/selftests/user_events/abi_test.c
index 85892b3b719cc..b71813eaf5c04 100644
--- a/tools/testing/selftests/user_events/abi_test.c
+++ b/tools/testing/selftests/user_events/abi_test.c
@@ -132,6 +132,33 @@ static int event_delete(void)
return ret;
}
+/*
+ * Deleting an event drops its last reference, but an unregister may defer
+ * that put (and the freeing of the associated enabler) past an RCU grace
+ * period. The delete can therefore transiently fail with -EBUSY while the
+ * previous reference is still being dropped. Retry only on that transient
+ * failure; treat an already-deleted event (-ENOENT) as success and return
+ * any other error immediately rather than spinning for the full timeout.
+ */
+static int wait_for_event_delete(void)
+{
+ int i, ret;
+
+ for (i = 0; i < 10000; ++i) {
+ ret = event_delete();
+
+ if (ret == 0 || errno == ENOENT)
+ return 0;
+
+ if (errno != EBUSY)
+ return ret;
+
+ usleep(1000);
+ }
+
+ return ret;
+}
+
static int reg_enable_multi(void *enable, int size, int bit, int flags,
char *args)
{
@@ -262,7 +289,7 @@ TEST_F(user, flags) {
ASSERT_TRUE(event_exists());
/* Ensure we can delete it */
- ASSERT_EQ(0, event_delete());
+ ASSERT_EQ(0, wait_for_event_delete());
/* USER_EVENT_REG_MAX or above is not allowed */
ASSERT_EQ(-1, reg_enable_flags(&self->check, sizeof(int), 0,
diff --git a/tools/testing/selftests/user_events/perf_test.c b/tools/testing/selftests/user_events/perf_test.c
index cafec0e52eb31..5727cb5b914cf 100644
--- a/tools/testing/selftests/user_events/perf_test.c
+++ b/tools/testing/selftests/user_events/perf_test.c
@@ -85,6 +85,7 @@ static int get_offset(void)
static int clear(int *check)
{
struct user_unreg unreg = {0};
+ int i, ret;
unreg.size = sizeof(unreg);
unreg.disable_bit = 31;
@@ -99,13 +100,32 @@ static int clear(int *check)
if (errno != ENOENT)
return -1;
- if (ioctl(fd, DIAG_IOCSDEL, "__test_event") == -1)
- if (errno != ENOENT)
+ /*
+ * Deleting the event drops its last reference, but the unregister
+ * above defers that put (and the freeing of the enabler) past an RCU
+ * grace period. The delete can therefore transiently fail with -EBUSY
+ * until that reference is dropped. Retry for up to ~10 seconds so the
+ * event is actually gone before the next test registers the same name.
+ */
+ for (i = 0; i < 10000; ++i) {
+ ret = ioctl(fd, DIAG_IOCSDEL, "__test_event");
+
+ if (ret == 0 || errno == ENOENT) {
+ ret = 0;
+ break;
+ }
+
+ if (errno != EBUSY) {
+ close(fd);
return -1;
+ }
+
+ usleep(1000);
+ }
close(fd);
- return 0;
+ return ret;
}
FIXTURE(user) {
--
2.53.0
^ permalink raw reply related
page: | 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