From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752576AbbIKKUV (ORCPT ); Fri, 11 Sep 2015 06:20:21 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:56322 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752191AbbIKKUS (ORCPT ); Fri, 11 Sep 2015 06:20:18 -0400 Message-ID: <55F2AAD3.7040506@huawei.com> Date: Fri, 11 Sep 2015 18:20:03 +0800 From: "Wangnan (F)" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Arnaldo Carvalho de Melo , "Liang, Kan" CC: Ingo Molnar , "linux-kernel@vger.kernel.org" , "Hunter, Adrian" , Borislav Petkov , David Ahern , "Frederic Weisbecker" , Jiri Olsa , Namhyung Kim , Stephane Eranian Subject: Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method References: <1441828225-667-1-git-send-email-acme@kernel.org> <1441828225-667-5-git-send-email-acme@kernel.org> <37D7C6CF3E00A74B8858931C1DB2F07701922457@SHSMSX103.ccr.corp.intel.com> <20150910131250.GW3475@kernel.org> In-Reply-To: <20150910131250.GW3475@kernel.org> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.111.66.109] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A0B0204.55F2AAD8.024A,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 3f7f1d1c7b1088264f1e3bf0bd2a8ea6 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2015/9/10 21:12, Arnaldo Carvalho de Melo wrote: [SNIP] > + > +int perf_env__read_cpu_topology_map(struct perf_env *env) > +{ > + int cpu, nr_cpus; > + > + if (env->cpu != NULL) > + return 0; > + > + if (env->nr_cpus_avail == 0) > + env->nr_cpus_avail = sysconf(_SC_NPROCESSORS_CONF); > + > + nr_cpus = env->nr_cpus_avail; > + if (nr_cpus == -1) > + return -EINVAL; > + > + env->cpu = calloc(nr_cpus, sizeof(env->cpu[0])); > + if (env->cpu == NULL) > + return -ENOMEM; > + > + for (cpu = 0; cpu < nr_cpus; ++cpu) { > + env->cpu[cpu].core_id = cpu_map__get_core_id(cpu); > + env->cpu[cpu].socket_id = cpu_map__get_socket_id(cpu); > + } Shouldn't we check the failure of these two functions? At this point perf_env__read_cpu_topology_map and build_cpu_topology are doing similar things. build_cpu_topology() reads /sys/xxxxx/cpu%d/topology/{core,thread}_siblings_list, perf_env__read_cpu_topology_map() reads /sys/xxxxx/cpu%d/topology/core_id, but build_cpu_topology() returns error if any read failed, but perf_env__read_cpu_topology_map() fills core_id and socket_id with -1 if read fail. I tried to offline a core between build_cpu_topology() and perf_env__read_cpu_topology_map(), and perf report say: # perf report -v --header-only -I build id event received for [kernel.kallsyms]: (...) core_id number is too big.You may need to upgrade the perf tool. <-- *see this warning* # ======== # captured on: Sun Feb 15 15:01:05 2009 # hostname : localhost ... # sibling cores : ... # sibling threads : 7 # Core ID and Socket ID information is not available # pmu mappings: not available # ======== Thank you. > + > + env->nr_cpus_avail = nr_cpus; > + return 0; > +} > diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h > index 70124d9a1624..c4e36323d91e 100644 > --- a/tools/perf/util/env.h > +++ b/tools/perf/util/env.h > @@ -38,4 +38,6 @@ void perf_env__exit(struct perf_env *env); > > int perf_env__set_cmdline(struct perf_env *env, int argc, const char *argv[]); > > +int perf_env__read_cpu_topology_map(struct perf_env *env); > + > #endif /* __PERF_ENV_H */ > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c > index 151b8310ac70..d6437465f70f 100644 > --- a/tools/perf/util/header.c > +++ b/tools/perf/util/header.c > @@ -415,8 +415,6 @@ struct cpu_topo { > u32 thread_sib; > char **core_siblings; > char **thread_siblings; > - int *core_id; > - int *phy_pkg_id; > }; > > static int build_cpu_topo(struct cpu_topo *tp, int cpu) > @@ -479,9 +477,6 @@ try_threads: > } > ret = 0; > done: > - tp->core_id[cpu] = cpu_map__get_core_id(cpu); > - tp->phy_pkg_id[cpu] = cpu_map__get_socket_id(cpu); > - > if(fp) > fclose(fp); > free(buf); > @@ -509,7 +504,7 @@ static struct cpu_topo *build_cpu_topology(void) > struct cpu_topo *tp; > void *addr; > u32 nr, i; > - size_t sz, sz_id; > + size_t sz; > long ncpus; > int ret = -1; > > @@ -520,9 +515,8 @@ static struct cpu_topo *build_cpu_topology(void) > nr = (u32)(ncpus & UINT_MAX); > > sz = nr * sizeof(char *); > - sz_id = nr * sizeof(int); > > - addr = calloc(1, sizeof(*tp) + 2 * sz + 2 * sz_id); > + addr = calloc(1, sizeof(*tp) + 2 * sz); > if (!addr) > return NULL; > > @@ -532,10 +526,6 @@ static struct cpu_topo *build_cpu_topology(void) > tp->core_siblings = addr; > addr += sz; > tp->thread_siblings = addr; > - addr += sz; > - tp->core_id = addr; > - addr += sz_id; > - tp->phy_pkg_id = addr; > > for (i = 0; i < nr; i++) { > ret = build_cpu_topo(tp, i); > @@ -554,7 +544,7 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused, > { > struct cpu_topo *tp; > u32 i; > - int ret; > + int ret, j; > > tp = build_cpu_topology(); > if (!tp) > @@ -579,11 +569,17 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused, > break; > } > > - for (i = 0; i < tp->cpu_nr; i++) { > - ret = do_write(fd, &tp->core_id[i], sizeof(int)); > + ret = perf_env__read_cpu_topology_map(&perf_env); > + if (ret < 0) > + goto done; > + > + for (j = 0; j < perf_env.nr_cpus_avail; j++) { > + ret = do_write(fd, &perf_env.cpu[j].core_id, > + sizeof(perf_env.cpu[j].core_id)); > if (ret < 0) > return ret; > - ret = do_write(fd, &tp->phy_pkg_id[i], sizeof(int)); > + ret = do_write(fd, &perf_env.cpu[j].socket_id, > + sizeof(perf_env.cpu[j].socket_id)); > if (ret < 0) > return ret; > }