All of lore.kernel.org
 help / color / mirror / Atom feed
From: Swapnil Sapkal <swapnil.sapkal@amd.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Shrikanth Hegde <sshegde@linux.ibm.com>, <mark.rutland@arm.com>,
	<alexander.shishkin@linux.intel.com>, <jolsa@kernel.org>,
	<adrian.hunter@intel.com>, <thomas.falcon@intel.com>,
	<ashelat@redhat.com>, <yu.c.chen@intel.com>,
	<gautham.shenoy@amd.com>, <ravi.bangoria@amd.com>,
	<linux-kernel@vger.kernel.org>,
	<linux-perf-users@vger.kernel.org>, <peterz@infradead.org>,
	<mingo@redhat.com>, <namhyung@kernel.org>, <irogers@google.com>,
	<james.clark@arm.com>
Subject: Re: [PATCH v1 1/6] perf header: Replace hardcoded max cpus by MAX_NR_CPUS
Date: Wed, 28 Jan 2026 23:59:29 +0530	[thread overview]
Message-ID: <4da60acc-cbce-4cd2-9600-8a151592d122@amd.com> (raw)
In-Reply-To: <aXpSSaMdBhKDRzsY@x1>

Hello Arnaldo,

On 28-01-2026 23:45, Arnaldo Carvalho de Melo wrote:
> On Wed, Jan 28, 2026 at 09:55:05PM +0530, Swapnil Sapkal wrote:
>> Hi Srikanth, Arnaldo,
>>
>> Thank you for reviewing the patches.
>>
>> On 28-01-2026 13:09, Shrikanth Hegde wrote:
>>>
>>>
>>> On 1/28/26 12:19 AM, Swapnil Sapkal wrote:
>>>> cpumask and cpulist from cpu-domain header have hardcoded max_cpus value
>>>> of 1024. Current systems have more cpus than this value. Replace it with
>>>> MAX_NR_CPUS. Also define a macro to represent domain name length.
>>>>
>>>> Fixes: d40c68a49f69 ("perf header: Support CPU DOMAIN relation info")
>>>> Reported-by: Shrikanth Hegde <sshegde@linux.ibm.com>
>>>> Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
>>>> ---
>>>>    tools/perf/util/header.c | 5 +++--
>>>>    1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
>>>> index eefd1cd73b6a..31c3bab1b10a 100644
>>>> --- a/tools/perf/util/header.c
>>>> +++ b/tools/perf/util/header.c
>>>> @@ -76,6 +76,7 @@ static const u64 __perf_magic2    =
>>>> 0x32454c4946524550ULL;
>>>>    static const u64 __perf_magic2_sw = 0x50455246494c4532ULL;
>>>>    #define PERF_MAGIC    __perf_magic2
>>>> +#define DNAME_LEN    16
>>>>    const char perf_version_string[] = PERF_VERSION;
>>>> @@ -1616,10 +1617,10 @@ static int write_pmu_caps(struct feat_fd *ff,
>>>>    struct cpu_domain_map **build_cpu_domain_map(u32
>>>> *schedstat_version, u32 *max_sched_domains, u32 nr)
>>>>    {
>>>> +    char dname[DNAME_LEN], cpumask[MAX_NR_CPUS];
>>>>        struct domain_info *domain_info;
>>>>        struct cpu_domain_map **cd_map;
>>>> -    char dname[16], cpumask[256];
>>>> -    char cpulist[1024];
>>>> +    char cpulist[MAX_NR_CPUS];
>>>>        char *line = NULL;
>>>>        u32 cpu, domain;
>>>>        u32 dcount = 0;
>>>
>>> Looking at
>>> https://lore.kernel.org/all/20260119175833.340369-3-swapnil.sapkal@amd.com/
>>>
>>> There was one more "char cpus[1024]"  in tools/perf/util/util.c.
>>> You may need to fix that too. It is unlikely but, if one has created
>>> exclusive
>>> cpusets comprising of only one cpu from a core, maybe you will run out
>>> the length.
>>> So better use the MAX_NR_CPUS there as well.
>>
>> Yes, I missed this.
>>
>> Arnaldo, can you please consider the below diff? Let me know if you'd like
>> me to respin the patch.
>>
>> --
>> Thanks and Regards,
>> Swapnil
>>
>> diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
>> index c83e59e8c787..3795d5182ce8 100644
>> --- a/tools/perf/util/util.c
>> +++ b/tools/perf/util/util.c
>> @@ -262,7 +262,7 @@ void cpumask_to_cpulist(char *cpumask, char *cpulist)
>>          int i, j, bm_size, nbits;
>>          int len = strlen(cpumask);
>>          unsigned long *bm;
>> -       char cpus[1024];
>> +       char cpus[MAX_NR_CPUS];
>>
>>          for (i = 0; i < len; i++) {
>>                  if (cpumask[i] == ',') {
>> diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
>> index 394dbfa944ac..c43f17137efd 100644
>> --- a/tools/perf/util/util.h
>> +++ b/tools/perf/util/util.h
>> @@ -6,6 +6,7 @@
>>   /* glibc 2.20 deprecates _BSD_SOURCE in favour of _DEFAULT_SOURCE */
>>   #define _DEFAULT_SOURCE 1
>>
>> +#include "perf.h"
> 
> Why add it to util.h? I'll add it to where it is used, util.c, ok?
> 

This doesn't need to be in util.h. Please move it to util.c

--
Thanks and Regards,
Swapnil

> - Arnaldo
> 
>>   #include <dirent.h>
>>   #include <fcntl.h>
>>   #include <stdbool.h>


  reply	other threads:[~2026-01-28 18:29 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-27 18:49 [PATCH v1 0/6] perf sched stats: Fixes and improvements Swapnil Sapkal
2026-01-27 18:49 ` [PATCH v1 1/6] perf header: Replace hardcoded max cpus by MAX_NR_CPUS Swapnil Sapkal
2026-01-28  7:39   ` Shrikanth Hegde
2026-01-28 16:25     ` Swapnil Sapkal
2026-01-28 18:13       ` Arnaldo Carvalho de Melo
2026-01-28 18:15       ` Arnaldo Carvalho de Melo
2026-01-28 18:29         ` Swapnil Sapkal [this message]
2026-01-27 18:49 ` [PATCH v1 2/6] perf util: Fix NULL check in cpumask_to_cpulist() Swapnil Sapkal
2026-01-27 18:49 ` [PATCH v1 3/6] perf sched stats: Add NULL check for cd_map Swapnil Sapkal
2026-01-27 18:49 ` [PATCH v1 4/6] perf sched stats: correct spelling of function name Swapnil Sapkal
2026-01-27 18:49 ` [PATCH v1 5/6] perf sched stats: Define macro for SEP_LEN Swapnil Sapkal
2026-01-27 18:50 ` [PATCH v1 6/6] perf sched stats: Fixes in man page Swapnil Sapkal
2026-01-28  8:03 ` [PATCH v1 0/6] perf sched stats: Fixes and improvements Shrikanth Hegde

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=4da60acc-cbce-4cd2-9600-8a151592d122@amd.com \
    --to=swapnil.sapkal@amd.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=ashelat@redhat.com \
    --cc=gautham.shenoy@amd.com \
    --cc=irogers@google.com \
    --cc=james.clark@arm.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=ravi.bangoria@amd.com \
    --cc=sshegde@linux.ibm.com \
    --cc=thomas.falcon@intel.com \
    --cc=yu.c.chen@intel.com \
    /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 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.