Re: [PATCH] perf tools: Replace MAX_NR_CPUS with nr_cpus_onln

From: Jiri Olsa
Date: Fri Aug 02 2019 - 09:17:15 EST

Next message: Leonardo Bras: "Re: [PATCH 1/1] pseries/hotplug-memory.c: Replace nested ifs by switch-case"
Previous message: Sergey Senozhatsky: "Re: [PATCH 2/2] i915: do not leak module ref counter"
Next in thread: Kyle Meyer: "[PATCH] perf tools: Replace MAX_NR_CPUS with nr_cpus_onln"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Fri, Jul 26, 2019 at 02:51:39PM -0500, Kyle Meyer wrote:
> The variables nr_cpus_onln and max_caches are dynamic alternatives for
> MAX_NR_CPUS and MAX_CACHES as they are initialized at runtime. MAX_NR_CPUS
> is still used by DECLARE_BITMAP() at compile time, however, nr_cpus_onln
> replaces it elsewhere throughout perf.
>
> This patch was tested using "perf record -a -g" on both an eight socket
> (288 CPUs) system and a single socket (36 CPUs) system. Each system was then
> rebooted single socket (36 CPUs) / eight socket (288 CPUs) and "perf
> report" used to read the perf.data file. "perf report --header" was used to
> confirm that each perf.data had information on 288 CPUs / 36 CPUs.
>
> This patch is meant to be built against perf tools: Increase MAX_NR_CPUS and
> MAX_CACHES (commit 9f94c7f947e919c343b30f080285af53d0fa9902).

hi,
could you please rebase to latest Arnaldo's perf/core,
the cpumap.c got changed recently and I can't apply
your patch anymore:

patching file perf.c
patching file perf.h
patching file util/cpumap.c
Hunk #1 FAILED at 72.
Hunk #2 FAILED at 83.
Hunk #3 FAILED at 170.
3 out of 3 hunks FAILED -- saving rejects to file util/cpumap.c.rej
patching file util/header.c
Hunk #1 succeeded at 1126 with fuzz 2 (offset 1 line).
patching file util/machine.c
patching file util/stat.c
Hunk #1 succeeded at 208 with fuzz 1.
patching file util/svghelper.c
Hunk #1 succeeded at 706 (offset 1 line).
Hunk #2 succeeded at 721 (offset 1 line).
Hunk #3 succeeded at 738 (offset 1 line).
Hunk #4 succeeded at 785 (offset 1 line).

thanks,
jirka

