[PATCH 0/2] perf: Revert direct PMU stop for refresh limit

[PATCH 0/2] perf: Revert direct PMU stop for refresh limit

[Leo Yan]

Commit 18dbcbfabfff ("perf: Fix the POLL_HUP delivery breakage") added a
direct pmu->stop() call when the refresh limit reaches zero. The change
was based on a test program [1] that reported missing POLL_HUP
notifications.

However, the test program used SIGIO, which is a standard signal and can
be coalesced. As a result, userspace may miss signal delivery even
though the signal was generated by the kernel. This is expected
behaviour for standard signals.

This series adds a selftest for the PERF_EVENT_IOC_REFRESH limit using a
real-time signal, which guarantees queued signal delivery and confirms
that POLL_HUP notifications are delivered reliably on arm64.

With that verified, the direct pmu->stop() call introduced by commit
18dbcbfabfff can be reverted, since event disabling is already handled
through perf_event_disable_inatomic().

[1] https://lore.kernel.org/lkml/aICYAqM5EQUlTqtX@li-2b55cdcc-350b-11b2-a85c-a78bff51fc11.ibm.com/

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
Leo Yan (2):
      selftests/perf_events: Add test for refresh limit signals
      Revert "perf: Fix the POLL_HUP delivery breakage"

 kernel/events/core.c                               |   1 -
 tools/testing/selftests/perf_events/.gitignore     |   1 +
 tools/testing/selftests/perf_events/Makefile       |   3 +-
 .../testing/selftests/perf_events/refresh_signal.c | 120 +++++++++++++++++++++
 4 files changed, 123 insertions(+), 2 deletions(-)
---
base-commit: 50897c955902c93ae71c38698abb910525ebdc89
change-id: 20260429-arm_cs_clean_perf_handle-763cc339c1f5

Best regards,
-- 
Leo Yan <leo.yan@arm.com>

[PATCH 1/2] selftests/perf_events: Add test for refresh limit signals

[Leo Yan]

perf reports POLL_IN for overflows while an event still has refreshes
left, and POLL_HUP when the refresh count reaches zero and the event is
disabled.

Add a test to verify PERF_EVENT_IOC_REFRESH with a task-clock software
event. Use a real-time signal so notifications are queued instead of
being coalesced.

Each iteration sets the refresh count to 5 and waits for a POLL_HUP
notification after 5 task-clock overflows at a 1 microsecond interval.
The sequence is repeated 100 times, verifying that exactly one POLL_HUP
notification is delivered per iteration. The test is bounded by
the default selftest timeout (30 seconds).

    POLL_IN             POLL_HUP
    e1   e2   e3   e4   e5        |
    |    |    |    |    |         |
  --+----+----+----+----+---------+----------------------
    <-----      Iter 1      -----> <--- Iter2 ... --->

POLL_IN is allowed to be lower than the theoretical maximum because
overflow delivery is deferred through irq_work. If timers expire too
close to each other, pending irq_work may not run before the next timer
expires, so some POLL_IN notifications can be missed.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/testing/selftests/perf_events/.gitignore     |   1 +
 tools/testing/selftests/perf_events/Makefile       |   3 +-
 .../testing/selftests/perf_events/refresh_signal.c | 120 +++++++++++++++++++++
 3 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/perf_events/.gitignore b/tools/testing/selftests/perf_events/.gitignore
index 4931b3b6bbd3971145b0e09b1fcdaf6cae9eb10e..e1bcf3ab8ab24bdb742a9cd593810c1c566d4b20 100644
--- a/tools/testing/selftests/perf_events/.gitignore
+++ b/tools/testing/selftests/perf_events/.gitignore
@@ -3,3 +3,4 @@ sigtrap_threads
 remove_on_exec
 watermark_signal
 mmap
+refresh_signal
diff --git a/tools/testing/selftests/perf_events/Makefile b/tools/testing/selftests/perf_events/Makefile
index 2e5d85770dfeadd909196dbf980fd334b9580477..e0591b1045f959476a0c5bb57e471a01006b66ee 100644
--- a/tools/testing/selftests/perf_events/Makefile
+++ b/tools/testing/selftests/perf_events/Makefile
@@ -2,5 +2,6 @@
 CFLAGS += -Wl,-no-as-needed -Wall $(KHDR_INCLUDES)
 LDFLAGS += -lpthread
 
