Date: Sun, 27 Sep 2015 03:59:53 +1000 (EST) From: Ian Smith <smithi@nimnet.asn.au> To: Colin Percival <cperciva@FreeBSD.org> Cc: Jung-uk Kim <jkim@FreeBSD.org>, John Baldwin <jhb@FreeBSD.org>, "freebsd-acpi@freebsd.org" <freebsd-acpi@FreeBSD.org>, Dan Lukes <dan@obluda.cz> Subject: Re: disabling sleep when shutting down Message-ID: <20150927024553.L29510@sola.nimnet.asn.au> In-Reply-To: <560648A7.4030708@freebsd.org> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <sys/stdint.h> /* 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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150927024553.L29510>