From owner-svn-src-head@freebsd.org Sat Jun 2 08:39:00 2018 Return-Path: Delivered-To: svn-src-head@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 84830FDF1C8; Sat, 2 Jun 2018 08:39:00 +0000 (UTC) (envelope-from bde@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 32DE3751D6; Sat, 2 Jun 2018 08:39:00 +0000 (UTC) (envelope-from bde@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 1405F56EA; Sat, 2 Jun 2018 08:39:00 +0000 (UTC) (envelope-from bde@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w528cxnV005332; Sat, 2 Jun 2018 08:38:59 GMT (envelope-from bde@FreeBSD.org) Received: (from bde@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w528cxgm005331; Sat, 2 Jun 2018 08:38:59 GMT (envelope-from bde@FreeBSD.org) Message-Id: <201806020838.w528cxgm005331@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: bde set sender to bde@FreeBSD.org using -f From: Bruce Evans Date: Sat, 2 Jun 2018 08:38:59 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r334526 - head/sys/dev/syscons X-SVN-Group: head X-SVN-Commit-Author: bde X-SVN-Commit-Paths: head/sys/dev/syscons X-SVN-Commit-Revision: 334526 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 02 Jun 2018 08:39:00 -0000 Author: bde Date: Sat Jun 2 08:38:59 2018 New Revision: 334526 URL: https://svnweb.freebsd.org/changeset/base/334526 Log: Fix low-level locking during panics. The SCHEDULER_STOPPED() hack breaks locking generally, and mtx_trylock_*() especially. When mtx_trylock_*() returns nonzero, naive code version here trusts it to have worked. But when SCHEDULER_STOPPED() is true, mtx_trylock_*() returns 1 without doing anything. Then mtx_unlock_*() crashes especially badly attempting to unlock iff the error is detected, since mutex unlocking functions don't check SCHEDULER_STOPPED(). syscons already didn't trust mtx_trylock_spin(), but it was missing the logic to turn on sp->kdb_locked when turning off sp->mtx_locked during panics. It also used panicstr instead of SCHEDULER_LOCKED because I thought that panicstr was more fragile. They only differ for a window of lines in panic(), and in broken cases where stop_cpus_hard() in panic() didn't work. Modified: head/sys/dev/syscons/syscons.c Modified: head/sys/dev/syscons/syscons.c ============================================================================== --- head/sys/dev/syscons/syscons.c Sat Jun 2 07:44:53 2018 (r334525) +++ head/sys/dev/syscons/syscons.c Sat Jun 2 08:38:59 2018 (r334526) @@ -1807,13 +1807,19 @@ sccnscrlock(sc_softc_t *sc, struct sc_cnstate *sp) * enough to ignore the protection even in the kdb_active case. */ if (kdb_active) { - sp->kdb_locked = sc->video_mtx.mtx_lock == MTX_UNOWNED || panicstr; + sp->kdb_locked = sc->video_mtx.mtx_lock == MTX_UNOWNED || + SCHEDULER_STOPPED(); sp->mtx_locked = FALSE; } else { sp->kdb_locked = FALSE; for (retries = 0; retries < 1000; retries++) { sp->mtx_locked = mtx_trylock_spin_flags(&sc->video_mtx, - MTX_QUIET) != 0 || panicstr; + MTX_QUIET) != 0; + if (SCHEDULER_STOPPED()) { + sp->kdb_locked = TRUE; + sp->mtx_locked = FALSE; + break; + } if (sp->mtx_locked) break; DELAY(1);