From: Don Zickus on
On Wed, Aug 04, 2010 at 05:21:10PM +0800, Lin Ming wrote:
> With nmi_watchdog enabled, perf_event_nmi_handler always return
> NOTIFY_STOP(active_events > 0), and the notifier call chain will not
> call further.
>
> If it was not perf NMI, does the perf nmi handler may stop the real NMI
> handler get called because NOTIFY_STOP is returned??

Yes I sent a cheap and dirty patch to address this a couple of weeks ago

http://lkml.indiana.edu/hypermail//linux/kernel/1007.2/02590.html

Unfortunately, no responded. :-( Of course, it could have been so gross
no one wanted to comment on it. :-)

Cheers,
Don
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Don Zickus on
On Wed, Aug 04, 2010 at 12:01:16PM +0200, Robert Richter wrote:
> There is no general mechanism for recording the NMI source (except if
> it was external triggered, e.g. by the southbridge). Also, all nmis
> are mapped to NMI vector 2 and therefore there is no way to find out
> the reason by using apic mask registers.

This is no different than a shared interrupt, no? All the nmi handlers
need to check their own sources to see if they triggered it. You can't
expect the generic nmi handler to determine this.

>
> Now, if multiple perfctrs trigger an nmi, it may happen that a handler
> has nothing to do because the counter was already handled by the
> previous one. Thus, it is valid to have unhandled nmis caused by
> perfctrs.
>
> So, with counters enabled we always have to return stop for *all* nmis
> as we cannot detect that it was an perfctr nmi. Otherwise we could
> trigger an unhandled nmi. To ensure that all other nmi handlers are
> called, the perfctr's nmi handler must have the lowest priority. Then,
> the handler will be the last in the chain.

But the cases this break are, external NMI buttons, broken firmware that
causes SERRs on the PCI bus, and any other general hardware failures.

So what the perf handler does is really unacceptable. The only reason we
are noticing this now is because I put the nmi_watchdog on top of the perf
subsystem, so it always has a user and will trigger NOTIFY_STOP. Before,
it never had a registerd user so instead returned NOTIFY_DONE and
everything worked great.

Cheers,
Don
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on
On Wed, 2010-08-04 at 10:00 -0400, Don Zickus wrote:
> On Wed, Aug 04, 2010 at 12:01:16PM +0200, Robert Richter wrote:
> > There is no general mechanism for recording the NMI source (except if
> > it was external triggered, e.g. by the southbridge). Also, all nmis
> > are mapped to NMI vector 2 and therefore there is no way to find out
> > the reason by using apic mask registers.
>
> This is no different than a shared interrupt, no? All the nmi handlers
> need to check their own sources to see if they triggered it. You can't
> expect the generic nmi handler to determine this.

Sure, but the problem is that the PMU can't reliably do that.

> > Now, if multiple perfctrs trigger an nmi, it may happen that a handler
> > has nothing to do because the counter was already handled by the
> > previous one. Thus, it is valid to have unhandled nmis caused by
> > perfctrs.
> >
> > So, with counters enabled we always have to return stop for *all* nmis
> > as we cannot detect that it was an perfctr nmi. Otherwise we could
> > trigger an unhandled nmi. To ensure that all other nmi handlers are
> > called, the perfctr's nmi handler must have the lowest priority. Then,
> > the handler will be the last in the chain.
>
> But the cases this break are, external NMI buttons, broken firmware that
> causes SERRs on the PCI bus, and any other general hardware failures.

It breaks broken firmware? :-) and you care?

> So what the perf handler does is really unacceptable. The only reason we
> are noticing this now is because I put the nmi_watchdog on top of the perf
> subsystem, so it always has a user and will trigger NOTIFY_STOP. Before,
> it never had a registerd user so instead returned NOTIFY_DONE and
> everything worked great.

