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

[sashiko-bot@kernel.org]

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