2014-04-03 05:48:59

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 1/4] perf: Allow ability to map cpus to nodes easily

Hi Don,

(Adding Boris to CC as he might be interested)

On Mon, 24 Mar 2014 15:32:54 -0400, Don Zickus wrote:
> This patch figures out the max number of cpus and nodes that are on the
> system and creates a map of cpu to node. This allows us to provide a cpu
> and quickly get the node associated with it.
>
> It was mostly copied from builtin-kmem.c and tweaked slightly to use less memory
> (use possible cpus instead of max). It also calculates the max number of nodes.
>
> V3: simplify function names
>
> Signed-off-by: Don Zickus <[email protected]>
> ---
> tools/perf/util/cpumap.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/cpumap.h | 35 ++++++++++++
> 2 files changed, 174 insertions(+)
>
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 7fe4994..2eb528e 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -317,3 +317,142 @@ int cpu_map__build_core_map(struct cpu_map *cpus, struct cpu_map **corep)
> {
> return cpu_map__build_map(cpus, corep, cpu_map__get_core);
> }
> +
> +/* setup simple routines to easily access node numbers given a cpu number */
> +static int __set_max_num(FILE *fp, int *max)
> +{
> + int num;
> + char buf[256];
> +
> + num = fread(&buf, 1, sizeof(buf), fp);
> + if (!num)
> + return -1;
> +
> + buf[num] = '\0';
> +
> + /* start on the right, to find highest node num */
> + while (--num) {
> + if ((buf[num] == ',') || (buf[num] == '-')) {
> + num++;
> + break;
> + }
> + }
> + if (sscanf(&buf[num], "%d", max) < 1)
> + return -1;
> +
> + /* convert from 0-based to 1-based */
> + (*max)++;
> +
> + return 0;
> +}
> +
> +/* Determine highest possible cpu in the system for sparse allocation */
> +static void set_max_cpu_num(void)
> +{
> + FILE *fp;
> + int ret = -1;
> +
> + /* set up default */
> + max_cpu_num = 4096;
> +
> + /* get the highest possible cpu number for a sparse allocation */
> + fp = fopen("/sys/devices/system/cpu/possible", "r");

More generally, this sysfs access needs to check actual mountpoint using
sysfs__mountpoint() IMHO.

Also this API can be generalized like reading int value from a sysfs
file as the filename itself represents the content in most cases.

So how about changing this way? It might reside on somewhere in tools/lib/api/fs/.

max_cpu_num = sysfs__read_int("devices/system/cpu/possible");

max_node_num = sysfs__read_int("devices/system/node/possible");


Hmm.. looking at the code, perf already has filename__read_{int,str} API
in util/util.c. Maybe you can just use it instead.

Thanks,
Namhyung


2014-04-06 12:16:13

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 1/4] perf: Allow ability to map cpus to nodes easily

On Thu, Apr 03, 2014 at 02:48:51PM +0900, Namhyung Kim wrote:
> Hi Don,

SNIP

>
> More generally, this sysfs access needs to check actual mountpoint using
> sysfs__mountpoint() IMHO.
>
> Also this API can be generalized like reading int value from a sysfs
> file as the filename itself represents the content in most cases.
>
> So how about changing this way? It might reside on somewhere in tools/lib/api/fs/.
>
> max_cpu_num = sysfs__read_int("devices/system/cpu/possible");
>
> max_node_num = sysfs__read_int("devices/system/node/possible");
>
>
> Hmm.. looking at the code, perf already has filename__read_{int,str} API
> in util/util.c. Maybe you can just use it instead.

I think those just read the file, while Don needs to parse
the map to get max cpu number

jirka

2014-04-07 05:28:40

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 1/4] perf: Allow ability to map cpus to nodes easily

On Sun, 6 Apr 2014 14:15:46 +0200, Jiri Olsa wrote:
> On Thu, Apr 03, 2014 at 02:48:51PM +0900, Namhyung Kim wrote:
>> More generally, this sysfs access needs to check actual mountpoint using
>> sysfs__mountpoint() IMHO.
>>
>> Also this API can be generalized like reading int value from a sysfs
>> file as the filename itself represents the content in most cases.
>>
>> So how about changing this way? It might reside on somewhere in tools/lib/api/fs/.
>>
>> max_cpu_num = sysfs__read_int("devices/system/cpu/possible");
>>
>> max_node_num = sysfs__read_int("devices/system/node/possible");
>>
>>
>> Hmm.. looking at the code, perf already has filename__read_{int,str} API
>> in util/util.c. Maybe you can just use it instead.
>
> I think those just read the file, while Don needs to parse
> the map to get max cpu number

Ah, missed that... sorry for noise.

Thanks,
Namhyung

2014-04-07 18:20:59

by Don Zickus

[permalink] [raw]
Subject: Re: [PATCH 1/4] perf: Allow ability to map cpus to nodes easily

On Mon, Apr 07, 2014 at 02:28:35PM +0900, Namhyung Kim wrote:
> On Sun, 6 Apr 2014 14:15:46 +0200, Jiri Olsa wrote:
> > On Thu, Apr 03, 2014 at 02:48:51PM +0900, Namhyung Kim wrote:
> >> More generally, this sysfs access needs to check actual mountpoint using
> >> sysfs__mountpoint() IMHO.
> >>
> >> Also this API can be generalized like reading int value from a sysfs
> >> file as the filename itself represents the content in most cases.
> >>
> >> So how about changing this way? It might reside on somewhere in tools/lib/api/fs/.
> >>
> >> max_cpu_num = sysfs__read_int("devices/system/cpu/possible");
> >>
> >> max_node_num = sysfs__read_int("devices/system/node/possible");
> >>
> >>
> >> Hmm.. looking at the code, perf already has filename__read_{int,str} API
> >> in util/util.c. Maybe you can just use it instead.
> >
> > I think those just read the file, while Don needs to parse
> > the map to get max cpu number
>
> Ah, missed that... sorry for noise.

Well, I already implemented using filename__read_str. It is just added
overhead, but the end result is the same, a string needs to be read from
the file. I just had to remember to free the memory at the end of the
function.

Cheers,
Don