From owner-svn-src-all@FreeBSD.ORG Tue Sep 21 16:50:43 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2F4E510657C3; Tue, 21 Sep 2010 16:50:43 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id EF7D38FC17; Tue, 21 Sep 2010 16:50:42 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 96B7246BA6; Tue, 21 Sep 2010 12:50:42 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 662518A04E; Tue, 21 Sep 2010 12:50:41 -0400 (EDT) From: John Baldwin To: mdf@freebsd.org Date: Tue, 21 Sep 2010 12:50:40 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: <201009211507.o8LF7iVv097676@svn.freebsd.org> <4C98D200.4040909@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009211250.40704.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Tue, 21 Sep 2010 12:50:41 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Andriy Gapon Subject: Re: svn commit: r212964 - head/sys/kern X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Sep 2010 16:50:43 -0000 On Tuesday, September 21, 2010 12:31:01 pm mdf@freebsd.org wrote: > On Tue, Sep 21, 2010 at 8:40 AM, Andriy Gapon wrote: > > on 21/09/2010 18:27 Andriy Gapon said the following: > >> on 21/09/2010 18:17 mdf@FreeBSD.org said the following: > >>> > >>> I'd recommend using stack_print_ddb(), as that avoids any locking > >>> which may hang depending on how the kernel panic'd. > >> > >> It seems that stack_print_ddb() depends on DDB? > > > > But the point about locking is very good. > > How do you suggest we can deal with it? > > > > A dirty hack would be to check panicstr in linker_search_symbol_name and avoid > > locking, but I don't like that at all. > > Perhaps, some code in subr_stack.c could be taken outside DDB ifdef? > > I keep forgetting, but actually _mtx_lock_sleep() will just return if > panicstr is set. _mtx_assert() is similarly guarded, so actually it > should be mostly okay. Err, I don't think _mtx_lock_sleep() is guarded in that fashion? I have an old patch to do that but have never committed it. If we want that we should probably change rwlocks and sxlocks to have also not block when panicstr is set. --- //depot/vendor/freebsd/src/sys/kern/kern_mutex.c 2010-05-11 18:31:25.000000000 0000 +++ //depot/projects/smpng/sys/kern/kern_mutex.c 2010-06-01 20:12:02.000000000 0000 @@ -348,6 +348,15 @@ return; } + /* + * If we have already panic'd and this is the thread that called + * panic(), then don't block on any mutexes but silently succeed. + * Otherwise, the kernel will deadlock since the scheduler isn't + * going to run the thread that holds the lock we need. + */ + if (panicstr != NULL && curthread->td_flags & TDF_INPANIC) + return; + lock_profile_obtain_lock_failed(&m->lock_object, &contested, &waittime); if (LOCK_LOG_TEST(&m->lock_object, opts)) @@ -664,6 +673,15 @@ } /* + * If we failed to unlock this lock and we are a thread that has + * called panic(), it may be due to the bypass in _mtx_lock_sleep() + * above. In that case, just return and leave the lock alone to + * avoid changing the state. + */ + if (panicstr != NULL && curthread->td_flags & TDF_INPANIC) + return; + + /* * We have to lock the chain before the turnstile so this turnstile * can be removed from the hash list if it is empty. */ -- John Baldwin