_mtx_lock_spin: obsolete historic handling of kdb_active and panicstr?

[Andriy Gapon]

_mtx_lock_spin has the following check in its retry loop:
if (i < 60000000 || kdb_active || panicstr != NULL)
        DELAY(1);
else
        _mtx_lock_spin_failed(m);

Which means that in the (!kdb_active && panicstr == NULL) case we will make at
most 60000000 iterations and then call _mtx_lock_spin_failed (which proceeds to
panic).  When either kdb_active or panicstr is set, then we are going to loop
forever.

I've done some digging through the lengthy history and many evolutions of the
code (unfortunately I haven't kept records during the research), and my
conclusion is that the kdb_active and panicstr checks were added at the quite
early era of FreeBSD SMP, where we didn't have a mechanism to stop/block other
CPUs when kdb or panic were entered.  We didn't even prevent parallel execution
of panic.
So the above code was a sort of defense where we hoped that "other" CPUs would
eventually stumble upon some held spinlock and would be captured there.  Maybe
there was a specific set of spinlocks, which were supposed to help.

Nowadays, we do try to stop other CPUs during panic and kdb activation and there
are good chances that they are indeed stopped.  In this circumstances, should
the main CPU be so unlucky as to run into the held spinlock, the above check
would do more harm than good - the main CPU would just spin there forever,
because a lock owner is also spinning in the stop loop and so can't release the
lock.
Actually, this is only true for the kdb case.  In the panic case we make a check
earlier and simply ignore/skip/bust all the locks.  That makes the panicstr
check in the code in question both harmless and useless.

So I'd like to propose to remove those checks altogether.  Or perhaps to
"reverse" them and immediately induce a (possibly secondary) panic if we ever
get to that wait loop and kdb_active || panicstr != NULL.

What do you think?
-- 
Andriy Gapon