From: Ian Rogers <irogers@google.com>
To: irogers@google.com, acme@kernel.org, adrian.hunter@intel.com,
james.clark@linaro.org, jolsa@kernel.org, mingo@redhat.com,
namhyung@kernel.org, peterz@infradead.org
Cc: alexander.shishkin@linux.intel.com, alexei.starovoitov@gmail.com,
andrii@kernel.org, ast@kernel.org, bpf@vger.kernel.org,
daniel@iogearbox.net, eddyz87@gmail.com, haoluo@google.com,
john.fastabend@gmail.com, kpsingh@kernel.org,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
martin.lau@linux.dev, memxor@gmail.com, sdf@fomichev.me,
song@kernel.org, yonghong.song@linux.dev
Subject: [PATCH v2 4/4] perf hashmap: Remove errptr usage from hashmap
Date: Sat, 21 Mar 2026 17:58:23 -0700 [thread overview]
Message-ID: <20260322005823.981079-5-irogers@google.com> (raw)
In-Reply-To: <20260322005823.981079-1-irogers@google.com>
The hashmap implementation in tools/perf/util was using ERR_PTR to
return errors from perf_hashmap__new. This is non-standard for most of
the perf codebase which prefers NULL on error and setting errno. As
such it was a frequent source of bugs:
commit d05073adda0f ("perf trace: Avoid an ERR_PTR in syscall_stats")
commit 96f202eab813 ("perf trace: Fix IS_ERR() vs NULL check bug")
commit 9f3c16a430e8 ("perf expr: Fix return value of ids__new()")
commit 4d4d00dd321f ("perf tools: Update copy of libbpf's hashmap.c")
Remove the dependency on linux/err.h in hashmap.c. Update
perf_hashmap__new to set errno = ENOMEM and return NULL on failure.
Update perf_hashmap__free to check for NULL instead of using
IS_ERR_OR_NULL.
Update all callers of perf_hashmap__new to check for NULL. In places
where IS_ERR_OR_NULL was used on hashmap pointers, switch to regular
NULL checks.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-trace.c | 2 +-
tools/perf/ui/browsers/annotate.c | 4 ++--
tools/perf/util/expr.c | 4 ++--
tools/perf/util/fncache.c | 10 +++++++---
tools/perf/util/hashmap.c | 9 +++++----
tools/perf/util/pmu.c | 6 ++++++
tools/perf/util/s390-sample-raw.c | 2 +-
tools/perf/util/stat.c | 2 +-
8 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 88b2fac16457..7b186c32cb0d 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1566,7 +1566,7 @@ static struct perf_hashmap *alloc_syscall_stats(void)
{
struct perf_hashmap *result = perf_hashmap__new(syscall_id_hash, syscall_id_equal, NULL);
- return IS_ERR(result) ? NULL : result;
+ return result;
}
static void delete_syscall_stats(struct perf_hashmap *syscall_stats)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index c61415295dda..5c0656421084 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -143,7 +143,7 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
if (!browser->navkeypressed)
ops.width += 1;
- if (!IS_ERR_OR_NULL(ab->type_hash))
+ if (ab->type_hash)
apd.type_hash = ab->type_hash;
annotation_line__write(al, notes, &ops, &apd);
@@ -1248,7 +1248,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
debuginfo__delete(browser.dbg);
- if (!IS_ERR_OR_NULL(browser.type_hash)) {
+ if (browser.type_hash) {
struct perf_hashmap_entry *cur;
size_t bkt;
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index dc10c335e378..f96812bae318 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -68,7 +68,7 @@ struct perf_hashmap *ids__new(void)
struct perf_hashmap *hash;
hash = perf_hashmap__new(key_hash, key_equal, NULL);
- if (IS_ERR(hash))
+ if (!hash)
return NULL;
return hash;
}
@@ -296,7 +296,7 @@ struct expr_parse_ctx *expr__ctx_new(void)
return NULL;
ctx->ids = perf_hashmap__new(key_hash, key_equal, NULL);
- if (IS_ERR(ctx->ids)) {
+ if (!ctx->ids) {
free(ctx);
return NULL;
}
diff --git a/tools/perf/util/fncache.c b/tools/perf/util/fncache.c
index 9c49a914e784..0970b0bbd1d5 100644
--- a/tools/perf/util/fncache.c
+++ b/tools/perf/util/fncache.c
@@ -36,9 +36,10 @@ static struct perf_hashmap *fncache__get(void)
static bool lookup_fncache(const char *name, bool *res)
{
+ struct perf_hashmap *map = fncache__get();
long val;
- if (!perf_hashmap__find(fncache__get(), name, &val))
+ if (!map || !perf_hashmap__find(map, name, &val))
return false;
*res = (val != 0);
@@ -47,11 +48,14 @@ static bool lookup_fncache(const char *name, bool *res)
static void update_fncache(const char *name, bool res)
{
+ struct perf_hashmap *map = fncache__get();
char *old_key = NULL, *key = strdup(name);
- if (key) {
- perf_hashmap__set(fncache__get(), key, res, &old_key, /*old_value*/NULL);
+ if (map && key) {
+ perf_hashmap__set(map, key, res, &old_key, /*old_value*/NULL);
free(old_key);
+ } else {
+ free(key);
}
}
diff --git a/tools/perf/util/hashmap.c b/tools/perf/util/hashmap.c
index d90ef4ed384d..147e011a547f 100644
--- a/tools/perf/util/hashmap.c
+++ b/tools/perf/util/hashmap.c
@@ -10,7 +10,6 @@
#include <stdio.h>
#include <string.h>
#include <errno.h>
-#include <linux/err.h>
#include "hashmap.h"
/* make sure libbpf doesn't use kernel-only integer typedefs */
@@ -55,8 +54,10 @@ struct perf_hashmap *perf_hashmap__new(perf_hashmap_hash_fn hash_fn,
{
struct perf_hashmap *map = malloc(sizeof(struct perf_hashmap));
- if (!map)
- return ERR_PTR(-ENOMEM);
+ if (!map) {
+ errno = ENOMEM;
+ return NULL;
+ }
perf_hashmap__init(map, hash_fn, equal_fn, ctx);
return map;
}
@@ -76,7 +77,7 @@ void perf_hashmap__clear(struct perf_hashmap *map)
void perf_hashmap__free(struct perf_hashmap *map)
{
- if (IS_ERR_OR_NULL(map))
+ if (!map)
return;
perf_hashmap__clear(map);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index ace354b23501..01a7c0098797 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1305,6 +1305,12 @@ struct perf_pmu *perf_pmu__create_placeholder_core_pmu(struct list_head *core_pm
INIT_LIST_HEAD(&pmu->format);
pmu->aliases = perf_hashmap__new(aliases__hash, aliases__equal, /*ctx=*/ NULL);
+ if (!pmu->aliases) {
+ perf_cpu_map__put(pmu->cpus);
+ free((char *)pmu->name);
+ free(pmu);
+ return NULL;
+ }
INIT_LIST_HEAD(&pmu->caps);
list_add_tail(&pmu->list, core_pmus);
return pmu;
diff --git a/tools/perf/util/s390-sample-raw.c b/tools/perf/util/s390-sample-raw.c
index 80a5535089e0..52ab84e53173 100644
--- a/tools/perf/util/s390-sample-raw.c
+++ b/tools/perf/util/s390-sample-raw.c
@@ -201,7 +201,7 @@ static char *get_counter_name(int set, int nr, struct perf_pmu *pmu)
get_counter_name_perf_hashmap_equal_fn,
/*ctx=*/NULL);
- if (!IS_ERR(tmp)) {
+ if (tmp) {
cache = tmp;
cache_pmu = pmu;
}
diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
index 81b266c5d4f5..1359a1f81562 100644
--- a/tools/perf/util/stat.c
+++ b/tools/perf/util/stat.c
@@ -320,7 +320,7 @@ static int check_per_pkg(struct evsel *counter, struct perf_counts_values *vals,
if (!mask) {
mask = perf_hashmap__new(pkg_id_hash, pkg_id_equal, NULL);
- if (IS_ERR(mask))
+ if (!mask)
return -ENOMEM;
counter->per_pkg_mask = mask;
--
2.53.0.959.g497ff81fa9-goog
prev parent reply other threads:[~2026-03-22 0:58 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-21 2:44 [PATCH v1 1/2] libbpf: Fix strict aliasing violations in hashmap Ian Rogers
2026-03-21 2:44 ` [PATCH v1 2/2] perf tools: " Ian Rogers
2026-03-21 12:37 ` [PATCH v1 1/2] libbpf: " sun jian
2026-03-21 15:40 ` Yonghong Song
2026-03-21 17:36 ` Kumar Kartikeya Dwivedi
2026-03-21 19:49 ` Alexei Starovoitov
2026-03-21 23:04 ` Ian Rogers
2026-03-21 23:08 ` Alexei Starovoitov
2026-03-21 23:10 ` Ian Rogers
2026-03-22 0:58 ` [PATCH v2 0/4] perf hashmap: Separate perf's hashmap code from libbpf Ian Rogers
2026-03-22 0:58 ` [PATCH v2 1/4] perf build: Don't check difference of perf and libbpf hashmap Ian Rogers
2026-03-22 0:58 ` [PATCH v2 2/4] perf hashmap: Rename hashmap to perf_hashmap to avoid libbpf conflict Ian Rogers
2026-03-22 0:58 ` [PATCH v2 3/4] perf hashmap: Fix strict aliasing violations in hashmap Ian Rogers
2026-03-22 0:58 ` Ian Rogers [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260322005823.981079-5-irogers@google.com \
--to=irogers@google.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=haoluo@google.com \
--cc=james.clark@linaro.org \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=sdf@fomichev.me \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox