All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/2] perf: move perf_pmus__find_core_pmu() prototype to pmus.h
@ 2025-06-12 16:36 Thomas Falcon
  2025-06-12 16:36 ` [PATCH v4 2/2] perf top: populate PMU capabilities data in perf_env Thomas Falcon
  2025-06-27 18:53 ` [PATCH v4 1/2] perf: move perf_pmus__find_core_pmu() prototype to pmus.h Namhyung Kim
  0 siblings, 2 replies; 5+ messages in thread
From: Thomas Falcon @ 2025-06-12 16:36 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang
  Cc: linux-kernel, linux-perf-users, Thomas Falcon

perf_pmus__find_core_pmu() is implemented in util/pmus.c but its
prototpye is in util/pmu.h. Move it to util/pmus.h.

Suggested-by: Ian Rogers <irogers@google.com>
Signed-off-by: Thomas Falcon <thomas.falcon@intel.com>
---
 tools/perf/util/pmu.h  | 1 -
 tools/perf/util/pmus.h | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index a1fdd6d50c53..d38a63ba4583 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -298,7 +298,6 @@ struct perf_pmu *perf_pmu__lookup(struct list_head *pmus, int dirfd, const char
 				  bool eager_load);
 struct perf_pmu *perf_pmu__create_placeholder_core_pmu(struct list_head *core_pmus);
 void perf_pmu__delete(struct perf_pmu *pmu);
-struct perf_pmu *perf_pmus__find_core_pmu(void);
 
 const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config);
 bool perf_pmu__is_fake(const struct perf_pmu *pmu);
diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h
index 8def20e615ad..d6a8d95af376 100644
--- a/tools/perf/util/pmus.h
+++ b/tools/perf/util/pmus.h
@@ -33,5 +33,6 @@ struct perf_pmu *perf_pmus__add_test_hwmon_pmu(int hwmon_dir,
 					       const char *sysfs_name,
 					       const char *name);
 struct perf_pmu *perf_pmus__fake_pmu(void);
+struct perf_pmu *perf_pmus__find_core_pmu(void);
 
 #endif /* __PMUS_H */
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v4 2/2] perf top: populate PMU capabilities data in perf_env
  2025-06-12 16:36 [PATCH v4 1/2] perf: move perf_pmus__find_core_pmu() prototype to pmus.h Thomas Falcon
