On Sat, Feb 15 2020, Jeremie Courreges-Anglas <[email protected]> wrote:
> On Fri, Feb 14 2020, Scott Cheloha <[email protected]> wrote:
>> On Thu, Feb 13, 2020 at 02:08:32PM +0100, Jeremie Courreges-Anglas wrote:
>>> On Wed, Feb 12 2020, Scott Cheloha <[email protected]> wrote:
>>> > On Wed, Feb 12, 2020 at 01:35:22PM +0100, Jeremie Courreges-Anglas wrote:
>>> >> On Wed, Feb 12 2020, Jeremie Courreges-Anglas <[email protected]> wrote:
[...]
>>> >> On top of the previous diff, here's a diff to block autoaction for 60
>>> >> seconds after:
>>> >> - autoaction triggers; this prevents apmd from sending multiple suspend
>>> >> requests before the system goes to sleep
>>> >> - a resume happens; this gives you 60 seconds to fetch and plug your AC
>>> >> cable if you notice you're low on power
>>> >>
>>> >> A side effect is that apmd now ignores stale acpiac(4) data at resume
>>> >> time, so it apmd doesn't suspend the system again when you resume with
>>> >> a low battery and AC plugged.
>>> >>
>>> >> cc'ing Scott since he has a thing for everything time-related. :)
>>> >
>>> > For the first case, is there any way you can detect that a suspend is
>>> > in-progress but not done yet?
>>>
>>> Well, apmd could record that it asked the kernel for a suspend/hibernate
>>> and skip autoaction as long as it doesn't get a resume event.
>>
>> Hmmm. So what happens if the suspend/hibernate fails? Could apmd(8)
>> get stuck waiting for a resume that will never happen?
>
> Well, if suspend fails maybe there's no point in having apmd retry
> a suspend. Also what would get stuck is only the autoaction behavior,
> the rest of apmd would keep on working as usual.
>
> acpi_sleep_state seems to properly send a RESUME event even if
> suspend/hibernate fails, except in one error case.
>
> But depending on a resume event is not portable, the APM code in i386
> and loongson doesn't notify userland about resumes. Something that ought
> to be fixed.
Looks like i386 apm(4) actually sends resume events, and I teached
loongson to send an APM_NORMAL_RESUME event too. So unthrottling
autoaction using resume events has a chance to work on all relevant
platforms.
If autoaction asks for a suspend and the suspend fails and the kernel
fails to send a resume event, autoaction will stay disabled in apmd(8).
I think that's reasonable, after all, why would a second suspend request
succeed?
The diff below (on top of -current):
- blocks autoaction after it kicks in, until a resume event is received.
This prevents autoaction from sending multiple suspend requests before
suspend happens, and avoids spurious suspend/resume cycles.
- blocks autoaction for 60 seconds after a resume event, so that the
user has time to control the system / disable apmd(8) if needed, etc
Thanks for the feedback so far.
Comments, tests and oks welcome.
Index: apmd.c
===================================================================
--- apmd.c.orig
+++ apmd.c
@@ -368,18 +368,20 @@ resumed(int ctl_fd)
}
#define TIMO (10*60) /* 10 minutes */
+#define AUTOACTION_GRACE_PERIOD (60) /* 1mn after resume */
int
main(int argc, char *argv[])
{
const char *fname = _PATH_APM_CTLDEV;
int ctl_fd, sock_fd, ch, suspends, standbys, hibernates, resumes;
- int autoaction = 0;
+ int autoaction = 0, autoaction_inflight = 0;
int autolimit = 0;
int statonly = 0;
int powerstatus = 0, powerbak = 0, powerchange = 0;
int noacsleep = 0;
struct timespec ts = {TIMO, 0}, sts = {0, 0};
+ struct timespec last_resume = { 0, 0 };
struct apm_power_info pinfo;
const char *sockname = _PATH_APM_SOCKET;
const char *errstr;
@@ -566,6 +568,8 @@ main(int argc, char *argv[])
powerstatus = powerbak;
powerchange = 1;
}
+ clock_gettime(CLOCK_MONOTONIC, &last_resume);
+ autoaction_inflight = 0;
resumes++;
break;
case APM_POWER_CHANGE:
@@ -577,17 +581,30 @@ main(int argc, char *argv[])
if (!powerstatus && autoaction &&
autolimit > (int)pinfo.battery_life) {
+ struct timespec graceperiod, now;
+
+ graceperiod = last_resume;
+ graceperiod.tv_sec += AUTOACTION_GRACE_PERIOD;
+ clock_gettime(CLOCK_MONOTONIC, &now);
+
logmsg(LOG_NOTICE,
"estimated battery life %d%%"
- " below configured limit %d%%",
- pinfo.battery_life,
- autolimit
+ " below configured limit %d%%%s%s",
+ pinfo.battery_life, autolimit,
+ !autoaction_inflight ? "" : ", in flight",
+ timespeccmp(&now, &graceperiod, >) ?
+ "" : ", grace period"
);
- if (autoaction == AUTO_SUSPEND)
- suspends++;
- else
- hibernates++;
+ if (!autoaction_inflight &&
+ timespeccmp(&now, &graceperiod, >)) {
+ if (autoaction == AUTO_SUSPEND)
+ suspends++;
+ else
+ hibernates++;
+ /* Block autoaction until next resume */
+ autoaction_inflight = 1;
+ }
}
break;
default:
Index: apmd.8
===================================================================
--- apmd.8.orig
+++ apmd.8
@@ -128,6 +128,7 @@ If both
and
.Fl z
are specified, the last one will supersede the other.
+After a resume, the effect of those options is inhibited for 60 seconds.
.El
.Pp
When a client requests a suspend or stand-by state,
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
