Re: [PATCH -v2 4/7] x86, NMI, Rewrite NMI handler

[Robert Richter]

On 27.09.10 21:03:52, Huang Ying wrote:
> Use priority to enforce the order has some issues except what Don
> pointed out (two registers for two call in chain):
> 
> - Almost all direct call in default_do_nmi() must be turned into
> notifier_block. I know this is what you want. But I am not a big fan of
> notifier chain :)
> 
> - This makes order of notifier call more implicitly. And I think the
> order is important for NMI handler to work properly.
> 
> - In your scheme, both die_val (DIE_NMI or DIE_NMI_UNKNOWN) and priority
> are used to determine the order of call. This makes code more complex
> and no additional benefit.
> 
> So I think it is better to use different die_val to determine the order,
> and insert some direct call between them.

Huang,

registering a notifier is the only clean way to add an NMI handler.
Modifying the NMI control flow in traps.c with global flags is not the
right approach. Currently, execution order depends on die_val, this is
not obvious when reading the code and we all were wondering in the
beginning why there was DIE_NMI, DIE_NMI_IPI and DIE_NMI_UNKNOWN, etc.

So, better we say instead, this is the notifier function with that
priority, add it to the call chain. With priorities we are able to
clearly specify the execution order even more fine grained than
now. And, code in traps.c will become fairly easy. Also, we don't need
two notifiers, because there is no need for different priorities
then. Give me one use case, I don't know of any.

And the setup of a notifier is not that much overhead or complex, sure
we need to change the code, but actually this the work to be done to
clean it up.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

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