-TEST_GEN_PROGS := sigtrap_threads remove_on_exec watermark_signal mmap
+TEST_GEN_PROGS := sigtrap_threads remove_on_exec watermark_signal mmap \
+		  refresh_signal
 include ../lib.mk
diff --git a/tools/testing/selftests/perf_events/refresh_signal.c b/tools/testing/selftests/perf_events/refresh_signal.c
new file mode 100644
index 0000000000000000000000000000000000000000..9bf05bcd888782fe8130579cc03602a8f39dd21c
--- /dev/null
+++ b/tools/testing/selftests/perf_events/refresh_signal.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+
+#include <fcntl.h>
+#include <linux/perf_event.h>
+#include <poll.h>
+#include <signal.h>
+#include <sys/ioctl.h>
+#include <time.h>
+
+#include "../kselftest_harness.h"
+
+#define RT_SIG			(SIGRTMIN + 1)
+
+#define EVENT_LIMIT		5
+#define ITERATIONS		100
+
+static struct {
+	volatile sig_atomic_t in;
+	volatile sig_atomic_t hup;
+} count;
+
+static void sigio_handler(int signo __maybe_unused, siginfo_t *info,
+			  void *ucontext __maybe_unused)
+{
+	switch (info->si_code) {
+	case POLL_IN:
+		count.in++;
+		break;
+	case POLL_HUP:
+		count.hup++;
+		break;
+	}
+}
+
+FIXTURE(refresh_signal)
+{
+	struct sigaction old_sa;
+	int fd;
+};
+
+FIXTURE_SETUP(refresh_signal)
+{
+	struct sigaction sa = { 0 };
+	struct perf_event_attr attr = { 0 };
+
+	sa.sa_sigaction = sigio_handler;
+	sa.sa_flags = SA_SIGINFO;
+	sigemptyset(&sa.sa_mask);
+
+	/* Use a real-time signal so notifications are reliably queued */
+	EXPECT_EQ(sigaction(RT_SIG, &sa, &self->old_sa), 0);
+
+	attr.size = sizeof(attr);
+	attr.type = PERF_TYPE_SOFTWARE;
+	attr.config = PERF_COUNT_SW_TASK_CLOCK;
+	attr.disabled = 1;
+	attr.sample_period = 1000;	/* 1 us */
+	attr.exclude_kernel = 1;
+	attr.exclude_hv = 1;
+
+	self->fd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, 0);
+	ASSERT_NE(self->fd, -1);
+
+	/* Enable async notification */
+	ASSERT_EQ(fcntl(self->fd, F_SETFL, fcntl(self->fd, F_GETFL) | O_ASYNC), 0);
+
+	/* Receive the signal for current process */
+	ASSERT_EQ(fcntl(self->fd, F_SETOWN, getpid()), 0);
+
+	/* Use signo instead of the default SIGIO */
+	ASSERT_EQ(fcntl(self->fd, F_SETSIG, RT_SIG), 0);
+}
+
+FIXTURE_TEARDOWN(refresh_signal)
+{
+	ASSERT_EQ(ioctl(self->fd, PERF_EVENT_IOC_DISABLE, 0), 0);
+
+	close(self->fd);
+	sigaction(RT_SIG, &self->old_sa, NULL);
+}
+
+TEST_F(refresh_signal, refresh_stress)
+{
+	int i;
+
+	ASSERT_EQ(ioctl(self->fd, PERF_EVENT_IOC_RESET, 0), 0);
+
+	for (i = 0; i < ITERATIONS; i++) {
+		sig_atomic_t old_count = count.hup;
+
+		/* Set event limit and the event is enabled */
+		ASSERT_EQ(ioctl(self->fd, PERF_EVENT_IOC_REFRESH, EVENT_LIMIT), 0);
+
+		/*
+		 * Wait for new the POLL_HUP notification. The test is bounded
+		 * by the default timeout.
+		 */
+		while (old_count == count.hup);
+	}
+
+	/*
+	 * Events before EVENT_LIMIT are reported as POLL_IN. When the limit
+	 * is reached, the final event is reported as POLL_HUP. The total
+	 * number of events is scaled by the number of iterations.
+	 *
+	 * Events are delivered via irq_work. If timers expire too close to
+	 * each other, irq_work may not run before the next timer fires,
+	 * causing some POLL_IN events to be missed. Therefore, use a
+	 * less-or-equal comparison for POLL_IN.
+	 *
+	 * The last event stops the timer, so the POLL_HUP signal must be
+	 * observed once per iteration when the limit is reached.
+	 */
+	EXPECT_LE(count.in, (EVENT_LIMIT - 1) * ITERATIONS);
+	EXPECT_GT(count.in, 0);
+	EXPECT_EQ(count.hup, ITERATIONS);
+}
+
+TEST_HARNESS_MAIN

