All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, laurent@vivier.eu,
	anthony@codemonkey.ws
Subject: Re: [RFC PATCH] fix select(2) race between main_loop_wait and qemu_aio_wait
Date: Mon, 05 Mar 2012 10:25:34 +0100	[thread overview]
Message-ID: <4F54868E.1040402@redhat.com> (raw)
In-Reply-To: <4F548263.1070905@siemens.com>

Il 05/03/2012 10:07, Jan Kiszka ha scritto:
> > This is quite ugly.  Two threads, one running main_loop_wait and
> > one running qemu_aio_wait, can race with each other on running the
> > same iohandler.  The result is that an iohandler could run while the
> > underlying socket is not readable or writable, with possibly ill effects.
> 
> Hmm, isn't it a problem already that a socket is polled by two threads
> at the same time? Can't that be avoided?

We still have synchronous I/O in the device models.  That's the root
cause of the bug, I suppose.

> Long-term, I'd like to cut out certain file descriptors from the main
> loop and process them completely in separate threads (for separate
> locking, prioritization etc.). Dunno how NBD works, but maybe it should
> be reworked like this already.

Me too, I even made a very simple proof of concept a couple of weeks ago
(search for a thread "switching the block layer from coroutines to
threads").  It worked, though it is obviously not upstreamable in any way.

In that world order EventNotifiers would replace
qemu_aio_set_fd_handler, and socket-based protocols such as NBD would
run with blocking I/O in their own thread.  In addition to one thread
per I/O request (from a thread pool), there would be one arbiter thread
that reads replies and dispatches them to the appropriate I/O request
thread.  The arbiter thread replaces the read callback in
qemu_aio_set_fd_handler.

The problem is, even though it worked, making this thread-safe is
another story.  I suspect that in practice it is very difficult to do
without resurrecting RCU patches.

Paolo

WARNING: multiple messages have this Message-ID (diff)
From: Paolo Bonzini <pbonzini@redhat.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: anthony@codemonkey.ws, qemu-devel@nongnu.org,
	kvm@vger.kernel.org, laurent@vivier.eu
Subject: Re: [Qemu-devel] [RFC PATCH] fix select(2) race between main_loop_wait and qemu_aio_wait
Date: Mon, 05 Mar 2012 10:25:34 +0100	[thread overview]
Message-ID: <4F54868E.1040402@redhat.com> (raw)
In-Reply-To: <4F548263.1070905@siemens.com>

Il 05/03/2012 10:07, Jan Kiszka ha scritto:
> > This is quite ugly.  Two threads, one running main_loop_wait and
> > one running qemu_aio_wait, can race with each other on running the
> > same iohandler.  The result is that an iohandler could run while the
> > underlying socket is not readable or writable, with possibly ill effects.
> 
> Hmm, isn't it a problem already that a socket is polled by two threads
> at the same time? Can't that be avoided?

We still have synchronous I/O in the device models.  That's the root
cause of the bug, I suppose.

> Long-term, I'd like to cut out certain file descriptors from the main
> loop and process them completely in separate threads (for separate
> locking, prioritization etc.). Dunno how NBD works, but maybe it should
> be reworked like this already.

Me too, I even made a very simple proof of concept a couple of weeks ago
(search for a thread "switching the block layer from coroutines to
threads").  It worked, though it is obviously not upstreamable in any way.

In that world order EventNotifiers would replace
qemu_aio_set_fd_handler, and socket-based protocols such as NBD would
run with blocking I/O in their own thread.  In addition to one thread
per I/O request (from a thread pool), there would be one arbiter thread
that reads replies and dispatches them to the appropriate I/O request
thread.  The arbiter thread replaces the read callback in
qemu_aio_set_fd_handler.

The problem is, even though it worked, making this thread-safe is
another story.  I suspect that in practice it is very difficult to do
without resurrecting RCU patches.

Paolo

  reply	other threads:[~2012-03-05  9:25 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-05  8:34 [RFC PATCH] fix select(2) race between main_loop_wait and qemu_aio_wait Paolo Bonzini
2012-03-05  8:34 ` [Qemu-devel] " Paolo Bonzini
2012-03-05  9:07 ` Jan Kiszka
2012-03-05  9:07   ` [Qemu-devel] " Jan Kiszka
2012-03-05  9:25   ` Paolo Bonzini [this message]
2012-03-05  9:25     ` Paolo Bonzini
2012-03-05 14:24   ` Avi Kivity
2012-03-05 14:24     ` [Qemu-devel] " Avi Kivity
2012-03-05 14:30     ` Paolo Bonzini
2012-03-05 14:30       ` [Qemu-devel] " Paolo Bonzini
2012-03-05 15:14       ` Avi Kivity
2012-03-05 15:14         ` [Qemu-devel] " Avi Kivity
2012-03-05 16:14         ` Paolo Bonzini
2012-03-05 16:14           ` [Qemu-devel] " Paolo Bonzini
2012-03-05 17:35           ` Avi Kivity
2012-03-05 17:35             ` [Qemu-devel] " Avi Kivity
2012-03-06  9:01             ` Paolo Bonzini
2012-03-06  9:01               ` [Qemu-devel] " Paolo Bonzini
2012-03-05 14:30     ` Jan Kiszka
2012-03-05 14:30       ` [Qemu-devel] " Jan Kiszka
2012-03-05 17:39       ` Avi Kivity
2012-03-05 17:39         ` [Qemu-devel] " Avi Kivity
2012-03-05 17:55         ` Jan Kiszka
2012-03-05 17:55           ` [Qemu-devel] " Jan Kiszka

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=4F54868E.1040402@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=jan.kiszka@siemens.com \
    --cc=kvm@vger.kernel.org \
    --cc=laurent@vivier.eu \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.