RE: [PATCH v11 7/9] test/pmu: enable test

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Tomasz Duszynski [mailto:tduszynski@marvell.com]
> Sent: Friday, 24 October 2025 07.48
> 
> Enable test to allow users to verify basic functionality. Due to
> varying
> configuration options across distributions and kernels user should
> ensure that all requirements are satisfied before starting test.
> 
> Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
> ---

With these changes, the test can never return TEST_FAILED, it returns TEST_SKIPPED when something doesn't work.

IMO, the perf_allowed_quirk() check belongs in rte_pmu_init() instead.

Then, rte_pmu_init() could return a variety of error values depending on the reason for the inability to init/start the PMU library.
Amongst the high level error values should be:
-EACCES (insufficient privileges)
-ENODEV (hardware has no PMU, or kernel is custom built without PMU)

The test should only return SKIPPED if rte_pmu_init() returns -ENODEV (no PMU present to test); any other non-success should cause the test to return FAILED.

Also, the possible return values from rte_pmu_init() should be documented in its function description.


>  app/test/test_pmu.c | 60 ++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 54 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test/test_pmu.c b/app/test/test_pmu.c
> index 10513bf9c9..7f450b3566 100644
> --- a/app/test/test_pmu.c
> +++ b/app/test/test_pmu.c
> @@ -2,10 +2,48 @@
>   * Copyright(C) 2025 Marvell International Ltd.
>   */
> 
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
>  #include <rte_pmu.h>
> 
>  #include "test.h"
> 
> +#define PERF_EVENT_PARANOID_PATH
> "/proc/sys/kernel/perf_event_paranoid"
> +
> +static bool perf_allowed_quirk(void)
> +{
> +	int level, ret;
> +	FILE *fp;
> +
> +	fp = fopen(PERF_EVENT_PARANOID_PATH, "r");
> +	if (!fp)
> +		return false;
> +
> +	ret = fscanf(fp, "%d", &level);
> +	fclose(fp);
> +	if (ret != 1)
> +		return false;
> +
> +	/* On vanilla Linux the default perf_event_paranoid level is 2,
> which allows non-privileged
> +	 * processes to access performance counters.
> +	 *
> +	 * Debian / Ubuntu and their derivatives apply patches that
> introduce
> +	 * additional paranoia levels:
> +	 *
> +	 * - Debian adds level 3, which restricts access to
> perf_event_open() for
> +	 *   monitoring other processes, but still allows unprivileged
> self-monitoring.
> +	 *   See: https://lore.kernel.org/all/1469630746-32279-1-git-
> send-email-jeffv@google.com/
> +	 * - Ubuntu adds level 4 (which is also the default), completely
> disabling perf_event_open()
> +	 *   for unprivileged users—effectively disabling self-
> monitoring.
> +	 *
> +	 * That said, check below should be sufficient to enable this
> test on most kernels.
> +	 */
> +	return level < 4;
> +}
> +
>  static int
>  test_pmu_read(void)
>  {
> @@ -24,8 +62,15 @@ test_pmu_read(void)
>  		return TEST_SKIPPED;
>  	}
> 
> -	if (rte_pmu_init() < 0)
> -		return TEST_FAILED;
> +	if ((getuid() != 0) && !perf_allowed_quirk()) {
> +		printf("self-monitoring disabled\n");
> +		return TEST_SKIPPED;
> +	}
> +
> +	if (rte_pmu_init() < 0) {
> +		printf("PMU not initialized\n");
> +		return TEST_SKIPPED;
> +	}
> 
>  	event = rte_pmu_add_event(name);
>  	while (tries--)
> @@ -33,7 +78,12 @@ test_pmu_read(void)
> 
>  	rte_pmu_fini();
> 
> -	return val ? TEST_SUCCESS : TEST_FAILED;
> +	/* rte_pmu_read() returns zero if it can't read perf counter.
> Thus series of zeros doesn't
> +	 * necessarily mean the counter is actually zero. It might just
> signal a problem with setup
> +	 * itself. So skip test to avoid testing failure and leave it to
> user to interpret this
> +	 * outcome.
> +	 */
> +	return val ? TEST_SUCCESS : TEST_SKIPPED;
>  }
> 
>  static struct unit_test_suite pmu_tests = {
> @@ -52,6 +102,4 @@ test_pmu(void)
>  	return unit_test_suite_runner(&pmu_tests);
>  }
> 
> -/* disabled because of reported failures, waiting for a fix
> - * REGISTER_FAST_TEST(pmu_autotest, true, true, test_pmu);
> - */
> +REGISTER_FAST_TEST(pmu_autotest, true, true, test_pmu);
> --
> 2.34.1