Re: Stop scheduler on panic

2011/12/2 Andriy Gapon <avg_at_freebsd.org>:
> on 02/12/2011 20:40 John Baldwin said the following:
>> On 12/2/11 12:18 PM, Attilio Rao wrote:
>>> 2011/12/2 John Baldwin<jhb_at_freebsd.org>:
>>>> On 12/2/11 5:05 AM, Andriy Gapon wrote:
>>>>>
>>>>> on 02/12/2011 06:36 John Baldwin said the following:
>>>>>>
>>>>>> Ah, ok (I had thought SCHEDULER_STOPPED was going to always be true when
>>>>>> kdb was
>>>>>> active).  But I think these two changes should cover critical_exit() ok.
>>>>>>
>>>>>
>>>>> I attempted to start a discussion about this a few times already :-)
>>>>> Should we treat kdb context the same as SCHEDULER_STOPPED context (in the
>>>>> current definition) ?  That is, skip all locks in the same fashion?
>>>>> There are pros and contras.
>>>>
>>>>
>>>> kdb should not block on locks, no.  Most debugger commands should not go
>>>> near locks anyway unless they are intended to carefully modify the existing
>>>> system in a safe manner (such as the 'kill' command which should only be
>>>> using try locks and fail if it cannot safely post the signal).
>>>
>>> The biggest problem to KDB as the same as panic is that doing proper
>>> 'continue' is impossible.
>>> One of the features of the 'skip-locking' path is that it doesn't take
>>> into account fast locking paths, where sometimes the lock can succeed
>>> and other fails and you don't know about them. Also the restarted CPUs
>>> can find corrupted datas (as they can be arbitrarely updated), I'm
>>> sure it is too much panic prone.
>>
>> Yes, my thought is that kdb commands, etc. should be using dedicated routines
>> that do not use locks whenever possible.  The problem of a user
>> calling an arbitrary routine is not solvable (so I don't think we should try to
>> solve that, you use 'call' at your own risk), but built-in commands should
>> explicitly either 1) not use locking, or 2) only use try locks and fail out
>> cleanly (including dropping any try locks acquired) if a try fails.  Now, that's
>> an ideal view, I don't know how close we are to that in practice or if it is a
>> realistically attainable goal.
>>
>
>
> I agree with what Attilio and you say.  Initially it was tempting for me to
> apply the same SCHEDULER_STOPPED stopped medicine to the kdb_active context, but
> after trying to deal with kdb_active x SCHEDULER_STOPPED x ukbd situation I
> really changed my mind.
>
>
> I would classify the code that can be called in kdb_active context as follows:
> o debugger code proper (kdb, ddb, gdb stub, etc) - this obviously must not
> (doesn't have to) use any locking
>
> o code that can be invoked via 'call' command - this is essentially any code and
> I don't think that it can/should do anything special for the kdb_active context [*]
>
> o debugger helper routines - those that do something trivial should not acquire
> any locks; those that access shared resources should try the relevant locks and
> bail out if a resource can be in inconsistent state, or should be equipped to
> deal correctly with such a state; this is the same as what you say above
>
> o common code that the debuggers have to use - most obviously this is console
> code and drivers that serve a particular console; on one hand those drivers can
> have a non-trivial state that must be lock protected during normal operation, on
> the other hand the debugger must disregard those locks and grab its console;
> this is the most complex case in my opinion.

Thanks for summarizing this up.
However, please note that code in 2 and 4 entries may have the same
issues or being the same thing, in practice.

Anyway, I'm thinking now that if we really want to stop CPUs when
entering KDB (and I think we do) functions at 2 and 4 should basically
just be totally lockless or in general being totally re-entrant
because when we restart CPUs we don't really want them finding datas
to be corrupted. Also, skipping locking, is totally broken for this
very specific reason.

Functions at point 2 and 4 should be totally lockless then and
possibly just work on read mode. For point 2, specifically, I think we
need an explicit KPI to define functions within the subsystem
themselves (something like DB_SHOW_COMMAND()) which marks undoublty
functions to be called within ddb (the only KDB backend we implement
right now) and likely for functions at point 4 we need to find a way
to stress their belonging to the KDB area.

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein