[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZvsE5IQtEkYooDA8@google.com>
Date: Mon, 30 Sep 2024 13:07:00 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: "Liang, Kan" <kan.liang@...ux.intel.com>
Cc: James Clark <james.clark@...aro.org>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Ian Rogers <irogers@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org,
Ravi Bangoria <ravi.bangoria@....com>,
Mark Rutland <mark.rutland@....com>,
James Clark <james.clark@....com>, Kajol Jain <kjain@...ux.ibm.com>,
Thomas Richter <tmricht@...ux.ibm.com>,
Atish Patra <atishp@...shpatra.org>,
Palmer Dabbelt <palmer@...osinc.com>,
Mingwei Zhang <mizhang@...gle.com>
Subject: Re: [PATCH 04/10] perf stat: Add --exclude-guest option
On Wed, Sep 25, 2024 at 09:49:14AM -0400, Liang, Kan wrote:
>
>
> On 2024-09-24 4:21 p.m., Namhyung Kim wrote:
> > On Mon, Sep 23, 2024 at 09:47:17AM +0100, James Clark wrote:
> >>
> >> On 06/09/2024 3:33 pm, Liang, Kan wrote:
> >>>
> >>> On 2024-09-05 4:24 p.m., Namhyung Kim wrote:
> >>>> This option is to support the old behavior of setting exclude_guest by
> >>>> default. Now it doesn't set the bit so users want the old behavior can
> >>>> use this option.
> >>>>
> >>>> $ perf stat true
> >>>>
> >>>> Performance counter stats for 'true':
> >>>>
> >>>> 0.86 msec task-clock:u # 0.443 CPUs utilized
> >>>> 0 context-switches:u # 0.000 /sec
> >>>> 0 cpu-migrations:u # 0.000 /sec
> >>>> 49 page-faults:u # 56.889 K/sec
> >>>> ...
> >>>>
> >>>> $ perf stat --exclude-guest true
> >>>>
> >>>> Performance counter stats for 'true':
> >>>>
> >>>> 0.79 msec task-clock:Hu # 0.490 CPUs utilized
> >>>> 0 context-switches:Hu # 0.000 /sec
> >>>> 0 cpu-migrations:Hu # 0.000 /sec
> >>>> 49 page-faults:Hu # 62.078 K/sec
> >>>> ...
> >>>>
> >>>> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> >>>> ---
> >>>> tools/perf/Documentation/perf-stat.txt | 7 +++++++
> >>>> tools/perf/builtin-stat.c | 2 ++
> >>>> 2 files changed, 9 insertions(+)
> >>>>
> >>>> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> >>>> index 2bc06367248691dd..d28d8370a856598f 100644
> >>>> --- a/tools/perf/Documentation/perf-stat.txt
> >>>> +++ b/tools/perf/Documentation/perf-stat.txt
> >>>> @@ -382,6 +382,13 @@ color the metric's computed value.
> >>>> Don't print output, warnings or messages. This is useful with perf stat
> >>>> record below to only write data to the perf.data file.
> >>>> +--exclude-guest::
> >>>> +Don't count event in the guest mode. It was the old behavior but the
> >>>> +default is changed to count guest events also. Use this option if you
> >>>> +want the old behavior (host only). Note that this option needs to be
> >>>> +before other events in case you added -e/--event option in the command
> >>>> +line.
> >>> I'm not sure if we really need this option. I think it may bring more
> >>> trouble than what we get.
> >>>
> >>> The name of the "--exclude-guest" sounds like a replacement of the event
> >>> modifier "H". But in fact, it's not. It should only affect the default.
> >>> It doesn't set the "H" for any events.
> > Well I think it's tricky but it'd set "H" modifier events after the
> > option. But I have to agree that it can bring more troubles.
>
> I may have miss-read something before. After some simple tests, yes, the
> "H" is applied with the option.
Ok.
>
> Since there is a limit for the "--exclude-guest" option, can we print a
> warning if the option becomes invalid because of the order?
Well.. I'm thinking of removing this option for now.
>
> >
> >>> Except for the perf kvm user, I don't think there are many users which
> >>> care the exclude_guest. The behavior of the perf kvm is not changed. So
> >>> the option seems not that important. If we really want an option to
> >>> restore the old behavior, it's better to choose a better name and update
> >>> the description.
> > Personally I don't want to this option but just worried if there's a
> > case where exclude_guest is preferred.
>
> The only case I can imagine is that, with the new vPMU passthrough
> introduced, some users may want to explicitly set the exclude_guest to
> avoid the fallback. But I'm not sure how useful it is for them.
Because of overhead? They'll get exclude_guest eventually, right?
So I think I can drop this patch for now. And consider this later when
we can find a real usecase.
Thanks,
Namhyung
Powered by blists - more mailing lists