From owner-freebsd-current@FreeBSD.ORG Thu Dec 1 21:42:36 2011 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 42653106566C; Thu, 1 Dec 2011 21:42:36 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id AACB68FC0A; Thu, 1 Dec 2011 21:42:27 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id XAA19075; Thu, 01 Dec 2011 23:42:23 +0200 (EET) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1RWEOc-0007mF-I1; Thu, 01 Dec 2011 23:42:22 +0200 Message-ID: <4ED7F4BC.3080206@FreeBSD.org> Date: Thu, 01 Dec 2011 23:42:20 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:8.0) Gecko/20111108 Thunderbird/8.0 MIME-Version: 1.0 To: John Baldwin References: <20111113083215.GV50300@deviant.kiev.zoral.com.ua> <201112011349.50502.jhb@freebsd.org> <4ED7E6B0.30400@FreeBSD.org> <201112011553.34432.jhb@freebsd.org> In-Reply-To: <201112011553.34432.jhb@freebsd.org> X-Enigmail-Version: undefined Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: freebsd-current@FreeBSD.org Subject: Re: Stop scheduler on panic X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Dec 2011 21:42:36 -0000 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 >> 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; -- Andriy Gapon