Ticket #2091 (new task)

Opened 10 years ago

Last modified 10 years ago

batget polls lots too often

Reported by: Viddy Owned by: openmoko-devel
Priority: lowest Milestone: FSO
Component: unknown Version: FSO-MS2
Severity: trivial Keywords: batget,polling
Cc: dmurrell@… Blocked By:
Blocking: Estimated Completion (week):
HasPatchForReview: no PatchReviewResult:


A strace of


while it is running produces the following output, repeated:

Command to get ouput:
ps ax|grep batget|grep -v grep|awk '{print $1}'|xargs -n1 strace -Ff -p

open("/sys/class/power_supply/bat/current_now", O_RDONLY|O_LARGEFILE) = 6
fstat64(6, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x4001e000
read(6, "-43125\n", 4096) = 7
close(6) = 0
munmap(0x4001e000, 4096) = 0
open("/sys/class/power_supply/bat/status", O_RDONLY|O_LARGEFILE) = 6
fstat64(6, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x4001e000
read(6, "Charging\n", 4096) = 9
close(6) = 0
munmap(0x4001e000, 4096) = 0
open("/sys/class/power_supply/bat/time_to_full_now", O_RDONLY|O_LARGEFILE) = 6
fstat64(6, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x4001e000
read(6, "0\n", 4096) = 2
close(6) = 0
munmap(0x4001e000, 4096) = 0
open("/sys/class/power_supply/bat/charge_now", O_RDONLY|O_LARGEFILE) = -1 ENOENT (No such file or directory)
gettimeofday({1225162448, 995725}, NULL) = 0
select(6, [4 5], [], [], {0, 254907}) = 0 (Timeout)
gettimeofday({1225162449, 257579}, NULL) = 0
gettimeofday({1225162449, 260369}, NULL) = 0
open("/sys/class/power_supply/bat/capacity", O_RDONLY|O_LARGEFILE) = 6
fstat64(6, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x4001e000
read(6, "100\n", 4096) = 4
close(6) = 0
munmap(0x4001e000, 4096) = 0

Count of calls, and time taken:
root@om-gta02:~# strace -fF -p 1416 -c
Process 1416 attached - interrupt to quit
Process 1416 detached
% time seconds usecs/call calls errors syscall

100.00 0.004264 102 42 mmap2

0.00 0.000000 0 41 read
0.00 0.000000 0 52 10 open
0.00 0.000000 0 41 close
0.00 0.000000 0 32 gettimeofday
0.00 0.000000 0 41 munmap
0.00 0.000000 0 11 select
0.00 0.000000 0 42 fstat64

100.00 0.004264 302 10 total

I appreciate that the easiest way to check the battery status is to poll the device every so often, and inotify is more challenging to use, but
a) polling multiple files n times per second isn't a good idea, and b) there really isn't any point in polling the battery while the screen is off

use sleep()
if screen is off, sleep() longer
if screen comes back on, wake up process, get it to update battery, go back to sleep for a few seconds/minutes

Given that battery indicator is a four bar or percentage indicator, a calculation on how long to sleep for given current capacity and discharge rate divided by a fudge factor should give the time to sleep for before any meaningful change in the indicator would be seen by the user.

Change History

comment:1 Changed 10 years ago by raster

  • Priority changed from normal to lowest
  • Type changed from defect to task
  • Severity changed from normal to trivial

first. 1. the default config for e (upstream) is 32 "ticks" (8 ticks per second) between polls. thats every 4 seconds. that is less waking up than almost anything else around on the system. second - it's a config option in the battery module config - increase the # of ticks between checks there is a gui for it. 2. you cannot use d/inotify on sysfs - you can't get change events. it doesn't work. if you could - i would have already. i use it where appropriate and possible already. 3. - there have been lots of bugs with emitting events on the uevent file for the battery when plugging/unplugging power and battery so poll time is "often enough" so that u'll catch a change (like plug in power and start charging) with a poll rather than a uevent - i don't know if this is fixed these days as the polling covers it up mostly. just to expand you can get events - but the event reports "ac plugged in" but battery wont report its charging for an arbitrary amount of time after so you have to poll to find this out. or that was the case last time i looked and was working on this code. 4. if the screen is blanked u likely want to be going to sleep soon anyway (suspend) so until zero-clock rolls around and all userspace stuff that polls needs to be aware of this - it's not work the effort as you have a problem of not having suspended, rather than there being occasional polling. if the system is not idle (eg playing music or making a call) while blanked, then you are constantly awake and consuming cycles anyway so it doesn't matter. the poll overhead is nigh zero except for kernel -> device overhead. 5. the battery meter is not a 4 bar limit - that is merely the theme that om wanted that reduces accuracy. reality is that the battery reports full detail and the batget abstraction will always report a value from 0 to 100 for battery then the gadget and ultimately theme are responsible for displaying it however the like (by putting up a group of chickens and the more chickens you have, the more power, or by color (green, yellow, red) or a dial, or... the ui designer entirely decides that. batget/code know nothing of their intentions and are just there to feed the current state of the machine :). the battery itself and other themes display full accuracy (eg e17(illume)'s default theme), so you can't calculate when to poll as power drain may be variable and the theme/ui abstracts the display. 6. do some actual power measurements. kill batget then run it by hand:

/usr/lib/enlightenment/modules/battery/linux-*/batget X

where X is ticks. there are 8 ticks per second. the default is 32. try it and actually do power drain measurements (run a idle system for lets say 10 minutes and calculate battery drain at the end of it). do it while running batget @ 32 ticks and with batget off. if you can show measurable differences (that are not entropy - so u'll have to run the test multiple times) it'd be maaaaybe worth a bit more code and effort - but really, it isn't. my bet is that you will not show any measurable difference.

remember - i really paid attention to this when writing the code. it goes to effort to minimise what it opens when it polls and how often it does and allows it to be configured. i know about all the details about power consumption, wakeups, (i've checked with powertop before). batget HAS to poll (has no choice) and even have a generic mechanism to indicate the polling interval. :)

so here's your challenge. test with batget off. run for a period of time (10 mins, 30) measure drain (from full), recharge to full then do it again with batget at 32 ticks, then maybe try it at 128 or 256 or 512 ticks (make it a power of 2 - it only allows power of 2). if you got a measurable difference from no batget to 32 ticks. if u see a difference < 5-10% it's probably entropy so u'll need to run the test many times to get an average.

if you can show real differences - report back here :) because there is nothing much u can do code-wise to improve this beyond the battery issuing interrupts via kernel to indicate a change in level, which the battery is not capable of doing as it literally needs to be polled to get its charge on the hardware level. so someone will do the polling - if not batget - it'll be a kernel thread. you just shuffle the problem out of your sight - but don't remove it or the overhead. what you do add is likely lack of control and configuration :)

Note: See TracTickets for help on using tickets.