Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1652203 > unrolled thread

Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Stephane Eranian <eranian@google.com> - 2017-05-28 22:40 +0200
    Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Peter Zijlstra <peterz@infradead.org> - 2017-05-30 11:30 +0200
      Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Andi Kleen <ak@linux.intel.com> - 2017-05-30 16:00 +0200
        Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Peter Zijlstra <peterz@infradead.org> - 2017-05-30 18:30 +0200
          Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Stephane Eranian <eranian@google.com> - 2017-05-30 18:50 +0200
          Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Andi Kleen <ak@linux.intel.com> - 2017-05-30 19:30 +0200
            Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Peter Zijlstra <peterz@infradead.org> - 2017-05-30 19:50 +0200
              Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Andi Kleen <ak@linux.intel.com> - 2017-05-30 20:00 +0200
                Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Peter Zijlstra <peterz@infradead.org> - 2017-05-30 21:10 +0200
                  Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Andi Kleen <ak@linux.intel.com> - 2017-05-30 21:50 +0200
      Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter Stephane Eranian <eranian@google.com> - 2017-05-30 18:50 +0200
        Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP  counter Thomas Gleixner <tglx@linutronix.de> - 2017-05-30 19:00 +0200

#1652203 — Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter

On Wed, May 24, 2017 at 9:01 AM, Vince Weaver <vincent.weaver@maine.edu> wrote:
>
> On Wed, 24 May 2017, Andi Kleen wrote:
>
> > > Right, I did not even consider the rdpmc, but yeah, you will get a count that
> > > is not relevant to the user visible event. Unless you fake it using the time
> > > scaling fields there but that's ugly.
> >
> > Could add another scaling field to the mmap page for this.
>
> The whole point of the rdpmc() implementation is to be low overhead.
> If you have to parse 10 different mmap() fields it starts to defeat the
> purpose.
>
> I already have people really grumpy that you have to have one mmap() page
> per event, meaning you sacrifice one TLB entry for each event you are
> measuring.
>
>
> If the watchdog counter is constantly running, can't you just modify
> perf_event to just grab start/stop values at context switch time and
> provide the difference to the user?  Sort of like the "always running"
> patchsets that float around? Though I guess that doesn't help much with
> sampling.
>
This goes back to my initial point of simply increasing the period to
avoid false positive and stay with the
core-cycle event. It is much simpler. And again, if you are
deadlocked, it should not matter if you detect
it after 2mn or 5mn or 10mn. The accuracy is not so critical. And by
choosing a large enough period, you
avoid the issue with Turbo, even if you consider Turbo being able to
double your reference frequency.

Ultimately, I would like to see the watchdog move out of the PMU. That
is the only sensible solution.
You just need a resource able to interrupt on NMI or you handle
interrupt masking in software as has
been proposed on LKML.

[toc] | [next] | [standalone]

#1653025

On Sun, May 28, 2017 at 01:31:09PM -0700, Stephane Eranian wrote:
> Ultimately, I would like to see the watchdog move out of the PMU. That
> is the only sensible solution.
> You just need a resource able to interrupt on NMI or you handle
> interrupt masking in software as has
> been proposed on LKML.

So even if we do the soft masking, we still need to deal with regions
where the interrupts are disabled. Once an interrupt hits the soft mask
we still hardware mask.

So to get full and reliable coverage we still need an NMI source.

I agree that it would be lovely to free up the one counter though.


One other approach is running the watchdog off of _any_ PMI, then all we
need to ensure is that PMIs happen semi regularly. There are two cases
where this becomes 'interesting':

 - we have only !sampling events; in this case we have PMIs but at the
   max period to properly account for counter overflow. This is too
   large a period. We'd have to muck with the max period of at least one
   counter.

 - we have _no_ events; in this case we need to somehow schedule an
   event anyway.

It might be possible to deal with both cases by fudging the state of one
of the fixed counters. Never clear the EN bit for that counter and
reduce the max period for that one counter.


I think a scheme like that was mentioned before, but I'm also afraid
that it'll turn into quite the mess if we try it. And by its very nature
it adds complexity and therefore risks reducing the reliability of the
thing :/

