Re: [PATCH] NMI request/release

From: Corey Minyard (cminyard@mvista.com)
Date: Mon Oct 21 2002 - 21:32:07 EST


John Levon wrote:

>On Mon, Oct 21, 2002 at 08:32:47PM -0500, Corey Minyard wrote:
>
>>The attached patch implements a way to request to receive an NMI if it
>>comes from an otherwise unknown source. I needed this for handling NMIs
>>with the IPMI watchdog. This function was discussed a little a while
>>
>>
>Then NMI watchdog and oprofile should be changed to use this too.
>
Maybe so. If we get the infrastructure into place, we can change that
afterwards.

> We
>also need priority and/or equivalent of NOTIFY_STOP_MASK so we can break
>out of calling all the handlers. Actually, why do you continue if one
>of the handlers returns 1 anyway ?
>
What if there's an NMI from multiple things? Not that it's likely, but
it's possible, right? I could easily add priority and sort the list by
it, and add a NOTIFY_STOP_MASK, if there is some benefit.

>>+ atomic_inc(&calling_nmi_handlers);
>>
>>
>Isn't this going to cause cacheline ping pong ?
>
This is an NMI, does it really matter? And is there another way to do
this without locks?

>>+ curr = nmi_handler_list;
>>+ while (curr) {
>>+ handled |= curr->handler(curr->dev_id, regs);
>>
>>
>dev_name is never used at all. What is it for ? Also, would be nice
>to do an smp_processor_id() just once and pass that in to prevent
>multiple calls to get_current().
>
dev_name could be removed, although it would be nice for reporting
later. Basically, for the same reason it's there for interrupts. And
again, this is an NMI, I don't think performance really matters (unless
we perhaps add the NMI watchdog to this).

>Couldn't you modify the notifier code to do the xchg()s (though that's
>not available on all CPU types ...)
>
I don't understand. The xchg()s are for atomicity between the
request/release code and the NMI handler. How could the notifier code
do it?

>>+#define HAVE_NMI_HANDLER 1
>>
>>
>What uses this ?
>
This is so the user code can know if it's available or not.

>>+ volatile struct nmi_handler *next;
>>
>>
>Hmm ...
>
>Is it not possible to use linux/rcupdate.h for this stuff ?
>
I'm not sure. It looks possible, but remember, this is an NMI, normal
rules may not apply. Particularly, you cannot block or spin waiting for
something else, the NMI code has to run. An NMI can happen at ANY time.
 If the rcu code can handle this, I could use it, but I have not looked
to see if it can.

Thanks,

-Corey

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Wed Oct 23 2002 - 22:00:56 EST