@ 2025-06-12 16:36 ` Thomas Falcon
  2025-06-26 16:47   ` Falcon, Thomas
  2025-06-27 18:53 ` [PATCH v4 1/2] perf: move perf_pmus__find_core_pmu() prototype to pmus.h Namhyung Kim
  1 sibling, 1 reply; 5+ messages in thread
From: Thomas Falcon @ 2025-06-12 16:36 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang
  Cc: linux-kernel, linux-perf-users, Thomas Falcon

Calling perf top with branch filters enabled on Intel CPU's
with branch counters logging (A.K.A LBR event logging [1]) support
results in a segfault.

$ perf top  -e '{cpu_core/cpu-cycles/,cpu_core/event=0xc6,umask=0x3,frontend=0x11,name=frontend_retired_dsb_miss/}' -j any,counter
...
Thread 27 "perf" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffafff76c0 (LWP 949003)]
perf_env__find_br_cntr_info (env=0xf66dc0 <perf_env>, nr=0x0, width=0x7fffafff62c0) at util/env.c:653
653			*width = env->cpu_pmu_caps ? env->br_cntr_width :
(gdb) bt
 #0  perf_env__find_br_cntr_info (env=0xf66dc0 <perf_env>, nr=0x0, width=0x7fffafff62c0) at util/env.c:653
 #1  0x00000000005b1599 in symbol__account_br_cntr (branch=0x7fffcc3db580, evsel=0xfea2d0, offset=12, br_cntr=8) at util/annotate.c:345
 #2  0x00000000005b17fb in symbol__account_cycles (addr=5658172, start=5658160, sym=0x7fffcc0ee420, cycles=539, evsel=0xfea2d0, br_cntr=8) at util/annotate.c:389
 #3  0x00000000005b1976 in addr_map_symbol__account_cycles (ams=0x7fffcd7b01d0, start=0x7fffcd7b02b0, cycles=539, evsel=0xfea2d0, br_cntr=8) at util/annotate.c:422
 #4  0x000000000068d57f in hist__account_cycles (bs=0x110d288, al=0x7fffafff6540, sample=0x7fffafff6760, nonany_branch_mode=false, total_cycles=0x0, evsel=0xfea2d0) at util/hist.c:2850
 #5  0x0000000000446216 in hist_iter__top_callback (iter=0x7fffafff6590, al=0x7fffafff6540, single=true, arg=0x7fffffff9e00) at builtin-top.c:737
 #6  0x0000000000689787 in hist_entry_iter__add (iter=0x7fffafff6590, al=0x7fffafff6540, max_stack_depth=127, arg=0x7fffffff9e00) at util/hist.c:1359
 #7  0x0000000000446710 in perf_event__process_sample (tool=0x7fffffff9e00, event=0x110d250, evsel=0xfea2d0, sample=0x7fffafff6760, machine=0x108c968) at builtin-top.c:845
 #8  0x0000000000447735 in deliver_event (qe=0x7fffffffa120, qevent=0x10fc200) at builtin-top.c:1211
 #9  0x000000000064ccae in do_flush (oe=0x7fffffffa120, show_progress=false) at util/ordered-events.c:245
 #10 0x000000000064d005 in __ordered_events__flush (oe=0x7fffffffa120, how=OE_FLUSH__TOP, timestamp=0) at util/ordered-events.c:324
 #11 0x000000000064d0ef in ordered_events__flush (oe=0x7fffffffa120, how=OE_FLUSH__TOP) at util/ordered-events.c:342
 #12 0x00000000004472a9 in process_thread (arg=0x7fffffff9e00) at builtin-top.c:1120
 #13 0x00007ffff6e7dba8 in start_thread (arg=<optimized out>) at pthread_create.c:448
 #14 0x00007ffff6f01b8c in __GI___clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78

The cause is that perf_env__find_br_cntr_info tries to access a
null pointer pmu_caps in the perf_env struct. A similar issue exists
for homogeneous core systems which use the cpu_pmu_caps structure.

Fix this by populating cpu_pmu_caps and pmu_caps structures with
values from sysfs when calling perf top with branch stack sampling
enabled.

[1], LBR event logging introduced here:
https://lore.kernel.org/all/20231025201626.3000228-5-kan.liang@linux.intel.com/

Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Thomas Falcon <thomas.falcon@intel.com>
---
v4: prefer calloc and zfree operations in addition to other
    cleanups suggested by Namhyung and Arnaldo

v3: constify struct perf_pmu *pmu in __perf_env__read_core_pmu_caps()
    use perf_pmus__find_core_pmu() instead of perf_pmus__scan_core(NULL)

v2: update commit message with more meaningful stack trace from
    gdb and indicate that affected systems are limited to CPU's
    with LBR event logging support and that both hybrid and
    non-hybrid core systems are affected.
---
 tools/perf/builtin-top.c |   8 +++
 tools/perf/util/env.c    | 110 +++++++++++++++++++++++++++++++++++++++
 tools/perf/util/env.h    |   1 +
 3 files changed, 119 insertions(+)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index f9f31391bddb..c9d679410591 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1729,6 +1729,14 @@ int cmd_top(int argc, const char **argv)
 	if (opts->branch_stack && callchain_param.enabled)
 		symbol_conf.show_branchflag_count = true;
 
+	if (opts->branch_stack) {
+		status = perf_env__read_core_pmu_caps(&perf_env);
+		if (status) {
+			pr_err("PMU capability data is not available\n");
+			goto out_delete_evlist;
+		}
+	}
+
 	sort__mode = SORT_MODE__TOP;
 	/* display thread wants entries to be collapsed in a different tree */
 	perf_hpp_list.need_collapse = 1;
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 36411749e007..b053d09438af 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -416,6 +416,116 @@ static int perf_env__read_nr_cpus_avail(struct perf_env *env)
 	return env->nr_cpus_avail ? 0 : -ENOENT;
 }
 
+static int __perf_env__read_core_pmu_caps(const struct perf_pmu *pmu,
+					  int *nr_caps, char ***caps,
+					  unsigned int *max_branches,
+					  unsigned int *br_cntr_nr,
+					  unsigned int *br_cntr_width)
+{
+	struct perf_pmu_caps *pcaps = NULL;
+	char *ptr, **tmp;
+	int ret = 0;
+
+	*nr_caps = 0;
+	*caps = NULL;
+
+	if (!pmu->nr_caps)
+		return 0;
+
+	*caps = calloc(pmu->nr_caps, sizeof(char *));
+	if (!*caps)
+		return -ENOMEM;
+
+	tmp = *caps;
+	list_for_each_entry(pcaps, &pmu->caps, list) {
+		if (asprintf(&ptr, "%s=%s", pcaps->name, pcaps->value) < 0) {
+			ret = -ENOMEM;
+			goto error;
+		}
+
+		*tmp++ = ptr;
+
+		if (!strcmp(pcaps->name, "branches"))
+			*max_branches = atoi(pcaps->value);
+		else if (!strcmp(pcaps->name, "branch_counter_nr"))
+			*br_cntr_nr = atoi(pcaps->value);
+		else if (!strcmp(pcaps->name, "branch_counter_width"))
+			*br_cntr_width = atoi(pcaps->value);
+	}
+	*nr_caps = pmu->nr_caps;
+	return 0;
+error:
+	while (tmp-- != *caps)
+		zfree(tmp);
+	zfree(caps);
+	*nr_caps = 0;
+	return ret;
+}
+
+int perf_env__read_core_pmu_caps(struct perf_env *env)
+{
+	struct pmu_caps *pmu_caps;
+	struct perf_pmu *pmu = NULL;
+	int nr_pmu, i = 0, j;
+	int ret;
+
+	nr_pmu = perf_pmus__num_core_pmus();
+
+	if (!nr_pmu)
+		return -ENODEV;
+
+	if (nr_pmu == 1) {
+		pmu = perf_pmus__find_core_pmu();
+		if (!pmu)
+			return -ENODEV;
+		ret = perf_pmu__caps_parse(pmu);
+		if (ret < 0)
+			return ret;
+		return __perf_env__read_core_pmu_caps(pmu, &env->nr_cpu_pmu_caps,
+						      &env->cpu_pmu_caps,
+						      &env->max_branches,
+						      &env->br_cntr_nr,
+						      &env->br_cntr_width);
+	}
+
+	pmu_caps = calloc(nr_pmu, sizeof(*pmu_caps));
+	if (!pmu_caps)
+		return -ENOMEM;
+
+	while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
+		if (perf_pmu__caps_parse(pmu) <= 0)
+			continue;
+		ret = __perf_env__read_core_pmu_caps(pmu, &pmu_caps[i].nr_caps,
+						     &pmu_caps[i].caps,
+						     &pmu_caps[i].max_branches,
+						     &pmu_caps[i].br_cntr_nr,
+						     &pmu_caps[i].br_cntr_width);
+		if (ret)
+			goto error;
+
+		pmu_caps[i].pmu_name = strdup(pmu->name);
+		if (!pmu_caps[i].pmu_name) {
+			ret = -ENOMEM;
+			goto error;
+		}
+		i++;
+	}
+
+	env->nr_pmus_with_caps = nr_pmu;
+	env->pmu_caps = pmu_caps;
+
+	return 0;
+error:
+	for (i = 0; i < nr_pmu; i++) {
+		for (j = 0; j < pmu_caps[i].nr_caps; j++)
+			zfree(&pmu_caps[i].caps[j]);
+		zfree(&pmu_caps[i].caps);
+		zfree(&pmu_caps[i].pmu_name);
+	}
+	zfree(&pmu_caps);
+	return ret;
+}
+
 const char *perf_env__raw_arch(struct perf_env *env)
 {
 	return env && !perf_env__read_arch(env) ? env->arch : "unknown";
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index d90e343cf1fa..135a1f714905 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -152,6 +152,7 @@ struct btf_node;
 
 extern struct perf_env perf_env;
 
+int perf_env__read_core_pmu_caps(struct perf_env *env);
 void perf_env__exit(struct perf_env *env);
 
 int perf_env__kernel_is_64_bit(struct perf_env *env);
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 2/2] perf top: populate PMU capabilities data in perf_env
  2025-06-12 16:36 ` [PATCH v4 2/2] perf top: populate PMU capabilities data in perf_env Thomas Falcon