[toc] | [prev] | [next] | [standalone]

#1653256

On Tue, May 30, 2017 at 11:25:23AM +0200, Peter Zijlstra wrote:
> On Sun, May 28, 2017 at 01:31:09PM -0700, Stephane Eranian wrote:
> > Ultimately, I would like to see the watchdog move out of the PMU. That
> > is the only sensible solution.
> > You just need a resource able to interrupt on NMI or you handle
> > interrupt masking in software as has
> > been proposed on LKML.
> 
> So even if we do the soft masking, we still need to deal with regions
> where the interrupts are disabled. Once an interrupt hits the soft mask
> we still hardware mask.
> 
> So to get full and reliable coverage we still need an NMI source.

You would only need a single one per system however, not one per CPU.
RCU already tracks all the CPUs, all we need is a single NMI watchdog
that makes sure RCU itself does not get stuck.

So we just have to find a single watchdog somewhere that can trigger
NMI.

> I agree that it would be lovely to free up the one counter though.

One option is to use the TCO watchdog in the chipset instead. 
Unfortunatley it's not an universal solution because some BIOS lock
the TCO watchdog for their own use. But if you have a BIOS that
doesn't do that it should work.

> One other approach is running the watchdog off of _any_ PMI, then all we
> need to ensure is that PMIs happen semi regularly. There are two cases
> where this becomes 'interesting':

Seems fairly complex.

-Andi

[toc] | [prev] | [next] | [standalone]

#1653357

On Tue, May 30, 2017 at 06:51:28AM -0700, Andi Kleen wrote:
> On Tue, May 30, 2017 at 11:25:23AM +0200, Peter Zijlstra wrote:
> > On Sun, May 28, 2017 at 01:31:09PM -0700, Stephane Eranian wrote:
> > > Ultimately, I would like to see the watchdog move out of the PMU. That
> > > is the only sensible solution.
> > > You just need a resource able to interrupt on NMI or you handle
> > > interrupt masking in software as has
> > > been proposed on LKML.
> > 
> > So even if we do the soft masking, we still need to deal with regions
> > where the interrupts are disabled. Once an interrupt hits the soft mask
> > we still hardware mask.
> > 
> > So to get full and reliable coverage we still need an NMI source.
> 
> You would only need a single one per system however, not one per CPU.
> RCU already tracks all the CPUs, all we need is a single NMI watchdog
> that makes sure RCU itself does not get stuck.
> 
> So we just have to find a single watchdog somewhere that can trigger
> NMI.

But then you have to IPI broadcast the NMI, which is less than ideal.

RCU doesn't have that problem because the quiescent state is a global
thing. CPU progress, which is what the NMI watchdog tests, is very much
per logical CPU though.

> > I agree that it would be lovely to free up the one counter though.
> 
> One option is to use the TCO watchdog in the chipset instead. 
> Unfortunatley it's not an universal solution because some BIOS lock
> the TCO watchdog for their own use. But if you have a BIOS that
> doesn't do that it should work.

I suppose you could also route the HPET to the NMI vector and other
similar things. Still, you're then stuck with IPI broadcasts, which
suck.

> > One other approach is running the watchdog off of _any_ PMI, then all we
> > need to ensure is that PMIs happen semi regularly. There are two cases
> > where this becomes 'interesting':
> 
> Seems fairly complex.

Yes.. :/

[toc] | [prev] | [next] | [standalone]

#1653389

