Closed TAAPArthur closed 3 years ago
epoll
was primarily chosen to prevent 'thundering herd' problem[0]. We must have multiple inotify listeners to avoid race condition which may cause uevent loss if uevent files being created very quickly.
[0] From epoll(7)
_If multiple threads (or processes, if child processes have inherited the epoll file descriptor across fork(2)) are blocked in epoll_wait(2) waiting on the same epoll file descriptor and a file descriptor in the interest list that is marked for edge- triggered (EPOLLET) notification becomes ready, just one of the threads (or processes) is awoken from epollwait(2). This provides a useful optimization for avoiding "thundering herd" wake-ups in some scenarios.
I don't see how this leads to uevent loss. Isn't the worst case that multiple threads wakeup and most see the FD is empty and go back to sleep. The current code handles a read of size 0 for the inotify fd just fine. I guess one could be concerned with the resource load for all these threads waking up, but don't think waking up, performing a single read and then going back to sleep is that problematic.
Btw, why inotify is used? Clients can also read data from netlink socket even if they are not run as root. Actually, this is how libudev itself works.
Btw, why inotify is used? Clients can also read data from netlink socket even if they are not run as root. Actually, this is how libudev itself works.
Directly reading uevent from netlink will cause race condition between device manager and libudev-zero. I briefly explained this in README. When uevent arrives to netlink, device manager needs to handle it(modprobe modalias, change permissions, etc...). If we listen on netlink directly, this may cause race condition when device manager did not have time to handle uevent yet. As a result, this will lead to inability to properly handle uevent in libudev-zero due to e.g broken permissions. I experienced this problem between mdev, xorg and libudev-zero while implementing hotplugging support. I chose to use inotify because that allows to handle uevent only after device manager handled it. Correct me if there is other race-free IPC.
Instead of using the Linux-specific epoll, we can use the more portable poll.
So as far as I can tell #23 isn't fixed. This commit also has the side effect of addressing that