Re: [PATCH v5 28/34] perf pmus: Split pmus list into core and other

[Ian Rogers]

On Thu, Jun 8, 2023 at 9:01 PM Ravi Bangoria <ravi.bangoria@xxxxxxx> wrote:
>
> Hi Ian,

Hi Ravi,

> On 27-May-23 12:52 PM, Ian Rogers wrote:
> > Split the pmus list into core and other. This will later allow for
> > the core and other pmus to be populated separately.
> >
> > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> > Reviewed-by: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
> > ---
> >  tools/perf/util/pmus.c | 52 ++++++++++++++++++++++++++++++------------
> >  1 file changed, 38 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> > index 58ff7937e9b7..4ef4fecd335f 100644
> > --- a/tools/perf/util/pmus.c
> > +++ b/tools/perf/util/pmus.c
> > @@ -12,13 +12,19 @@
> >  #include "pmu.h"
> >  #include "print-events.h"
> >
> > -static LIST_HEAD(pmus);
> > +static LIST_HEAD(core_pmus);
> > +static LIST_HEAD(other_pmus);
>
> AMD ibs_fetch// and ibs_op// PMUs are per SMT-thread and are independent of
> core hw pmu. I wonder where does IBS fit. Currently it's part of other_pmus.
> So, is it safe to assume that other_pmus are not just uncore pmus? In that
> case shall we add a comment here?

I'm a fan of comments. The code has landed in perf-tools-next:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/pmus.c?h=perf-tools-next
Do you have any suggestions on wording? I've had limited success
adding glossary terms, for example, offcore vs uncore:
https://perf.wiki.kernel.org/index.php/Glossary#Offcore
I think offcore is a more interconnect related term, but I'd prefer
not to be inventing the definitions. I'd like it if we could be less
ambiguous in the code and provide useful information on the wiki, so
help appreciated :-)

Thanks,
Ian

> Thanks,
> Ravi