@ 2025-06-26 16:47   ` Falcon, Thomas
  2025-06-26 17:48     ` Namhyung Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Falcon, Thomas @ 2025-06-26 16:47 UTC (permalink / raw)
  To: alexander.shishkin@linux.intel.com, peterz@infradead.org,
	acme@kernel.org, mingo@redhat.com, mark.rutland@arm.com,
	Hunter, Adrian, namhyung@kernel.org, irogers@google.com,
	jolsa@kernel.org, kan.liang@linux.intel.com
  Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org

Hi, is there anything still needed from me before this can be accepted?

Thanks,
Tom


On Thu, 2025-06-12 at 11:36 -0500, Thomas Falcon wrote:
> Calling perf top with branch filters enabled on Intel CPU's
> with branch counters logging (A.K.A LBR event logging [1]) support
> results in a segfault.
> 
> $ perf top  -e '{cpu_core/cpu-cycles/,cpu_core/event=0xc6,umask=0x3,frontend=0x11,name=frontend_retired_dsb_miss/}' -j any,counter
> ...
> Thread 27 "perf" received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7fffafff76c0 (LWP 949003)]
> perf_env__find_br_cntr_info (env=0xf66dc0 <perf_env>, nr=0x0, width=0x7fffafff62c0) at util/env.c:653
> 653			*width = env->cpu_pmu_caps ? env->br_cntr_width :
> (gdb) bt
>  #0  perf_env__find_br_cntr_info (env=0xf66dc0 <perf_env>, nr=0x0, width=0x7fffafff62c0) at util/env.c:653
>  #1  0x00000000005b1599 in symbol__account_br_cntr (branch=0x7fffcc3db580, evsel=0xfea2d0, offset=12, br_cntr=8) at util/annotate.c:345
>  #2  0x00000000005b17fb in symbol__account_cycles (addr=5658172, start=5658160, sym=0x7fffcc0ee420, cycles=539, evsel=0xfea2d0, br_cntr=8) at util/annotate.c:389
>  #3  0x00000000005b1976 in addr_map_symbol__account_cycles (ams=0x7fffcd7b01d0, start=0x7fffcd7b02b0, cycles=539, evsel=0xfea2d0, br_cntr=8) at util/annotate.c:422
>  #4  0x000000000068d57f in hist__account_cycles (bs=0x110d288, al=0x7fffafff6540, sample=0x7fffafff6760, nonany_branch_mode=false, total_cycles=0x0, evsel=0xfea2d0) at util/hist.c:2850
>  #5  0x0000000000446216 in hist_iter__top_callback (iter=0x7fffafff6590, al=0x7fffafff6540, single=true, arg=0x7fffffff9e00) at builtin-top.c:737
>  #6  0x0000000000689787 in hist_entry_iter__add (iter=0x7fffafff6590, al=0x7fffafff6540, max_stack_depth=127, arg=0x7fffffff9e00) at util/hist.c:1359
>  #7  0x0000000000446710 in perf_event__process_sample (tool=0x7fffffff9e00, event=0x110d250, evsel=0xfea2d0, sample=0x7fffafff6760, machine=0x108c968) at builtin-top.c:845
>  #8  0x0000000000447735 in deliver_event (qe=0x7fffffffa120, qevent=0x10fc200) at builtin-top.c:1211
>  #9  0x000000000064ccae in do_flush (oe=0x7fffffffa120, show_progress=false) at util/ordered-events.c:245
>  #10 0x000000000064d005 in __ordered_events__flush (oe=0x7fffffffa120, how=OE_FLUSH__TOP, timestamp=0) at util/ordered-events.c:324
>  #11 0x000000000064d0ef in ordered_events__flush (oe=0x7fffffffa120, how=OE_FLUSH__TOP) at util/ordered-events.c:342
>  #12 0x00000000004472a9 in process_thread (arg=0x7fffffff9e00) at builtin-top.c:1120
>  #13 0x00007ffff6e7dba8 in start_thread (arg=<optimized out>) at pthread_create.c:448
>  #14 0x00007ffff6f01b8c in __GI___clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78
> 
> The cause is that perf_env__find_br_cntr_info tries to access a
> null pointer pmu_caps in the perf_env struct. A similar issue exists
> for homogeneous core systems which use the cpu_pmu_caps structure.
> 
> Fix this by populating cpu_pmu_caps and pmu_caps structures with
> values from sysfs when calling perf top with branch stack sampling
> enabled.
> 
> [1], LBR event logging introduced here:
> https://lore.kernel.org/all/20231025201626.3000228-5-kan.liang@linux.intel.com/
> 
> Reviewed-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Thomas Falcon <thomas.falcon@intel.com>
> ---
> v4: prefer calloc and zfree operations in addition to other
>     cleanups suggested by Namhyung and Arnaldo
> 
> v3: constify struct perf_pmu *pmu in __perf_env__read_core_pmu_caps()
>     use perf_pmus__find_core_pmu() instead of perf_pmus__scan_core(NULL)
> 
> v2: update commit message with more meaningful stack trace from
>     gdb and indicate that affected systems are limited to CPU's
>     with LBR event logging support and that both hybrid and
>     non-hybrid core systems are affected.
> ---
>  tools/perf/builtin-top.c |   8 +++
>  tools/perf/util/env.c    | 110 +++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/env.h    |   1 +
>  3 files changed, 119 insertions(+)
> 
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index f9f31391bddb..c9d679410591 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1729,6 +1729,14 @@ int cmd_top(int argc, const char **argv)
>  	if (opts->branch_stack && callchain_param.enabled)
>  		symbol_conf.show_branchflag_count = true;
>  
> +	if (opts->branch_stack) {
> +		status = perf_env__read_core_pmu_caps(&perf_env);
> +		if (status) {
> +			pr_err("PMU capability data is not available\n");
> +			goto out_delete_evlist;
> +		}
> +	}
> +
>  	sort__mode = SORT_MODE__TOP;
>  	/* display thread wants entries to be collapsed in a different tree */
>  	perf_hpp_list.need_collapse = 1;
> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
> index 36411749e007..b053d09438af 100644
> --- a/tools/perf/util/env.c
> +++ b/tools/perf/util/env.c
> @@ -416,6 +416,116 @@ static int perf_env__read_nr_cpus_avail(struct perf_env *env)
>  	return env->nr_cpus_avail ? 0 : -ENOENT;
>  }
>  
> +static int __perf_env__read_core_pmu_caps(const struct perf_pmu *pmu,
> +					  int *nr_caps, char ***caps,
> +					  unsigned int *max_branches,
> +					  unsigned int *br_cntr_nr,
> +					  unsigned int *br_cntr_width)
> +{
> +	struct perf_pmu_caps *pcaps = NULL;
> +	char *ptr, **tmp;
> +	int ret = 0;
> +
> +	*nr_caps = 0;
> +	*caps = NULL;
> +
> +	if (!pmu->nr_caps)
> +		return 0;
> +
> +	*caps = calloc(pmu->nr_caps, sizeof(char *));
> +	if (!*caps)
> +		return -ENOMEM;
> +
> +	tmp = *caps;
> +	list_for_each_entry(pcaps, &pmu->caps, list) {
> +		if (asprintf(&ptr, "%s=%s", pcaps->name, pcaps->value) < 0) {
> +			ret = -ENOMEM;
> +			goto error;
> +		}
> +
> +		*tmp++ = ptr;
> +
> +		if (!strcmp(pcaps->name, "branches"))
> +			*max_branches = atoi(pcaps->value);
> +		else if (!strcmp(pcaps->name, "branch_counter_nr"))
> +			*br_cntr_nr = atoi(pcaps->value);
> +		else if (!strcmp(pcaps->name, "branch_counter_width"))
> +			*br_cntr_width = atoi(pcaps->value);
> +	}
> +	*nr_caps = pmu->nr_caps;
> +	return 0;
> +error:
> +	while (tmp-- != *caps)
> +		zfree(tmp);
> +	zfree(caps);
> +	*nr_caps = 0;
> +	return ret;
> +}
> +
> +int perf_env__read_core_pmu_caps(struct perf_env *env)
> +{
> +	struct pmu_caps *pmu_caps;
> +	struct perf_pmu *pmu = NULL;
> +	int nr_pmu, i = 0, j;
> +	int ret;
> +
> +	nr_pmu = perf_pmus__num_core_pmus();
> +
> +	if (!nr_pmu)
> +		return -ENODEV;
> +
> +	if (nr_pmu == 1) {
> +		pmu = perf_pmus__find_core_pmu();
> +		if (!pmu)
> +			return -ENODEV;
> +		ret = perf_pmu__caps_parse(pmu);
> +		if (ret < 0)
> +			return ret;
> +		return __perf_env__read_core_pmu_caps(pmu, &env->nr_cpu_pmu_caps,
> +						      &env->cpu_pmu_caps,
> +						      &env->max_branches,
> +						      &env->br_cntr_nr,
> +						      &env->br_cntr_width);
> +	}
> +
> +	pmu_caps = calloc(nr_pmu, sizeof(*pmu_caps));
> +	if (!pmu_caps)
> +		return -ENOMEM;
> +
> +	while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
> +		if (perf_pmu__caps_parse(pmu) <= 0)
> +			continue;
> +		ret = __perf_env__read_core_pmu_caps(pmu, &pmu_caps[i].nr_caps,
> +						     &pmu_caps[i].caps,
> +						     &pmu_caps[i].max_branches,
> +						     &pmu_caps[i].br_cntr_nr,
> +						     &pmu_caps[i].br_cntr_width);
> +		if (ret)
> +			goto error;
> +
> +		pmu_caps[i].pmu_name = strdup(pmu->name);
> +		if (!pmu_caps[i].pmu_name) {
> +			ret = -ENOMEM;
> +			goto error;
> +		}
> +		i++;
> +	}
> +
> +	env->nr_pmus_with_caps = nr_pmu;
> +	env->pmu_caps = pmu_caps;
> +
> +	return 0;
> +error:
> +	for (i = 0; i < nr_pmu; i++) {
> +		for (j = 0; j < pmu_caps[i].nr_caps; j++)
> +			zfree(&pmu_caps[i].caps[j]);
> +		zfree(&pmu_caps[i].caps);
> +		zfree(&pmu_caps[i].pmu_name);
> +	}
> +	zfree(&pmu_caps);
> +	return ret;
> +}
> +
>  const char *perf_env__raw_arch(struct perf_env *env)
>  {
>  	return env && !perf_env__read_arch(env) ? env->arch : "unknown";
> diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
> index d90e343cf1fa..135a1f714905 100644
> --- a/tools/perf/util/env.h
> +++ b/tools/perf/util/env.h
> @@ -152,6 +152,7 @@ struct btf_node;
>  
>  extern struct perf_env perf_env;
>  
> +int perf_env__read_core_pmu_caps(struct perf_env *env);
>  void perf_env__exit(struct perf_env *env);
>  
>  int perf_env__kernel_is_64_bit(struct perf_env *env);


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 2/2] perf top: populate PMU capabilities data in perf_env
  2025-06-26 16:47   ` Falcon, Thomas
@ 2025-06-26 17:48     ` Namhyung Kim
  0 siblings, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2025-06-26 17:48 UTC (permalink / raw)
  To: Falcon, Thomas
  Cc: alexander.shishkin@linux.intel.com, peterz@infradead.org,
	acme@kernel.org, mingo@redhat.com, mark.rutland@arm.com,
	Hunter, Adrian, irogers@google.com, jolsa@kernel.org,
	kan.liang@linux.intel.com, linux-kernel@vger.kernel.org,
	linux-perf-users@vger.kernel.org

