Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1587220 > unrolled thread

[PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask() Namhyung Kim <namhyung@kernel.org> - 2017-02-24 02:20 +0100
    Re: [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask() Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-24 22:20 +0100
      Re: [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask() Namhyung Kim <namhyung@kernel.org> - 2017-02-25 05:40 +0100

#1587220 — [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask()

The cpu_map__snprint_mask() is to generate string representation of
cpumask bitmap.  For cpu 0 to 11, it'll return "fff".

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/cpumap.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/cpumap.h |  1 +
 2 files changed, 47 insertions(+)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 8c7504939113..08540ab2a891 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -673,3 +673,49 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
 	pr_debug("cpumask list: %s\n", buf);
 	return ret;
 }
+
+static char hex_char(char val)
+{
+	if (0 <= val && val <= 9)
+		return val + '0';
+	if (10 <= val && val < 16)
+		return val - 10 + 'a';
+	return '?';
+}
+
+size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
+{
+	int i, cpu;
+	char *ptr = buf;
+	unsigned char *bitmap;
+	int last_cpu = cpu_map__cpu(map, map->nr - 1);
+
+	bitmap = zalloc((last_cpu + 7) / 8);
+	if (bitmap == NULL) {
+		buf[0] = '\0';
+		return 0;
+	}
+
+	for (i = 0; i < map->nr; i++) {
+		cpu = cpu_map__cpu(map, i);
+		bitmap[cpu / 8] |= 1 << (cpu % 8);
+	}
+
+	for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
+		unsigned char bits = bitmap[cpu / 8];
+
+		if (cpu % 8)
+			bits >>= 4;
+		else
+			bits &= 0xf;
+
+		*ptr++ = hex_char(bits);
+		if ((cpu % 32) == 0 && cpu > 0)
+			*ptr++ = ',';
+	}
+	*ptr = '\0';
+	free(bitmap);
+
+	buf[size - 1] = '\0';
+	return ptr - buf;
+}
diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
index 1a0549af8f5c..4d231c6eac0c 100644
--- a/tools/perf/util/cpumap.h
+++ b/tools/perf/util/cpumap.h
@@ -20,6 +20,7 @@ struct cpu_map *cpu_map__dummy_new(void);
 struct cpu_map *cpu_map__new_data(struct cpu_map_data *data);
 struct cpu_map *cpu_map__read(FILE *file);
 size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size);
+size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size);
 size_t cpu_map__fprintf(struct cpu_map *map, FILE *fp);
 int cpu_map__get_socket_id(int cpu);
 int cpu_map__get_socket(struct cpu_map *map, int idx, void *data);
-- 
2.11.1

[toc] | [next] | [standalone]

#1587942

Em Fri, Feb 24, 2017 at 10:12:49AM +0900, Namhyung Kim escreveu:
> The cpu_map__snprint_mask() is to generate string representation of
> cpumask bitmap.  For cpu 0 to 11, it'll return "fff".
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/cpumap.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/cpumap.h |  1 +
>  2 files changed, 47 insertions(+)
> 
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 8c7504939113..08540ab2a891 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -673,3 +673,49 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
>  	pr_debug("cpumask list: %s\n", buf);
>  	return ret;
>  }
> +
> +static char hex_char(char val)

Why do you use 'char' above and...

> +{
> +	if (0 <= val && val <= 9)
> +		return val + '0';
> +	if (10 <= val && val < 16)
> +		return val - 10 + 'a';
> +	return '?';
> +}
> +size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
<SNIP>
> +	for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
> +		unsigned char bits = bitmap[cpu / 8];

'unsigned char' here?

Some compilers don't like it, for instance:

  19 fedora:24-x-ARC-uClibc: FAIL

  CC       /tmp/build/perf/util/cpumap.o
util/cpumap.c: In function 'hex_char':
util/cpumap.c:679:2: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  if (0 <= val && val <= 9)
  ^
cc1: all warnings being treated as errors

And:

  10 debian:experimental-x-arm64: FAIL

  CC       /tmp/build/perf/util/cpumap.o
util/cpumap.c: In function 'hex_char':
util/cpumap.c:679:8: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  if (0 <= val && val <= 9)
        ^~

