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

[Peter Crosthwaite]

On Wed, Jun 27, 2012 at 6:33 PM, Markus Armbruster <address@hidden> wrote:
> Stefan Hajnoczi <address@hidden> writes:
>
>> On Wed, Jun 27, 2012 at 8:59 AM, Peter Maydell <address@hidden> wrote:
>>> On 27 June 2012 08:48, Stefan Hajnoczi <address@hidden> wrote:
>>>> I'd like to see your code though because I still don't understand why
>>>> it relies on the exact yield behavior.  Have you pushed it to a public
>>>> git repo?
>>>
>>> I haven't seen Peter's code either, but his complaint makes sense
>>> to me -- the whole point of coroutines is that you can rely on
>>> the exact yield behaviour, surely.
>>
>> Not if you call coroutine_fn functions - these are explicitly marked
>> as functions that yield.  For example block or net I/O.

Thats what I mean by "assuming ownership of coroutines". If block
marks half its API as coroutine_fn, then essentially you are saying
block is mutually exclusive with other users of coroutine. Is it
really that hard for the block layer to keep track of its own little
collection of coroutines and just check that it owns the current
context before taking the fast path?

>
> I think you two are in violent agreement :)
>
> With coroutines, you can rely on the exact yield behavior.  Of course,
> that doesn't do you any good unless you know which functions can yield.
> We make that easy by giving such functions distinctive names.

That brings a new problem, best illustrated by example. Suppose I
ensure myself against yielding something like this:

foo(void *opaque) {
   bdrv_foo(...); //this may yield!
   *((int*)opaque)++; //state++
   .... /* some coroutine behaviour */
}

<<from my device>>
int state = 0;
foo_co = qemu_couroutine_new(foo_fn);
qemu_coroutine_enter(foo_co, &state);
while (state < 1) {
    qemu_coroutined_enter(foo_co);
}
... /* the other half of some coroutine behvaiour */

I have insured myself against bdrv_yielding by renentering it if im
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.

>
> Except when we don't:
>
>> The issue here is that there are some block.h functions that are not
>> marked coroutine_fn but actually yield if running inside coroutine
>> context.  I think we could get rid of them with a little bit of work.
>
> Sounds like a good idea.