From owner-freebsd-acpi@freebsd.org Sat Sep 26 18:00:04 2015 Return-Path: Delivered-To: freebsd-acpi@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8137CA09B51 for ; Sat, 26 Sep 2015 18:00:04 +0000 (UTC) (envelope-from smithi@nimnet.asn.au) Received: from sola.nimnet.asn.au (paqi.nimnet.asn.au [115.70.110.159]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id B967AF19; Sat, 26 Sep 2015 18:00:02 +0000 (UTC) (envelope-from smithi@nimnet.asn.au) Received: from localhost (localhost [127.0.0.1]) by sola.nimnet.asn.au (8.14.2/8.14.2) with ESMTP id t8QHxr3t095641; Sun, 27 Sep 2015 03:59:53 +1000 (EST) (envelope-from smithi@nimnet.asn.au) Date: Sun, 27 Sep 2015 03:59:53 +1000 (EST) From: Ian Smith To: Colin Percival cc: Jung-uk Kim , John Baldwin , "freebsd-acpi@freebsd.org" , Dan Lukes Subject: Re: disabling sleep when shutting down In-Reply-To: <560648A7.4030708@freebsd.org> Message-ID: <20150927024553.L29510@sola.nimnet.asn.au> References: <55FA3848.7090802@freebsd.org> <55FB233D.2080000@FreeBSD.org> <55FB48E3.20401@freebsd.org> <55FC4F13.3090603@FreeBSD.org> <55FC57F9.3050702@yahoo.com> <55FE5D54.1030806@freebsd.org> <5601A863.5070406@FreeBSD.org> <560262BF.7090107@freebsd.org> <5602DE8D.3020102@FreeBSD.org> <560648A7.4030708@freebsd.org> MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY=------------050603020603000908060907 Content-ID: <20150927024553.U29510@sola.nimnet.asn.au> X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 26 Sep 2015 18:00:04 -0000 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --------------050603020603000908060907 Content-Type: TEXT/PLAIN; CHARSET=US-ASCII Content-ID: <20150927024553.P29510@sola.nimnet.asn.au> On Sat, 26 Sep 2015 00:26:31 -0700, Colin Percival wrote: > Hi all, > > I think there is consensus for: > * Using a sysctl (simpler than a device node), > * Setting this sysctl on all architectures, > * Calling the sysctl kern.suspend_blocked, > * Consulting the sysctl from the ACPI code (for now) and possibly from > other platform-specific forms of sleeping (in the indefinite future). Sounds all good to me, FWTW. > Points without consensus: > * jkim thinks we should prevent suspend when we're dropping to single-user > mode; I'm not sure I see the point, but I don't think there's any harm in > doing that too. We should prevent suspending _during_ shutdown to SU, of course. Once happily in SU, is there any reason to disallow suspend? I've done it. > * Ian Smith would like to have suspend blocked for the last 5 minutes before > shutdown(8) signals init to shut the system down. I don't think anyone else > has expressed a desire for this, and some people have raised concerns about > blocking suspend for too long in case a system is running out of battery; so > I'm inclined to leave this out at this point. (It would be easy enough to > add the sysctl-frobbing to shutdown(8) if desired later.) Sorry, but that rather misrepresents my position; I was trying to deal specifically with the LID foot-shooting potential, in the case of user- initiated shutdown, so looking at possible mechanisms. Not to worry, I clearly didn't express myself clearly :^\ As jkim pointed out, code speaks for itself, and C is read-only here at best .. though after 30-odd years I can usually follow it around .. > With the above in mind, I've written (but not yet actually compiled or tested, > so beware of typos) a patch which I think makes sense. If nobody is violently > opposed to this I'll make sure I got the details right and then commit this in > a few days. +static void +block_suspend(int block) +{ + + sysctlbyname("kern.suspend_blocked", NULL, NULL, &block, sizeof(block)); +} This doesn't appear to get called? Any idea if any of this may not be straightforward to MFC, to 9 maybe? Thanks for going for the generic solution. Hope it wins that race :) cheers, Ian --------------050603020603000908060907 Content-Type: TEXT/X-PATCH; NAME=block_suspend.patch Content-ID: <20150927024553.X29510@sola.nimnet.asn.au> Content-Description: Content-Disposition: ATTACHMENT; FILENAME=block_suspend.patch Index: sbin/init/init.c =================================================================== --- sbin/init/init.c (revision 288249) +++ sbin/init/init.c (working copy) @@ -1481,6 +1481,16 @@ } /* + * Block (if block == 1) or unblock (if block == 0) suspend. + */ +static void +block_suspend(int block) +{ + + sysctlbyname("kern.suspend_blocked", NULL, NULL, &block, sizeof(block)); +} + +/* * Bring the system down to single user. */ static state_func_t @@ -1487,7 +1497,16 @@ death(void) { session_t *sp; + int block, blocked; + size_t len; + /* Temporarily block suspend. */ + len = sizeof(blocked); + block = 1; + if (sysctlbyname("kern.suspend_blocked", &blocked, &len, + &block, sizeof(block))) + blocked = 0; + /* * Also revoke the TTY here. Because runshutdown() may reopen * the TTY whose getty we're killing here, there is no guarantee @@ -1503,6 +1522,11 @@ /* Try to run the rc.shutdown script within a period of time */ runshutdown(); + /* Unblock suspend if we blocked it. */ + if (!blocked) + sysctlbyname(kern.suspend_blocked", NULL, NULL, + &blocked, sizeof(blocked)); + return (state_func_t) death_single; } Index: sys/dev/acpica/acpi.c =================================================================== --- sys/dev/acpica/acpi.c (revision 288249) +++ sys/dev/acpica/acpi.c (working copy) @@ -2574,8 +2574,11 @@ if (!acpi_sleep_states[state]) return (EOPNOTSUPP); - /* If a suspend request is already in progress, just return. */ - if (sc->acpi_next_sstate != 0) { + /* + * If a reboot/shutdown/suspend request is already in progress or + * suspend is blocked due to an upcoming shutdown, just return. + */ + if (rebooting || sc->acpi_next_sstate != 0 || suspend_blocked) { return (0); } Index: sys/kern/kern_shutdown.c =================================================================== --- sys/kern/kern_shutdown.c (revision 288249) +++ sys/kern/kern_shutdown.c (working copy) @@ -137,6 +137,10 @@ SYSCTL_INT(_kern_shutdown, OID_AUTO, show_busybufs, CTLFLAG_RW, &show_busybufs, 0, ""); +int suspend_blocked = 0; +SYSCTL_INT(_kern, OID_AUTO, suspend_blocked, CTLFLAG_RW, + &suspend_blocked, 0, "Block suspend due to a pending shutdown"); + /* * Variable panicstr contains argument to first call to panic; used as flag * to indicate that the kernel has already called panic. Index: sys/sys/systm.h =================================================================== --- sys/sys/systm.h (revision 288249) +++ sys/sys/systm.h (working copy) @@ -46,6 +46,7 @@ #include /* for people using printf mainly */ extern int cold; /* nonzero if we are doing a cold boot */ +extern int suspend_blocked; /* block suspend due to pending shutdown */ extern int rebooting; /* kern_reboot() has been called. */ extern const char *panicstr; /* panic message */ extern char version[]; /* system version */ --------------050603020603000908060907--