From: Stephane Eranian <eranian@google.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
mingo@elte.hu, Ian Rogers <irogers@google.com>,
"Phillips, Kim" <kim.phillips@amd.com>,
Jiri Olsa <jolsa@redhat.com>
Subject: Re: [PATCH 3/3] perf/x86/rapl: add AMD Fam17h RAPL support
Date: Wed, 20 May 2020 01:34:50 -0700 [thread overview]
Message-ID: <CABPqkBQ8esgB1QiLGi68YM6CNxwY-78E7MHNtrxDjZNAhiSw0g@mail.gmail.com> (raw)
In-Reply-To: <CABPqkBQt0a7Pi7X2cLz601TJEqasDXrOZcwuBhDCjcZ+ishPMw@mail.gmail.com>
Hi,
On Mon, May 18, 2020 at 1:16 PM Stephane Eranian <eranian@google.com> wrote:
>
> On Mon, May 18, 2020 at 2:34 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Fri, May 15, 2020 at 02:57:33PM -0700, Stephane Eranian wrote:
> >
> > > +static struct perf_msr amd_rapl_msrs[] = {
> > > + [PERF_RAPL_PP0] = { 0, &rapl_events_cores_group, NULL},
> > > + [PERF_RAPL_PKG] = { MSR_AMD_PKG_ENERGY_STATUS, &rapl_events_pkg_group, test_msr },
> > > + [PERF_RAPL_RAM] = { 0, &rapl_events_ram_group, NULL},
> > > + [PERF_RAPL_PP1] = { 0, &rapl_events_gpu_group, NULL},
> > > + [PERF_RAPL_PSYS] = { 0, &rapl_events_psys_group, NULL},
> > > +};
> >
> > Why have those !PKG things initialized? Wouldn't they default to 0
> > anyway? If not, surely { 0, } is sufficient.
>
> Yes, but that assumes that perf_msr_probe() is fixed to not expect a grp.
> I think it is best to fix perf_msr_probe(). I already fixed one
> problem, I'll fix this one as well.
Well, now I remember what I did it the way it is in the patch. This
grp is going to sysfs, i.e., visible vs. not_visible callback.
Even if I fix the handling of NULL grp in perf_msr_probe(), the rest
of the rapl code pushes every event to sysfs and if the visible
callback is set to NULL this means the event is visible for sysfs! We
can fix that in init_rapl_pmus() but that is not pretty or leave it
as is, your call.
next prev parent reply other threads:[~2020-05-20 8:35 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-15 21:57 [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h Stephane Eranian
2020-05-15 21:57 ` [PATCH 1/3] perf/x86/rapl: move RAPL support to common x86 code Stephane Eranian
2020-05-18 9:08 ` Peter Zijlstra
2020-05-15 21:57 ` [PATCH 2/3] perf/x86/rapl: refactor code for Intel/AMD sharing Stephane Eranian
2020-05-18 9:23 ` Peter Zijlstra
2020-05-15 21:57 ` [PATCH 3/3] perf/x86/rapl: add AMD Fam17h RAPL support Stephane Eranian
2020-05-18 9:34 ` Peter Zijlstra
2020-05-18 20:16 ` Stephane Eranian
2020-05-20 8:34 ` Stephane Eranian [this message]
2020-05-16 16:56 ` [PATCH 0/3] perf/x86/rapl: Enable RAPL for AMD Fam17h Alexander Monakov
2020-05-18 8:50 ` Jiri Olsa
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=CABPqkBQ8esgB1QiLGi68YM6CNxwY-78E7MHNtrxDjZNAhiSw0g@mail.gmail.com \
--to=eranian@google.com \
--cc=irogers@google.com \
--cc=jolsa@redhat.com \
--cc=kim.phillips@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).