* [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).