Skip to contentSkip to conversation

eliminte NMI entry/ exit code

PATCH In review George Anzinger 4 participants 6 messages Aug 13, 2005

lore
Share & tools
lore

George Anzinger

author
#0 lore raw

PATCH 1/1 eliminte NMI entry/ exit code

The NMI entry and exit code fiddles with bits in the preempt count. If an NMI happens while some other code is doing the same, bits will be lost. This patch removes this modify code from the NMI path till we can come up with something better.

Signature · 2 lines
George Anzinger   [email protected]
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

Source: MontaVista Software, Inc. George Anzinger <[email protected]>
Type: Defect Fix

Description:

Modifying a word from NMI code runs the very real risk of loosing either then new or the old bits. Remember, we can not prevent an NMI interrupt from ANYWHERE, inparticular between the read and the write of a read modify write sequence.

This patch removes the update of the preempt count from the NMI path.

Signed-off-by George Anzinger<[email protected]>
 hardirq.h |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)
include/linux/hardirq.h +6 -3
Index: linux-2.6.13-rc/include/linux/hardirq.h
===================================================================
@@ -98,9 +98,12 @@ extern void synchronize_irq(unsigned int
 #else
 # define synchronize_irq(irq)	barrier()
 #endif
-
-#define nmi_enter()		irq_enter()
-#define nmi_exit()		sub_preempt_count(HARDIRQ_OFFSET)
+/*
+ * Re think these.  NMI _must_not_ share data words with non-nmi code
+ * Meanwhile, just do a no-op.
+ */
+#define nmi_enter()	/*	irq_enter()  */
+#define nmi_exit()	/*	sub_preempt_count(HARDIRQ_OFFSET) */
 
 #ifndef CONFIG_VIRT_CPU_ACCOUNTING
 static inline void account_user_vtime(struct task_struct *tsk)
#1 lore raw
Quoted 2 lines · George Anzinger

The NMI entry and exit code fiddles with bits in the preempt count. If an NMI happens while some other code is doing the same, bits will be lost. This patch removes this modify code from the NMI path till we can come up with something better.

Humour me for a minute here...
NMI restores preempt_count back to its old value upon exit, right? So what does a race case look like?

Nick

Signature · 3 lines
SUSE Labs, Novell Inc.

Send instant messages to your online friends http://au.messenger.yahoo.com
author
#2 lore raw
Quoted 8 lines · Nick Piggin
Quoted 2 lines · George Anzinger

The NMI entry and exit code fiddles with bits in the preempt count. If an NMI happens while some other code is doing the same, bits will be lost. This patch removes this modify code from the NMI path till we can come up with something better.

Humour me for a minute here...
NMI restores preempt_count back to its old value upon exit, right? So what does a race case look like?

Normal code NMI
fetch preempt_count
add <----- interrupt here add and store then subtract and store, darn!
store preempt_count

Ok, no problem.

The problem is in the RT code when PREEMPT_DEBUG is on. The tests for reasonable counts fail because of the rather undefined state when NMI picks up the word. The failure is on the NMI side...

Quoted 1 line
Signature · 2 lines
George Anzinger   [email protected]
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/
#3 lore raw
Quoted 12 lines · George Anzinger
Quoted 8 lines · Nick Piggin
Quoted 2 lines · George Anzinger

The NMI entry and exit code fiddles with bits in the preempt count. If an NMI happens while some other code is doing the same, bits will be lost. This patch removes this modify code from the NMI path till we can come up with something better.

Humour me for a minute here...
NMI restores preempt_count back to its old value upon exit, right? So what does a race case look like?

Quoted 8 lines

Normal code NMI
fetch preempt_count
add <----- interrupt here add and store then subtract and store, darn!
store preempt_count

Ok, no problem.

The problem is in the RT code when PREEMPT_DEBUG is on. The tests for reasonable counts fail because of the rather undefined state when NMI picks up the word. The failure is on the NMI side...

So NMI changing the preempt count and restoring in the middle of a RWM is not the problem. Thus I don't understand what the issue is. NMI must undo all side effects. Does the PREEMPT_DEBUG code check the count somewhere within the NMI handler? If so, shouldn't the proper fix be to make that code aware that it could be running inside of an NMI and/or ensure that code is not called from within the NMI handler?

Zach

author
#4 lore raw
Quoted 20 lines · Zachary Amsden
Quoted 18 lines · George Anzinger
Quoted 8 lines · Nick Piggin
Quoted 2 lines · George Anzinger

The NMI entry and exit code fiddles with bits in the preempt count. If an NMI happens while some other code is doing the same, bits will be lost. This patch removes this modify code from the NMI path till we can come up with something better.

Humour me for a minute here...
NMI restores preempt_count back to its old value upon exit, right? So what does a race case look like?

Normal code NMI
fetch preempt_count
add <----- interrupt here add and store then subtract and store, darn!
store preempt_count

Quoted 7 lines
Quoted 3 lines

Ok, no problem.

The problem is in the RT code when PREEMPT_DEBUG is on. The tests for reasonable counts fail because of the rather undefined state when NMI picks up the word. The failure is on the NMI side...

So NMI changing the preempt count and restoring in the middle of a RWM is not the problem. Thus I don't understand what the issue is. NMI must undo all side effects. Does the PREEMPT_DEBUG code check the count somewhere within the NMI handler? If so, shouldn't the proper fix be to make that code aware that it could be running inside of an NMI and/or ensure that code is not called from within the NMI handler?

Yes that is the problem. The sanity check in PREEMPT_DEBUG fails when called from the NMI handler.

Quoted 1 line
Signature · 2 lines
George Anzinger   [email protected]
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/
#5 lore raw
In reply to George Anzinger
Quoted 4 lines · George Anzinger

The NMI entry and exit code fiddles with bits in the preempt count. If an NMI happens while some other code is doing the same, bits will be lost.

Why?

Even if an NMI happens in the middle of a read-modify-write sequence that is critical, the NMI exit is supposed to undo whatever it was that the NMI entry did, so the preempt counters are "safe" wrt NMI: they may change, but they always change back by the time anybody cares.

This, btw, is something we depend on wrt _normal_ interrupts too. It's why people can read/modify/write preempt count without having to disable interrupts.

Linus