On Tue, May 30, 2017 at 9:28 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, May 30, 2017 at 06:51:28AM -0700, Andi Kleen wrote:
>> On Tue, May 30, 2017 at 11:25:23AM +0200, Peter Zijlstra wrote:
>> > On Sun, May 28, 2017 at 01:31:09PM -0700, Stephane Eranian wrote:
>> > > Ultimately, I would like to see the watchdog move out of the PMU. That
>> > > is the only sensible solution.
>> > > You just need a resource able to interrupt on NMI or you handle
>> > > interrupt masking in software as has
>> > > been proposed on LKML.
>> >
>> > So even if we do the soft masking, we still need to deal with regions
>> > where the interrupts are disabled. Once an interrupt hits the soft mask
>> > we still hardware mask.
>> >
>> > So to get full and reliable coverage we still need an NMI source.
>>
>> You would only need a single one per system however, not one per CPU.
>> RCU already tracks all the CPUs, all we need is a single NMI watchdog
>> that makes sure RCU itself does not get stuck.
>>
>> So we just have to find a single watchdog somewhere that can trigger
>> NMI.
>
> But then you have to IPI broadcast the NMI, which is less than ideal.
>
> RCU doesn't have that problem because the quiescent state is a global
> thing. CPU progress, which is what the NMI watchdog tests, is very much
> per logical CPU though.
>
>> > I agree that it would be lovely to free up the one counter though.
>>
>> One option is to use the TCO watchdog in the chipset instead.
>> Unfortunatley it's not an universal solution because some BIOS lock
>> the TCO watchdog for their own use. But if you have a BIOS that
>> doesn't do that it should work.
>
> I suppose you could also route the HPET to the NMI vector and other
> similar things. Still, you're then stuck with IPI broadcasts, which
> suck.
>
Can the HPET interrupt (whatever vector) be broadcast to all CPUs by hw?

>> > One other approach is running the watchdog off of _any_ PMI, then all we
>> > need to ensure is that PMIs happen semi regularly. There are two cases
>> > where this becomes 'interesting':
>>
>> Seems fairly complex.
>
> Yes.. :/

[toc] | [prev] | [next] | [standalone]

#1653422

> > You would only need a single one per system however, not one per CPU.
> > RCU already tracks all the CPUs, all we need is a single NMI watchdog
> > that makes sure RCU itself does not get stuck.
> > 
> > So we just have to find a single watchdog somewhere that can trigger
> > NMI.
> 
> But then you have to IPI broadcast the NMI, which is less than ideal.

Only when the watchdog times out to print the backtraces.

> 
> RCU doesn't have that problem because the quiescent state is a global
> thing. CPU progress, which is what the NMI watchdog tests, is very much
> per logical CPU though.

RCU already has a CPU stall detector. It should work (and usually
triggers before the NMI watchdog in my experience unless the
whole system is dead)

-Andi

[toc] | [prev] | [next] | [standalone]

#1653437

On Tue, May 30, 2017 at 10:22:08AM -0700, Andi Kleen wrote:
> > > You would only need a single one per system however, not one per CPU.
> > > RCU already tracks all the CPUs, all we need is a single NMI watchdog
> > > that makes sure RCU itself does not get stuck.
> > > 
> > > So we just have to find a single watchdog somewhere that can trigger
> > > NMI.
> > 
> > But then you have to IPI broadcast the NMI, which is less than ideal.
> 
> Only when the watchdog times out to print the backtraces.

The current NMI watchdog has a per-cpu state. So that means either doing
for_all_cpu() loops or IPI broadcasts from the NMI tickle. Neither is
something you really want.

> > RCU doesn't have that problem because the quiescent state is a global
> > thing. CPU progress, which is what the NMI watchdog tests, is very much
> > per logical CPU though.
> 
> RCU already has a CPU stall detector. It should work (and usually
> triggers before the NMI watchdog in my experience unless the
> whole system is dead)

It only goes look at CPU state once it detects the global QS is stalled
I think. But I've not had much luck with the RCU one -- although I think
its been improved since I last had a hard problem.

[toc] | [prev] | [next] | [standalone]

#1653446

On Tue, May 30, 2017 at 07:40:14PM +0200, Peter Zijlstra wrote:
> On Tue, May 30, 2017 at 10:22:08AM -0700, Andi Kleen wrote:
> > > > You would only need a single one per system however, not one per CPU.
> > > > RCU already tracks all the CPUs, all we need is a single NMI watchdog
> > > > that makes sure RCU itself does not get stuck.
> > > > 
> > > > So we just have to find a single watchdog somewhere that can trigger
> > > > NMI.
> > > 
> > > But then you have to IPI broadcast the NMI, which is less than ideal.
> > 
> > Only when the watchdog times out to print the backtraces.
> 
> The current NMI watchdog has a per-cpu state. So that means either doing
> for_all_cpu() loops or IPI broadcasts from the NMI tickle. Neither is
> something you really want.

