[PATCH] perf arm-spe: Don't warn about the discard bit if it doesn't exist

James Clark posted 1 patch 2 months, 1 week ago
tools/perf/arch/arm64/util/arm-spe.c |  3 ++-
tools/perf/tests/pmu.c               | 11 +++++++++--
tools/perf/util/evsel.c              |  6 ++++++
tools/perf/util/evsel.h              |  1 +
4 files changed, 18 insertions(+), 3 deletions(-)
[PATCH] perf arm-spe: Don't warn about the discard bit if it doesn't exist
Posted by James Clark 2 months, 1 week ago
Opening an SPE event shows a warning that doesn't concern the user:

  $ perf record -e arm_spe
  Unknown/empty format name: discard

Perf only wants to know if the discard bit is set for configuring the
event, not in response to anything the user has done. Fix it by adding
another helper that returns if a config bit exists without warning.

We should probably keep the warning in evsel__get_config_val() to avoid
having every caller having to do it, and most format bits should never
be missing.

Add a test for the new helper. Rename the parent test function to be
more generic rather than adding a new one as it requires a lot of
boilerplate.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/arch/arm64/util/arm-spe.c |  3 ++-
 tools/perf/tests/pmu.c               | 11 +++++++++--
 tools/perf/util/evsel.c              |  6 ++++++
 tools/perf/util/evsel.h              |  1 +
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index f00d72d087fc..91bb28cad79a 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -428,7 +428,8 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
 	evlist__for_each_entry_safe(evlist, tmp, evsel) {
 		if (evsel__is_aux_event(evsel)) {
 			arm_spe_setup_evsel(evsel, cpus);
-			if (!evsel__get_config_val(evsel, "discard", &discard_bit))
+			if (evsel__config_exists(evsel, "discard") &&
+			    !evsel__get_config_val(evsel, "discard", &discard_bit))
 				discard = !!discard_bit;
 		}
 	}
diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
index 0ebf2d7b2cb4..d7be9d1c6f52 100644
--- a/tools/perf/tests/pmu.c
+++ b/tools/perf/tests/pmu.c
@@ -201,7 +201,8 @@ static int test__pmu_format(struct test_suite *test __maybe_unused, int subtest
 	return ret;
 }
 
-static int test__pmu_usr_chgs(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
+static int test__pmu_config_helpers(struct test_suite *test __maybe_unused,
+				    int subtest __maybe_unused)
 {
 	const char *event = "perf-pmu-test/config=15,config1=4,krava02=170,"
 			    "krava03=1,krava11=27,krava12=1/";
@@ -236,6 +237,12 @@ static int test__pmu_usr_chgs(struct test_suite *test __maybe_unused, int subtes
 	}
 	evsel = evlist__first(evlist);
 
+	/* Test evsel__config_exists() */
+	TEST_ASSERT_EQUAL("krava01 should exist",
+			  evsel__config_exists(evsel, "krava01"), true);
+	TEST_ASSERT_EQUAL("krava99 should not exist",
+			  evsel__config_exists(evsel, "krava99"), false);
+
 	/*
 	 * Set via config=15, krava01 bits 0-1
 	 * Set via config1=4, krava11 bit 1
@@ -629,7 +636,7 @@ static struct test_case tests__pmu[] = {
 	TEST_CASE("PMU name combining", name_len),
 	TEST_CASE("PMU name comparison", name_cmp),
 	TEST_CASE("PMU cmdline match", pmu_match),
-	TEST_CASE("PMU user config changes", pmu_usr_chgs),
+	TEST_CASE("PMU config helpers", pmu_config_helpers),
 	{	.name = NULL, }
 };
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 2ee87fd84d3e..685c4118aef3 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1396,6 +1396,12 @@ void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,
 	perf_pmu__format_pack(format->bits, val, vp, /*zero=*/true);
 }
 
+bool evsel__config_exists(const struct evsel *evsel, const char *config_name)
+{
+	struct perf_pmu_format *format = pmu_find_format(&evsel->pmu->format, config_name);
+
+	return format && !bitmap_empty(format->bits, PERF_PMU_FORMAT_BITS);
+}
 
 int evsel__get_config_val(const struct evsel *evsel, const char *config_name,
 			  u64 *val)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 339b5c08a33d..a255ae2b1f8e 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -578,6 +578,7 @@ void evsel__uniquify_counter(struct evsel *counter);
 	((((src) >> (pos)) & ((1ull << (size)) - 1)) << (63 - ((pos) + (size) - 1)))
 
 u64 evsel__bitfield_swap_branch_flags(u64 value);
