Re: [PATCH v4 3/4] perf parse-events: Set is_pmu_core for legacy hardware events

[Namhyung Kim]

On Tue, Jan 07, 2025 at 09:34:27PM -0800, Ian Rogers wrote:
> Also set the CPU map to all online CPU maps. This is done so the
> behavior of legacy hardware and hardware cache events better matches
> that of sysfs and json events during
> __perf_evlist__propagate_maps. Fix missing cpumap put in "Synthesize
> attr update" test.
> 
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
>  tools/perf/tests/event_update.c |  1 +
>  tools/perf/util/parse-events.c  | 37 ++++++++++++++++++++-------------
>  2 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
> index d6b4ce3ef4ee..9301fde11366 100644
> --- a/tools/perf/tests/event_update.c
> +++ b/tools/perf/tests/event_update.c
> @@ -109,6 +109,7 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes
>  	TEST_ASSERT_VAL("failed to synthesize attr update name",
>  			!perf_event__synthesize_event_update_name(&tmp.tool, evsel, process_event_name));
>  
> +	perf_cpu_map__put(evsel->core.own_cpus);
>  	evsel->core.own_cpus = perf_cpu_map__new("1,2,3");

Looks like it needs to be in a separate commit.

Thanks,
Namhyung

>  
>  	TEST_ASSERT_VAL("failed to synthesize attr update cpus",
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 1e23faa364b1..4bda4141e9e7 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -231,21 +231,30 @@ __add_event(struct list_head *list, int *idx,
>  	    struct perf_cpu_map *cpu_list, u64 alternate_hw_config)
>  {
>  	struct evsel *evsel;
> -	struct perf_cpu_map *cpus = perf_cpu_map__is_empty(cpu_list) && pmu ? pmu->cpus : cpu_list;
> +	bool is_pmu_core;
> +	struct perf_cpu_map *cpus;
>  
> -	cpus = perf_cpu_map__get(cpus);
> -	if (pmu)
> +	if (pmu) {
> +		is_pmu_core = pmu->is_core;
> +		cpus = perf_cpu_map__get(perf_cpu_map__is_empty(cpu_list) ? pmu->cpus : cpu_list);
>  		perf_pmu__warn_invalid_formats(pmu);
> -
> -	if (pmu && (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX)) {
> -		perf_pmu__warn_invalid_config(pmu, attr->config, name,
> -					      PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> -		perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> -					      PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> -		perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> -					      PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> -		perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> -					      PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> +		if (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX) {
> +			perf_pmu__warn_invalid_config(pmu, attr->config, name,
> +						PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> +			perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> +						PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> +			perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> +						PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> +			perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> +						PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> +		}
> +	} else {
> +		is_pmu_core = (attr->type == PERF_TYPE_HARDWARE ||
> +			       attr->type == PERF_TYPE_HW_CACHE);
> +		if (perf_cpu_map__is_empty(cpu_list))
> +			cpus = is_pmu_core ? perf_cpu_map__new_online_cpus() : NULL;
> +		else
> +			cpus = perf_cpu_map__get(cpu_list);
>  	}
>  	if (init_attr)
>  		event_attr_init(attr);
> @@ -260,7 +269,7 @@ __add_event(struct list_head *list, int *idx,
>  	evsel->core.cpus = cpus;
>  	evsel->core.own_cpus = perf_cpu_map__get(cpus);
>  	evsel->core.requires_cpu = pmu ? pmu->is_uncore : false;
> -	evsel->core.is_pmu_core = pmu ? pmu->is_core : false;
> +	evsel->core.is_pmu_core = is_pmu_core;
>  	evsel->auto_merge_stats = auto_merge_stats;
>  	evsel->pmu = pmu;
>  	evsel->alternate_hw_config = alternate_hw_config;
> -- 
> 2.47.1.613.gc27f4b7a9f-goog
>