On Sun, 12 Nov 2017, Bruce Evans wrote:

On Sun, 12 Nov 2017, Mateusz Guzik wrote:

Log:
 amd64: stop nesting preemption counter in spinlock_enter

 Discussed with:        jhb

This seems to break it.  i386 still has the old code.
...
The main broken case is:
- both levels initially 0
- disable interrupts
- raise spinlock count to 1
- bad race window until critical_enter() is called.  Disabling hardware
 interrupts doesn't prevent exceptions like debugger traps or NMIs.
 Debuuger trap handlers shouldn't use critical sections or (spin)
 mutexes, but NMI handlers might.  When an exception handler calls
 spinlock_enter(), this no longer gives a critical section and bad
 things like context switches occur if the handler calls critical_enter/
 exit().
...
I don't like this change.  The nested counting is easier to understand,
and the nested case is very rare and the critical section part of it is
very efficient (then critical_exit() is just lowering the level).  Old
...
I think the nested case is only for recursive spinlocks.  So NMI handlers
should not use any spinlocks and the new bug is small (NMI handlers should
not use non-recursive spinlocks since they would deadlock, and should not
use recursive spinlocks since they don't work).  NMI handlers aren't that
careful.  They call printf(), and even the message buffer has been broken
to use non-recursive spinlocks.

Actually, it is valid for NMI handlers to use spinlocks via mtx_trylock_spin()
in the non-kdb non-panic case, and that is what my fixes for printf() do.

I had confused "nesting preemption counter" (td_critnest) with interrupt
nesting (the bogus td_intr_nesting_level).  td_critnest was incremented
for every concurrently held spinlock, so it could grow quite large without
any interrupt/exception recursion.  So the micro-optimization of td_critnest
is useful if it is faster and fixed to work.

Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to