qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] Re: [5578] Increase default IO timeout from 10ms to 5s
Date: Wed, 5 Nov 2008 20:28:52 +0000	[thread overview]
Message-ID: <20081105202852.GZ25523@redhat.com> (raw)
In-Reply-To: <fb249edb0811051216q2a9549e4sbef428cfcdbe51cf@mail.gmail.com>

On Wed, Nov 05, 2008 at 09:16:59PM +0100, andrzej zaborowski wrote:
> 2008/11/5 Jan Kiszka <jan.kiszka@web.de>:
> > M. Warner Losh wrote:
> >> In message: <20081105150042.GJ13630@shareable.org>
> >>             Jamie Lokier <jamie@shareable.org> writes:
> >> : M. Warner Losh wrote:
> >> : > : In other words, don't use pselect() if you might run on a kernel older
> >> : > : than 2.6.16, or on a host architecture which adds pselect() in a later
> >> : > : kernel version.  Also, I wouldn't be surprised if older versions of
> >> : > : some BSDs have similar dodgy wrappers.
> >> : >
> >> : > Which ones have a good kernel implementation of it?  FreeBSD's is
> >> : > currently approximately:
> >> : >
> >> : >   if (!mask)
> >> : >           _sigprocmask(mask, &oldmask);
> >> : >   /* here */
> >> : >   select();
> >> : >   if (!mask)
> >> : >           _sigprocmask(oldmask, NULL);
> >> : >
> >> : > I'm assuming that the problem is due to a signal arriving at /* here */.
> >> :
> >> : If that's _kernel_ code and the kernel behaves like Linux, it's not a
> >> : problem because signals don't affect the control flow until returning
> >> : to userspace, meaning the select() will return EINTR.
> >>
> >> It is currently user level code, and I'm looking at moving it into the
> >> kernel, but I need to understand the race being talked about here.
> >
> > From the Linux man page on [p]select:
> >
> > "The reason that pselect() is needed is that if one wants to wait for
> > either a signal or for a file descriptor to become ready, then an atomic
> > test is needed to prevent race conditions. (Suppose the signal handler
> > sets a global flag and returns. Then a test of this global flag followed
> > by a call of select() could hang indefinitely if the signal arrived just
> > after the test but just before the call. By contrast, pselect() allows
> > one to first block signals, handle the signals that have come in, then
> > call pselect() with the desired sigmask, avoiding the race.)"
> >
> > So the unmasking and possible blocking on select must be done
> > atomically. And that is only feasible in kernel land.
> 
> To be exact, it *was* possible for glibc to implement a pselect free of races:
> that is by using the same trick as your patch, i.e. making a pipe and
> adding it to select()ed fd's and mangling the sigmask.

Yes & no. The trouble with glibc using pipes behind your back is that
then it creates a totally different race in threaded apps, where a FD
could be leaked to a child process between glibc opening its secret 
pipe and setting the O_CLOSEXEC flag. Indeed it already suffers from
this problem with name resolving

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

  reply	other threads:[~2008-11-05 20:28 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-31 18:40 [Qemu-devel] [5578] Increase default IO timeout from 10ms to 5s Anthony Liguori
2008-10-31 18:52 ` Anthony Liguori
2008-11-02 19:08   ` [Qemu-devel] " Jan Kiszka
2008-11-03 20:04     ` Anthony Liguori
2008-11-03 20:36       ` Jan Kiszka
2008-11-03 21:50         ` Jan Kiszka
2008-11-03 22:00           ` Anthony Liguori
2008-11-03 22:03             ` Jan Kiszka
2008-11-04  8:07             ` andrzej zaborowski
2008-11-04  8:22               ` Jan Kiszka
2008-11-04  8:33                 ` andrzej zaborowski
2008-11-04 11:32                   ` Jamie Lokier
2008-11-04 16:22                     ` M. Warner Losh
2008-11-04 17:10                       ` Jan Kiszka
2008-11-04 17:55                         ` M. Warner Losh
2008-11-04 19:08                           ` Jan Kiszka
2008-11-05 15:00                       ` Jamie Lokier
2008-11-05 16:10                         ` M. Warner Losh
2008-11-05 18:21                           ` Jan Kiszka
2008-11-05 18:41                             ` Daniel P. Berrange
2008-11-05 20:16                             ` andrzej zaborowski
2008-11-05 20:28                               ` Daniel P. Berrange [this message]
2008-11-05 23:38                                 ` Jamie Lokier
2008-11-06  0:53                           ` Jamie Lokier
2008-11-06  5:19                             ` M. Warner Losh
2008-11-04  8:29           ` Avi Kivity
2008-10-31 19:41 ` [Qemu-devel] " Jamie Lokier
2008-10-31 20:13   ` Anthony Liguori

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=20081105202852.GZ25523@redhat.com \
    --to=berrange@redhat.com \
    --cc=qemu-devel@nongnu.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).