Re: [GIT] Experimental threaded udev

[Alan Jenkins <alan-jenkins@tuffmail.co.uk>]

Kay Sievers wrote:
> On Fri, 2009-05-29 at 20:11 +0200, Kay Sievers wrote:
>   
>> On Fri, May 29, 2009 at 19:53, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>>
>>     
>>>  Maybe just recycling the event processes would bring
>>> similar gains, with less of the risks of threads.
>>>       
>> Yeah, I thought that too, without having tested anything, it could be,
>> that we just want to keep a pipe to the event process, and let the
>> event process send a signal back to the main daemon, that it has
>> handled the event, and it goes to sleep after that. The main daemon
>> can recycle a sleeping event process and push a new event over the
>> pipe to it. If no events are queued anymore, the main daemon just
>> closes the pipe, and the event process will exit.
>>
>> With that model we might be able to reduce the number of fork()s
>> significantly. And we would still have the process separation, it's
>> robustness, and the lock-free behavior for malloc, cloexec and all
>> these issues.
>>     
>
> Here is a rough hack to check how it behaves. It boots my box, nothing
> else I really checked.
>
> It clones the event processes as the current version does, but the event
> process stays around to get re-used as a worker for later events.
> Further messages are send over netlink to the worker processes, and the
> worker process signals its state back with sigqueue() rt-signals. When
> the events have settled, the workers get killed after a few seconds of
> idle time.
>
> The current git version:
>   $ time (udevadm trigger; udevadm settle)
>   real	0m1.566s
>   ...
>
>   $ time /sbin/udevd.orig
>   []
>   user	0m0.420s
>   sys	0m1.412s
>
>
> The thread-version:
>   $ time (udevadm trigger -Snet; udevadm settle)
>   real 0m1.336s
>
>   $ time udev/udevd
>   []
>   user	0m0.310s
>   sys	0m0.679s
>
>
> The worker-version:
>   $ time (udevadm trigger; udevadm settle)
>   real	0m1.171s
>   ...
>
>   $ time udev/udevd
>   []
>   user	0m0.057s
>   sys	0m0.095s
>
>
> The thread- and worker-versions do not create as many COW page-faults in
> the daemon after every cloned event-process, and therefore need much
> less CPU.
>
> At least on the dual-core laptop here, the pool of workers seems to be
> faster than the threads.
>   

Great, you beat me to it.  It makes sense that this would be _bit_ 
faster than threads.  As I say I tried to avoid the page faults caused 
(somehow) by forking the extra thread-stack mappings, but it didn't 
really work out.  I'm surprised by quite how much faster it is though!

I have some thoughts which may help in bringing the code up from "rough 
hack" quality :-).

Aren't signal queues unreliable?  If you exceed the maximum number of 
queued signals, sigqueue will fail with EAGAIN, and I don't think 
there's a blocking version :-).  I think a  pipe would be better, or 
maybe you can do something with netlink.


+                       /* wait for more device messages from udevd */
+                       do
+                               dev = udev_monitor_receive_device(monitor);
+                       while (dev = NULL);

I think this loop should finish when the signal handler sets 
worker_exit?  But maybe you didn't actually install a handler for 
SIGTERM and it's still being reset to the default action:

               /* set signal handlers */
                memset(&act, 0x00, sizeof(act));
                act.sa_handler = event_sig_handler;
 @@ -154,66 +203,135 @@ static void event_fork(struct udev_event *event)
                sigaction(SIGTERM, &act, NULL);



I'm not sure you're handling worker processes crashing, it looks like 
you might leave the event in limbo if you get SIGCHLD for a worker in 
state WORKER_RUNNING.  I'm sure you'll test that though.

Talking of unexpected crashes.  I anticipated using a 
connection-oriented socketpair for passing events.  Using netlink for 
this means the worker threads don't get a nice notification if the 
daemon dies without killing them.  Unless this is covered by udevd being 
the session leader, or something like that?  I'll test this empirically 
- maybe it's not important, but I think it's good to know what will happen.

BTW I had a go at this too, but a couple of my workers fail in about 1 
run out of 20, apparently because calloc() returns NULL without setting 
errno.  I'm sure it's my fault but I'll try to track it down.  Obviously 
I'll let you know if your patch could be affected by the same problem.

Thanks
Alan