+bool evsel__config_exists(const struct evsel *evsel, const char *config_name);
 int evsel__get_config_val(const struct evsel *evsel, const char *config_name,
 			  u64 *val);
 void evsel__set_config_if_unset(struct evsel *evsel, const char *config_name,

---
base-commit: 4cf1f549bbcdfea9c20df52994bb342677472dcd
change-id: 20260410-james-spe-discard-warning-049b27a53aac

Best regards,
-- 
James Clark <james.clark@linaro.org>
Re: [PATCH] perf arm-spe: Don't warn about the discard bit if it doesn't exist
Posted by Leo Yan 1 month, 1 week ago
On Fri, Apr 10, 2026 at 12:05:12PM +0100, James Clark wrote:
> Opening an SPE event shows a warning that doesn't concern the user:
> 
>   $ perf record -e arm_spe
>   Unknown/empty format name: discard
> 
> Perf only wants to know if the discard bit is set for configuring the
> event, not in response to anything the user has done. Fix it by adding
> another helper that returns if a config bit exists without warning.
> 
> We should probably keep the warning in evsel__get_config_val() to avoid
> having every caller having to do it, and most format bits should never
> be missing.
> 
> Add a test for the new helper. Rename the parent test function to be
> more generic rather than adding a new one as it requires a lot of
> boilerplate.
> 
> Signed-off-by: James Clark <james.clark@linaro.org>

Reviewed-by: Leo Yan <leo.yan@arm.com>
Re: [PATCH] perf arm-spe: Don't warn about the discard bit if it doesn't exist
Posted by Ian Rogers 1 month, 1 week ago
On Wed, May 13, 2026 at 3:20 AM Leo Yan <leo.yan@arm.com> wrote:
>
> On Fri, Apr 10, 2026 at 12:05:12PM +0100, James Clark wrote:
> > Opening an SPE event shows a warning that doesn't concern the user:
> >
> >   $ perf record -e arm_spe
> >   Unknown/empty format name: discard
> >
> > Perf only wants to know if the discard bit is set for configuring the
> > event, not in response to anything the user has done. Fix it by adding
> > another helper that returns if a config bit exists without warning.
> >
> > We should probably keep the warning in evsel__get_config_val() to avoid
> > having every caller having to do it, and most format bits should never
> > be missing.
> >
> > Add a test for the new helper. Rename the parent test function to be
> > more generic rather than adding a new one as it requires a lot of
> > boilerplate.
> >
> > Signed-off-by: James Clark <james.clark@linaro.org>
>
> Reviewed-by: Leo Yan <leo.yan@arm.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian
Re: [PATCH] perf arm-spe: Don't warn about the discard bit if it doesn't exist
Posted by James Clark 3 weeks, 1 day ago

On 13/05/2026 1:55 pm, Ian Rogers wrote:
> On Wed, May 13, 2026 at 3:20 AM Leo Yan <leo.yan@arm.com> wrote:
>>
>> On Fri, Apr 10, 2026 at 12:05:12PM +0100, James Clark wrote:
>>> Opening an SPE event shows a warning that doesn't concern the user:
>>>
>>>    $ perf record -e arm_spe
>>>    Unknown/empty format name: discard
>>>
>>> Perf only wants to know if the discard bit is set for configuring the
>>> event, not in response to anything the user has done. Fix it by adding
>>> another helper that returns if a config bit exists without warning.
>>>
>>> We should probably keep the warning in evsel__get_config_val() to avoid
>>> having every caller having to do it, and most format bits should never
>>> be missing.
>>>
>>> Add a test for the new helper. Rename the parent test function to be
>>> more generic rather than adding a new one as it requires a lot of
>>> boilerplate.
>>>
>>> Signed-off-by: James Clark <james.clark@linaro.org>
>>
>> Reviewed-by: Leo Yan <leo.yan@arm.com>
> 
> Reviewed-by: Ian Rogers <irogers@google.com>
> 
> Thanks,
> Ian

Ping, thanks

Re: [PATCH] perf arm-spe: Don't warn about the discard bit if it doesn't exist
Posted by Arnaldo Carvalho de Melo 3 weeks ago
On Fri, May 29, 2026 at 10:42:34AM +0100, James Clark wrote:
> On 13/05/2026 1:55 pm, Ian Rogers wrote:
> > On Wed, May 13, 2026 at 3:20 AM Leo Yan <leo.yan@arm.com> wrote:
> > > On Fri, Apr 10, 2026 at 12:05:12PM +0100, James Clark wrote:
> > > > Opening an SPE event shows a warning that doesn't concern the user:

> > > >    $ perf record -e arm_spe
> > > >    Unknown/empty format name: discard

> > > > Perf only wants to know if the discard bit is set for configuring the
> > > > event, not in response to anything the user has done. Fix it by adding
> > > > another helper that returns if a config bit exists without warning.

> > > > We should probably keep the warning in evsel__get_config_val() to avoid
> > > > having every caller having to do it, and most format bits should never
> > > > be missing.

> > > > Add a test for the new helper. Rename the parent test function to be
> > > > more generic rather than adding a new one as it requires a lot of
> > > > boilerplate.

> > > > Signed-off-by: James Clark <james.clark@linaro.org>

> > > Reviewed-by: Leo Yan <leo.yan@arm.com>

> > Reviewed-by: Ian Rogers <irogers@google.com>

> Ping, thanks

Thanks, applied to perf-tools-next, for v7.2.

- Arnaldo