linux-hotplug.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
To: linux-hotplug@vger.kernel.org
Subject: Re: Report: Threaded udevd
Date: Wed, 22 Oct 2008 16:12:12 +0000	[thread overview]
Message-ID: <48FF50DC.3000803@tuffmail.co.uk> (raw)
In-Reply-To: <48FF3458.6030909@tuffmail.co.uk>

Scott James Remnant wrote:
> On Wed, 2008-10-22 at 15:10 +0100, Alan Jenkins wrote:
>
>   
>> = Justification for udev-exec =
>>
>> External programs are run from a "udev-exec" subprocess.  This was
>> measured as reducing the fork() overhead.  To reduce overheads further,
>> it allowed me to use of vfork() without temporarily suspending all the
>> threads.  The commands to run are passed over a unix datagram socket;
>> pipes for stdout / stderr pipes are passed as ancillary data.  This
>> udev-exec accounts for over half the code.  The message passing
>> shouldn't impose too much overhead.  I think udev-exec should be used
>> even if threading has been disabled, otherwise there would be too many
>> ugly #ifdefs.
>>
>>     
> This sounds odd to me; udev-exec still needs to fork() for each child,
> and there's a cost of passing the command over the socket -- so how is
> this cheaper than just fork()ing inside udevd?
>
>   

My guess was that it avoids cloning the mappings for the thread stacks. 
I can test this again and compare the two profiles.  There was
definitely a significant difference, but I should have another look and
find out why.

I also measured an improvement from using vfork().  I don't know how
that horror interacts with threading, and I don't want to find out :).

>> udev-exec also avoids running external programs with extra file
>> descriptors - the problem "close on exec" was designed to solve.  I
>> don't think I could insist on a kernel recent enough to provide "close
>> on exec" support.  Plus I don't think "close on exec" helps with pipes
>> used to communicate with child processes.  Without this, the Ubuntu
>> "watershed" command seemed to complain about extra file descriptors.
>>
>>     
> I see no problem with depending on the very latest kernels; using a
> latest udev version goes hand-in-hand with using a latest kernel.
>
>   

Hmm.  I guess libudev ought to use it either way.  (libudev can test if
the flag is supported; I don't think it needs the new system calls).

> The pipe2() syscall allows you to specify flags for the returned file
> descriptors:
>
> 	pipe2 (&fds, O_CLOEXEC)
>   

Ah!  I see how it's supposed to be used now.

pipe2 (&fds, O_CLOEXEC)
if (fork() = 0) {
   dup2(&fds[WRITE_END], STDOUT_FILENO);
   /* insert call to fcntl to remove O_CLOEXEC */
   exec(program_name);
...

I was missing the call to fcntl().  It's doesn't really work without it :).

>> Lastly, udev-exec helped with implementing timeouts - event threads just
>> call select() with a timeout, waiting to read the exitcode of the
>> command from a pipe.  None of the wait() family of commands include a
>> timeout.  alarm() wouldn't work because it's per-process.  Without
>> udev-exec, I would have to create posix timers with callbacks to signal
>> the correct thread - and disable SA_RESTART before waiting.
>>
>>     
> Ironically, I have a long-standing proposal to fix this ;)  I've just
> got to get around to persuading a kernel developer to do it, or do it
> myself.
>
> The idea was you'd do:
>
>   fd = waitfd (P_PID, pid, 0);
>
>   FD_SET (fd, &readfds);
>
>   select (fd + 1, &readfds, NULL, NULL, &timeout);
>
>   read (fd, &siginfo, sizeof (struct siginfo_t));
>
> ie. a version of waitid() that returns the data through a socket - then
> all daemon main loop events could be done with select() [we already have
> signalfd(), timerfd_*(), etc.)
>   

I saw a similar complaint by Bernstein when I looked up the self-pipe
trick.  "Of course, the Right Thing would be to have fork() return a
file descriptor".  FWIW, wayback says the page was put up in 2000.

Thanks for the comments
Alan

  parent reply	other threads:[~2008-10-22 16:12 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-22 14:10 Report: Threaded udevd Alan Jenkins
2008-10-22 14:32 ` Scott James Remnant
2008-10-22 15:15 ` Marco d'Itri
2008-10-22 16:12 ` Alan Jenkins [this message]
2008-10-22 16:22 ` Scott James Remnant
2008-10-22 16:24 ` Greg KH
2008-10-22 17:02 ` Marco d'Itri
2008-10-22 17:11 ` Scott James Remnant
2008-10-22 17:14 ` Marco d'Itri
2008-10-23 16:25 ` Karl O. Pinc
2008-10-23 16:29 ` Marco d'Itri
2008-10-23 16:55 ` Karl O. Pinc

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48FF50DC.3000803@tuffmail.co.uk \
    --to=alan-jenkins@tuffmail.co.uk \
    --cc=linux-hotplug@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).