Received-SPF: pass (google.com: domain of linux-kernel+bounces-1543-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <14957171-6abd-4fd6-9e05-669b4945c0df@linux.intel.com>
Date: Fri, 15 Dec 2023 13:53:12 -0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] perf top: Use evsel's cpus to replace user_requested_cpus
Content-Language: en-US
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>, Ian Rogers <irogers@google.com>,
 Namhyung Kim <namhyung@kernel.org>, maz@kernel.org, marcan@marcan.st,
 linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
References: <20231208210855.407580-1-kan.liang@linux.intel.com>
 <ZXd7ZuxbNNsjAyqm@kernel.org>
 <07677ab2-c29b-499b-b473-f7535fb27a8c@linux.intel.com>
 <CAM9d7ci-VVhubefMqkSQgK-B2e2z4QU1=TLJtC49wbWW=VNc8g@mail.gmail.com>
 <CAP-5=fVd-0aSovYVsOmTo2dfKb5_PHz1KV7ePipi35_JbfJ6qQ@mail.gmail.com>
 <ZXim6U5251q0_bB2@FVFF77S0Q05N.cambridge.arm.com>
 <ZXxyanyZgWBTOnoK@kernel.org>
 <4658ca16-9749-434e-9296-3893aa2a34da@linux.intel.com>
 <ZXyaZjwrpy6toGf6@kernel.org>
From: "Liang, Kan" <kan.liang@linux.intel.com>
In-Reply-To: <ZXyaZjwrpy6toGf6@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


On 2023-12-15 1:26 p.m., Arnaldo Carvalho de Melo wrote:
> Em Fri, Dec 15, 2023 at 12:59:22PM -0500, Liang, Kan escreveu:
>> On 2023-12-15 10:36 a.m., Arnaldo Carvalho de Melo wrote:
>>
>> #perf report --header-only | grep event
>> # event : name = cpu_atom/cycles:P/, , id = { 7360, 7361, 7362, 7363,
>> 7364, 7365, 7366, 7367, 7368, 7369 }, type = 0 (PERF_TYPE_HARDWARE),
>> size = 136, config = 0xa00000000, { sample_period, sample_freq } = 3000,
>> sample_type = IP|TID|TIME|PERIOD|IDENTIFIER, read_format = ID|LOST,
>> disabled = 1, inherit = 1, freq = 1, enable_on_exec = 1, precise_ip = 3,
>> sample_id_all = 1
>> # event : name = cpu_core/cycles:P/, , id = { 7370, 7371, 7372, 7373,
>> 7374, 7375, 7376, 7377, 7378, 7379, 7380, 7381 }, type = 0
>> (PERF_TYPE_HARDWARE), size = 136, config = 0x400000000, { sample_period,
>> sample_freq } = 3000, sample_type = IP|TID|TIME|PERIOD|IDENTIFIER,
>> read_format = ID|LOST, disabled = 1, inherit = 1, freq = 1,
>> enable_on_exec = 1, precise_ip = 3, sample_id_all = 1
>>
>> I think we should move all the modifiers after the "/". The below patch
>> can fix it.
>>
>> https://lore.kernel.org/lkml/20231215175455.1300261-1-kan.liang@linux.intel.com/
> 
> Right, I implemented it in a slightly different way, but end result
> should be the same:
> 
> From 5dd1b7ab1ba69ebb8e070923dcc214b7b489ffc2 Mon Sep 17 00:00:00 2001
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date: Fri, 15 Dec 2023 15:23:30 -0300
> Subject: [PATCH 1/1] perf evlist: Move event attributes to after the / when
>  uniquefying using the PMU name
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Looks good to me and verified.

Tested-by: Kan Liang <kan.liang@linux.intel.com>

Thanks,
Kan
> ---
>  tools/perf/util/evlist.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 6f0892803c2249af..3a9505c99490b372 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -2522,7 +2522,7 @@ void evlist__warn_user_requested_cpus(struct evlist *evlist, const char *cpu_lis
>  void evlist__uniquify_name(struct evlist *evlist)
>  {
>  	struct evsel *pos;
> -	char *new_name;
> +	char *new_name, *attributes;
>  	int ret;
>  
>  	if (perf_pmus__num_core_pmus() == 1)
> @@ -2535,8 +2535,16 @@ void evlist__uniquify_name(struct evlist *evlist)
>  		if (strchr(pos->name, '/'))
>  			continue;
>  
> -		ret = asprintf(&new_name, "%s/%s/",
> -			       pos->pmu_name, pos->name);
> +		attributes = strchr(pos->name, ':');
> +		if (attributes)
> +			*attributes = '\0';
> +
> +		ret = asprintf(&new_name, "%s/%s/%s",
> +			       pos->pmu_name, pos->name, attributes ? attributes + 1 : "");
> +
> +		if (attributes)
> +			*attributes = ':';
> +
>  		if (ret) {
>  			free(pos->name);
>  			pos->name = new_name;