acpid hangs with full cpu usage when removing USB input devices
Natanael Copa
natanael.copa at gmail.com
Mon Jan 9 15:58:10 UTC 2012
Hi,
On Mon, Jan 9, 2012 at 5:02 AM, Denys Vlasenko <vda.linux at googlemail.com> wrote:
> On Friday 06 January 2012 14:52, Natanael Copa wrote:
>> Looking a bit at the code, it appears that it never checks for POLLERR
>> or POLLHUP and thus enters an endless loop.
>>
>> Doh... it just hit me.
>>
>> What happens is, I have an USB keyboard and USB mouse attached to the
>> screen. When I power off the screen the keyboard and mouse input
>> devices disappear and the file handle in acpid becomes invalid. Since
>> nothing checks for POLLERR|POLLHUP it ends up in an endless loop since
>> POLLIN was not set.
>
> This is weird, because usually POLLIN would be set too in such a case -
> since read() would usually be possible (in a sense that it would not block,
> but return EOF or error _immediately_), and therefore poll must indicate that.
>
> Oh well...
>
>> I wonder what the proper way to handle this is. Probably do some
>> netlink stuff to detect new devices for acpid?
>
> Another solution is just to restart acpid.
> Best to do it automatically from hotplug helper...
That'd be a simpler solution yes, and more than good enough.
>> Quick workaround would be to just close fd on POLLHUP or POLLERR:
>
>> diff --git a/util-linux/acpid.c b/util-linux/acpid.c
>> index 6e7321b..3d723c0 100644
>> --- a/util-linux/acpid.c
>> +++ b/util-linux/acpid.c
>> @@ -292,6 +292,11 @@ int acpid_main(int argc UNUSED_PARAM, char **argv)
>> for (i = 0; i < nfd; i++) {
>> const char *event = NULL;
>>
>> + if (pfd[i].revents & (POLLERR | POLLHUP)) {
>> + close(pfd[i].fd);
>> + pfd[i].fd = -1;
>> + }
>> +
>
> I'm a bit surprised this would work: I don't remember any special case
> in poll() to ignore fds < 0, so it _might_ start returning POLLNVAL
> on this pfd[i] from now on.
>From http://pubs.opengroup.org/onlinepubs/009695399/functions/poll.html:
"If the value of fd is less than 0, events shall be ignored, and
revents shall be set to 0 in that entry on return from poll()."
I tested adding a
+ if (pfd[i].revents & (POLLNVAL))
+ printf("pollnval\n");
The POLLNVAL never happens after fd is set to -1.
> Does it work if instead of closing pfd[i].fd, you set pfd[i].events = 0?
No. Still hangs with full CPU usage - and I get lots of POLLNVAL.
> A cleaner solution is to shift all pfd[] elements one element down
> starting from [i], and do nfd--. Something like this:
>
> if (!(pfd[i].revents & POLLIN)) {
> if (pfd[i].revents == 0)
> continue; /* this fd has nothing */
>
> /* Likely POLLERR, POLLHUP, POLLNVAL.
> * Do not listen on this fd anymore.
> */
> close(pfd[i].fd);
> nfd--;
> for (; i < nfd; i++)
> pfd[i].fd = pfd[i + 1].fd;
> break; /* do poll() again */
> }
I think the "if less than zero" thingy is there so you don't need to
the shifting.
> With your fix, what happens when you power your screen back on?
CPU goes back to normal and everything seems to work as it did ...
> I guess the keyboard and mouse ACPI events are no longer detected by acpid,
> right?
... except for the keyboard and mouse. I havent really checked but
yes, I assume it stops to detect mouse and keyboard events since those
fd's are closed and never reopened.
fork/exec'ing itself again would solve that. Do you have other applets
that re-start themselves? I could send a new patch - including a
comment with a reference for the ignore fd == -1 thingy. Or should we
just exit the acpid and let a hook script restart it?
Maybe could use re_exec() to do it together with CLOEXEC?
I am pretty sure you can reproduce it yourself with an USB keybaord
connected when starting up acpid and unplug it afterwards.
> --
> vda
Thanks!
--
Natanael Copa
More information about the busybox
mailing list