Skip site navigation (1)Skip section navigation (2)
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>