Are you ok with the patch below?

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 6ab8699f0233..405f56ad5c24 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -687,7 +687,7 @@ size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
 {
 	int i, cpu;
 	char *ptr = buf;
-	unsigned char *bitmap;
+	char *bitmap;
 	int last_cpu = cpu_map__cpu(map, map->nr - 1);
 
 	bitmap = zalloc((last_cpu + 7) / 8);
@@ -702,7 +702,7 @@ size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
 	}
 
 	for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
-		unsigned char bits = bitmap[cpu / 8];
+		char bits = bitmap[cpu / 8];
 
 		if (cpu % 8)
 			bits >>= 4;


> +		if (cpu % 8)
> +			bits >>= 4;
> +		else
> +			bits &= 0xf;
> +
> +		*ptr++ = hex_char(bits);
> +		if ((cpu % 32) == 0 && cpu > 0)
> +			*ptr++ = ',';
> +	}
> +	*ptr = '\0';
> +	free(bitmap);
> +
> +	buf[size - 1] = '\0';
> +	return ptr - buf;
> +}
> diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
> index 1a0549af8f5c..4d231c6eac0c 100644
> --- a/tools/perf/util/cpumap.h
> +++ b/tools/perf/util/cpumap.h
> @@ -20,6 +20,7 @@ struct cpu_map *cpu_map__dummy_new(void);
>  struct cpu_map *cpu_map__new_data(struct cpu_map_data *data);
>  struct cpu_map *cpu_map__read(FILE *file);
>  size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size);
> +size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size);
>  size_t cpu_map__fprintf(struct cpu_map *map, FILE *fp);
>  int cpu_map__get_socket_id(int cpu);
>  int cpu_map__get_socket(struct cpu_map *map, int idx, void *data);
> -- 
> 2.11.1

[toc] | [prev] | [next] | [standalone]

#1588062

Hi Arnaldo,

On Fri, Feb 24, 2017 at 06:08:53PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 24, 2017 at 10:12:49AM +0900, Namhyung Kim escreveu:
> > The cpu_map__snprint_mask() is to generate string representation of
> > cpumask bitmap.  For cpu 0 to 11, it'll return "fff".
> > 
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/cpumap.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> >  tools/perf/util/cpumap.h |  1 +
> >  2 files changed, 47 insertions(+)
> > 
> > diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> > index 8c7504939113..08540ab2a891 100644
> > --- a/tools/perf/util/cpumap.c
> > +++ b/tools/perf/util/cpumap.c
> > @@ -673,3 +673,49 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
> >  	pr_debug("cpumask list: %s\n", buf);
> >  	return ret;
> >  }
> > +
> > +static char hex_char(char val)
> 
> Why do you use 'char' above and...

My bad.  The argument should be unsigned.  The semantics is that it
converts a bitmap value into a character (string).

> 
> > +{
> > +	if (0 <= val && val <= 9)
> > +		return val + '0';
> > +	if (10 <= val && val < 16)
> > +		return val - 10 + 'a';
> > +	return '?';
> > +}
> > +size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
> <SNIP>
> > +	for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
> > +		unsigned char bits = bitmap[cpu / 8];
> 
> 'unsigned char' here?
> 
> Some compilers don't like it, for instance:
> 
>   19 fedora:24-x-ARC-uClibc: FAIL
> 
>   CC       /tmp/build/perf/util/cpumap.o
> util/cpumap.c: In function 'hex_char':
> util/cpumap.c:679:2: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>   if (0 <= val && val <= 9)
>   ^
> cc1: all warnings being treated as errors
> 
> And:
> 
>   10 debian:experimental-x-arm64: FAIL
> 
>   CC       /tmp/build/perf/util/cpumap.o
> util/cpumap.c: In function 'hex_char':
> util/cpumap.c:679:8: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>   if (0 <= val && val <= 9)
>         ^~
> 
> Are you ok with the patch below?

I'd rather change hex_char() instead.  How about this?

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 6ab8699f0233..061018b42393 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -674,11 +674,11 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, size_t size)
        return ret;
 }
 
-static char hex_char(char val)
+static char hex_char(unsigned char val)
 {
-       if (0 <= val && val <= 9)
+       if (val < 10)
                return val + '0';
-       if (10 <= val && val < 16)
+       if (val < 16)
                return val - 10 + 'a';
        return '?';
 }

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web