[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread |
Date: |
Thu, 04 Apr 2013 14:54:41 -0500 |
User-agent: |
Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
Paolo Bonzini <address@hidden> writes:
> Il 04/04/2013 18:59, Anthony Liguori ha scritto:
>>> >
>>> > The right thing to use would be g_source_add_child_source() and
>>> > g_source_remove_child_source(), but that is only present since glib 2.28
>>> > and we currently require 2.12 (2.20 on Windows).
>> I don't think a child source fixes the problem. The backend definitely
>> has work to do. What we don't know is whether the front end is capable
>> of processing the work.
>>
>> The problem here is that we use polling on the front-end. IOW:
>>
>> 1) Char backend has data and is ready to write.
>> 2) Asks front end if it can write
>> 3) Front end says no
>> 4) Goto (1)
>
> What we used to do is this, however:
>
> 1) Char backend asks front end if it can write
> 2) Front end says no
You're missing a step here. Previously we would drop the data silently.
> 3) poll() goes on without char backend's descriptor
Which is what enabled this.
> 4) Goto (1), then:
> 5) Front end is now available, calls qemu_notify_event()
This still exists FWIW.
> 6) Char backend asks front end if it can write
> 7) Front end says yes
>
> No busy waiting here. Though we don't really do (5) everywhere, because
> for example this patch slipped through the cracks:
Ack.
> http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg03208.html (but
> for the monitor and every consumer running in iothread context it's ok).
>
> You could achieve the same thing by making the io watch a child source
> of the QEMU wrapper. All the wrapper does is add/remove the io watch
> source from its children, depending on can_read.
I don't understand how this solves the problem. There aren't two
sources. There is only a single source (the GIOChannel source).
We override the function pointers to basically do monkey patching of the
GSource.
If I understand your suggestion, it's to add the source as a child of
itself.
>
> Perhaps it's possible without child sources though, by setting the
> callbacks on the glib source directly.
That's what we're doing...
src = g_io_create_watch(channel, G_IO_IN | G_IO_ERR | G_IO_HUP);
g_source_set_funcs(src, &io_watch_poll_funcs);
g_source_set_callback(src, (GSourceFunc)fd_read, user_data, NULL);
There is only one GSource active here.
Regards,
Anthony Liguori
> Paolo
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, (continued)
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, Paolo Bonzini, 2013/04/02
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, Peter Crosthwaite, 2013/04/02
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, Paolo Bonzini, 2013/04/03
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, Peter Crosthwaite, 2013/04/03
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, Paolo Bonzini, 2013/04/04
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, Anthony Liguori, 2013/04/04
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, Anthony Liguori, 2013/04/04
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, Peter Maydell, 2013/04/04
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, Anthony Liguori, 2013/04/04
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, Paolo Bonzini, 2013/04/04
- Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread,
Anthony Liguori <=
[Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread, Peter Crosthwaite, 2013/04/02