[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAM9d7ci0Dn3hAX9dp1UMGK7SN-w1BoRqQz6hk8Oykfaa=LnWwg@mail.gmail.com>
Date: Tue, 6 Sep 2022 11:15:47 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Rob Herring <robh@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
James Clark <james.clark@....com>,
linux-perf-users <linux-perf-users@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] perf: Ignore format attributes with an unknown
perf_event_attr field
On Fri, Sep 2, 2022 at 8:25 AM Rob Herring <robh@...nel.org> wrote:
>
> On Fri, Sep 2, 2022 at 1:53 AM Namhyung Kim <namhyung@...nel.org> wrote:
> >
> > Hello,
> >
> > On Thu, Sep 1, 2022 at 11:55 AM Rob Herring <robh@...nel.org> wrote:
> > >
> > > If the kernel exposes a new perf_event_attr field in a format attr, perf
> > > will return an error stating the specified PMU can't be found. For
> > > example, a format attr with 'config3:0-63' causes an error if config3 is
> > > unknown to perf. This causes a compatibility issue between a newer
> > > kernel and an older perf tool.
> > >
> > > The addition here makes any attr string up to the ':' ignored, but
> > > still checks the 'bits' portion.
> > >
> > > Signed-off-by: Rob Herring <robh@...nel.org>
> > > ---
> > > This is the YACC mud I threw and seems to stick. Maybe there's a better
> > > way to handle this. It doesn't seem like there's a way to do wildcards
> > > (i.e. config.*) in YACC.
> > >
> > > This is needed for this series[1]. Unfortunately the best we do to avoid
> > > the issue is applying this to stable. I think there's some time before
> > > v8.7 h/w is deployed, too.
> >
> > Maybe you could change the format_term rule to take an identifier instead
> > of PP_CONFIG* directly and pass it to perf_pmu__new_format(). Then
> > it could check the string and create an appropriate PERF_PMU_FORMAT_VALUE_*
> > or ignore it according to the PERF_ATTR_SIZE_VER*.
>
> That only moves parsing of configN from YACC to strcmp in C. In doing
> so, we'd be left with just the 'error' token case which seems a bit
> odd (if there's another way to do it, I don't know. yacc is not my
> thing). Is that really better?
I thought we could do more flexible handling and detailed error reporting
in the C code. But it could be done in the lex/yacc as well..
I think the general idea is that we want to run a more recent version of
perf tools than the kernel. So if it detects the tool is older, it can show
a warning message like:
"config3 is not in the perf_event_attr.. skipping.
Maybe you're running on a newer kernel. Please upgrade the perf tool."
> Unless there is some way to retrieve
> the PERF_ATTR_SIZE_VER* from the kernel at runtime?
Even if it can retrieve the info at runtime, perf tool might not know
how to use the new config term.
Thanks,
Namhyung
Powered by blists - more mailing lists