* [PATCH] perf cpumap: Make counter as unsigned ints @ 2023-01-23 21:13 Khem Raj 2023-01-24 17:02 ` Ian Rogers 0 siblings, 1 reply; 4+ messages in thread From: Khem Raj @ 2023-01-23 21:13 UTC (permalink / raw) To: linux-perf-users Cc: Khem Raj, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim These are loop counters which is inherently unsigned. Therefore make them unsigned. Moreover it also fixes alloc-size-larger-than error with gcc-13, where malloc can be called with (-1) due to tmp_len being an int type. Fixes | cpumap.c:366:20: error: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=] | 366 | tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu)); | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Signed-off-by: Khem Raj <raj.khem@gmail.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Namhyung Kim <namhyung@kernel.org> --- tools/lib/perf/cpumap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c index 6cd0be7c1bb4..d960880dd903 100644 --- a/tools/lib/perf/cpumap.c +++ b/tools/lib/perf/cpumap.c @@ -351,8 +351,8 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, struct perf_cpu_map *other) { struct perf_cpu *tmp_cpus; - int tmp_len; - int i, j, k; + unsigned int tmp_len; + unsigned int i, j, k; struct perf_cpu_map *merged; if (perf_cpu_map__is_subset(orig, other)) @@ -369,7 +369,7 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, /* Standard merge algorithm from wikipedia */ i = j = k = 0; - while (i < orig->nr && j < other->nr) { + while (i < (unsigned int)orig->nr && j < (unsigned int)other->nr) { if (orig->map[i].cpu <= other->map[j].cpu) { if (orig->map[i].cpu == other->map[j].cpu) j++; @@ -378,10 +378,10 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, tmp_cpus[k++] = other->map[j++]; } - while (i < orig->nr) + while (i < (unsigned int)orig->nr) tmp_cpus[k++] = orig->map[i++]; - while (j < other->nr) + while (j < (unsigned int)other->nr) tmp_cpus[k++] = other->map[j++]; assert(k <= tmp_len); -- 2.39.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] perf cpumap: Make counter as unsigned ints 2023-01-23 21:13 [PATCH] perf cpumap: Make counter as unsigned ints Khem Raj @ 2023-01-24 17:02 ` Ian Rogers 2023-01-25 19:04 ` Khem Raj 0 siblings, 1 reply; 4+ messages in thread From: Ian Rogers @ 2023-01-24 17:02 UTC (permalink / raw) To: Khem Raj Cc: linux-perf-users, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim On Mon, Jan 23, 2023 at 1:13 PM Khem Raj <raj.khem@gmail.com> wrote: > > These are loop counters which is inherently unsigned. Therefore make > them unsigned. Moreover it also fixes alloc-size-larger-than > error with gcc-13, where malloc can be called with (-1) due to tmp_len > being an int type. > > Fixes > | cpumap.c:366:20: error: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=] > | 366 | tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu)); > | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Signed-off-by: Khem Raj <raj.khem@gmail.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Cc: Jiri Olsa <jolsa@kernel.org> > Cc: Namhyung Kim <namhyung@kernel.org> > --- > tools/lib/perf/cpumap.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c > index 6cd0be7c1bb4..d960880dd903 100644 > --- a/tools/lib/perf/cpumap.c > +++ b/tools/lib/perf/cpumap.c > @@ -351,8 +351,8 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > struct perf_cpu_map *other) > { > struct perf_cpu *tmp_cpus; > - int tmp_len; > - int i, j, k; > + unsigned int tmp_len; > + unsigned int i, j, k; > struct perf_cpu_map *merged; > > if (perf_cpu_map__is_subset(orig, other)) > @@ -369,7 +369,7 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > > /* Standard merge algorithm from wikipedia */ > i = j = k = 0; > - while (i < orig->nr && j < other->nr) { > + while (i < (unsigned int)orig->nr && j < (unsigned int)other->nr) { Rather than cast 'nr' why not add the unsigned to the declaration? Thanks, Ian > if (orig->map[i].cpu <= other->map[j].cpu) { > if (orig->map[i].cpu == other->map[j].cpu) > j++; > @@ -378,10 +378,10 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > tmp_cpus[k++] = other->map[j++]; > } > > - while (i < orig->nr) > + while (i < (unsigned int)orig->nr) > tmp_cpus[k++] = orig->map[i++]; > > - while (j < other->nr) > + while (j < (unsigned int)other->nr) > tmp_cpus[k++] = other->map[j++]; > assert(k <= tmp_len); > > -- > 2.39.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf cpumap: Make counter as unsigned ints 2023-01-24 17:02 ` Ian Rogers @ 2023-01-25 19:04 ` Khem Raj 2023-01-25 20:59 ` Ian Rogers 0 siblings, 1 reply; 4+ messages in thread From: Khem Raj @ 2023-01-25 19:04 UTC (permalink / raw) To: Ian Rogers Cc: linux-perf-users, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim On Tue, Jan 24, 2023 at 9:02 AM Ian Rogers <irogers@google.com> wrote: > > On Mon, Jan 23, 2023 at 1:13 PM Khem Raj <raj.khem@gmail.com> wrote: > > > > These are loop counters which is inherently unsigned. Therefore make > > them unsigned. Moreover it also fixes alloc-size-larger-than > > error with gcc-13, where malloc can be called with (-1) due to tmp_len > > being an int type. > > > > Fixes > > | cpumap.c:366:20: error: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=] > > | 366 | tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu)); > > | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > Signed-off-by: Khem Raj <raj.khem@gmail.com> > > Cc: Peter Zijlstra <peterz@infradead.org> > > Cc: Ingo Molnar <mingo@redhat.com> > > Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > > Cc: Jiri Olsa <jolsa@kernel.org> > > Cc: Namhyung Kim <namhyung@kernel.org> > > --- > > tools/lib/perf/cpumap.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c > > index 6cd0be7c1bb4..d960880dd903 100644 > > --- a/tools/lib/perf/cpumap.c > > +++ b/tools/lib/perf/cpumap.c > > @@ -351,8 +351,8 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > > struct perf_cpu_map *other) > > { > > struct perf_cpu *tmp_cpus; > > - int tmp_len; > > - int i, j, k; > > + unsigned int tmp_len; > > + unsigned int i, j, k; > > struct perf_cpu_map *merged; > > > > if (perf_cpu_map__is_subset(orig, other)) > > @@ -369,7 +369,7 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > > > > /* Standard merge algorithm from wikipedia */ > > i = j = k = 0; > > - while (i < orig->nr && j < other->nr) { > > + while (i < (unsigned int)orig->nr && j < (unsigned int)other->nr) { > > Rather than cast 'nr' why not add the unsigned to the declaration? > if type of nr is changed from int to unsigned int, this will ensue a lot of changes including function signature changes, I am not sure if that is the best approach. > Thanks, > Ian > > > if (orig->map[i].cpu <= other->map[j].cpu) { > > if (orig->map[i].cpu == other->map[j].cpu) > > j++; > > @@ -378,10 +378,10 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > > tmp_cpus[k++] = other->map[j++]; > > } > > > > - while (i < orig->nr) > > + while (i < (unsigned int)orig->nr) > > tmp_cpus[k++] = orig->map[i++]; > > > > - while (j < other->nr) > > + while (j < (unsigned int)other->nr) > > tmp_cpus[k++] = other->map[j++]; > > assert(k <= tmp_len); > > > > -- > > 2.39.1 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf cpumap: Make counter as unsigned ints 2023-01-25 19:04 ` Khem Raj @ 2023-01-25 20:59 ` Ian Rogers 0 siblings, 0 replies; 4+ messages in thread From: Ian Rogers @ 2023-01-25 20:59 UTC (permalink / raw) To: Khem Raj Cc: linux-perf-users, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim On Wed, Jan 25, 2023 at 11:04 AM Khem Raj <raj.khem@gmail.com> wrote: > > On Tue, Jan 24, 2023 at 9:02 AM Ian Rogers <irogers@google.com> wrote: > > > > On Mon, Jan 23, 2023 at 1:13 PM Khem Raj <raj.khem@gmail.com> wrote: > > > > > > These are loop counters which is inherently unsigned. Therefore make > > > them unsigned. Moreover it also fixes alloc-size-larger-than > > > error with gcc-13, where malloc can be called with (-1) due to tmp_len > > > being an int type. > > > > > > Fixes > > > | cpumap.c:366:20: error: argument 1 range [18446744065119617024, 18446744073709551612] exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=] > > > | 366 | tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu)); > > > | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > Signed-off-by: Khem Raj <raj.khem@gmail.com> > > > Cc: Peter Zijlstra <peterz@infradead.org> > > > Cc: Ingo Molnar <mingo@redhat.com> > > > Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > > > Cc: Mark Rutland <mark.rutland@arm.com> > > > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > > > Cc: Jiri Olsa <jolsa@kernel.org> > > > Cc: Namhyung Kim <namhyung@kernel.org> > > > --- > > > tools/lib/perf/cpumap.c | 10 +++++----- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c > > > index 6cd0be7c1bb4..d960880dd903 100644 > > > --- a/tools/lib/perf/cpumap.c > > > +++ b/tools/lib/perf/cpumap.c > > > @@ -351,8 +351,8 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > > > struct perf_cpu_map *other) > > > { > > > struct perf_cpu *tmp_cpus; > > > - int tmp_len; > > > - int i, j, k; > > > + unsigned int tmp_len; > > > + unsigned int i, j, k; > > > struct perf_cpu_map *merged; > > > > > > if (perf_cpu_map__is_subset(orig, other)) > > > @@ -369,7 +369,7 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > > > > > > /* Standard merge algorithm from wikipedia */ > > > i = j = k = 0; > > > - while (i < orig->nr && j < other->nr) { > > > + while (i < (unsigned int)orig->nr && j < (unsigned int)other->nr) { > > > > Rather than cast 'nr' why not add the unsigned to the declaration? > > > > if type of nr is changed from int to unsigned int, this will ensue a > lot of changes including function signature changes, I am not > sure if that is the best approach. All non-libperf accesses to 'nr' should go through perf_cpu_map__nr: https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/lib/perf/include/perf/cpumap.h?h=perf/core#n25 Fwiw, this is done as there is some hope of 1 day landing a better reference count checker. We should migrate all __nr users to being unsigned. The use of __nr is somewhat confusing, the dummy "program me on any CPU" cpu map has an nr of 1, while the nr for being on all CPUs will be the number of CPUs in the system, possibly +1 for the dummy case. I'm reminded that cpumap needs its API clearing up, perf_cpu_map__empty being especially confusing. Thanks, Ian > > Thanks, > > Ian > > > > > if (orig->map[i].cpu <= other->map[j].cpu) { > > > if (orig->map[i].cpu == other->map[j].cpu) > > > j++; > > > @@ -378,10 +378,10 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > > > tmp_cpus[k++] = other->map[j++]; > > > } > > > > > > - while (i < orig->nr) > > > + while (i < (unsigned int)orig->nr) > > > tmp_cpus[k++] = orig->map[i++]; > > > > > > - while (j < other->nr) > > > + while (j < (unsigned int)other->nr) > > > tmp_cpus[k++] = other->map[j++]; > > > assert(k <= tmp_len); > > > > > > -- > > > 2.39.1 > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-01-25 21:00 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-23 21:13 [PATCH] perf cpumap: Make counter as unsigned ints Khem Raj 2023-01-24 17:02 ` Ian Rogers 2023-01-25 19:04 ` Khem Raj 2023-01-25 20:59 ` Ian Rogers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).