The normal case is that the RCU stall only prints the backtrace for
the CPU that stalled.

The extra NMI watchdog should only kick in when RCU is broken too,
or the CPU that owns the stall detection stalled too, which should be rare.

In this case it's reasonable to print backtrace for all, like sysrq would do.
In theory could try to figure out what the current CPU that would own stall
detection is, but it's probably safer to do it for all.

BTW there's an alternative solution in cycling the NMI watchdog over
all available CPUs. Then it would eventually cover all. But that's
less real time friendly than relying on RCU.

> > > RCU doesn't have that problem because the quiescent state is a global
> > > thing. CPU progress, which is what the NMI watchdog tests, is very much
> > > per logical CPU though.
> > 
> > RCU already has a CPU stall detector. It should work (and usually
> > triggers before the NMI watchdog in my experience unless the
> > whole system is dead)
> 
> It only goes look at CPU state once it detects the global QS is stalled
> I think. But I've not had much luck with the RCU one -- although I think
> its been improved since I last had a hard problem.

I've seen it trigger.

-Andi

[toc] | [prev] | [next] | [standalone]

#1653501

On Tue, May 30, 2017 at 10:51:51AM -0700, Andi Kleen wrote:
> On Tue, May 30, 2017 at 07:40:14PM +0200, Peter Zijlstra wrote:
> > On Tue, May 30, 2017 at 10:22:08AM -0700, Andi Kleen wrote:
> > > > > You would only need a single one per system however, not one per CPU.
> > > > > RCU already tracks all the CPUs, all we need is a single NMI watchdog
> > > > > that makes sure RCU itself does not get stuck.
> > > > > 
> > > > > So we just have to find a single watchdog somewhere that can trigger
> > > > > NMI.
> > > > 
> > > > But then you have to IPI broadcast the NMI, which is less than ideal.
> > > 
> > > Only when the watchdog times out to print the backtraces.
> > 
> > The current NMI watchdog has a per-cpu state. So that means either doing
> > for_all_cpu() loops or IPI broadcasts from the NMI tickle. Neither is
> > something you really want.
> 
> The normal case is that the RCU stall only prints the backtrace for
> the CPU that stalled.
> 
> The extra NMI watchdog should only kick in when RCU is broken too,
> or the CPU that owns the stall detection stalled too, which should be rare.

Well, if we can drive the RCU watchdog from NMI (using RTC/HPET or
whatever) it might be good enough if we can convince ourselves there's
no holes in it otherwise.

The obvious hole being locked up while RCU isn't considering the CPU
'interesting'.

> In this case it's reasonable to print backtrace for all, like sysrq would do.
> In theory could try to figure out what the current CPU that would own stall
> detection is, but it's probably safer to do it for all.
> 
> BTW there's an alternative solution in cycling the NMI watchdog over
> all available CPUs. Then it would eventually cover all. But that's
> less real time friendly than relying on RCU.

I don't think we need to worry too much about the watchdog being rt
friendly. Robustness is the thing that worries me most.

> > > > RCU doesn't have that problem because the quiescent state is a global
> > > > thing. CPU progress, which is what the NMI watchdog tests, is very much
> > > > per logical CPU though.
> > > 
> > > RCU already has a CPU stall detector. It should work (and usually
> > > triggers before the NMI watchdog in my experience unless the
> > > whole system is dead)
> > 
> > It only goes look at CPU state once it detects the global QS is stalled
> > I think. But I've not had much luck with the RCU one -- although I think
> > its been improved since I last had a hard problem.
> 
> I've seen it trigger.

Oh, I've seen it trigger plenty,.. just not when I needed it and/or it
didn't contain useful bits.

[toc] | [prev] | [next] | [standalone]

#1653531

