All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leo Yan <leo.yan@arm.com>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Kan Liang <kan.liang@linux.intel.com>,
	James Clark <james.clark@linaro.org>,
	Kyle Meyer <kyle.meyer@hpe.com>, Ben Gainey <ben.gainey@arm.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/8] perf: Increase MAX_NR_CPUS to 4096
Date: Fri, 6 Dec 2024 23:03:21 +0000	[thread overview]
Message-ID: <20241206230321.GA5430@e132581.arm.com> (raw)
In-Reply-To: <CAP-5=fVDH6k7rW3_LXK5Y9Givs3WO5MQ8XMKsuUXXY5nQ66qDg@mail.gmail.com>

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

  reply	other threads:[~2024-12-06 23:03 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-06  4:40 [PATCH v1 0/8] Cpumap improvements for large MAX_NR_CPUS Ian Rogers
2024-12-06  4:40 ` [PATCH v1 1/8] perf: Increase MAX_NR_CPUS to 4096 Ian Rogers
2024-12-06 10:24   ` Leo Yan
2024-12-06 16:25     ` Ian Rogers
2024-12-06 23:03       ` Leo Yan [this message]
2024-12-07  5:24         ` Ian Rogers
2024-12-09 21:35           ` David Laight
2024-12-09 21:47             ` Ian Rogers
2024-12-06  4:40 ` [PATCH v1 2/8] perf cpumap: Reduce transitive dependencies on libperf MAX_NR_CPUS Ian Rogers
2024-12-06  4:40 ` [PATCH v1 3/8] libperf cpumap: Hide/reduce scope of MAX_NR_CPUS Ian Rogers
2024-12-06  4:40 ` [PATCH v1 4/8] libperf cpumap: Be tolerant of newline at the end of a cpumask Ian Rogers
2024-12-06  4:40 ` [PATCH v1 5/8] perf pmu: Remove use of perf_cpu_map__read Ian Rogers
2024-12-06  4:40 ` [PATCH v1 6/8] libperf cpumap: " Ian Rogers
2024-12-06  4:40 ` [PATCH v1 7/8] libperf cpumap: Remove perf_cpu_map__read Ian Rogers
2024-12-06  4:40 ` [PATCH v1 8/8] libperf cpumap: Grow array of read CPUs in smaller increments Ian Rogers
2024-12-06 23:03 ` [PATCH v1 0/8] Cpumap improvements for large MAX_NR_CPUS Leo Yan
2024-12-09 16:04   ` Arnaldo Carvalho de Melo

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=20241206230321.GA5430@e132581.arm.com \
    --to=leo.yan@arm.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=ben.gainey@arm.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=kyle.meyer@hpe.com \
    --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 \
    /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.