mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Cyrill Gorcunov <gorcunov@gmail.com>,
	Don Zickus <dzickus@redhat.com>,
	Stephane Eranian <eranian@google.com>,
	Lin Ming <ming.m.lin@intel.com>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH -tip, final] perf, x86: Add hw_watchdog_set_attr() in a sake of nmi-watchdog on P4
Date: Tue, 5 Jul 2011 15:31:05 +0200	[thread overview]
Message-ID: <20110705133105.GB5843@elte.hu> (raw)
In-Reply-To: <1309871841.3282.148.camel@twins>


* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> > So the question is, why does the NMI watchdog prevent 'perf top' 
> > from working on a P4?
> 
> because the NMI watchdog is a pinned event, you don't want to share 
> the counter, that would be very bad, suppose you lock up when the 
> NMI watchdog was scheduled out. Unreliably debug tools are worse 
> than no tools.

Yeah, indeed that explains the symptom.

Firstly, we should fix/enhance perf top to print out an error message 
in this case, not just hang there doing nothing.

Secondly, the proper solution would be to allow the multiplexing of 
like-minded hw events. Here if we have two events:

  - pinned NMI watchdog, set to a period of 2 billion cycles
  - perf top with a default of 1 khz auto-freq cycles

We should first change the NMI watchdog to use auto-freq samples - 
the hw_nmi_get_sample_period() looks unnecessary - if we set the NMI 
watchdog to 1 Hz by default it should be more than enough.

Thus we'd have two events:

  - pinned NMI watchdog, set to 1 Hz
  - perf top, set to 1000 Hz

The 1000 Hz event could serve the 1 Hz event just fine.

Thirdly, even if we mucked the NMI event to be somewhat different 
from the real cycles event (so the P4 PMU can co-schedule it with 
perf top), the real solution there would *still* be to express this 
as a variation of a cycles event: use the same trick to allow up to 
two cycles events to be present in the PMU - but hide this from the 
generic interfaces, just allow up to two cycles events to be 
scheduled at once.

This will have the advantage of not only fixing the NMI watchdog, but 
other users as well.

Now, if there's some real behavioral difference between the two 
events (one is halted cycles the other is unhalted cycles), then i'd 
suggest to use PERF_COUNT_HW_BUS_CYCLES in the NMI watchdog - that is 
the generic 'constant frequency' cycles event.

So there's lots of options to fix/improve this more intelligently.

Thanks,

	Ingo

  reply	other threads:[~2011-07-05 13:31 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-05 10:03 Cyrill Gorcunov
2011-07-05 10:20 ` Ingo Molnar
2011-07-05 10:34   ` Cyrill Gorcunov
2011-07-05 10:59     ` Ingo Molnar
2011-07-05 11:05       ` Cyrill Gorcunov
2011-07-05 11:20         ` Ingo Molnar
2011-07-05 11:36           ` Cyrill Gorcunov
2011-07-05 11:44             ` Ingo Molnar
2011-07-05 11:49               ` Cyrill Gorcunov
2011-07-05 12:14                 ` Cyrill Gorcunov
2011-07-05 13:10                   ` Ingo Molnar
2011-07-05 13:17                     ` Peter Zijlstra
2011-07-05 13:31                       ` Ingo Molnar [this message]
2011-07-05 14:19                         ` Cyrill Gorcunov
2011-07-08 12:44                           ` Ingo Molnar
2011-07-05 14:20                         ` Peter Zijlstra
2011-07-05 14:40                         ` Peter Zijlstra
2011-07-05 14:56                           ` Ingo Molnar
2011-07-05 15:25                             ` Cyrill Gorcunov
2011-07-06  7:01                               ` Cyrill Gorcunov
2011-07-08 12:49                               ` Ingo Molnar
2011-07-08 13:01                                 ` Cyrill Gorcunov
2011-07-08 13:09                                   ` Ingo Molnar
2011-07-08 13:12                                   ` Cyrill Gorcunov
2011-07-05 13:26                     ` Cyrill Gorcunov
2011-07-05 12:24               ` Don Zickus
2011-07-05 12:26                 ` Cyrill Gorcunov
2011-07-05 12:44                   ` Don Zickus
2011-07-05 12:56                     ` Cyrill Gorcunov

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=20110705133105.GB5843@elte.hu \
    --to=mingo@elte.hu \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@redhat.com \
    --cc=dzickus@redhat.com \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=gorcunov@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.m.lin@intel.com \
    /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

all inboxes | Powered by JetHome®