-- 
2.34.1

Re: [PATCH 1/2] selftests/perf_events: Add test for refresh limit signals

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Using ASSERT_EQ inside FIXTURE_TEARDOWN leads to infinite recursion and masks test failures.
- [Medium] The assertion EXPECT_GT(count.in, 0) will deterministically fail on architectures without fast irq_work support.
--

commit b2c9c1a07e0f0b3e81140a87060db6e67e7d721a
Author: Leo Yan <leo.yan@arm.com>

selftests/perf_events: Add test for refresh limit signals

This commit adds a test to verify PERF_EVENT_IOC_REFRESH with a task-clock
software event. Each iteration sets the refresh count to 5 and waits for a
POLL_HUP notification after 5 task-clock overflows at a 1 microsecond interval.
The sequence is repeated 100 times to verify exactly one POLL_HUP is delivered
per iteration.

> diff --git a/tools/testing/selftests/perf_events/refresh_signal.c b/tools/testing/selftests/perf_events/refresh_signal.c
> new file mode 100644
> index 0000000000000..9bf05bcd88878
> --- /dev/null
> +++ b/tools/testing/selftests/perf_events/refresh_signal.c
[ ... ]
> +FIXTURE_TEARDOWN(refresh_signal)
> +{
> +	ASSERT_EQ(ioctl(self->fd, PERF_EVENT_IOC_DISABLE, 0), 0);

Will this lead to infinite recursion if the test fails during setup?

Under kselftest_harness.h, a failing ASSERT_* macro invokes __bail(), which
calls the test's teardown function. If this ioctl fails (e.g., if self->fd is
invalid because perf_event_open failed during setup), the ASSERT_EQ will invoke
teardown again, causing an infinite recursion that crashes the test.

Should EXPECT_EQ be used here instead to ensure cleanup continues and the
original error is properly reported?

> +
> +	close(self->fd);
> +	sigaction(RT_SIG, &self->old_sa, NULL);
> +}
[ ... ]
> +	/*
> +	 * Events before EVENT_LIMIT are reported as POLL_IN. When the limit
> +	 * is reached, the final event is reported as POLL_HUP. The total
> +	 * number of events is scaled by the number of iterations.
> +	 *
> +	 * Events are delivered via irq_work. If timers expire too close to
> +	 * each other, irq_work may not run before the next timer fires,
> +	 * causing some POLL_IN events to be missed. Therefore, use a
> +	 * less-or-equal comparison for POLL_IN.
> +	 *
> +	 * The last event stops the timer, so the POLL_HUP signal must be
> +	 * observed once per iteration when the limit is reached.
> +	 */
> +	EXPECT_LE(count.in, (EVENT_LIMIT - 1) * ITERATIONS);
> +	EXPECT_GT(count.in, 0);

Can this deterministically fail on architectures without fast irq_work support?

On architectures where arch_irq_work_has_interrupt() is false or self-IPI
is not supported, irq_work is executed during the timer tick. Because this
test uses a 1 microsecond hrtimer period, all 5 hrtimer callbacks could fire
within the first ~5 microseconds of the iteration, before the next timer tick.

The final callback would overwrite the pending signal to POLL_HUP before the
irq_work has a chance to run. When the tick finally executes the irq_work,
only the POLL_HUP signal is delivered. This would cause count.in to remain 0
across all iterations, failing this assertion.

