qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, Anthony Liguori <anthony@codemonkey.ws>
Subject: Re: [Qemu-devel] [RFC PATCH 00/17] Support for multiple "AIO contexts"
Date: Thu, 27 Sep 2012 09:11:55 +0200	[thread overview]
Message-ID: <5063FC3B.1020209@redhat.com> (raw)
In-Reply-To: <506323BE.2000504@redhat.com>

Am 26.09.2012 17:48, schrieb Paolo Bonzini:
> Il 26/09/2012 16:31, Kevin Wolf ha scritto:
> 
>>>> In fact, after removing io_flush, I don't really see what makes AIO
>>>> fd handlers special any more.
>>>
>>> Note that while the handlers aren't that special indeed, there is still
>>> some magic because qemu_aio_wait() bottom halves.
>>
>> Do you mean the qemu_bh_poll() call? But the normal main loop does the
>> same, so I don't see what would be special about it.
> 
> That's an abstraction leakage, IMHO.  After this series the normal main
> loop does not need anymore to call bottom halves.

This is something that I find hard to believe. Bottom halves aren't an
invention of the block layer, but used throughout qemu.

> (Most usage of bottom halves in hw/* is pointless and also falls under
> the category of leaked abstractions.  The other uses could also in
> principle be called at the wrong time inside monitor commands.  Many
> would be served better by a thread pool if it wasn't for our beloved big
> lock).

Possibly, but with the current infrastructure, I'm almost sure that most
of them are needed and you can't directly call them. Nobody uses BHs
just for fun.

>>>> qemu_aio_wait() only calls these handlers, but would it do any harm if
>>>> we called all fd handlers?
>>>
>>> Unfortunately yes.  You could get re-entrant calls from the monitor
>>> while a monitor command drains the AIO queue for example.
>>
>> Hm, that's true... Who's special here - is it the block layer or the
>> monitor? I'm not quite sure. If it's the monitor, maybe we should plan
>> to change that sometime when we have some spare time... ;-)
> 
> It feels like it's the monitor.  But I think in general it is better if
> as little QEMU infrastructure as possible is used by the block layer,
> because you end up with impossibly-knotted dependencies.  Using things
> such as GSource to mediate between the block layer and everything else
> is also better with an eye to libqblock.

I guess my expectation was that if GSource is an improvement for AIO fd
handlers, it would also be an improvement for the rest of fd handlers.

It's well known that qemu as a whole suffers from the NIH syndrome, but
should we really start introducing another NIH wall between the block
layer an the rest of qemu?

> Also, consider that under Windows there's a big difference: after this
> series, qemu_aio_wait() only works with EventNotifiers, while
> qemu_set_fd_handler2 only works with sockets.  Networked block drivers
> are disabled for Windows by these patches, there's really no way to move
> forward without sacrificing them.

Is it really only networked block drivers that you lose this way?

Kevin

  reply	other threads:[~2012-09-27  7:12 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-25 12:55 [Qemu-devel] [RFC PATCH 00/17] Support for multiple "AIO contexts" Paolo Bonzini
2012-09-25 12:55 ` [Qemu-devel] [PATCH 01/17] build: do not rely on indirect inclusion of qemu-config.h Paolo Bonzini
2012-09-25 12:55 ` [Qemu-devel] [PATCH 02/17] event_notifier: enable it to use pipes Paolo Bonzini
2012-10-08  7:03   ` Stefan Hajnoczi
2012-09-25 12:55 ` [Qemu-devel] [PATCH 03/17] event_notifier: add Win32 implementation Paolo Bonzini
2012-09-25 12:55 ` [Qemu-devel] [PATCH 04/17] aio: change qemu_aio_set_fd_handler to return void Paolo Bonzini
2012-09-25 21:47   ` Anthony Liguori
2012-09-25 12:55 ` [Qemu-devel] [PATCH 05/17] aio: provide platform-independent API Paolo Bonzini
2012-09-25 21:48   ` Anthony Liguori
2012-09-25 12:55 ` [Qemu-devel] [PATCH 06/17] aio: introduce AioContext, move bottom halves there Paolo Bonzini
2012-09-25 21:51   ` Anthony Liguori
2012-09-26  6:30     ` Paolo Bonzini
2012-09-25 12:55 ` [Qemu-devel] [PATCH 07/17] aio: add I/O handlers to the AioContext interface Paolo Bonzini
2012-09-25 12:55 ` [Qemu-devel] [PATCH 08/17] aio: add non-blocking variant of aio_wait Paolo Bonzini
2012-09-25 21:56   ` Anthony Liguori
2012-09-25 12:55 ` [Qemu-devel] [PATCH 09/17] aio: prepare for introducing GSource-based dispatch Paolo Bonzini
2012-09-25 22:01   ` Anthony Liguori
2012-09-26  6:36     ` Paolo Bonzini
2012-09-26  6:48     ` Paolo Bonzini
2012-09-29 11:28   ` Blue Swirl
2012-10-01  6:40     ` Paolo Bonzini
2012-09-25 12:55 ` [Qemu-devel] [PATCH 10/17] aio: add Win32 implementation Paolo Bonzini
2012-09-25 12:55 ` [Qemu-devel] [PATCH 11/17] aio: make AioContexts GSources Paolo Bonzini
2012-09-25 22:06   ` Anthony Liguori
2012-09-26  6:40     ` Paolo Bonzini
2012-09-25 12:55 ` [Qemu-devel] [PATCH 12/17] aio: add aio_notify Paolo Bonzini
2012-09-25 22:07   ` Anthony Liguori
2012-09-25 12:55 ` [Qemu-devel] [PATCH 13/17] aio: call aio_notify after setting I/O handlers Paolo Bonzini
2012-09-25 22:07   ` Anthony Liguori
2012-09-25 12:56 ` [Qemu-devel] [PATCH 14/17] main-loop: use GSource to poll AIO file descriptors Paolo Bonzini
2012-09-25 22:09   ` Anthony Liguori
2012-09-26  6:38     ` Paolo Bonzini
2012-09-25 12:56 ` [Qemu-devel] [PATCH 15/17] main-loop: use aio_notify for qemu_notify_event Paolo Bonzini
2012-09-25 22:10   ` Anthony Liguori
2012-09-25 12:56 ` [Qemu-devel] [PATCH 16/17] aio: clean up now-unused functions Paolo Bonzini
2012-09-25 22:11   ` Anthony Liguori
2012-09-25 12:56 ` [Qemu-devel] [PATCH 17/17] linux-aio: use event notifiers Paolo Bonzini
2012-09-26 12:28 ` [Qemu-devel] [RFC PATCH 00/17] Support for multiple "AIO contexts" Kevin Wolf
2012-09-26 13:32   ` Paolo Bonzini
2012-09-26 14:31     ` Kevin Wolf
2012-09-26 15:48       ` Paolo Bonzini
2012-09-27  7:11         ` Kevin Wolf [this message]
2012-09-27  7:43           ` Paolo Bonzini
2012-10-08 11:39 ` Stefan Hajnoczi
2012-10-08 13:00   ` Paolo Bonzini
2012-10-09  9:08     ` [Qemu-devel] Block I/O outside the QEMU global mutex was "Re: [RFC PATCH 00/17] Support for multiple "AIO contexts"" Stefan Hajnoczi
2012-10-09  9:26       ` Avi Kivity
2012-10-09 10:36         ` Paolo Bonzini
2012-10-09 10:52           ` Avi Kivity
2012-10-09 11:08             ` Paolo Bonzini
2012-10-09 11:55               ` Avi Kivity
2012-10-09 12:01                 ` Paolo Bonzini
2012-10-09 12:18                   ` Jan Kiszka
2012-10-09 12:28                     ` Avi Kivity
2012-10-09 12:22                   ` Avi Kivity
2012-10-09 13:11                     ` Paolo Bonzini
2012-10-09 13:21                       ` Avi Kivity
2012-10-09 13:50                         ` Paolo Bonzini
2012-10-09 14:24                           ` Avi Kivity
2012-10-09 14:35                             ` Paolo Bonzini
2012-10-09 14:41                               ` Avi Kivity
2012-10-09 14:05                   ` Stefan Hajnoczi
2012-10-09 15:02       ` Anthony Liguori
2012-10-09 15:06         ` Paolo Bonzini
2012-10-09 15:37           ` Anthony Liguori
2012-10-09 16:26             ` Paolo Bonzini
2012-10-09 18:26               ` Anthony Liguori
2012-10-10  7:11                 ` Paolo Bonzini
2012-10-10 12:25                   ` Anthony Liguori
2012-10-10 13:31                     ` Paolo Bonzini
2012-10-10 14:44                       ` Anthony Liguori
2012-10-11 12:28         ` Kevin Wolf

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=5063FC3B.1020209@redhat.com \
    --to=kwolf@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=pbonzini@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).