> > BTW there's an alternative solution in cycling the NMI watchdog over
> > all available CPUs. Then it would eventually cover all. But that's
> > less real time friendly than relying on RCU.
> 
> I don't think we need to worry too much about the watchdog being rt
> friendly. Robustness is the thing that worries me most.

Ok. Then just cycling is the most robust method, and it's very
simple.

You're right. Perhaps it's better than to rely on the RCU machinery which
seems to become ever more and more complex.

People who care extremely about latencies can just turn it off.

The main problem with the proposal is that it depends on the BIOS not 
locking the TCO watchdog. On the systems were it is locked we would
either need to continue using the PMU for the watchdog, or find some other 
watchdog source that can be programmed to be a NMI.

-Andi

[toc] | [prev] | [next] | [standalone]

#1653383

On Tue, May 30, 2017 at 2:25 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Sun, May 28, 2017 at 01:31:09PM -0700, Stephane Eranian wrote:
>> Ultimately, I would like to see the watchdog move out of the PMU. That
>> is the only sensible solution.
>> You just need a resource able to interrupt on NMI or you handle
>> interrupt masking in software as has
>> been proposed on LKML.
>
> So even if we do the soft masking, we still need to deal with regions
> where the interrupts are disabled. Once an interrupt hits the soft mask
> we still hardware mask.
>
What I was thinking is that you never hardware mask, software always
catches the hw interrupts and keeps them pending or deliver them
depending on sw mask.

> So to get full and reliable coverage we still need an NMI source.
>
> I agree that it would be lovely to free up the one counter though.
>
>
> One other approach is running the watchdog off of _any_ PMI, then all we
> need to ensure is that PMIs happen semi regularly. There are two cases
> where this becomes 'interesting':
>
>  - we have only !sampling events; in this case we have PMIs but at the
>    max period to properly account for counter overflow. This is too
>    large a period. We'd have to muck with the max period of at least one
>    counter.
>
>  - we have _no_ events; in this case we need to somehow schedule an
>    event anyway.
>
> It might be possible to deal with both cases by fudging the state of one
> of the fixed counters. Never clear the EN bit for that counter and
> reduce the max period for that one counter.
>
>
> I think a scheme like that was mentioned before, but I'm also afraid
> that it'll turn into quite the mess if we try it. And by its very nature
> it adds complexity and therefore risks reducing the reliability of the
> thing :/

[toc] | [prev] | [next] | [standalone]

#1653395 — Re: [PATCH 1/2] perf/x86/intel: enable CPU ref_cycles for GP counter

On Tue, 30 May 2017, Stephane Eranian wrote:

> On Tue, May 30, 2017 at 2:25 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Sun, May 28, 2017 at 01:31:09PM -0700, Stephane Eranian wrote:
> >> Ultimately, I would like to see the watchdog move out of the PMU. That
> >> is the only sensible solution.
> >> You just need a resource able to interrupt on NMI or you handle
> >> interrupt masking in software as has
> >> been proposed on LKML.
> >
> > So even if we do the soft masking, we still need to deal with regions
> > where the interrupts are disabled. Once an interrupt hits the soft mask
> > we still hardware mask.
> >
> What I was thinking is that you never hardware mask, software always
> catches the hw interrupts and keeps them pending or deliver them
> depending on sw mask.

That does not work.

  1) You still need some sections where you must have hardware masking.

  2) Software cannot keep them pending in hardware other than disabling
     interrupts at the CPU level and returning to the interrupted sections
     without issuing EOI. Reenabling them at the CPU level will reinject
     them in hardware.

     If we actually want send the EOI and keep a irq pending mask in
     software then we cannot do that quick in the low level entry code. We
     need to go up into the generic interrupt code, take a bunch of locks
     which expect interrupts disable and figure out what to do with the
     interrupt.

     That's required because the low level entry code has no idea how to
     deal with that interrupt, i.e. which physical interrupt controller to
     talk to. Due to our stacked irq chips we can end up with rather
     complex call chains there.

     Of course this involves locking which will deadlock if such a lock has
     been taken at some other place with interrupts just soft disabled.

Thanks,

	tglx

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web