linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: "Jin, Yao" <yao.jin@linux.intel.com>
Cc: acme@kernel.org, jolsa@kernel.org, peterz@infradead.org,
	mingo@redhat.com, alexander.shishkin@linux.intel.com,
	Linux-kernel@vger.kernel.org, ak@linux.intel.com,
	kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH v3 3/3] perf tools: Enable on a list of CPUs for hybrid
Date: Thu, 22 Jul 2021 12:19:40 +0200	[thread overview]
Message-ID: <YPlGPC3OkPihS91A@krava> (raw)
In-Reply-To: <598463ae-0bb0-7609-407b-4822112b2093@linux.intel.com>

On Wed, Jul 21, 2021 at 12:30:11PM +0800, Jin, Yao wrote:
> Hi Jiri,
> 
> On 7/20/2021 5:16 PM, Jiri Olsa wrote:
> > On Tue, Jul 20, 2021 at 03:07:02PM +0800, Jin, Yao wrote:
> > 
> > SNIP
> > 
> > > 
> > > OK, evlist__fix_cpus() is better, use this name in v4.
> > > 
> > > > > +{
> > > > > +	struct perf_cpu_map *cpus;
> > > > > +	struct evsel *evsel, *tmp;
> > > > > +	struct perf_pmu *pmu;
> > > > > +	int ret, unmatched_count = 0, events_nr = 0;
> > > > > +
> > > > > +	if (!perf_pmu__has_hybrid() || !cpu_list)
> > > > > +		return 0;
> > > > > +
> > > > > +	cpus = perf_cpu_map__new(cpu_list);
> > > > > +	if (!cpus)
> > > > > +		return -1;
> > > > > +
> > > > > +	evlist__for_each_entry_safe(evlist, tmp, evsel) {
> > > > > +		struct perf_cpu_map *matched_cpus, *unmatched_cpus;
> > > > > +		char buf1[128], buf2[128];
> > > > > +
> > > > > +		pmu = perf_pmu__find_hybrid_pmu(evsel->pmu_name);
> > > > > +		if (!pmu)
> > > > > +			continue;
> > > > > +
> > > > > +		ret = perf_pmu__cpus_match(pmu, cpus, &matched_cpus,
> > > > > +					   &unmatched_cpus);
> > > > > +		if (ret)
> > > > > +			goto out;
> > > > > +
> > > > > +		events_nr++;
> > > > > +
> > > > > +		if (matched_cpus->nr > 0 && (unmatched_cpus->nr > 0 ||
> > > > > +		    matched_cpus->nr < cpus->nr ||
> > > > > +		    matched_cpus->nr < pmu->cpus->nr)) {
> > > > > +			perf_cpu_map__put(evsel->core.cpus);
> > > > > +			perf_cpu_map__put(evsel->core.own_cpus);
> > > > > +			evsel->core.cpus = perf_cpu_map__get(matched_cpus);
> > > > > +			evsel->core.own_cpus = perf_cpu_map__get(matched_cpus);
> > > > 
> > > > I'm bit confused in here.. AFAIUI there's 2 evsel objects create
> > > > for hybrid 'cycles' ... should they have already proper cpus set?
> > > > 
> > > 
> > > For 'cycles', yes two evsels are created automatically. One is for atom CPU
> > > (e.g. 8-11), the other is for core CPU (e.g. 0-7). In this example, these 2
> > > evsels have already the cpus set.
> > 
> > hum, so those evsels are created with pmu's cpus, right?
> > 
> 
> Yes, that's right. But we also check and adjust the evsel->cpus by using
> user's cpu list on hybrid (what the evlist__use_cpu_list() does).
> 
> > > 
> > > While the 'cpus' here is just the user specified cpu list.
> > > cpus = perf_cpu_map__new(cpu_list);
> > 
> > then I think they will be changed by evlist__create_maps
> > with whatever user wants?
> > 
> 
> No, it will not be changed by evlist__create_maps.
> 
> In evlist__create_maps(),
> evlist->core.has_user_cpus = !!target->cpu_list && !target->hybrid;
> 
> It disables has_user_cpus for hybrid.
> 
> So in __perf_evlist__propagate_maps, they will not be changed by evlist->cpus.
> 
> if (!evsel->own_cpus || evlist->has_user_cpus) {
> 	perf_cpu_map__put(evsel->cpus);
> 	evsel->cpus = perf_cpu_map__get(evlist->cpus);
> 	
> > could we just change __perf_evlist__propagate_maps to follow
> > pmu's cpus?
> > 
> 
> In __perf_evlist__propagate_maps, it has already followed pmu's cpus because
> the evlist->has_user_cpus is false for hybrid.

sorry for delay

ok, so we first fix the cpus on hybrid events and then
propagate maps.. I guess it's ok, because it's in libperf
and that has no notion of hybrid so far

could you please rename that function so it's also obvious
it's for hybrid only

  evlist__fix_hybrid_cpus ? not sure ;-)

and add some comment with example to explain what the
function is doing

thanks,
jirka


  reply	other threads:[~2021-07-22 10:19 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-12  7:12 [PATCH v3 0/3] perf tool: Enable cpu list " Jin Yao
2021-07-12  7:12 ` [PATCH v3 1/3] libperf: Add perf_cpu_map__default_new() Jin Yao
2021-07-12  7:12 ` [PATCH v3 2/3] perf tools: Create hybrid flag in target Jin Yao
2021-07-12  7:12 ` [PATCH v3 3/3] perf tools: Enable on a list of CPUs for hybrid Jin Yao
2021-07-19 19:36   ` Jiri Olsa
2021-07-20  7:07     ` Jin, Yao
2021-07-20  9:16       ` Jiri Olsa
2021-07-21  4:30         ` Jin, Yao
2021-07-22 10:19           ` Jiri Olsa [this message]
2021-07-23  0:49             ` Jin, Yao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YPlGPC3OkPihS91A@krava \
    --to=jolsa@redhat.com \
    --cc=Linux-kernel@vger.kernel.org \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@intel.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=yao.jin@intel.com \
    --cc=yao.jin@linux.intel.com \
    --subject='Re: [PATCH v3 3/3] perf tools: Enable on a list of CPUs for hybrid' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).