发自我的 iPhone
> 在 2015年9月8日，下午9:13，Jiri Olsa <[email protected]> 写道： > >> On Tue, Sep 08, 2015 at 04:12:55PM +0800, Wangnan (F) wrote: >> >> >>> On 2015/9/8 15:37, Jiri Olsa wrote: >>> 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; >> >> Now I understand your suggestion. You mean we can build env->cpu before >> processing the first sample, then init al.socket using that map instead > > hum, that should be the case anyway.. features are read before events > >> of calling cpu_map__get_socket_id() unconditionally in an ad-hoc way. >> >> And I have another question that, since build_cpu_topo() and >> perf_event__preprocess_sample() are more or less doing similar things, >> why we need both of them? > > perf_event__preprocess_sample is called for each sample, > while build_cpu_topo is part of storing topology feature Sorry, what I wanted to say should be: cpu_map__get_socket_id() and build_cpu_topo()... > > jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
