svn commit: r222060 - in user/avg/xcpu/sys: kern sys

[Attilio Rao]

2011/5/26 Attilio Rao <attilio at freebsd.org>:
> 2011/5/26 Andriy Gapon <avg at freebsd.org>:
>> on 25/05/2011 17:36 Andriy Gapon said the following:
>>> on 18/05/2011 23:06 Attilio Rao said the following:
>>>> However I think that TDF_INPANIC handling is not optimal.
>>>> You should really acquire thread_lock otherwise you are going to break
>>>> choosethread() concurrency.
>>>>
>>>> I would prefer to make TDF_INPANIC a private flag and just use it with
>>>> curthread, if possible, but I still don't have a good way to resolve
>>>> choosethread() (I would dig the runqueue adding path and resolve the
>>>> problem later in the codeflow, I think).
>>>
>>> I've been thinking about this.
>>> I think that in the new world where only one thread runs after panic we could just
>>> reduce TD_IS_INPANIC to panicstr != NULL, TDF_INPANIC could be removed altogether
>>> along with the check in  choosethread().  But for some initial period I would like
>>> to have an option to disable CPU stopping (to protect from possible bugs,
>>> regressions, etc) and for that I would like to keep TDF_INPANIC.  The flag could
>>> be set without thread_lock() because we still allow only one thread to be in/after
>>> panic.  But I completely agree with you that it is cleaner to move TDF_INPANIC to
>>> private flags.
>>>
>>> So the first step:
>>> TDF_INPANIC => to private flags
>>>
>>> Some time in the future:
>>> TDF_INPANIC => removed
>>> TD_IS_INPANIC => panicstr != NULL
>>>
>>
>> Ehm...  After discussing this issue with you on IRC I realized absurdity of my
>> interim suggestion.
>>
>> New proposal:
>> #define TD_IS_INPANIC() \
>>        (panicstr != NULL && stop_cpus_on_panic)
>>
>> When/if stop_cpus_on_panic knob is removed, then TD_IS_INPANIC will naturally be
>> reduced to (panicstr != NULL) and TDF_INPANIC flag will also go as we will be
>> guaranteed that the scheduler will not be running.
>
> Yes, that is a much better proposal.
>
>> Given the above, maybe TD_IS_INPANIC should change name again as it doesn't check
>> properties of a particular thread, but rather the whole system state?  Also,
>> sys/proc.h doesn't seem like the best location for it anymore.
>
> Yes, I think it would be better something like SYSTEM_IN_PANIC() or such.
> The natural location for this would be kern_shutdown.c but it really
> doesn't have a corresponding header (maybe sys/reboot.h could be, with
> some more lifting, but for the moment, no), thus you can still pickup
> something easy to use like proc.h or systm.h.

I think that systm.h may be the best place as long as it is where
panic() is declared.

Attilio


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