Hello,

On Thu, Jun 26, 2025 at 04:47:18PM +0000, Falcon, Thomas wrote:
> Hi, is there anything still needed from me before this can be accepted?
 
Sorry I just missed it.  I'll add it to the tmp branch for testing.

Thanks,
Namhyung

> 
> On Thu, 2025-06-12 at 11:36 -0500, Thomas Falcon wrote:
> > Calling perf top with branch filters enabled on Intel CPU's
> > with branch counters logging (A.K.A LBR event logging [1]) support
> > results in a segfault.
> > 
> > $ perf top  -e '{cpu_core/cpu-cycles/,cpu_core/event=0xc6,umask=0x3,frontend=0x11,name=frontend_retired_dsb_miss/}' -j any,counter
> > ...
> > Thread 27 "perf" received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 0x7fffafff76c0 (LWP 949003)]
> > perf_env__find_br_cntr_info (env=0xf66dc0 <perf_env>, nr=0x0, width=0x7fffafff62c0) at util/env.c:653
> > 653			*width = env->cpu_pmu_caps ? env->br_cntr_width :
> > (gdb) bt
> >  #0  perf_env__find_br_cntr_info (env=0xf66dc0 <perf_env>, nr=0x0, width=0x7fffafff62c0) at util/env.c:653
> >  #1  0x00000000005b1599 in symbol__account_br_cntr (branch=0x7fffcc3db580, evsel=0xfea2d0, offset=12, br_cntr=8) at util/annotate.c:345
> >  #2  0x00000000005b17fb in symbol__account_cycles (addr=5658172, start=5658160, sym=0x7fffcc0ee420, cycles=539, evsel=0xfea2d0, br_cntr=8) at util/annotate.c:389
> >  #3  0x00000000005b1976 in addr_map_symbol__account_cycles (ams=0x7fffcd7b01d0, start=0x7fffcd7b02b0, cycles=539, evsel=0xfea2d0, br_cntr=8) at util/annotate.c:422
> >  #4  0x000000000068d57f in hist__account_cycles (bs=0x110d288, al=0x7fffafff6540, sample=0x7fffafff6760, nonany_branch_mode=false, total_cycles=0x0, evsel=0xfea2d0) at util/hist.c:2850
> >  #5  0x0000000000446216 in hist_iter__top_callback (iter=0x7fffafff6590, al=0x7fffafff6540, single=true, arg=0x7fffffff9e00) at builtin-top.c:737
> >  #6  0x0000000000689787 in hist_entry_iter__add (iter=0x7fffafff6590, al=0x7fffafff6540, max_stack_depth=127, arg=0x7fffffff9e00) at util/hist.c:1359
> >  #7  0x0000000000446710 in perf_event__process_sample (tool=0x7fffffff9e00, event=0x110d250, evsel=0xfea2d0, sample=0x7fffafff6760, machine=0x108c968) at builtin-top.c:845
> >  #8  0x0000000000447735 in deliver_event (qe=0x7fffffffa120, qevent=0x10fc200) at builtin-top.c:1211
> >  #9  0x000000000064ccae in do_flush (oe=0x7fffffffa120, show_progress=false) at util/ordered-events.c:245
> >  #10 0x000000000064d005 in __ordered_events__flush (oe=0x7fffffffa120, how=OE_FLUSH__TOP, timestamp=0) at util/ordered-events.c:324
> >  #11 0x000000000064d0ef in ordered_events__flush (oe=0x7fffffffa120, how=OE_FLUSH__TOP) at util/ordered-events.c:342
> >  #12 0x00000000004472a9 in process_thread (arg=0x7fffffff9e00) at builtin-top.c:1120
> >  #13 0x00007ffff6e7dba8 in start_thread (arg=<optimized out>) at pthread_create.c:448
> >  #14 0x00007ffff6f01b8c in __GI___clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78
> > 
> > The cause is that perf_env__find_br_cntr_info tries to access a
> > null pointer pmu_caps in the perf_env struct. A similar issue exists
> > for homogeneous core systems which use the cpu_pmu_caps structure.
> > 
> > Fix this by populating cpu_pmu_caps and pmu_caps structures with
> > values from sysfs when calling perf top with branch stack sampling
> > enabled.
> > 
> > [1], LBR event logging introduced here:
> > https://lore.kernel.org/all/20231025201626.3000228-5-kan.liang@linux.intel.com/
> > 
> > Reviewed-by: Ian Rogers <irogers@google.com>
> > Signed-off-by: Thomas Falcon <thomas.falcon@intel.com>
> > ---
> > v4: prefer calloc and zfree operations in addition to other
> >     cleanups suggested by Namhyung and Arnaldo
> > 
> > v3: constify struct perf_pmu *pmu in __perf_env__read_core_pmu_caps()
> >     use perf_pmus__find_core_pmu() instead of perf_pmus__scan_core(NULL)
> > 
> > v2: update commit message with more meaningful stack trace from
> >     gdb and indicate that affected systems are limited to CPU's
> >     with LBR event logging support and that both hybrid and
> >     non-hybrid core systems are affected.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/2] perf: move perf_pmus__find_core_pmu() prototype to pmus.h
  2025-06-12 16:36 [PATCH v4 1/2] perf: move perf_pmus__find_core_pmu() prototype to pmus.h Thomas Falcon
  2025-06-12 16:36 ` [PATCH v4 2/2] perf top: populate PMU capabilities data in perf_env Thomas Falcon
@ 2025-06-27 18:53 ` Namhyung Kim
  1 sibling, 0 replies; 5+ messages in thread
From: Namhyung Kim @ 2025-06-27 18:53 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Kan Liang, Thomas Falcon
  Cc: linux-kernel, linux-perf-users

On Thu, 12 Jun 2025 11:36:58 -0500, Thomas Falcon wrote:
> perf_pmus__find_core_pmu() is implemented in util/pmus.c but its
> prototpye is in util/pmu.h. Move it to util/pmus.h.
> 
> 
Applied to perf-tools-next, thanks!

Best regards,
Namhyung



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-06-27 18:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-12 16:36 [PATCH v4 1/2] perf: move perf_pmus__find_core_pmu() prototype to pmus.h Thomas Falcon
2025-06-12 16:36 ` [PATCH v4 2/2] perf top: populate PMU capabilities data in perf_env Thomas Falcon
2025-06-26 16:47   ` Falcon, Thomas
2025-06-26 17:48     ` Namhyung Kim
2025-06-27 18:53 ` [PATCH v4 1/2] perf: move perf_pmus__find_core_pmu() prototype to pmus.h Namhyung Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.