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

Attilio Rao attilio at freebsd.org
Thu May 26 16:41:26 UTC 2011


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.

It is your decision, anyway, I don't have strong opinion on where to put this.

Attilio


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


More information about the svn-src-user mailing list