From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757669AbbCDAPk (ORCPT ); Tue, 3 Mar 2015 19:15:40 -0500 Received: from mail.kernel.org ([198.145.29.136]:56414 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757443AbbCDAPj (ORCPT ); Tue, 3 Mar 2015 19:15:39 -0500 Date: Tue, 3 Mar 2015 21:15:33 -0300 From: Arnaldo Carvalho de Melo To: "Liang, Kan" Cc: "a.p.zijlstra@chello.nl" , "linux-kernel@vger.kernel.org" , "ak@linux.intel.com" , Stephane Eranian Subject: Re: [PATCH 2/5] perf,tools: check and re-organize evsel cpu maps Message-ID: <20150304001533.GP5187@kernel.org> References: <1425372886-33397-1-git-send-email-kan.liang@intel.com> <1425372886-33397-2-git-send-email-kan.liang@intel.com> <20150303160929.GI5187@kernel.org> <20150303161139.GJ5187@kernel.org> <37D7C6CF3E00A74B8858931C1DB2F0770170C75A@SHSMSX103.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <37D7C6CF3E00A74B8858931C1DB2F0770170C75A@SHSMSX103.ccr.corp.intel.com> X-Url: http://acmel.wordpress.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 Em Tue, Mar 03, 2015 at 05:09:29PM +0000, Liang, Kan escreveu: > > > > Em Tue, Mar 03, 2015 at 01:09:29PM -0300, Arnaldo Carvalho de Melo > > escreveu: > > > Em Tue, Mar 03, 2015 at 03:54:43AM -0500, kan.liang@intel.com escreveu: > > > > From: Kan Liang > > > > > > > > With the patch 1/5, it's possible to group read events from > > > > different pmus. "-C" can be used to set cpu list. The cpu list may > > > > be incompatible with pmu's cpumask. > > > > This patch checks the event's cpu maps, and discard the incompatible > > > > cpu maps. > > > > event's cpu maps is saved in evsel->cpus during option parse. Then > > > > the evlist's cpu maps is created in perf_evlist__create_maps. So the > > > > cpu maps can be check and re-organized in perf_evlist__create_maps. > > > > Only cpu_list need to check the cpu maps. > > > > > > Humm, I had something done in this area... > > > > > > Stephane complained about the confusion about which cpumap to use > > with > > > pmus, so I wrote a patch and sent an RFC, which I think I got no > > > comments, lemme dig it... > > > > Here it is, can you take a look? Stephane? > > > > Your patch is more like my 3/5 patch. The difference is your patch force > the evsel->cpus = evlist->cpus, if evsel->cpus == NULL. > My patch handle the evsel->cpus == NULL case when using it. Idea is to use evsel->cpus always, not having to special case it and fallback to evlist->cpus, so that we don't have to pass evlist around that often. > > @@ -1216,8 +1206,8 @@ static void print_aggr(char *prefix) > > evlist__for_each(evsel_list, counter) { > > val = ena = run = 0; > > nr = 0; > > - for (cpu = 0; cpu < perf_evsel__nr_cpus(counter); > > cpu++) { > > - cpu2 = perf_evsel__cpus(counter)- > > >map[cpu]; > > + for (cpu = 0; cpu < cpu_map__nr(counter->cpus); > > cpu++) { > > + cpu2 = counter->cpus->map[cpu]; > > s2 = aggr_get_id(evsel_list->cpus, cpu2); > > if (s2 != id) > > continue; > > print_aggr also need to be special handled. In the past, all events use > evlist's cpu map,so it uses index to find the real cpu id. > Now, event's cpu map are different. The s2 could be wrong. > For example, evlist's cpu map is 0,4,5,18. Event's cpu map could be 0,18. > When cpu == 1, the return of aggr_get_id must be wrong, since it > still use index to find s2. > My 3/5 patch introduce a function perf_evsel__get_cpumap_index > to handle it. > > Only your patch is not enough, we still need 2/5 and 4/5. > 2/5 is used to check if the event's cpu maps are compatible as evlist's > cpu map. For example, evlist's cpu map is 1,2,17. Event's cpu map > could be 0,18. We can error out earlier. > 4/5 is used to special handle the open and mmap. We need to do > the same thing as what we did in print_aggr. I'll try to go thru this tomorrow, thanks for checking. - Arnaldo