From owner-svn-src-all@freebsd.org Thu Mar 22 06:39:20 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 22EAA323; Thu, 22 Mar 2018 06:39:20 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id 94B9B8362A; Thu, 22 Mar 2018 06:39:19 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id 443E8101576; Thu, 22 Mar 2018 17:39:16 +1100 (AEDT) Date: Thu, 22 Mar 2018 17:39:14 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Warner Losh cc: Warner Losh , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r331298 - head/sys/dev/syscons In-Reply-To: Message-ID: <20180322170345.J1053@besplex.bde.org> References: <201803211447.w2LElDcK091988@repo.freebsd.org> <20180322024846.S4293@besplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=VJytp5HX c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=ujyv94cRbLZRN1sVS_cA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 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: Thu, 22 Mar 2018 06:39:20 -0000 On Wed, 21 Mar 2018, Warner Losh wrote: > On Wed, Mar 21, 2018 at 11:53 AM, Bruce Evans wrote: > >> On Wed, 21 Mar 2018, Warner Losh wrote: >> >> Log: >>> Unlock giant when calling shutdown_nice() >> >> This breaks the driver. Giant is syscons' driver lock, and also the >> interrupt handler lock for at least the atkbd keyboard driver, so vt >> sometimes holds the lock for. > > OK. I got carried away. You're right. The proper fix is to unlock Giant at > the top of kern_reboot() instead. This handles the case where we call > shutdown_nice() with no init running and have to call kern_reboot directly. > Otherwise it's perfectly fine to just call shutdown_nice() with Giant held > since we just signal init from there. So I'll revert this change and make > that other change instead. > ... > Good point. I think the following change is good for everything except > calling shutdown_nice() from a fast interrupt handler with noinit running: > > diff --git a/sys/kern/kern_shutdown.c b/sys/kern/kern_shutdown.c > index e5ea9644ad3f..564aecd811be 100644 > --- a/sys/kern/kern_shutdown.c > +++ b/sys/kern/kern_shutdown.c > @@ -366,6 +366,12 @@ kern_reboot(int howto) > { > static int once = 0; > > + /* > + * Drop Giant once and for all. > + */ > + while (mtx_owned(&Giant)) > + mtx_unlock(&Giant); > + > #if defined(SMP) > /* > * Bind us to the first CPU so that all shutdown code runs there. > Some > > Comments? Try putting this in vfs_mountroot_parse() and/or the second clause of shutdown_nice() only. The only other calls are from is from sys_reboot() and vpanic(). sys_reboot() is either MPSAFE or not, and it should know when it is safe to drop its Giant lock if at all. vpanic() sets SCHEDULER_STOPPED() to avoid seeing problems with Giant or any other mutex. Bruce