From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>,
Ingo Molnar <mingo@kernel.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Stephane Eranian <eranian@google.com>,
LKML <linux-kernel@vger.kernel.org>,
Andi Kleen <andi@firstfloor.org>, Ian Rogers <irogers@google.com>
Subject: Re: [PATCHSET 0/4] perf stat: Add --multiply-cgroup option
Date: Thu, 10 Sep 2020 08:10:20 -0300 [thread overview]
Message-ID: <20200910111020.GA4018363@kernel.org> (raw)
In-Reply-To: <20200910091542.GD1627030@krava>
Em Thu, Sep 10, 2020 at 11:15:42AM +0200, Jiri Olsa escreveu:
> On Tue, Sep 08, 2020 at 01:42:24PM +0900, Namhyung Kim wrote:
> > When we profile cgroup events with perf stat, it's very annoying to
> > specify events and cgroups on the command line as it requires the
> > mapping between events and cgroups. (Note that perf record can use
> > cgroup sampling but it's not usable for perf stat).
> > I guess most cases we just want to use a same set of events (N) for
> > all cgroups (M), but we need to specify NxM events and NxM cgroups.
> > This is not good especially when profiling large number of cgroups:
> > say M=200.
> > So I added --multiply-cgroup option to make it easy for that case. It
> > will create NxM events from N events and M cgroups. One more upside
> > is that it can handle metrics too.
> agreed that it's PITA to use -G option ;-)
yeah, its great that someone is looking at cgroups improvements, thanks
Namyung, its great to have you working on this!
More below.
> > For example, the following example measures IPC metric for 3 cgroups
> > $ cat perf-multi-cgrp.sh
> > #!/bin/sh
> > METRIC=${1:-IPC}
> > CGROUP_DIR=/sys/fs/cgroup/perf_event
> > sudo mkdir $CGROUP_DIR/A $CGROUP_DIR/B $CGROUP_DIR/C
> > # add backgroupd workload for each cgroup
> > echo $$ | sudo tee $CGROUP_DIR/A/cgroup.procs > /dev/null
> > yes > /dev/null &
> > echo $$ | sudo tee $CGROUP_DIR/B/cgroup.procs > /dev/null
> > yes > /dev/null &
> > echo $$ | sudo tee $CGROUP_DIR/C/cgroup.procs > /dev/null
> > yes > /dev/null &
> > # run 'perf stat' in the root cgroup
> > echo $$ | sudo tee $CGROUP_DIR/cgroup.procs > /dev/null
> > perf stat -a -M $METRIC --multiply-cgroup -G A,B,C sleep 1
>
> would it be easier to have new option for this? like:
>
> perf stat -a -M $METRIC --for-cgroup A,B,C
> perf stat -a -M $METRIC --for-each-cgroup A,B,C
> perf stat -a -M $METRIC --attach-cgroup A,B,C
> perf stat -a -M $METRIC --attach-to-cgroup A,B,C
>
> I'm still not sure how the --multiply-cgroup deals with empty
> cgroup A,,C but looks like we don't need this behaviour now?
Yeah, I also didn't like the --multiply-cgroup thing, perhaps we can use
a per-event term? or per group, for example:
perf stat -a -M $METRIC/cgroups=A,B,C/
perf stat -a -e '{cycles,instructions,cache-misses}/cgroups=A,B,C/'
Allowing wildcards or regexps would help with some use cases.
We already have several terms that allows us to control per event knobs,
this would be one more.
- Arnaldo
next prev parent reply other threads:[~2020-09-10 21:59 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-08 4:42 [PATCHSET 0/4] perf stat: Add --multiply-cgroup option Namhyung Kim
2020-09-08 4:42 ` [PATCH 1/4] perf evsel: Add evsel__clone() function Namhyung Kim
2020-09-10 8:59 ` Jiri Olsa
2020-09-10 13:18 ` Namhyung Kim
2020-09-08 4:42 ` [PATCH 2/4] perf stat: Add --multiply-cgroup option Namhyung Kim
2020-09-08 4:42 ` [PATCH 3/4] perf tools: Copy metric events properly when multiply cgroups Namhyung Kim
2020-09-23 9:14 ` [perf tools] 77b66fd551: perf-sanity-tests.'import_perf'_in_python.fail kernel test robot
2020-09-24 3:04 ` Namhyung Kim
2020-09-25 11:55 ` Arnaldo Carvalho de Melo
2020-09-08 4:42 ` [PATCH 4/4] perf test: Add multiply cgroup event test Namhyung Kim
2020-09-10 9:15 ` [PATCHSET 0/4] perf stat: Add --multiply-cgroup option Jiri Olsa
2020-09-10 11:10 ` Arnaldo Carvalho de Melo [this message]
2020-09-10 13:32 ` Namhyung Kim
2020-09-10 15:57 ` Andi Kleen
2020-09-10 17:11 ` Ian Rogers
2020-09-11 2:54 ` Namhyung Kim
2020-09-11 2:35 ` Namhyung Kim
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=20200910111020.GA4018363@kernel.org \
--to=acme@kernel.org \
--cc=a.p.zijlstra@chello.nl \
--cc=alexander.shishkin@linux.intel.com \
--cc=andi@firstfloor.org \
--cc=eranian@google.com \
--cc=irogers@google.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox