[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241206230321.GA5430@e132581.arm.com>
Date: Fri, 6 Dec 2024 23:03:21 +0000
From: Leo Yan <leo.yan@....com>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>,
James Clark <james.clark@...aro.org>,
Kyle Meyer <kyle.meyer@....com>, Ben Gainey <ben.gainey@....com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/8] perf: Increase MAX_NR_CPUS to 4096
Hi Ian,
On Fri, Dec 06, 2024 at 08:25:06AM -0800, Ian Rogers wrote:
[...]
> > This series is fine for me. Just wandering if we can use a central
> > place to maintain the macro, e.g. lib/perf/include/perf/cpumap.h. It
> > is pointless to define exactly same macros in different headers. As
> > least, I think we can unify this except the kwork bpf program?
> >
> > P.s. for dynamically allocating per CPU maps in eBPF program, we can
> > refer to the code samples/bpf/xdp_sample_user.c, but this is another
> > topic.
>
> Thanks Leo,
>
> can I take this as an acked-by?
Yeah. I will give my review tags in the cover letter.
> Wrt a single constant I agree,
> following these changes MAX_NR_CPUS is just used for a warning in
> libperf's cpumap.c. I think we're agreed that getting rid of the
> constant would be best. I also think the cpumap logic is duplicating
> something that libc is providing in cpu_set.
>
> And we have more than one representation in perf for the sake of the
> disk representation:
Thanks for sharing the info.
> Just changing the int to be a s16 would lower the memory overhead,
> which is why I'd kind of like the abstraction to be minimal.
Here I am not clear what for "changing the int to be a s16". Could you
elaberate a bit for this?
Lastly, I also found multiple files use "MAX_CPUS" rather than
"MAX_NR_CPUS". Polish them in a new series?
Thanks,
Leo
Powered by blists - more mailing lists