> +	EXPECT_EQ(count.hup, ITERATIONS);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-arm_cs_clean_perf_handle-v1-0-75ff373ecd22@arm.com?part=1

[PATCH 2/2] Revert "perf: Fix the POLL_HUP delivery breakage"

[Leo Yan]

This reverts commit 18dbcbfabfffc4a5d3ea10290c5ad27f22b0d240.

The original issue was reported in [1], it shared a test program with
SIGIO. Since SIGIO is a standard signal, multiple notifications can be
coalesced, so userspace may miss a signal even though the signal was
generated by the event core.

Using the Ftrace signal tracepoints on arm64 shows that POLL_HUP is
generated, but the corresponding delivery trace event ("signal_deliver")
may be absent when SIGIO is used. In contrast, the kselftest
refresh_signal demonstrates that POLL_HUP delivery is reliable when using
a real-time signal, and the test passes without the extra pmu->stop()
call introduced by commit 18dbcbfabfff.

When the refresh limit reaches zero, __perf_event_overflow() already
calls perf_event_disable_inatomic(), which schedules the event disable
through irq_work. Calling the PMU stop callback directly here is
redundant and unrelated to signal delivery.

Remove the extra stop callback.

[1] https://lore.kernel.org/lkml/aICYAqM5EQUlTqtX@li-2b55cdcc-350b-11b2-a85c-a78bff51fc11.ibm.com/

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 kernel/events/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 7935d5663944ee1cbaf38cf8018c3347635e8d31..7d98a56cd91c47e6ac0e4f8ace1ab494ba2b0501 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10750,7 +10750,6 @@ static int __perf_event_overflow(struct perf_event *event,
 		ret = 1;
 		event->pending_kill = POLL_HUP;
 		perf_event_disable_inatomic(event);
-		event->pmu->stop(event, 0);
 	}
 
 	if (event->attr.sigtrap) {

-- 
2.34.1

Re: [PATCH 2/2] Revert "perf: Fix the POLL_HUP delivery breakage"

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the synchronous `event->pmu->stop(event, 0)` re-introduces a race condition where the PMU hardware is not stopped synchronously when `event_limit` reaches 0, allowing spurious hardware interrupts to clobber the `POLL_HUP` signal.
--

commit 0345ee403573bb4c49df3d0db2209160312a4bd6
Author: Leo Yan <leo.yan@arm.com>
Revert "perf: Fix the POLL_HUP delivery breakage"

This commit reverts the addition of a synchronous PMU stop call during
event overflow when the event limit reaches zero. It relies instead on
the asynchronous irq_work scheduled by perf_event_disable_inatomic() to
disable the event, removing the direct hardware stop.

Link: https://lore.kernel.org/lkml/aICYAqM5EQUlTqtX@li-2b55cdcc-350b-11b2-a85c-a78bff51fc11.ibm.com/

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -10750,7 +10750,6 @@ static int __perf_event_overflow(struct perf_event *event,
>  		ret = 1;
>  		event->pending_kill = POLL_HUP;
>  		perf_event_disable_inatomic(event);
> -		event->pmu->stop(event, 0);

Does removing this synchronous stop introduce a race condition with
subsequent hardware interrupts?

Looking at perf_event_disable_inatomic(), it does not synchronously stop
the PMU hardware. Instead, it queues an asynchronous irq_work. Because
architecture PMU interrupt handlers generally re-enable the PMU globally
before returning from the NMI/IRQ, the counter can remain active and
overflow again before the scheduled irq_work actually executes.

If a subsequent overflow occurs, __perf_event_overflow() is re-entered
and unconditionally sets pending_kill to POLL_IN:

__perf_event_overflow() {
    ...
    event->pending_kill = POLL_IN;
    if (events && atomic_dec_and_test(&event->event_limit)) {
        ret = 1;
        event->pending_kill = POLL_HUP;
        perf_event_disable_inatomic(event);
    }
    ...
}

Since event_limit is already 0 at this point, the if condition fails, and
pending_kill remains set to POLL_IN.

When the irq_work finally runs, could this sequence cause it to deliver
POLL_IN instead of the expected POLL_HUP, silently breaking userspace
expectations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-arm_cs_clean_perf_handle-v1-0-75ff373ecd22@arm.com?part=2