Right so I looked up your thing and while that limits the damage in that
at some point it will let NMIs pass, it will still consume too many.
Meaning that Yinghai will have to potentially press his NMI button
several times before it registers.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Don Zickus on
On Wed, Aug 04, 2010 at 04:11:33PM +0200, Peter Zijlstra wrote:
> On Wed, 2010-08-04 at 10:00 -0400, Don Zickus wrote:
> > On Wed, Aug 04, 2010 at 12:01:16PM +0200, Robert Richter wrote:
> > > There is no general mechanism for recording the NMI source (except if
> > > it was external triggered, e.g. by the southbridge). Also, all nmis
> > > are mapped to NMI vector 2 and therefore there is no way to find out
> > > the reason by using apic mask registers.
> >
> > This is no different than a shared interrupt, no? All the nmi handlers
> > need to check their own sources to see if they triggered it. You can't
> > expect the generic nmi handler to determine this.
>
> Sure, but the problem is that the PMU can't reliably do that.

Right, but that is because there is no bit that says the PMU generated the
nmi. But for the most part checking to see if the PMU is >0 is good
enough, no?

>
> > > Now, if multiple perfctrs trigger an nmi, it may happen that a handler
> > > has nothing to do because the counter was already handled by the
> > > previous one. Thus, it is valid to have unhandled nmis caused by
> > > perfctrs.
> > >
> > > So, with counters enabled we always have to return stop for *all* nmis
> > > as we cannot detect that it was an perfctr nmi. Otherwise we could
> > > trigger an unhandled nmi. To ensure that all other nmi handlers are
> > > called, the perfctr's nmi handler must have the lowest priority. Then,
> > > the handler will be the last in the chain.
> >
> > But the cases this break are, external NMI buttons, broken firmware that
> > causes SERRs on the PCI bus, and any other general hardware failures.
>
> It breaks broken firmware? :-) and you care?

Absolutely. When a customer complains they upgraded their RHEL kernel and
the box suddenly hangs on boot trying to access the storage device, yes I
care. Because a flood of NMIs would indiciate something is fishy with
the firmware (in this case it was a network card though it hung on storage
access). Swallowing the NMIs would just cause everyone to waste weeks of
their time trying to figure it out (you don't want to know how many weeks
were wasted in RHEL-6 across multiple machines only to find out it was
broken firmware on a card that no one suspected as being the culprit).

As much as I hate broken firmware, it is becoming common place, and the
faster the kernel can point it out through unknown nmis, the faster we can
get the vendor involved to fix it.

>
> > So what the perf handler does is really unacceptable. The only reason we
> > are noticing this now is because I put the nmi_watchdog on top of the perf
> > subsystem, so it always has a user and will trigger NOTIFY_STOP. Before,
> > it never had a registerd user so instead returned NOTIFY_DONE and
> > everything worked great.
>
> Right so I looked up your thing and while that limits the damage in that
> at some point it will let NMIs pass, it will still consume too many.
> Meaning that Yinghai will have to potentially press his NMI button
> several times before it registers.

Ok. Thanks for reviewing. How does it consume to many? I probably don't
understand how perf is being used in the non-simple scenarios.

Cheers,
Don

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on
On Wed, 2010-08-04 at 10:52 -0400, Don Zickus wrote:
> > Right so I looked up your thing and while that limits the damage in that
> > at some point it will let NMIs pass, it will still consume too many.
> > Meaning that Yinghai will have to potentially press his NMI button
> > several times before it registers.
>
> Ok. Thanks for reviewing. How does it consume to many? I probably don't
> understand how perf is being used in the non-simple scenarios.

Suppose you have 4 counters (AMD, intel-nhm+), when more than 2 overflow
the first will raise the PMI, if the other 2+ overflow before we disable
the PMU it will try to raise 2+ more PMIs, but because hardware only has
a single interrupt pending bit it will at most cause a single extra
interrupt after we finish servicing the first one.

So then the first interrupt will see 3+ overflows, return 3+, and will
thus eat 2+ NMIs, only one of which will be the pending interrupt,
leaving 1+ NMIs from other sources to consume unhandled.

In which case Yinghai will have to press his NMI button 2+ times before
it registers.

That said, that might be a better situation than always consuming
unknown NMIs..
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/