Re: [Qemu-devel] [RFC] block: Removed coroutine ownership assumption

[Peter Crosthwaite]

Hi Stefan.

On Thu, Jun 28, 2012 at 11:03 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Thu, Jun 28, 2012 at 4:17 AM, Peter Crosthwaite
> <address@hidden> wrote:
>>>> not at a valid yield point. But whats really wrong here is the block
>>>> layer will be making assumption on re-entry of the coroutine, so I
>>>> cant re-enter it witout wildly changing the behaviour of the block
>>>> layer. If you adopt this "mark it as coroutine" poilcy, you end up
>>>> with a system where half the functions in QEMU:
>>>>
>>>> A: may yield your coroutine
>>>> B: if it does yield it, your not allowed to restart it
>>>>
>>>> Making them completely uncallable from coroutine context.
>>>
>>> Please link to real code, I'm having trouble understanding the example
>>> code and your argument.
>>>
>>
>> Hi Stefan,
>>
>> Please see:
>>
>> git://developer.petalogix.com/public/qemu.git for-stefan/fdt-generic.next
>>
>> Its a generic machine model that creates an arbitrary machine from a
>> dtb. Yes, the work has many issues, some already discussed on the
>> mailing list, but it illustrates the issue with block. Top commit I
>> add some comments re this discussion. git diff HEAD^ will show you
>> where to look in the code.
>
> Thanks, that is very useful.
>
> Why are you using coroutines to build the machine?  I think the answer
> is because you are trying to do parallel init with a wait when a
> dependency is hit.

You got it. I could do multi passes of the tree to serialise but it is
messy. When I implement support for phandles as QOM links the number
of passes is no longer finite, so Id have to:

while (!everything_inited) {
    parse_entire_tree_again();
}

Which I dont want to do. Threads are a natural solution this this.
Coroutines are even better as I dont have to worry about synchronising
edits to the device tree (or other things for that matter) - which way
back was my original motivation for converting my thread based
solution to coroutines.

>
> The problem I see is:
>
> FDTMachineInfo *fdt_generic_create_machine()
> {
>    ...
>    while (qemu_co_queue_enter_next(fdti->cq));
> }
>
> The problem you have is not that the block layer is yielding.  The
> problem is that you need to run aio processing

And this is what Im trying to say is wrong. My usage of coroutine has
nothing to do with block, AIO or any other existing client of
coroutine. I shouldnt have to make API calls into things I dont care
about just to make my coroutines work. The QOM people should be able
to come along and rewrite the AIO subsystem tommorow and it shouldnt
affect my coroutines.

Maybe I need to generalise away from block. This problem goes a level
higher to AIO. AIO is assuming it owns all coroutines - I.E. if you
are in coroutines context (qemu_in_coroutine()) then that coroutine
can be pre-emented and queued up in AIO's work queue. Im saying this
is flawed - because it means coroutines are not generic, they are for
AIO use only.

So change subject to "Remove assumption that AIO owns coroutines".

(i.e. the main loop) in
> order for that bdrv_read() to make progress.  Please try:
>
> while (qemu_co_queue_enter_next(fdti->cq)) {
>    qemu_aio_wait();
> }

It still doesnt solve the yield point problem though. My coroutine
will yield at qdev_init() time which is not a valid yield point for my
coroutines.

Thanks for taking the time to look into this,
Regards,
Peter

>
> Stefan