sunil+b...@nimmagadda.net wrote:
> >Synopsis: apmd(8) poll timer off by 10x
> >Category: system
> >Environment:
> System : OpenBSD 6.3
> Details : OpenBSD 6.3-current (GENERIC.MP) #54: Wed May 30 23:03:50
> MDT 2018
>
> dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>
> Architecture: OpenBSD.amd64
> Machine : amd64
> >Description:
> With apmd_flags="-Az10", expected apmd(8) to suspend when
> battery is at 10%, however, it didn't check in time and
> laptop ran of out power.
> >How-To-Repeat:
> Disconnect A/C adapter and run with -z percent greater
> than current estimated battery life reported by apm(8);
> poll every minute, for example...
> # rcctl stop apmd
> # apmd -A -z90 -t60
> should suspend in a minute, however, it suspends after 10
> minutes.
> >Fix:
> The following diff...
>
> 1. Provides a dedicated timer that fires every 10 seconds
> instead of relying on EVFILT_READ freqency.
>
> 2. Increments a counter and checks against timeout value,
> if it exceeds, invokes auto-action.
>
> 3. Wraps a few long lines that exceed 80 cols upon code
> shuffle.
Revised diff without whitespace changes and...
1. While querying for changed objects using kevent(2), request changes
for all objects registered instead of just 1 and handle them.
Index: apmd.c
===================================================================
RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
retrieving revision 1.81
diff -u -p -w -r1.81 apmd.c
--- apmd.c 15 Oct 2017 15:14:49 -0000 1.81
+++ apmd.c 8 Jun 2018 10:16:53 -0000
@@ -56,6 +56,9 @@
#define AUTO_SUSPEND 1
#define AUTO_HIBERNATE 2
+#define TIMO (10*60) /* 10 minutes */
+#define TIMER_ID 1
+
const char apmdev[] = _PATH_APM_CTLDEV;
const char sockfile[] = _PATH_APM_SOCKET;
@@ -341,8 +344,6 @@ hibernate(int ctl_fd)
ioctl(ctl_fd, APM_IOC_HIBERNATE, 0);
}
-#define TIMO (10*60) /* 10 minutes */
-
int
main(int argc, char *argv[])
{
@@ -353,13 +354,12 @@ main(int argc, char *argv[])
int statonly = 0;
int powerstatus = 0, powerbak = 0, powerchange = 0;
int noacsleep = 0;
- struct timespec ts = {TIMO, 0}, sts = {0, 0};
struct apm_power_info pinfo;
- time_t apmtimeout = 0;
+ time_t apmtimeout = TIMO, counter = 0;
const char *sockname = sockfile;
const char *errstr;
int kq, nchanges;
- struct kevent ev[2];
+ struct kevent ev[3];
int ncpu_mib[2] = { CTL_HW, HW_NCPU };
int ncpu;
size_t ncpu_sz = sizeof(ncpu);
@@ -379,8 +379,8 @@ main(int argc, char *argv[])
sockname = optarg;
break;
case 't':
- ts.tv_sec = strtoul(optarg, NULL, 0);
- if (ts.tv_sec == 0)
+ apmtimeout = strtoul(optarg, NULL, 0);
+ if (apmtimeout == 0)
usage();
break;
case 's': /* status only */
@@ -466,57 +466,30 @@ main(int argc, char *argv[])
EV_SET(&ev[0], sock_fd, EVFILT_READ, EV_ADD | EV_ENABLE | EV_CLEAR,
0, 0, NULL);
+ EV_SET(&ev[1], TIMER_ID, EVFILT_TIMER, EV_ADD, 0, 10000, NULL);
if (ctl_fd == -1)
- nchanges = 1;
+ nchanges = 2;
else {
- EV_SET(&ev[1], ctl_fd, EVFILT_READ, EV_ADD | EV_ENABLE |
+ EV_SET(&ev[2], ctl_fd, EVFILT_READ, EV_ADD | EV_ENABLE |
EV_CLEAR, 0, 0, NULL);
- nchanges = 2;
+ nchanges = 3;
}
- if (kevent(kq, ev, nchanges, NULL, 0, &sts) < 0)
+ if (kevent(kq, ev, nchanges, NULL, 0, NULL) < 0)
error("kevent", NULL);
if (sysctl(ncpu_mib, 2, &ncpu, &ncpu_sz, NULL, 0) < 0)
error("cannot read hw.ncpu", NULL);
for (;;) {
- int rv;
+ int i, rv;
- sts = ts;
-
- apmtimeout += 1;
- if ((rv = kevent(kq, NULL, 0, ev, 1, &sts)) < 0)
+ if ((rv = kevent(kq, NULL, 0, ev, nchanges, NULL)) < 0)
break;
- if (apmtimeout >= ts.tv_sec) {
- apmtimeout = 0;
-
- /* wakeup for timeout: take status */
- powerbak = power_status(ctl_fd, 0, &pinfo);
- if (powerstatus != powerbak) {
- powerstatus = powerbak;
- powerchange = 1;
- }
-
- if (!powerstatus && autoaction &&
- autolimit > (int)pinfo.battery_life) {
- syslog(LOG_NOTICE,
- "estimated battery life %d%%, "
- "autoaction limit set to %d%% .",
- pinfo.battery_life,
- autolimit
- );
-
- if (autoaction == AUTO_SUSPEND)
- suspend(ctl_fd);
- else
- hibernate(ctl_fd);
- }
- }
-
- if (!rv)
- continue;
-
+ for (i = 0; i < rv; i++) {
+ struct kevent *ke = &ev[i];
+ switch (ke->filter) {
+ case EVFILT_READ:
if (ev->ident == ctl_fd) {
suspends = standbys = hibernates = resumes = 0;
syslog(LOG_DEBUG, "apmevent %04x index %d",
@@ -600,6 +573,42 @@ main(int argc, char *argv[])
hibernate(ctl_fd);
break;
}
+
+ break;
+ case EVFILT_TIMER:
+ if (ev->ident != TIMER_ID)
+ error("invalid timer id", NULL);
+
+ counter += 10;
+ if (counter >= apmtimeout) {
+ counter = 0;
+
+ /* wakeup for timeout: take status */
+ powerbak = power_status(ctl_fd, 0,
&pinfo);
+ if (powerstatus != powerbak) {
+ powerstatus = powerbak;
+ powerchange = 1;
+ }
+
+ if (!powerstatus && autoaction &&
+ autolimit >
(int)pinfo.battery_life) {
+ syslog(LOG_NOTICE,
+ "estimated battery life
%d%%, "
+ "autoaction limit set to
%d%% .",
+ pinfo.battery_life,
+ autolimit
+ );
+
+ if (autoaction == AUTO_SUSPEND)
+ suspend(ctl_fd);
+ else
+ hibernate(ctl_fd);
+ }
+ }
+
+ break;
+ }
+ }
}
error("kevent loop", NULL);
