Date: Thu, 01 Dec 2011 23:36:54 -0500 From: John Baldwin <jhb@FreeBSD.org> To: Andriy Gapon <avg@FreeBSD.org> Cc: freebsd-current@FreeBSD.org Subject: Re: Stop scheduler on panic Message-ID: <4ED855E6.20207@FreeBSD.org> In-Reply-To: <4ED7F4BC.3080206@FreeBSD.org> References: <20111113083215.GV50300@deviant.kiev.zoral.com.ua> <201112011349.50502.jhb@freebsd.org> <4ED7E6B0.30400@FreeBSD.org> <201112011553.34432.jhb@freebsd.org> <4ED7F4BC.3080206@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 12/1/11 4:42 PM, Andriy Gapon wrote:
> on 01/12/2011 22:53 John Baldwin said the following:
>> On Thursday, December 01, 2011 3:42:24 pm Andriy Gapon wrote:
>>> Returning to critical_exit, what do you think about the following patch?
>>> I guess that it could be committed independently of / before the
>>> SCHEDULER_STOPPED thing.
>>>
>>> commit ee3d1a04985e86911a68d854439ae8c5429b7bd5
>>> Author: Andriy Gapon<avg@icyb.net.ua>
>>> Date: Thu Dec 1 18:53:36 2011 +0200
>>>
>>> critical_exit: ignore td_owepreempt if kdb_active
>>>
>>> calling mi_switch in such a context result in a recursion via
>>> kdb_switch
>>>
>>> diff --git a/sys/kern/kern_switch.c b/sys/kern/kern_switch.c
>>> index 93cbf7b..885dc22 100644
>>> --- a/sys/kern/kern_switch.c
>>> +++ b/sys/kern/kern_switch.c
>>> @@ -200,7 +200,7 @@ critical_exit(void)
>>>
>>> if (td->td_critnest == 1) {
>>> td->td_critnest = 0;
>>> - if (td->td_owepreempt) {
>>> + if (td->td_owepreempt&& !kdb_active) {
>>> td->td_critnest = 1;
>>> thread_lock(td);
>>> td->td_critnest--;
>>
>> I think this is fine, but I'd probably change this to SCHEDULER_STOPPED()
>> in the SCHEDULER_STOPPED() patch.
>
> I don't understand why... What if kdb is entered for some other reason, not
> because of panic? In that case SCHEDULER_STOPPED() would be false, but it is
> still possible to find a way into mi_switch.
>
> The SCHEDULER_STOPPED patch adds this:
> @@ -428,6 +428,8 @@ mi_switch(int flags, struct thread *newtd)
> */
> if (kdb_active)
> kdb_switch();
> + if (SCHEDULER_STOPPED())
> + return;
> if (flags& SW_VOL) {
> td->td_ru.ru_nvcsw++;
> td->td_swvoltick = ticks;
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.
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4ED855E6.20207>