qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Michael Roth <mdroth@linux.vnet.ibm.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Stefan Hajnoczi <stefanha@gmail.com>,
	Anthony Liguori <aliguori@us.ibm.com>,
	Ping Fan Liu <pingfank@linux.vnet.ibm.com>,
	qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [RFC] Convert AioContext to Gsource sub classes
Date: Mon, 12 Aug 2013 12:01:18 -0500	[thread overview]
Message-ID: <20130812170118.22636.27943@loki> (raw)
In-Reply-To: <788101843.267798.1376292628614.JavaMail.root@redhat.com>

Quoting Paolo Bonzini (2013-08-12 02:30:28)
> > 1) rename AioContext to AioSource.
> >   This is my major purpose, which declare it is not a "context" concept,
> > and GMainContext is the entity represent the thread's activity.
> 
> Note that the nested event loops in QEMU are _very_ different from
> glib nested event loops.  In QEMU, nested event loops only run block
> layer events.  In glib, they run all events.  That's why you need
> AioContext.

Would it be possible to use glib for our nested loops as well by just
setting a higher priority for the AioContext GSource?

Stefan and I were considering how we could make use of his "drop
ioflush" patches to use a common mechanism to register fd events, but
still allow us to distinguish between AioContext and non-AioContext
for nested loops. I was originally thinking of using prepare() functions
to filter out non-AioContext events, but that requires we implement
on GSource's with that in mind, and non make use of pre-baked ones
like GIOChannel's, and bakes block stuff into every event source
implementation.

Priorities didn't cross my mind though, but it seems pretty
straightfoward...

AioContext could then just be a container of sorts for managing
bottom-halves and AioContext FDs and binding them to the proper
GMainContext/MainLoop, but the underlying GSources could
still be driven by a normal glib-based mainloop, just with a specific
priority in the nested case.

> 
> > 2) Break AioSource into FdSource and BhSource.
> >   This make custom code less and simpler, one Gsource for one kind of
> > job. It is not necessary but IMHO it will make things clear when add
> > more things into main loop: add a new Gsource sub class, avoid to
> > always have relationship with AioContext.
> 
> But this is only complicating things work since users rely on both file-
> descriptor APIs and bottom half APIs.

Taking things a step further, maybe AioContext can stop being a
block-specific construct, but actually be the "QContext" we've
discussed in the past for managing multiple event loops. All
the block stuff would be hidden away in the GSource priority.

For instance,

#ifndef _WIN32

qemu_aio_set_fd_handler(fd, ...):
    aio_set_fd_handler(qemu_aio_context, fd, ..., QEMU_PRIORITY_BLOCK)

qemu_set_fd_handler(fd, ...):
    aio_set_fd_handler(qemu_aio_context, fd, ..., G_PRIORITY_DEFAULT)

#else

qemu_add_wait_object(fd, ...):
    add_wait_object(qemu_aio_context, fd, ...)

qemu_set_fd_handler(fd, ...):
    set_socket_handler(qemu_aio_context, fd, ..., G_PRIORITY_DEFAULT)

#endif

qemu_bh_schedule:
    bh_schedule(qemu_aio_context, ...)

etc...

I'll be sending patches this week for moving
add_wait_object/qemu_set_fd_handler to GSources, the non-global ones use
GMainContext * to specify a non-default thread/context, but can be easily
changed, or we can just do aioctx->g_main_context at the call sites.
There's some nice possibilities in using the former though: avoiding
O(n) lookups for stuff like finding the GSource for a particular
event/event type, for instance, by storing pointers to the GSource or
some kind of hashmap lookup. But probably better to discuss that aspect
with some context so I'll try to get those patches out soon.

> 
> > >>    More reasons:
> > >>    When I thinking how to bind library code to a thread context, it may
> > >> need to add Context's concept into API of block.c. If I use AioContext,
> > >> there will need a wrapper API to run the event loop. But If I got
> > >> glib's GmainContext, things become simple.
> 
> You already have it because AioContext is a GSource.  You do not need
> to expose the AioContext, except as a GSource.
> 
> Paolo

  reply	other threads:[~2013-08-12 17:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-10  3:24 [Qemu-devel] [RFC] Convert AioContext to Gsource sub classes Wenchao Xia
2013-08-10  8:03 ` Paolo Bonzini
2013-08-12  6:46   ` Wenchao Xia
2013-08-12  7:30     ` Paolo Bonzini
2013-08-12 17:01       ` Michael Roth [this message]
2013-08-13  8:44         ` Wenchao Xia
2013-08-15 15:23           ` Michael Roth
2013-08-15 16:32             ` Michael Roth
2013-08-16  7:15               ` Wenchao Xia
2013-08-16  8:12                 ` Wenchao Xia
2013-08-20  9:59                   ` Wenchao Xia
2013-08-20 17:54                     ` Alex Bligh
2013-08-21  8:45                     ` Stefan Hajnoczi
2013-08-21  9:33                       ` Wenchao Xia
2013-08-22 11:40                         ` Stefan Hajnoczi
2013-08-21 10:06                       ` Alex Bligh
2013-08-10 10:15 ` Alex Bligh

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=20130812170118.22636.27943@loki \
    --to=mdroth@linux.vnet.ibm.com \
    --cc=aliguori@us.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=pingfank@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=xiawenc@linux.vnet.ibm.com \
    /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).