GNOME Bugzilla – Bug 88553
Problems querying /proc/acpi/..../info
Last modified: 2004-12-22 21:47:04 UTC
Reading /proc/acpi/battery/BAT1/info freezes my laptop (Dell Inspiron 8k, BIOS A19) for some milliseconds every time battstat-applet-2 updates its view, so I patched acpi-linux.c that it would only read this file once. I hope this is not a bad idea... Also with acpi version 20020709, the ac kernel module creates the /proc node in /proc/acpi/ac_adapter/AC/state and not /proc/acpi/ac_adapter/ACAD/state with my BIOS. grusz pHilipp Zabel post scriptum: Here's what I did. diff -u -r gnome-applets2-2.0.0.orig/battstat/acpi-linux.c gnome-applets2-2.0.0/battstat/acpi-linux.c --- gnome-applets2-2.0.0.orig/battstat/acpi-linux.c 2002-07-18 22:55:50.000000000 +0200 +++ gnome-applets2-2.0.0/battstat/acpi-linux.c 2002-07-18 22:52:36.000000000 +0200 @@ -141,7 +141,8 @@ */ gboolean acpi_linux_read(struct apm_info *apminfo) { - guint32 max_capacity, low_capacity, critical_capacity, remain; + static guint32 max_capacity = 0, low_capacity = 0, critical_capacity = 0; + guint32 remain; gboolean charging, ac_online; gulong acpi_ver; char buf[BUFSIZ]; @@ -157,10 +158,6 @@ g_assert(apminfo); - max_capacity = 0; - low_capacity = 0; - critical_capacity = 0; - hash = read_file ("/proc/acpi/info", buf, sizeof (buf)); if (!hash) return FALSE; @@ -177,20 +174,22 @@ } else { batt_info = "/proc/acpi/battery/BAT1/info"; batt_state = "/proc/acpi/battery/BAT1/state"; - ac_state = "/proc/acpi/ac_adapter/ACAD/state"; + ac_state = "/proc/acpi/ac_adapter/AC/state"; ac_state_state = "state"; charging_state = "charging state"; } - hash = read_file (batt_info, buf, sizeof (buf)); - if (hash) - { - max_capacity = read_long (hash, "design capacity"); - low_capacity = read_long (hash, "design capacity warning"); - critical_capacity = read_long (hash, "design capacity low"); - g_hash_table_destroy (hash); - } - + if (!max_capacity) { + hash = read_file (batt_info, buf, sizeof (buf)); + if (hash) + { + max_capacity = read_long (hash, "design capacity"); + low_capacity = read_long (hash, "design capacity warning"); + critical_capacity = read_long (hash, "design capacity low"); + g_hash_table_destroy (hash); + } + } + if (!max_capacity) return FALSE;
The patch is absolutely a requirement for quite a few ACPI laptops [e.g. on my Sony Vaio FX-705] The ACPI-20020815 still keeps the state under "/proc/acpi/ACAD/state" so that change in the patch might be dropped michele@laptop:/proc/acpi$ cat info version: 20020815 states: S0 S3 S4 S5 michele@laptop:/proc/acpi$ cat ac_adapter/ACAD/state state: on-line michele@laptop:/proc/acpi$
hardcoding acpi device names is a bad idea... Some notebooks might have BAT0 and BAT1, others even BAT2... looking at this code snipped the device names are hard-coded... Ouch.
A patch for this issue was posted to the debian-laptop mailing list. http://lists.debian.org/debian-laptop/2002/debian-laptop-200209/msg00460.html Which will also address 93875 but only supports one battery. (i.e. probably only my media bay socket my cd writer is in, not the socket my actual battery resides)
for the ac-adapter issue and the BAT0/BAT1 etc. issue, i have posted a patch at bug 89681. You can also get it at http://www.gentoo.org/~hanno/battstat_two_batteries_v3.diff
I read this in the release notes of the last acpi-patch: "If you've been having problems with reading battery or other information being very slow, please try this release and report if problems persist." I think your problem is not the gnome battery applet, but the kernel. Can you try the latest acpi-patch and tell us if it fixes your issue?
The clock problem is not specific to ACPI. I run RH8 which uses apmd (uses /dev/apm_bios, not /proc/acpi/*) and the battery applet does the same thing to the clock there - I lose about a minute an hour. There appears to be a much lower level bug/problem with system slow-down and/or loss of interrupts when you read the BIOS. See also RedHat Bugzilla report #74645. Others report that turning off the battery applet but leaving apmd running still results in a clock drift, but stopping apmd too seems to keep the clocks in sync. My laptop lets me jump to the BIOS (using Fn-F2 key sequence) while the kernel is running. Doing this really shifts the system time (by hours), but HW clock is OK. Changing the battery applet frequency of battery checking may reduce the impact, but it is not going to fix the root cause of the problem. Dell Inspiron 8200, RH8, kernel 2.4.18-17.8.0 (update from RH)
Patch->high. Kevin, going to have any time to look at this?
Luis: I've applied Hanno's patch already. None of these patches solve the clock thing though is an adequate fashion. Can someone please explain to me what info/ and status/ contain exactly and what they read when no battery is present or when connected to AC etc? I don't have a laptop nor acpi installed so I don't really know how this is supposed to work. Is there a way we can infer whether or not a battery is connected from status/ without having to read /info?
*** Bug 94990 has been marked as a duplicate of this bug. ***
*** Bug 93875 has been marked as a duplicate of this bug. ***
Retitling to reflect the remaining problem
From the release notes of the latest ACPI-Patch: >This includes a LOT of fixes for longstanding bugs. If you have had >issues in the past with long delays or oopses on reads from the battery >interface, hangs on boot, or excessive ACPI interrupts causing system >slowness, please try this patch. So can people with this problem please try out the latest ACPI-patch at www.sourceforge.net/projects/acpi and tell if the problem still occurs.
Removing PATCH keyword since patch has been applied
*** Bug 109911 has been marked as a duplicate of this bug. ***
Created attachment 17003 [details] [review] Patch to use ACPI events to inform us when info has changed.
With the latest kernel ACPI patch, this is still a problem for me on my Dell Latitute D600 laptop. I've attached a patch, above, to battstat-applet that makes use of ACPI events to only read /proc/acpi/battery/*/info when a battery is changed, added, or removed. This is done for AC power also. With these additions, the battery percentage charge (in /proc/acpi/battery/*/state) need only be updated every 30 seconds or so, which I have changed as well. I've posted more information on this patch at http://www.its.caltech.edu/~dmoore/battstat/ including Redhat 9 RPMS so those too lazy to compile can give it a try.
David Moore: Did you think about that /proc/acpi/events is usually locked by acpid? The idea about such an event-patch was there before, but it doesn't make much sense as long as you don't work together with the several powermanagement-daemons such as acpid or ospmd.
Yes, I thought of that. If it can't read /proc/acpi/event, it falls back on /var/run/acpid.socket which is created by acpid. This is necessary anyway, because /proc/acpi/event is usually readable only by root. Does ospmd export a different socket that I should be looking for?
Thanks for looking at this David! And what if /var/run/acpid.socket is not created? I think there was discussion that not all users have acpid running.
If there is no /var/run/acpid.socket, my patch will fall back to apm (which probably won't provide battery info on an ACPI laptop). I can see several possibilities for what to do when acpid is not running: 1) Revert to the old behavior. 2) Read /info once at startup. 3) Pop up an error message explaining they should be running acpid. I like option 3 best, because that way it either works well, or not at all. With option 1 or 2, it would work poorly in the fallback case, and the user wouldn't really know what to do about it. Anyone have thoughts on this?
Re: recent changes to battstat-applet I have an uptime of 3 days using the new applet (via RH9 rpm). With the previous version (as shipped with RH8 & 9) it would cause a kernel panic after as little as 4 hours. I don't know whether the problem lies with my (Fujitsu Amilo D) BIOS, the Linux kernel, ACPI subsystem or batt-stat... but it looks like the problem has gone away. With regard to the applet's behaviour if acpid isn't running: yes, I'd also favour option 3. Whenever I'm configuring hardware under GNU/linux, there are always so many ways to do things... it's often not clear what the 'proper' way is. And it may also help reduce ACPI mailing-list traffic :-)
*** Bug 117947 has been marked as a duplicate of this bug. ***
will be included this patch from http://www.its.caltech.edu/~dmoore/battstat/ in the 2.4 gnome release??? It really improves a lot the applet, that was completelly unusable without this patch...
This patch no longer works on gnome-applets 2.3.90. This is a more current version of the patch anywere? The 2.3.90 battstat works, but takes a constant 5% of CPU, and the previous version with this patch works better. patching file battstat/acpi-linux.c Hunk #2 FAILED at 138. Hunk #3 FAILED at 200. Hunk #4 FAILED at 221. 3 out of 4 hunks FAILED -- saving rejects to file battstat/acpi-linux.c.rej patching file battstat/acpi-linux.h patching file battstat/battstat_applet.c Hunk #5 succeeded at 1508 with fuzz 2 (offset -4 lines). Hunk #6 succeeded at 1551 (offset -3 lines). patching file battstat/battstat.h Hunk #1 succeeded at 118 (offset -1 lines). (and obviously it doesn't compile)
Created attachment 19807 [details] [review] Updated ACPI event patch for gnome-applets 2.3.90
Attached above is an updated version of my ACPI-events patch for the 2.3.90 release of gnome-applets. Note that I've added two features to the patch which will hopefully make it more likely to be merged into a release sometime: 1. If ACPI events cannot be listened to and no APM information is available, yet ACPI seems to be installed on the machine, an error dialog pops up, explaining that the user should start acpid in order for the applet to work properly. 2. If acpid is restarted or is started after the applet, the applet will handle this case and reconnect to the events socket. Any more comments are appreciated. It seems that this patch is necessary for many users to get acceptable performance from the applet. I hope it can be merged into a release soon.
Is this patch the same that is in the other report? #123545?
Yes, bug 123545 contains the same patch. It should probably marked as a dupe of this. I always keep the latest version of the patch on this website: http://www.its.caltech.edu/~dmoore/battstat/ So if there's ever a doubt of what the most recent version is, check the website. On that website the 2.3.90 version of the patch contains some new features over the 2.2.0 version as I've described in my above comments.
Is this patch reasonable to apply?
I've been running with it for some time now and I haven't seen any problems running normally or under valgrind for that matter.
Yah, we should integrate this patch into cvs. I'm not sure if it should go into 2.4.x since I know nothing about acpi. How many people have tested this? Does it work with multiple batteries? Also it adds a string so it would break a string freeze. WRT to the error message perhaps it could be more informative and tell the user how to start the acpid daemon?
I think a lot of people have tested it after it has been announced in various places like the fedora-beta mailing list etc, but I guess the author is the only one who can say how many have downloaded it. It works for me, and reportedly fixes or improves the situation for a lot of people who are seeing battstat eating a lot of CPU. Not sure if it helps for the people who see mouse pointer problems though, maybe that's a different issue.
My patch should support multiple batteries, although I cannot test that. Feel free to change the error string to whatever seems best. On my distribution, acpid is started with: /etc/init.d/acpid start I'm not sure of exactly how many have tested the patch because I don't have access to the webserver logs.
The string addition is still a problem for 2.4.1 at least, but I guess that could be worked around somehow on the stable branch or until after 2.4.1 is out? - don't mark the string for translation - don't make it a dialog, but a g_warning or something? - remove it all together? Christian, what do you think? It seems this patch makes life easier for a lot of people so living with one untranslated string in an obscure corner case where the applet isn't working anyway seems to be a fair tradeoff to me.
Agreed, I vote for applying the patch to both branches, but with the difference that the string is not marked for translation on the gnome-2-4 branch.
I do think we should propose marking it right after the release to have it translated in 2.4.2 though.
Ok then. In any case, I suggest the message be made more general by replacing the filename and path in it with %s. That will help avoiding accidental translations or mistranslations of the filename, too.
Also, avoid hard-coding line breaks in the message (http://developer.gnome.org/doc/tutorials/gnome-i18n/developer.html#line-breaks).
*** Bug 122128 has been marked as a duplicate of this bug. ***
*** Bug 124946 has been marked as a duplicate of this bug. ***
*** Bug 123545 has been marked as a duplicate of this bug. ***
Patch finally applied. Thanks guys for putting in the effort here. I apologize for waiting so long to get this in.