All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Andi Kleen <andi@firstfloor.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Jiri Olsa <jolsa@redhat.com>, 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>
Subject: Re: [PATCHSET 0/4] perf stat: Add --multiply-cgroup option
Date: Fri, 11 Sep 2020 11:54:45 +0900	[thread overview]
Message-ID: <CAM9d7cgxrhBfNbFKg6HfMV4SXPe9G1nE6Fy6Uj=Bb5h9LOGUoA@mail.gmail.com> (raw)
In-Reply-To: <CAP-5=fUp90QrY+HcetpgZJeUn2fSqndRCZ8f1m=rFcEwO7ZZew@mail.gmail.com>

Hi Ian,

On Fri, Sep 11, 2020 at 2:11 AM Ian Rogers <irogers@google.com> wrote:
> Just to throw in some 2c worth. A nice thing about Namhyung's approach
> is that it can apply for metrics, events and pfm-events. It would be
> nice if there were a single option for specifying these, perhaps we
> can figure one out.

Right!  Thanks for sharing your opinion. :)

>
> One thought that came to mind based on Namhyung's multiply name was
> and assuming a cgroup could be a modifier:
>
> perf record -e cycles:G1
>
> there could be equivalent to a new syntax of lists and multiplies of:
>
> perf record -e [cycles]*[:G1]
>
> This would allow expressions like:
>
> perf record -e [{instructions,cycles},branches]*[:u,:k]
>
> which would be equivalent to (showing the effect on sibling groups):
>
> perf record -e {instructions:u,cycles:u},branches:u,{instructions:k,cycles:k},branches:k
>
> Adding in cgroups makes a longer list of events:
>
> perf record -e [{instructions,cycles},branches]*[:u,:k]*[:G1,:G2]
>
> becomes:
>
> perf record -e {instructions:u:G1,cycles:u:G1},branches:u:G1,{instructions:k:G1,cycles:k:G1},branches:k:G1,{instructions:u:G2,cycles:u:G2},branches:u:G2,{instructions:k:G2,cycles:k:G2},branches:k:G2
>
> This is somewhat similar to Arnaldo's proposal but trying to make
> things a bit more generic, avoiding overloading the use of sibling
> groups, .. Perhaps there is a syntax that others prefer or could be
> borrowed from a familiar source like a programming language.

Yes, but this is like a radical change.  This would be nice to deal
with repetition in the event list.  But I guess mostly we only use a
small number of events (beside the cgroups)...

>
> I think for Namhyung's sake it is important not to get too distracted
> by desires for better syntax, as this change makes a benefit in a way
> that works with the existing flags. If it is accepted, the man pages
> need to be updated.

Right.  I wonder whether they're gonna take the new option
(maybe --for-each-cgroup ?) or not regardless of the syntax change.

Thanks
Namhyung

  reply	other threads:[~2020-09-11  2:55 UTC|newest]

Thread overview: 20+ 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-23  9:14     ` kernel test robot
2020-09-24  3:04     ` Namhyung Kim
2020-09-24  3:04       ` Namhyung Kim
2020-09-25 11:55       ` Arnaldo Carvalho de Melo
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
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 [this message]
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='CAM9d7cgxrhBfNbFKg6HfMV4SXPe9G1nE6Fy6Uj=Bb5h9LOGUoA@mail.gmail.com' \
    --to=namhyung@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --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 \
    /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.