From owner-cvs-src@FreeBSD.ORG Wed May 31 15:51:19 2006 Return-Path: X-Original-To: cvs-src@freebsd.org Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 95CDB16A656; Wed, 31 May 2006 15:51:19 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (66-23-211-162.clients.speedfactory.net [66.23.211.162]) by mx1.FreeBSD.org (Postfix) with ESMTP id F27D443D55; Wed, 31 May 2006 15:51:18 +0000 (GMT) (envelope-from jhb@freebsd.org) Received: from [131.106.61.215] (72-255-64-170.client.stsn.net [72.255.64.170]) (authenticated bits=0) by server.baldwin.cx (8.13.4/8.13.4) with ESMTP id k4VFpAY8068654; Wed, 31 May 2006 11:51:16 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: Scott Long Date: Wed, 31 May 2006 11:23:01 -0400 User-Agent: KMail/1.9.1 References: <200605252306.k4PN6cCS081708@repoman.freebsd.org> <200605311050.12156.jhb@freebsd.org> <447DAF19.3050901@samsco.org> In-Reply-To: <447DAF19.3050901@samsco.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200605311123.02334.jhb@freebsd.org> X-Virus-Scanned: ClamAV 0.87.1/1501/Wed May 31 06:23:26 2006 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.1 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.0 X-Spam-Checker-Version: SpamAssassin 3.1.0 (2005-09-13) on server.baldwin.cx Cc: cvs-src@freebsd.org, src-committers@freebsd.org, Warner Losh , cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/dev/syscons/apm apm_saver.c src/sys/i386/bios apm.c apm.h X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 May 2006 15:51:25 -0000 On Wednesday 31 May 2006 10:58, Scott Long wrote: > John Baldwin wrote: > > On Thursday 25 May 2006 23:01, Scott Long wrote: > > > >>Warner Losh wrote: > >> > >> > >>>imp 2006-05-25 23:06:38 UTC > >>> > >>> FreeBSD src repository > >>> > >>> Modified files: > >>> sys/dev/syscons/apm apm_saver.c > >>> sys/i386/bios apm.c apm.h > >>> Log: > >>> APM was calling the suspend process from a timeout. This meant that > >>> other timeouts could not happen while suspending, including timeouts > >>> for things like msleep. This caused the system to hang on suspend > >>> when the cbb was enabled, since its suspend path powered down the > >>> socket which used a timeout to wait for it to be done. > >>> > >>> APM now creates a thread when it is enabled, and deletes the thread > >>> when it is disabled. This thread takes the place of the timeout by > >>> doing its polling every ~.9s. When the thread is disabled, it will > >>> wakeup early, otherwise it times out and polls the varius things the > >>> old timeout polled (APM events, suspend delays, etc). > >>> > >>> This makes my Sony VAIO 505TS suspend/resume correctly when APM is > >>> enabled (ACPI is black listed on my 505TS). > >>> > >>> This will likely fix other problems with the suspend path where > >>> drivers would sleep with msleep and/or do other timeouts. Maybe > >>> there's some special case code that would use DELAY while suspending > >>> and msleep otherwise that can be revisited and removed. > >>> > >>> This was also tested by glebius@, who pointed out that in the patch I > >>> sent him, I'd forgotten apm_saver.c > >>> > >>> MFC After: 3 weeks > >> > >>In the past, I've been against mandating that callouts/timeouts/generic > >>taskqueues should not be allowed to sleep. However, after looking over > >>the history of this problem as well as others, it seems that it's just > >>too easy for driver authors to make bad assumptions and wind up with a > >>priority inversion/deadlock like this. It would be relatively trivial > >>to mark these contexts as being non-sleepable and have the msleep code > >>enforce it, like is done with ithreads. What do you think? Anyways, > >>thanks for looking at this and fixing it. > > > > > > We already do for timeouts if INVARIANTS is on: > > > > softclock() > > { > > ... > > THREAD_NO_SLEEPING(); > > c_func(c_arg); > > THREAD_SLEEPING_OK(); > > ... > > } > > > > That has been in place since 6.0 IIRC. > > > > I thought that it was only enabled for DIAGNOSTIC. > > Scott Nah, that was phk's other timekeeping code to see which timeouts take a long time to execute. The THREAD_NO_SLEEPING() stuff was added just before 6.0 was released and replaced a couple of "special" mutexes that were held just to provoke WITNESS warnings. -- John Baldwin