Received-SPF: pass (google.com: domain of linux-kernel+bounces-195312-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <53c7cbf0-ebf9-455b-bcec-d61830c7999b@gmail.com>
Date: Thu, 30 May 2024 20:43:32 +0900
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] perf stat: allocation check when calculating cache
 instance ID
To: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>,
 Arnaldo Carvalho de Melo <acme@kernel.org>,
 Mark Rutland <mark.rutland@arm.com>,
 Alexander Shishkin <alexander.shishkin@linux.intel.com>,
 Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
 Adrian Hunter <adrian.hunter@intel.com>,
 "Liang, Kan" <kan.liang@linux.intel.com>,
 Gautham Shenoy <gautham.shenoy@amd.com>,
 K Prateek Nayak <kprateek.nayak@amd.com>, skhan@linuxfoundation.org,
 Austin Kim <austindh.kim@gmail.com>, shjy180909@gmail.com,
 linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
 linux-kernel-mentees@lists.linuxfoundation.org
References: <20240527090300.17864-1-yskelg@gmail.com>
 <CAM9d7cjhwO99e0GLKf54V+1g7cdppL3hwjpgB3cdid7YzytkMA@mail.gmail.com>
Content-Language: en-US
From: Yunseong Kim <yskelg@gmail.com>
In-Reply-To: <CAM9d7cjhwO99e0GLKf54V+1g7cdppL3hwjpgB3cdid7YzytkMA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


On 5/29/24 3:56 오전, Namhyung Kim wrote:
> Hello Yunseong,
> 
> On Mon, May 27, 2024 at 2:06 AM <yskelg@gmail.com> wrote:
>>
>> From: Yunseong Kim <yskelg@gmail.com>
>>
>> Adds an allocation check for cpu_map before perf_cpu_map__min() accessing
> 
> I think perf_cpu_map__min() works well with NULL.
> 
> Thanks,
> Namhyung

Thank you for your code review Namhyung.

I'll try to find a more useful improvement and patch it next time!

Warm Regards,
Yunseong Kim

>>
>> Signed-off-by: Yunseong Kim <yskelg@gmail.com>
>> ---
>>  tools/perf/builtin-stat.c | 24 +++++++++++++-----------
>>  1 file changed, 13 insertions(+), 11 deletions(-)
>>
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index 35f79b48e8dc..1f238824abb2 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -1291,20 +1291,22 @@ static struct option stat_options[] = {
>>   */
>>  static int cpu__get_cache_id_from_map(struct perf_cpu cpu, char *map)
>>  {
>> -       int id;
>> +       int id = cpu.cpu;
>>         struct perf_cpu_map *cpu_map = perf_cpu_map__new(map);
>>
>> -       /*
>> -        * If the map contains no CPU, consider the current CPU to
>> -        * be the first online CPU in the cache domain else use the
>> -        * first online CPU of the cache domain as the ID.
>> -        */
>> -       id = perf_cpu_map__min(cpu_map).cpu;
>> -       if (id == -1)
>> -               id = cpu.cpu;
>> +       if (cpu_map) {
>> +               /*
>> +                * If the map contains no CPU, consider the current CPU to
>> +                * be the first online CPU in the cache domain else use the
>> +                * first online CPU of the cache domain as the ID.
>> +                */
>> +               id = perf_cpu_map__min(cpu_map).cpu;
>> +               if (id == -1)
>> +                       id = cpu.cpu;
>>
>> -       /* Free the perf_cpu_map used to find the cache ID */
>> -       perf_cpu_map__put(cpu_map);
>> +               /* Free the perf_cpu_map used to find the cache ID */
>> +               perf_cpu_map__put(cpu_map);
>> +       }
>>
>>         return id;
>>  }
>> --
>> 2.34.1
>>