Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200908044228.61197-1-namhyung@kernel.org> <20200910155736.jadhmqvnqquammpn@two.firstfloor.org>
 <CAP-5=fUp90QrY+HcetpgZJeUn2fSqndRCZ8f1m=rFcEwO7ZZew@mail.gmail.com>
In-Reply-To: <CAP-5=fUp90QrY+HcetpgZJeUn2fSqndRCZ8f1m=rFcEwO7ZZew@mail.gmail.com>
From:   Namhyung Kim <namhyung@kernel.org>
Date:   Fri, 11 Sep 2020 11:54:45 +0900
Message-ID: <CAM9d7cgxrhBfNbFKg6HfMV4SXPe9G1nE6Fy6Uj=Bb5h9LOGUoA@mail.gmail.com>
Subject: Re: [PATCHSET 0/4] perf stat: Add --multiply-cgroup option
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>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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