[email protected] wrote: > [email protected] 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 > > > > [email protected]:/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. >
Ping! Could someone familiar with apmd(8) please take a look and comment; thanks. Prior discussion on bugs@ about the problem... https://marc.info/?t=150784125800002&r=1&w=2 > 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); >