>
> Signed-off-by: Kyle Meyer <kyle.meyer@xxxxxxx>
> Cc: Russ Anderson <russ.anderson@xxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> ---
> tools/perf/perf.c | 10 ++++++++++
> tools/perf/perf.h | 1 +
> tools/perf/util/cpumap.c | 6 +++---
> tools/perf/util/header.c | 5 +++--
> tools/perf/util/machine.c | 11 +++++------
> tools/perf/util/stat.c | 4 ++--
> tools/perf/util/svghelper.c | 10 +++++-----
> 7 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index 97e2628ea5dd..0d0162fb4e24 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -428,6 +428,16 @@ int main(int argc, const char **argv)
> const char *cmd;
> char sbuf[STRERR_BUFSIZE];
>
> + nr_cpus_onln = sysconf(_SC_NPROCESSORS_ONLN);
> + if (nr_cpus_onln < 0) {
> + fprintf(stderr, "Cannot determine the number of CPUs currently online.\n");
> + goto out;
> + }
> + if (nr_cpus_onln > MAX_NR_CPUS) {
> + fprintf(stderr, "The number of CPUs currently online is too large, consider raising MAX_NR_CPUS.\n");
> + nr_cpus_onln = MAX_NR_CPUS;
> + }
> +
> /* libsubcmd init */
> exec_cmd_init("perf", PREFIX, PERF_EXEC_PATH, EXEC_PATH_ENVIRONMENT);
> pager_init(PERF_PAGER_ENVIRONMENT);
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index 74d0124d38f3..603391cac85b 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -29,6 +29,7 @@ static inline unsigned long long rdclock(void)
> #define MAX_NR_CPUS 2048
> #endif
>
> +int nr_cpus_onln;
> extern const char *input_name;
> extern bool perf_host, perf_guest;
> extern const char perf_version_string[];
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 3acfbe34ebaf..f634c56b1388 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -72,7 +72,7 @@ struct cpu_map *cpu_map__read(FILE *file)
> int new_max = nr_cpus + cpu - prev - 1;
>
> if (new_max >= max_entries) {
> - max_entries = new_max + MAX_NR_CPUS / 2;
> + max_entries = new_max + nr_cpus_onln / 2;
> tmp = realloc(tmp_cpus, max_entries * sizeof(int));
> if (tmp == NULL)
> goto out_free_tmp;
> @@ -83,7 +83,7 @@ struct cpu_map *cpu_map__read(FILE *file)
> tmp_cpus[nr_cpus++] = prev;
> }
> if (nr_cpus == max_entries) {
> - max_entries += MAX_NR_CPUS;
> + max_entries += nr_cpus_onln;
> tmp = realloc(tmp_cpus, max_entries * sizeof(int));
> if (tmp == NULL)
> goto out_free_tmp;
> @@ -170,7 +170,7 @@ struct cpu_map *cpu_map__new(const char *cpu_list)
> goto invalid;
>
> if (nr_cpus == max_entries) {
> - max_entries += MAX_NR_CPUS;
> + max_entries += nr_cpus_onln;
> tmp = realloc(tmp_cpus, max_entries * sizeof(int));
> if (tmp == NULL)
> goto invalid;
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index c24db7f4909c..030c0a8f6664 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1125,11 +1125,12 @@ static int build_caches(struct cpu_cache_level caches[], u32 size, u32 *cntp)
> static int write_cache(struct feat_fd *ff,
> struct perf_evlist *evlist __maybe_unused)
> {
> - struct cpu_cache_level caches[MAX_CACHES];
> + u32 max_caches = (nr_cpus_onln * 4);
> + struct cpu_cache_level caches[max_caches];
> u32 cnt = 0, i, version = 1;
> int ret;
>
> - ret = build_caches(caches, MAX_CACHES, &cnt);
> + ret = build_caches(caches, max_caches, &cnt);
> if (ret)
> goto out;
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index cf826eca3aaf..92720240676d 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -2615,7 +2615,7 @@ int __machine__synthesize_threads(struct machine *machine, struct perf_tool *too
>
> pid_t machine__get_current_tid(struct machine *machine, int cpu)
> {
> - if (cpu < 0 || cpu >= MAX_NR_CPUS || !machine->current_tid)
> + if (cpu < 0 || cpu >= nr_cpus_onln || !machine->current_tid)
> return -1;
>
> return machine->current_tid[cpu];
> @@ -2632,16 +2632,15 @@ int machine__set_current_tid(struct machine *machine, int cpu, pid_t pid,
> if (!machine->current_tid) {
> int i;
>
> - machine->current_tid = calloc(MAX_NR_CPUS, sizeof(pid_t));
> + machine->current_tid = calloc(nr_cpus_onln, sizeof(pid_t));
> if (!machine->current_tid)
> return -ENOMEM;
> - for (i = 0; i < MAX_NR_CPUS; i++)
> + for (i = 0; i < nr_cpus_onln; i++)
> machine->current_tid[i] = -1;
> }
>
> - if (cpu >= MAX_NR_CPUS) {
> - pr_err("Requested CPU %d too large. ", cpu);
> - pr_err("Consider raising MAX_NR_CPUS\n");
> + if (cpu >= nr_cpus_onln) {
> + pr_err("Requested CPU %d too large, there are %d CPUs currently online.\n", cpu, nr_cpus_onln);
> return -EINVAL;
> }
>
> diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> index db8a6cf336be..f87bdc140a4b 100644
> --- a/tools/perf/util/stat.c
> +++ b/tools/perf/util/stat.c
> @@ -208,7 +208,7 @@ void perf_evlist__reset_stats(struct perf_evlist *evlist)
> static void zero_per_pkg(struct perf_evsel *counter)
> {
> if (counter->per_pkg_mask)
> - memset(counter->per_pkg_mask, 0, MAX_NR_CPUS);
> + memset(counter->per_pkg_mask, 0, nr_cpus_onln);
> }
>
> static int check_per_pkg(struct perf_evsel *counter,
> @@ -227,7 +227,7 @@ static int check_per_pkg(struct perf_evsel *counter,
> return 0;
>
> if (!mask) {
> - mask = zalloc(MAX_NR_CPUS);
> + mask = zalloc(nr_cpus_onln);
> if (!mask)
> return -ENOMEM;
>
> diff --git a/tools/perf/util/svghelper.c b/tools/perf/util/svghelper.c
> index 76cc54000483..4c38f70520c5 100644
> --- a/tools/perf/util/svghelper.c
> +++ b/tools/perf/util/svghelper.c
> @@ -705,7 +705,7 @@ static void scan_thread_topology(int *map, struct topology *t, int cpu, int *pos
>
> for_each_set_bit(thr,
> cpumask_bits(&t->sib_thr[i]),
> - MAX_NR_CPUS)
> + nr_cpus_onln)
> if (map[thr] == -1)
> map[thr] = (*pos)++;
> }
> @@ -720,7 +720,7 @@ static void scan_core_topology(int *map, struct topology *t)
> for (i = 0; i < t->sib_core_nr; i++)
> for_each_set_bit(cpu,
> cpumask_bits(&t->sib_core[i]),
> - MAX_NR_CPUS)
> + nr_cpus_onln)
> scan_thread_topology(map, t, cpu, &pos);
> }
>
> @@ -737,7 +737,7 @@ static int str_to_bitmap(char *s, cpumask_t *b)
>
> for (i = 0; i < m->nr; i++) {
> c = m->map[i];
> - if (c >= MAX_NR_CPUS) {
> + if (c >= nr_cpus_onln) {
> ret = -1;
> break;
> }
> @@ -784,13 +784,13 @@ int svg_build_topology_map(char *sib_core, int sib_core_nr,
> sib_thr += strlen(sib_thr) + 1;
> }
>
> - topology_map = malloc(sizeof(int) * MAX_NR_CPUS);
> + topology_map = malloc(sizeof(int) * nr_cpus_onln);
> if (!topology_map) {
> fprintf(stderr, "topology: no memory\n");
> goto exit;
> }
>
> - for (i = 0; i < MAX_NR_CPUS; i++)
> + for (i = 0; i < nr_cpus_onln; i++)
> topology_map[i] = -1;
>
> scan_core_topology(topology_map, &t);
> --
> 2.12.3
>

Next message: Leonardo Bras: "Re: [PATCH 1/1] pseries/hotplug-memory.c: Replace nested ifs by switch-case"
Previous message: Sergey Senozhatsky: "Re: [PATCH 2/2] i915: do not leak module ref counter"
Next in thread: Kyle Meyer: "[PATCH] perf tools: Replace MAX_NR_CPUS with nr_cpus_onln"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]