Re: [PATCH v1 1/8] perf: Increase MAX_NR_CPUS to 4096

[Ian Rogers]

On Fri, Dec 6, 2024 at 3:03 PM Leo Yan <leo.yan@xxxxxxx> wrote:
>
> 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?

I meant this :-)
https://lore.kernel.org/lkml/20241207052133.102829-1-irogers@xxxxxxxxxx/

> Lastly, I also found multiple files use "MAX_CPUS" rather than
> "MAX_NR_CPUS".  Polish them in a new series?

Makes sense.

Thanks,
Ian