RE: [PATCH V9 1/6] perf,tools: introduce generic FEAT for CPU attributes

[Liang, Kan]

> 
> Em Thu, Sep 10, 2015 at 10:58:38AM -0300, Arnaldo Carvalho de Melo
> escreveu:
> > Em Tue, Sep 08, 2015 at 03:32:44PM -0400, kan.liang@xxxxxxxxx escreveu:
> > > This patch introduces generic FEAT for CPU attributes. For the patch
> > > set, we only need cpu max frequency. But it can be easily extented
> > > to support more other CPU attributes.
> > > The cpu max frequency is from the first online cpu.
> 
> > Ok, but don't we have to do error handling? i.e. you are returning 0
> > for any error in trying to read the cpu max freq, shouldn't we bail
> > out somewhere?
> 
> > And please move this get_cpu_max_freq() thing out of the cpumap.[ch]
> > files, it is not even a need completely specific to perf tooling,
> > there must be somewhere in tools/lib/api/ (kernel APIs) where this fits,
> no?
> 
> So, I've updated my perf/env branch with routines to do that, that uses
> infrastructure to read files from virtual dirs that was there plus a few I just
> introduced, in the same vein

The two new patches in perf/env are good to me.
Commit 4ee5cc5708d89f380ab5371181b65dd74935352d
Commit 57d54be1e9074049c8695c522a499f8a7d62ef2d
Acked-by: Kan Liang <kan.liang@xxxxxxxxx>

> I will update your patches to use them and
> put there as well, for your consideration.
> 

Thanks. Please let me know, when you finished.
I will update the rest of the per-sample freq patches, and repost them
for review then.

Thanks,
Kan

> The HEAD there is the one below:
> 
> - Arnaldo
> 
> commit 2bc1fae4ed8a842f52dc374449d37c3ec1fa1986
> Author: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Date:   Thu Sep 10 12:20:14 2015 -0300
> 
>     tools lib cpu: Introduce cpu.[ch] to read sysfs cpu related information
> 
>     E.g.:
> 
>      $ ./cpu__get_max_freq
>      3200000
> 
>     It does that, as Kan's patch does, by looking at these files:
> 
>       $ cat /sys/devices/system/cpu/online
>       0-3
>       $ ./sysfs__read_ull
>       devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq
>       /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq=3200000
>       $
> 
>     I.e. find out the first online CPU, then read its cpufreq info.
> 
>     But do it in tools/lib/api/, so that other tools living code can use
>     it, not just perf.
> 
>     Based-on-a-patch-by: Kan Liang <kan.liang@xxxxxxxxx>
>     Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>     Cc: Borislav Petkov <bp@xxxxxxx>
>     Cc: David Ahern <dsahern@xxxxxxxxx>
>     Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
>     Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
>     Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
>     Cc: Stephane Eranian <eranian@xxxxxxxxxx>
>     Link: http://lkml.kernel.org/n/tip-
> 915v4cvxqplaub8qco66b9mv@xxxxxxxxxxxxxx
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> 
> diff --git a/tools/lib/api/Build b/tools/lib/api/Build index
> 3653965cf481..e8b8a23b9bf4 100644
> --- a/tools/lib/api/Build
> +++ b/tools/lib/api/Build
> @@ -1,2 +1,3 @@
>  libapi-y += fd/
>  libapi-y += fs/
> +libapi-y += cpu.o
> diff --git a/tools/lib/api/cpu.c b/tools/lib/api/cpu.c new file mode 100644
> index 000000000000..8c6489356e3a
> --- /dev/null
> +++ b/tools/lib/api/cpu.c
> @@ -0,0 +1,18 @@
> +#include <stdio.h>
> +
> +#include "cpu.h"
> +#include "fs/fs.h"
> +
> +int cpu__get_max_freq(unsigned long long *freq) {
> +	char entry[PATH_MAX];
> +	int cpu;
> +
> +	if (sysfs__read_int("devices/system/cpu/online", &cpu) < 0)
> +		return -1;
> +
> +	snprintf(entry, sizeof(entry),
> +		 "devices/system/cpu/cpu%d/cpufreq/cpuinfo_max_freq",
> cpu);
> +
> +	return sysfs__read_ull(entry, freq);
> +}
> diff --git a/tools/lib/api/cpu.h b/tools/lib/api/cpu.h new file mode 100644
> index 000000000000..81e9d3955961
> --- /dev/null
> +++ b/tools/lib/api/cpu.h
> @@ -0,0 +1,6 @@
> +#ifndef __API_CPU__
> +#define __API_CPU__
> +
> +int cpu__get_max_freq(unsigned long long *freq);
> +
> +#endif /* __API_CPU__ */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/