From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753927AbbIHHh5 (ORCPT ); Tue, 8 Sep 2015 03:37:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34583 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753824AbbIHHhv (ORCPT ); Tue, 8 Sep 2015 03:37:51 -0400 Date: Tue, 8 Sep 2015 09:37:47 +0200 From: Jiri Olsa To: "Wangnan (F)" Cc: acme@kernel.org, kan.liang@intel.com, linux-kernel@vger.kernel.org, lizefan@huawei.com, pi3orama@163.com, Adrian Hunter , Andi Kleen , Jiri Olsa , Namhyung Kim , Stephane Eranian , Arnaldo Carvalho de Melo Subject: Re: [PATCH] perf report: Fix invalid memory accessing Message-ID: <20150908073747.GA2038@krava.brq.redhat.com> References: <1441630315-189525-1-git-send-email-wangnan0@huawei.com> <55ED90BE.9090706@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55ED90BE.9090706@huawei.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Sep 07, 2015 at 09:27:26PM +0800, Wangnan (F) wrote: SNIP > > I found the problem. > > perf relies on build_cpu_topology() to fetch CPU_TOPOLOGY from sysfs. It > depend on > the existance of > > /sys/devices/system/cpu/cpu%d/topology/core_siblings_list > > However, CPU can be canceled by hotcpu subsystem. After that the directory > of > /sys/devices/system/cpu/cpu%d/topology is gone, which causes perf's > write_cpu_topology() --> uild_cpu_topology() to fail, result in the above > perf.data. > > So I think my patch is required. no question there.. I just meant it should be placed in perf_event__preprocess_sample function with the rest of the 'al' initialization, like in the patch below? it does not compile, because there're many places calling it and it'd need changing all callers to pass env, which seems to require more changes.. also I'm not sure about removing: - al->socket = cpu_map__get_socket_id(al->cpu); Does any command actually need this initialized from current system? thanks, jirka --- diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c index 0bf8c9889fc0..3339d2579bfc 100644 --- a/tools/perf/util/event.c +++ b/tools/perf/util/event.c @@ -990,7 +990,8 @@ void thread__find_addr_location(struct thread *thread, int perf_event__preprocess_sample(const union perf_event *event, struct machine *machine, struct addr_location *al, - struct perf_sample *sample) + struct perf_sample *sample, + struct perf_env *env) { u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK; struct thread *thread = machine__findnew_thread(machine, sample->pid, @@ -1021,7 +1022,10 @@ int perf_event__preprocess_sample(const union perf_event *event, al->sym = NULL; al->cpu = sample->cpu; - al->socket = cpu_map__get_socket_id(al->cpu); + + al.socket = -1; + if (env->cpu && al->cpu >= 0) + al.socket = env->cpu[al->cpu].socket_id; if (al->map) { struct dso *dso = al->map->dso;