Fwd: svn commit: r288446 - in head: sbin/init sys/dev/acpica sys/kern sys/sys
Thanks to everybody who helped with the design and discussion of this.
Forwarded Message
Subject: svn commit: r288446 - in head: sbin/init sys/dev/acpica sys/kern
sys/sys
Date: Thu, 1 Oct 2015 10:52:27 + (UTC)
From: Colin Percival <cperc...@freebsd.org>
To: src-committ...@freebsd.org, svn-src-...@freebsd.org,
svn-src-h...@freebsd.org
Author: cperciva
Date: Thu Oct 1 10:52:26 2015
New Revision: 288446
URL: https://svnweb.freebsd.org/changeset/base/288446
Log:
Disable suspend when we're shutting down. This solves the "tell FreeBSD
to shut down; close laptop lid" scenario which otherwise tended to end
with a laptop overheating or the battery dying.
The implementation uses a new sysctl, kern.suspend_blocked; init(8) sets
this while rc.suspend runs, and the ACPI sleep code ignores requests while
the sysctl is set.
Discussed on: freebsd-acpi (35 emails)
MFC after:1 week
Modified:
head/sbin/init/init.c
head/sys/dev/acpica/acpi.c
head/sys/kern/kern_shutdown.c
head/sys/sys/systm.h
Modified: head/sbin/init/init.c
==
--- head/sbin/init/init.c Thu Oct 1 10:43:40 2015(r288445)
+++ head/sbin/init/init.c Thu Oct 1 10:52:26 2015(r288446)
@@ -1487,6 +1487,15 @@ static state_func_t
death(void)
{
session_t *sp;
+ int block, blocked;
+ size_t len;
+
+ /* Temporarily block suspend. */
+ len = sizeof(blocked);
+ block = 1;
+ if (sysctlbyname("kern.suspend_blocked", , ,
+ , sizeof(block)) == -1)
+ blocked = 0;
/*
* Also revoke the TTY here. Because runshutdown() may reopen
@@ -1503,6 +1512,11 @@ death(void)
/* 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,
+ , sizeof(blocked));
+
return (state_func_t) death_single;
}
Modified: head/sys/dev/acpica/acpi.c
==
--- head/sys/dev/acpica/acpi.c Thu Oct 1 10:43:40 2015(r288445)
+++ head/sys/dev/acpica/acpi.c Thu Oct 1 10:52:26 2015(r288446)
@@ -2574,8 +2574,11 @@ acpi_ReqSleepState(struct acpi_softc *sc
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);
}
Modified: head/sys/kern/kern_shutdown.c
==
--- head/sys/kern/kern_shutdown.c Thu Oct 1 10:43:40 2015
(r288445)
+++ head/sys/kern/kern_shutdown.c Thu Oct 1 10:52:26 2015
(r288446)
@@ -137,6 +137,10 @@ static int show_busybufs = 1;
SYSCTL_INT(_kern_shutdown, OID_AUTO, show_busybufs, CTLFLAG_RW,
_busybufs, 0, "");
+int suspend_blocked = 0;
+SYSCTL_INT(_kern, OID_AUTO, suspend_blocked, CTLFLAG_RW,
+ _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.
Modified: head/sys/sys/systm.h
==
--- head/sys/sys/systm.hThu Oct 1 10:43:40 2015(r288445)
+++ head/sys/sys/systm.hThu Oct 1 10:52:26 2015(r288446)
@@ -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 */
___
freebsd-acpi@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
Re: disabling sleep when shutting down
On 09/26/15 00:42, Hans Petter Selasky wrote: > On 09/26/15 09:26, Colin Percival wrote: >> I think there is consensus for: >> * Using a sysctl (simpler than a device node), > > Presumably a read/write tunable sysctl, RWTUN? I suppose it could be, but the intention here was to block suspend while we're shutting down, not to have it always blocked. The sysctl isn't a configuration setting, it's a userland-kernel communications channel. -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
Re: disabling sleep when shutting down
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).
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.
* 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.)
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.
--
Colin Percival
Security Officer Emeritus, FreeBSD | The power to serve
Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid
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, , 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", , ,
+ , 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,
+ , 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,
_busybufs, 0, "");
+int suspend_blocked = 0;
+SYSCTL_INT(_kern, OID_AUTO, suspend_blocked, CTLFLAG_RW,
+ _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 */
___
freebsd-acpi@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
Re: disabling sleep when shutting down
On 09/26/15 01:54, Dan Lukes wrote: > Colin Percival wrote: >> I've written (but not yet actually compiled or tested, so beware of typos) a >> patch which I think makes sense. > > It look good. > > But it seems to me the goal has been lost during discussion. It has been > started because of unwilling interaction of LID close and ongoing > shutdown, isn't it ? Yes, I tripped over this in the context of "tell FreeBSD to shut down, then close the lid of my laptop". > Unless I misunderstood the code, your patch seems to solve it for > special case only: > hw.acpi.lid_switch_state=S3 > > But what about Sx states for x -ne 3 ? > > I don't feel to be expert on ACPI code, thus it's possible those states > are handled (=ignored) correctly as well. In such case ignore this message. I may be wrong, but I believe all the ACPI Sx states go through the same function which is checking for suspend_blocked in my patch. -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
Re: disabling sleep when shutting down
On 09/20/15 03:04, Ian Smith wrote: > On Sun, 20 Sep 2015 00:16:36 -0700, Colin Percival wrote: > > On 09/18/15 11:29, Anthony Jenkins wrote: > > > Is it possible for /etc/rc.shutdown to complete, but shutdown not > > > occur? If so, there should be a mechanism to restore the ability to > > > suspend. Other than that, I like it. > > > > Hmm... well, rc.shutdown runs before the system drops into single-user > > mode. Which makes me think that maybe we should be making the kernel > > call from inside init instead of from rc.shutdown. > > I still think disabling suspend from shutdown.c, at the same time as > creating /var/run/nologin might be the best way to go, to avoid any > possibility of untimely suspending once committed to shutting down. So you think we should disable suspend for the last 5 minutes before a scheduled shutdown? This seems a bit strange to me... and I honestly can't imagine a situation where you'd need to announce an imminent shutdown of your laptop to logged-in users. > For one thing, shutdown's -o flag bypasses using init and calls halt or > reboot directly, though I don't know if anyone uses that. Right, I figured it wasn't worth worrying about that case since anyone who uses that hopefully knows what they're doing; also since that skips running rc.shutdown there's a much smaller race window. On the other hand, "send a signal to init" and the sysv compatible approach of running `init [runlevel]` are likely to be used by other tools (e.g., desktop environments), so I don't think we should assume that reboot/poweroff requests always go through shutdown(8). > For another, > if shutdown fails for any reason, or is cancelled by signal by the user > .. or in any case, I gather .. finish() removes /var/run/nologin, and > could also there reenable suspend, covering Anthony's point. This would also be accomplished by having the suspend-disabling done by init; if you tell shutdown(8) to not shut the system down, then it never sends the relevant signal to init. The shutdown(8) utility doesn't do any shutting down itself; it's just a front-end which makes an announcement, sets a timer, and disables logins, and then ultimately asks init(8) to do the real work (including spawning rc.shutdown). -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
Re: disabling sleep when shutting down
On 09/18/15 11:29, Anthony Jenkins wrote: > Is it possible for /etc/rc.shutdown to complete, but shutdown not > occur? If so, there should be a mechanism to restore the ability to > suspend. Other than that, I like it. Hmm... well, rc.shutdown runs before the system drops into single-user mode. Which makes me think that maybe we should be making the kernel call from inside init instead of from rc.shutdown. -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
Re: disabling sleep when shutting down
On 09/17/15 13:31, Jung-uk Kim wrote: > On 09/16/2015 23:49, Colin Percival wrote: >> I ran into an interesting glitch recently: I told my laptop to shut >> down, then closed the lid... and it promptly went into S3. When I >> opened the lid a couple days later, it resumed... and then finished >> the shutdown which it had started 2 days earlier. > > Please try the attached patch. No, this doesn't do what I wanted. It might be a good idea anyway, but your patch only disables suspend once the kernel is trying to reboot; what I want is to disable suspend a bit earlier -- once rc.shutdown is running and the userland is trying to shut down, because at that point unless something breaks horribly we're *about to* tell the kernel to shut down even though we haven't gotten there quite yet. -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
disabling sleep when shutting down
Hi ACPI people, I ran into an interesting glitch recently: I told my laptop to shut down, then closed the lid... and it promptly went into S3. When I opened the lid a couple days later, it resumed... and then finished the shutdown which it had started 2 days earlier. Meanwhile with two days of sitting in S3 instead of S5, the battery had almost completely drained. The obvious answer here is to disable Suspend if we're in the shutdown path. My first thought was to make rc.suspend slightly smarter, but that isn't good enough since there's a 10 second timeout after which the suspend will happen even if rc.suspend doesn't send the expected acknowledgment. So I think we need to get the kernel ACPI bits to disable Suspend. It looks to me like adding a sysctl to dev/acpica/acpi.c and checking it in acpi_ReqSleepState would work; then we just need a line in /etc/rc.shutdown to set the sysctl. But ACPI code scares me, so I figured I should check with you guys first... am I missing anything? -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
Re: ENXIOing non-present battery
On 12/12/14 07:21, John Baldwin wrote: On Thursday, December 11, 2014 01:05:49 PM Colin Percival wrote: On 12/11/14 11:08, John Baldwin wrote: Does setting hint.battery.1.disabled=1 work for you? That fixes the dev.battery sysctls and KDE's battery monitor. The hw.acpi.battery.units sysctl still reports 2, and `acpiconf -i 1` still reports the phantom battery; but I suppose those don't matter much... Ok. That is the generic thing we already have in place to disable devices, so I'd probably prefer to use that as the known workaround rather than adding another knob. OK, I'll stick to using that one. My original thinking was that disabling whatever isn't present would avoid the need for a user to figure out which number it was; but it's probably safe to assume that batteries will always be probed in the same order... That said, it looks like we report the userland state of not present correctly. I wonder if the bug is in KDE itself and its FreeBSD-specific power management bits (rather than hald)? The FreeBSD-specific userland bits are in hald. -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to freebsd-acpi-unsubscr...@freebsd.org
Re: ENXIOing non-present battery
On 12/11/14 11:08, John Baldwin wrote: On Sunday, December 07, 2014 2:53:37 am Colin Percival wrote: On my Dell Latitude E7440 laptop, the ACPI reports two batteries: First the battery which exists; and second, a Not Present battery with zeroed statistics. FreeBSD, not realizing that this second battery is a complete myth -- the E7440 only has one battery, and there is nowhere to add another -- faithfully reports the data from ACPI to userland. Does setting hint.battery.1.disabled=1 work for you? That fixes the dev.battery sysctls and KDE's battery monitor. The hw.acpi.battery.units sysctl still reports 2, and `acpiconf -i 1` still reports the phantom battery; but I suppose those don't matter much... -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to freebsd-acpi-unsubscr...@freebsd.org
Re: ENXIOing non-present battery
On 12/08/14 15:27, Adrian Chadd wrote: What's the output of acpiconf -i0 and acpiconf -i1? acpiconf -i0 is exactly what you'd expect. acpiconf -i1 shows blank for the model/serial/type/OEM info strings, and 0 mAh/mV for the rest except State which is not present. I wonder if changing 'state' to something else would keep everything happy. I can't see how it would. The hald code uses hw.acpi.battery.units to see how many batteries there are, then reads the _BIF data via ioctl; the state isn't even in _BIF. -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to freebsd-acpi-unsubscr...@freebsd.org
Re: ENXIOing non-present battery
On 12/08/14 21:33, Ian Smith wrote: And what does 'grep battery /var/run/dmesg.boot' have to say? Normally with 2 batteries catered for and only one fitted you'd expect to see eg: ./nicks_acpi/dmesg-bootwithacpi-2-part.txt:battery0: ACPI Control Method Battery on acpi0 ./nicks_acpi/dmesg-bootwithacpi-2-part.txt:battery1: ACPI Control Method Battery on acpi0 ./nicks_acpi/dmesg-bootwithacpi-2-part.txt:battery0: battery initialization start ./nicks_acpi/dmesg-bootwithacpi-2-part.txt:battery1: battery initialization start ./nicks_acpi/dmesg-bootwithacpi-2-part.txt:battery0: battery initialization done, tried 1 times ./nicks_acpi/dmesg-bootwithacpi-2-part.txt:battery1: battery initialization failed, giving up Yes, that's exactly what I see. -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to freebsd-acpi-unsubscr...@freebsd.org
Re: ENXIOing non-present battery
On 12/07/14 08:03, Adrian Chadd wrote: How's this work on other systems? KDE on Linux doesn't lose its mind if the second battery is totally flat. I just booted Ubuntu 14.04, and both batteries appear in /proc/acpi/battery; but BAT1 just shows present: no without any statistics, and the GUI shows the correct state for the single present battery. -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to freebsd-acpi-unsubscr...@freebsd.org
Re: ENXIOing non-present battery
On 12/07/14 00:28, Dan Lukes wrote: Colin Percival wrote: it seems like not attaching a non-present battery would be a good idea. This shouldn't be the default behaviour, since there are plenty of systems where a non-present battery might be inserted at a later time; but I see nothing wrong with adding an option. The attached patch adds a acpi.cmbat.hide_not_present loader tunable which, as the name suggests, hides non-present batteries; this is done in the probe code by returning ENXIO Any objections to me committing this? No, but it may be more useful to create more generalized interface. I agree in theory, but do you have any idea how to do this? A remember broken ACPI with issues related to ghost floppy drive when no real floppy drive has been present. Not only battery, but any device should be considered non-attachable upon administrator request. In the case of my laptop, the only way I can see to distinguish between the real battery and the ghost battery is that the latter returns zero from acpi_BatteryIsPresent; and that won't work for non-battery devices. There is, of course, the option of using debug.apci.disabled to disable complete subsystems, but I only want to hide the ghost battery, not the real one. -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to freebsd-acpi-unsubscr...@freebsd.org
Re: ENXIOing non-present battery
On 12/07/14 08:03, Adrian Chadd wrote: Wait - so it reports a battery with 0% in it, but not that it's not present? It reports all zeroes: Not Present, 0% power, 0V, 0mA design capacity, etc. How's this work on other systems? KDE on Linux doesn't lose its mind if the second battery is totally flat. Good question. I'll download an Ubuntu image and find out. Given that KDE gets this information via hald, it's possible that hald's linux code has a workaround for this though -- the battery-status-reading code is entirely separate between FreeBSD and Linux. -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid ___ freebsd-acpi@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-acpi To unsubscribe, send any mail to freebsd-acpi-unsubscr...@freebsd.org
