From: Andi Kleen <ak@linux.intel.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Andi Kleen <andi@firstfloor.org>,
acme@kernel.org, linux-kernel@vger.kernel.org, jolsa@kernel.org,
eranian@google.com, kan.liang@linux.intel.com,
peterz@infradead.org
Subject: Re: [PATCH v2 9/9] perf stat: Use affinity for enabling/disabling events
Date: Wed, 23 Oct 2019 06:07:13 -0700 [thread overview]
Message-ID: <20191023130713.GG4660@tassilo.jf.intel.com> (raw)
In-Reply-To: <20191023103048.GK22919@krava>
On Wed, Oct 23, 2019 at 12:30:48PM +0200, Jiri Olsa wrote:
> On Sun, Oct 20, 2019 at 10:52:02AM -0700, Andi Kleen wrote:
>
> SNIP
>
> >
> > void evlist__enable(struct evlist *evlist)
> > {
> > struct evsel *pos;
> > + struct affinity affinity;
> > + struct perf_cpu_map *cpus;
> > + int i;
> > +
> > + if (affinity__setup(&affinity) < 0)
> > + return;
> > +
> > + cpus = evlist__cpu_iter_start(evlist);
> > + for (i = 0; i < cpus->nr; i++) {
> > + int cpu = cpus->map[i];
> > + affinity__set(&affinity, cpu);
> >
> > + evlist__for_each_entry(evlist, pos) {
> > + if (evlist__cpu_iter_skip(pos, cpu))
> > + continue;
> > + if (!perf_evsel__is_group_leader(pos) || !pos->core.fd)
> > + continue;
>
> all the previous patches and this one have this code in common,
> could we make this a single function, that would call a callback
> that would have affinity set.. sort of like what we do in
> cpu_function_call in the kernel
I'm personally not a big friend of call backs. They usually make
the code harder to read and reason about.
Prefer to use callable libraries of common code.
Also the event open code has some more complex variants of this pattern
which would need multiple call backs.
I already factored the common code into the iterator.
I guess the for loop could be a macro, and affinity_set() could
perhaps accept the map and return the cpu. I'll add that in
the next version. This will reduce the common code by a few lines
more.
-Andi
next prev parent reply other threads:[~2019-10-23 13:07 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-20 17:51 Optimize perf stat for large number of events/cpus v2 Andi Kleen
2019-10-20 17:51 ` [PATCH v2 1/9] perf evsel: Always preserve errno while cleaning up perf_event_open failures Andi Kleen
2019-10-22 8:01 ` Jiri Olsa
2019-11-12 11:18 ` [tip: perf/core] " tip-bot2 for Andi Kleen
2019-10-20 17:51 ` [PATCH v2 2/9] perf evsel: Avoid close(-1) Andi Kleen
2019-10-22 8:01 ` Jiri Olsa
2019-11-12 11:18 ` [tip: perf/core] " tip-bot2 for Andi Kleen
2019-10-20 17:51 ` [PATCH v2 3/9] perf pmu: Use file system cache to optimize sysfs access Andi Kleen
2019-10-23 9:47 ` Jiri Olsa
2019-10-20 17:51 ` [PATCH v2 4/9] perf affinity: Add infrastructure to save/restore affinity Andi Kleen
2019-10-23 9:59 ` Jiri Olsa
2019-10-23 13:02 ` Andi Kleen
2019-10-23 14:30 ` Jiri Olsa
2019-10-23 14:52 ` Andi Kleen
2019-10-23 16:16 ` Alexey Budankov
2019-10-23 17:19 ` Andi Kleen
2019-10-23 18:08 ` Alexey Budankov
2019-10-23 22:37 ` Andi Kleen
2019-10-24 8:46 ` Alexey Budankov
2019-10-20 17:51 ` [PATCH v2 5/9] perf evsel: Add iterator to iterate over events ordered by CPU Andi Kleen
2019-10-20 17:51 ` [PATCH v2 6/9] perf stat: Use affinity for closing file descriptors Andi Kleen
2019-10-20 17:52 ` [PATCH v2 7/9] perf stat: Use affinity for opening events Andi Kleen
2019-10-20 17:52 ` [PATCH v2 8/9] perf stat: Use affinity for reading Andi Kleen
2019-10-20 17:52 ` [PATCH v2 9/9] perf stat: Use affinity for enabling/disabling events Andi Kleen
2019-10-23 10:30 ` Jiri Olsa
2019-10-23 13:07 ` Andi Kleen [this message]
2019-10-22 8:02 ` Optimize perf stat for large number of events/cpus v2 Jiri Olsa
2019-10-22 14:11 ` Arnaldo Carvalho de Melo
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=20191023130713.GG4660@tassilo.jf.intel.com \
--to=ak@linux.intel.com \
--cc=acme@kernel.org \
--cc=andi@firstfloor.org \
--cc=eranian@google.com \
--cc=jolsa@kernel.org \
--cc=jolsa@redhat.com \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.