tools/perf/util/s390-sample-raw.c | 56 ++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 5 deletions(-)
Searching all event names is slower now that legacy names are
included. Add a cache to avoid long iterative searches.
Reported-by: Thomas Richter <tmricht@linux.ibm.com>
Closes: https://lore.kernel.org/linux-perf-users/09943f4f-516c-4b93-877c-e4a64ed61d38@linux.ibm.com/
Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Thomas Richter <tmricht@linux.ibm.com>
---
v3: Fix minor comment typo, add Thomas' tag.
v2: Small tweak to the cache_key, just make it match the wanted event value.
---
tools/perf/util/s390-sample-raw.c | 56 ++++++++++++++++++++++++++++---
1 file changed, 51 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/s390-sample-raw.c b/tools/perf/util/s390-sample-raw.c
index 335217bb532b..4f269ea7c93b 100644
--- a/tools/perf/util/s390-sample-raw.c
+++ b/tools/perf/util/s390-sample-raw.c
@@ -19,12 +19,14 @@
#include <sys/stat.h>
#include <linux/compiler.h>
+#include <linux/err.h>
#include <asm/byteorder.h>
#include "debug.h"
#include "session.h"
#include "evlist.h"
#include "color.h"
+#include "hashmap.h"
#include "sample-raw.h"
#include "s390-cpumcf-kernel.h"
#include "util/pmu.h"
@@ -132,8 +134,8 @@ static int get_counterset_start(int setnr)
}
struct get_counter_name_data {
- int wanted;
- char *result;
+ long wanted;
+ const char *result;
};
static int get_counter_name_callback(void *vdata, struct pmu_event_info *info)
@@ -151,12 +153,22 @@ static int get_counter_name_callback(void *vdata, struct pmu_event_info *info)
rc = sscanf(event_str, "event=%x", &event_nr);
if (rc == 1 && event_nr == data->wanted) {
- data->result = strdup(info->name);
+ data->result = info->name;
return 1; /* Terminate the search. */
}
return 0;
}
+static size_t get_counter_name_hash_fn(long key, void *ctx __maybe_unused)
+{
+ return key;
+}
+
+static bool get_counter_name_hashmap_equal_fn(long key1, long key2, void *ctx __maybe_unused)
+{
+ return key1 == key2;
+}
+
/* Scan the PMU and extract the logical name of a counter from the event. Input
* is the counter set and counter number with in the set. Construct the event
* number and use this as key. If they match return the name of this counter.
@@ -164,17 +176,51 @@ static int get_counter_name_callback(void *vdata, struct pmu_event_info *info)
*/
static char *get_counter_name(int set, int nr, struct perf_pmu *pmu)
{
+ static struct hashmap *cache;
+ static struct perf_pmu *cache_pmu;
+ long cache_key = get_counterset_start(set) + nr;
struct get_counter_name_data data = {
- .wanted = get_counterset_start(set) + nr,
+ .wanted = cache_key,
.result = NULL,
};
+ char *result = NULL;
if (!pmu)
return NULL;
+ if (cache_pmu == pmu && hashmap__find(cache, cache_key, &result))
+ return strdup(result);
+
perf_pmu__for_each_event(pmu, /*skip_duplicate_pmus=*/ true,
&data, get_counter_name_callback);
- return data.result;
+
+ if (data.result)
+ result = strdup(data.result);
+
+ if (cache_pmu == NULL) {
+ struct hashmap *tmp = hashmap__new(get_counter_name_hash_fn,
+ get_counter_name_hashmap_equal_fn,
+ /*ctx=*/NULL);
+
+ if (!IS_ERR(cache)) {
+ cache = tmp;
+ cache_pmu = pmu;
+ }
+ }
+
+ if (cache_pmu == pmu) {
+ char *old_value = NULL, *new_value = strdup(result);
+
+ if (new_value) {
+ hashmap__set(cache, cache_key, new_value, /*old_key=*/NULL, &old_value);
+ /*
+ * Free in case of a race, but resizing would be broken
+ * in that case.
+ */
+ free(old_value);
+ }
+ }
+ return result;
}
static void s390_cpumcfdg_dump(struct perf_pmu *pmu, struct perf_sample *sample)
--
2.51.1.930.gacf6e81ea2-googHi Ian,
On Fri, Oct 31, 2025 at 09:12:13AM -0700, Ian Rogers wrote:
> Searching all event names is slower now that legacy names are
> included. Add a cache to avoid long iterative searches.
>
> Reported-by: Thomas Richter <tmricht@linux.ibm.com>
> Closes: https://lore.kernel.org/linux-perf-users/09943f4f-516c-4b93-877c-e4a64ed61d38@linux.ibm.com/
> Signed-off-by: Ian Rogers <irogers@google.com>
> Tested-by: Thomas Richter <tmricht@linux.ibm.com>
> ---
> v3: Fix minor comment typo, add Thomas' tag.
> v2: Small tweak to the cache_key, just make it match the wanted event value.
> ---
> tools/perf/util/s390-sample-raw.c | 56 ++++++++++++++++++++++++++++---
> 1 file changed, 51 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/s390-sample-raw.c b/tools/perf/util/s390-sample-raw.c
> index 335217bb532b..4f269ea7c93b 100644
> --- a/tools/perf/util/s390-sample-raw.c
> +++ b/tools/perf/util/s390-sample-raw.c
> @@ -19,12 +19,14 @@
>
> #include <sys/stat.h>
> #include <linux/compiler.h>
> +#include <linux/err.h>
> #include <asm/byteorder.h>
>
> #include "debug.h"
> #include "session.h"
> #include "evlist.h"
> #include "color.h"
> +#include "hashmap.h"
> #include "sample-raw.h"
> #include "s390-cpumcf-kernel.h"
> #include "util/pmu.h"
> @@ -132,8 +134,8 @@ static int get_counterset_start(int setnr)
> }
>
> struct get_counter_name_data {
> - int wanted;
> - char *result;
> + long wanted;
> + const char *result;
> };
>
> static int get_counter_name_callback(void *vdata, struct pmu_event_info *info)
> @@ -151,12 +153,22 @@ static int get_counter_name_callback(void *vdata, struct pmu_event_info *info)
>
> rc = sscanf(event_str, "event=%x", &event_nr);
> if (rc == 1 && event_nr == data->wanted) {
> - data->result = strdup(info->name);
> + data->result = info->name;
> return 1; /* Terminate the search. */
> }
> return 0;
> }
>
> +static size_t get_counter_name_hash_fn(long key, void *ctx __maybe_unused)
> +{
> + return key;
> +}
> +
> +static bool get_counter_name_hashmap_equal_fn(long key1, long key2, void *ctx __maybe_unused)
> +{
> + return key1 == key2;
> +}
> +
> /* Scan the PMU and extract the logical name of a counter from the event. Input
> * is the counter set and counter number with in the set. Construct the event
> * number and use this as key. If they match return the name of this counter.
> @@ -164,17 +176,51 @@ static int get_counter_name_callback(void *vdata, struct pmu_event_info *info)
> */
> static char *get_counter_name(int set, int nr, struct perf_pmu *pmu)
> {
> + static struct hashmap *cache;
> + static struct perf_pmu *cache_pmu;
> + long cache_key = get_counterset_start(set) + nr;
> struct get_counter_name_data data = {
> - .wanted = get_counterset_start(set) + nr,
> + .wanted = cache_key,
> .result = NULL,
> };
> + char *result = NULL;
>
> if (!pmu)
> return NULL;
>
> + if (cache_pmu == pmu && hashmap__find(cache, cache_key, &result))
> + return strdup(result);
> +
> perf_pmu__for_each_event(pmu, /*skip_duplicate_pmus=*/ true,
> &data, get_counter_name_callback);
> - return data.result;
> +
> + if (data.result)
> + result = strdup(data.result);
> +
> + if (cache_pmu == NULL) {
> + struct hashmap *tmp = hashmap__new(get_counter_name_hash_fn,
> + get_counter_name_hashmap_equal_fn,
> + /*ctx=*/NULL);
> +
> + if (!IS_ERR(cache)) {
Shouldn't it be if (!IS_ERR(tmp)) ?
And it's not released.
Thanks,
Namhyung
> + cache = tmp;
> + cache_pmu = pmu;
> + }
> + }
> +
> + if (cache_pmu == pmu) {
> + char *old_value = NULL, *new_value = strdup(result);
> +
> + if (new_value) {
> + hashmap__set(cache, cache_key, new_value, /*old_key=*/NULL, &old_value);
> + /*
> + * Free in case of a race, but resizing would be broken
> + * in that case.
> + */
> + free(old_value);
> + }
> + }
> + return result;
> }
>
> static void s390_cpumcfdg_dump(struct perf_pmu *pmu, struct perf_sample *sample)
> --
> 2.51.1.930.gacf6e81ea2-goog
>
On Fri, Oct 31, 2025 at 11:39 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Ian,
>
> On Fri, Oct 31, 2025 at 09:12:13AM -0700, Ian Rogers wrote:
> > Searching all event names is slower now that legacy names are
> > included. Add a cache to avoid long iterative searches.
> >
> > Reported-by: Thomas Richter <tmricht@linux.ibm.com>
> > Closes: https://lore.kernel.org/linux-perf-users/09943f4f-516c-4b93-877c-e4a64ed61d38@linux.ibm.com/
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > Tested-by: Thomas Richter <tmricht@linux.ibm.com>
> > ---
> > v3: Fix minor comment typo, add Thomas' tag.
> > v2: Small tweak to the cache_key, just make it match the wanted event value.
> > ---
> > tools/perf/util/s390-sample-raw.c | 56 ++++++++++++++++++++++++++++---
> > 1 file changed, 51 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/perf/util/s390-sample-raw.c b/tools/perf/util/s390-sample-raw.c
> > index 335217bb532b..4f269ea7c93b 100644
> > --- a/tools/perf/util/s390-sample-raw.c
> > +++ b/tools/perf/util/s390-sample-raw.c
> > @@ -19,12 +19,14 @@
> >
> > #include <sys/stat.h>
> > #include <linux/compiler.h>
> > +#include <linux/err.h>
> > #include <asm/byteorder.h>
> >
> > #include "debug.h"
> > #include "session.h"
> > #include "evlist.h"
> > #include "color.h"
> > +#include "hashmap.h"
> > #include "sample-raw.h"
> > #include "s390-cpumcf-kernel.h"
> > #include "util/pmu.h"
> > @@ -132,8 +134,8 @@ static int get_counterset_start(int setnr)
> > }
> >
> > struct get_counter_name_data {
> > - int wanted;
> > - char *result;
> > + long wanted;
> > + const char *result;
> > };
> >
> > static int get_counter_name_callback(void *vdata, struct pmu_event_info *info)
> > @@ -151,12 +153,22 @@ static int get_counter_name_callback(void *vdata, struct pmu_event_info *info)
> >
> > rc = sscanf(event_str, "event=%x", &event_nr);
> > if (rc == 1 && event_nr == data->wanted) {
> > - data->result = strdup(info->name);
> > + data->result = info->name;
> > return 1; /* Terminate the search. */
> > }
> > return 0;
> > }
> >
> > +static size_t get_counter_name_hash_fn(long key, void *ctx __maybe_unused)
> > +{
> > + return key;
> > +}
> > +
> > +static bool get_counter_name_hashmap_equal_fn(long key1, long key2, void *ctx __maybe_unused)
> > +{
> > + return key1 == key2;
> > +}
> > +
> > /* Scan the PMU and extract the logical name of a counter from the event. Input
> > * is the counter set and counter number with in the set. Construct the event
> > * number and use this as key. If they match return the name of this counter.
> > @@ -164,17 +176,51 @@ static int get_counter_name_callback(void *vdata, struct pmu_event_info *info)
> > */
> > static char *get_counter_name(int set, int nr, struct perf_pmu *pmu)
> > {
> > + static struct hashmap *cache;
> > + static struct perf_pmu *cache_pmu;
> > + long cache_key = get_counterset_start(set) + nr;
> > struct get_counter_name_data data = {
> > - .wanted = get_counterset_start(set) + nr,
> > + .wanted = cache_key,
> > .result = NULL,
> > };
> > + char *result = NULL;
> >
> > if (!pmu)
> > return NULL;
> >
> > + if (cache_pmu == pmu && hashmap__find(cache, cache_key, &result))
> > + return strdup(result);
> > +
> > perf_pmu__for_each_event(pmu, /*skip_duplicate_pmus=*/ true,
> > &data, get_counter_name_callback);
> > - return data.result;
> > +
> > + if (data.result)
> > + result = strdup(data.result);
> > +
> > + if (cache_pmu == NULL) {
> > + struct hashmap *tmp = hashmap__new(get_counter_name_hash_fn,
> > + get_counter_name_hashmap_equal_fn,
> > + /*ctx=*/NULL);
> > +
> > + if (!IS_ERR(cache)) {
>
> Shouldn't it be if (!IS_ERR(tmp)) ?
Yeah, that's me trying to make the code less racy. Will fix in v4.
> And it's not released.
It's just a global leak for the sake of performance, similar to pmus, ...
Thanks,
Ian
> Thanks,
> Namhyung
>
> > + cache = tmp;
> > + cache_pmu = pmu;
> > + }
> > + }
> > +
> > + if (cache_pmu == pmu) {
> > + char *old_value = NULL, *new_value = strdup(result);
> > +
> > + if (new_value) {
> > + hashmap__set(cache, cache_key, new_value, /*old_key=*/NULL, &old_value);
> > + /*
> > + * Free in case of a race, but resizing would be broken
> > + * in that case.
> > + */
> > + free(old_value);
> > + }
> > + }
> > + return result;
> > }
> >
> > static void s390_cpumcfdg_dump(struct perf_pmu *pmu, struct perf_sample *sample)
> > --
> > 2.51.1.930.gacf6e81ea2-goog
> >
© 2016 - 2026 Red Hat, Inc.