[RFC] perf: ref-cycle useless with watchdog changes

[RFC] perf: ref-cycle useless with watchdog changes

[Stephane Eranian]

Hi,

Since Andi added:

commit 2c95afc1e83d93fac3be6923465e1753c2c53b0a
Author: Andi Kleen <ak@linux.intel.com>
Date:   Thu Jun 9 06:14:38 2016 -0700

   perf/x86/intel, watchdog: Switch NMI watchdog to ref cycles on x86


$ perf stat -e ref-cycles ls
  <not counted> ....

fails systematically because the ref-cycles is now used by the
watchdog and given this is a system-wide pinned event, it monopolizes
the fixed counter 2 which is the
only counter able to measure this event.

So we either redirect ref-cycles towards 0x013c
(cpu_clk_unhalted:xlck) or another event maybe from the msr PMU, such
as msr/tsc/ or we revert Andi's patch. Each of these solutions has
problems:

   - 0x013c is that is counts supposedly at a fixed frequency (used to
be 100Mhz)
      but this is not guaranteed and user would still need to figure
out ref frequency
      of CPU to compare with core cycles

- msr/tsc/ counts in halted state. Recent paranoid default changes, make it not
  supported by default. msr/tsc/ counts user/kernel, with paranoid=2,
it does not
  work

- Revert Andi's patch, makes everything working again at the cost of
multiplexing
  for topdown.


It is not clear to me why topdown would be so sensitive to
multiplexing compared to other metrics.

Any comment?
Thanks

Re: [RFC] perf: ref-cycle useless with watchdog changes

[Peter Zijlstra]

On Sun, Jul 10, 2016 at 11:48:11AM -0700, Stephane Eranian wrote:
> So we either redirect ref-cycles towards 0x013c
> (cpu_clk_unhalted:xlck) or another event maybe

Another solution is us introducing (another) fake event, say 0x0400,
which will have a constrained mask of: 0x0F | (6 << 32) and varies in
actual encoding depending on which counter it lands on.

That way we have more flexibility in scheduling the NMI watchdog, and
its exact period isn't _that_ important; although we could obviously
also fix up some of that if we wanted.

Re: [RFC] perf: ref-cycle useless with watchdog changes

[Stephane Eranian]

Hi,

On Mon, Jul 11, 2016 at 3:33 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Sun, Jul 10, 2016 at 11:48:11AM -0700, Stephane Eranian wrote:
>> So we either redirect ref-cycles towards 0x013c
>> (cpu_clk_unhalted:xlck) or another event maybe
>
> Another solution is us introducing (another) fake event, say 0x0400,
> which will have a constrained mask of: 0x0F | (6 << 32) and varies in
> actual encoding depending on which counter it lands on.
>
But if you do this, you cannot give a good definition for that new event.
It may count core cycles or ref-cycles depending on event scheduling.
How can you make sense of the result?

> That way we have more flexibility in scheduling the NMI watchdog, and
> its exact period isn't _that_ important; although we could obviously
> also fix up some of that if we wanted.
>
Or you're saying 0x0400 is an "internal" event, never exposed to users that is
used only by the NMI watchdog. Note that the watchdog is timing-sensitive.
I tend to agree with you that if you use core or ref cycles it will work as well
given it needs to be setup t seconds granularity. But then each event scheduling
you'd have to readjust the sampling period to be uniform despite the
events it is
backed with may be different. Wouldn't you need yet another callback for this?
Also given that the watchdog is always system-wide pinned and how the scheduling
works it would tend to give the watchdog the fixed counter for core
cycles first.

Re: [RFC] perf: ref-cycle useless with watchdog changes

[Peter Zijlstra]

On Tue, Jul 12, 2016 at 01:24:27AM -0700, Stephane Eranian wrote:
> Hi,
> 
> On Mon, Jul 11, 2016 at 3:33 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Sun, Jul 10, 2016 at 11:48:11AM -0700, Stephane Eranian wrote:
> >> So we either redirect ref-cycles towards 0x013c
> >> (cpu_clk_unhalted:xlck) or another event maybe
> >
> > Another solution is us introducing (another) fake event, say 0x0400,
> > which will have a constrained mask of: 0x0F | (6 << 32) and varies in
> > actual encoding depending on which counter it lands on.
> >
> But if you do this, you cannot give a good definition for that new event.
> It may count core cycles or ref-cycles depending on event scheduling.
> How can you make sense of the result?

As you say below, never expose this to userspace.

> > That way we have more flexibility in scheduling the NMI watchdog, and
> > its exact period isn't _that_ important; although we could obviously
> > also fix up some of that if we wanted.
> >
> Or you're saying 0x0400 is an "internal" event, never exposed to users that is
> used only by the NMI watchdog.

Yes.

> Note that the watchdog is timing-sensitive.

Up to a point, there's a lot of leeway.

> I tend to agree with you that if you use core or ref cycles it will
> work as well given it needs to be setup t seconds granularity. But
> then each event scheduling you'd have to readjust the sampling period
> to be uniform despite the events it is backed with may be different.

Right, we'd have to adjust the period every time we switch to a
different actual event. But its not too horrible if they drift between
CPUs due to such scheduling artifacts. So we can be a bit slow/fast
etc., as long as we're mostly around the right period.

> Wouldn't you need yet another callback for this?  Also given that the
> watchdog is always system-wide pinned and how the scheduling works it
> would tend to give the watchdog the fixed counter for core cycles
> first.

Don't think so, event scheduling is done for every additional counter,
if you add an event with a tighter constraint that might well win from
this special event, since it will have a hweight of 6.

x86_schedule_events() -> perf_assign_events() -> perf_sched_next_event()
iterates through the events starting with the most constrained (wmin)
event and tries adding the more constrained events on top.

So a hweight of 6 will almost guarantee we'll try and fit this event
last, getting whatever option remains open at that time.

But yes, we might need a new callback for this to adjust the actual
encoding and period. I've not really thought through the entire ordeal
too well, it was just a hare brained idea that I threw out there as a
possible solution to look into.