Re: [Qemu-devel] [PATCH 3/5] Convert BlockDriver to explicit coroutine annotations

[Gabriel Kerneis]

On Wed, Aug 07, 2013 at 09:30:25PM +0200, Stefan Hajnoczi wrote:
> I have mixed feelings about removing coroutine_fn annotations from a
> function when it does not yield or call other coroutine_fn functions.
> 
> These functions were probably written as part of a coroutine code path.
> The coroutine_fn annotation tells me I'm in coroutine context.

No, it tells you it is forbidden to call this function from outside coroutine
context. Which is false: if the function never yields, it is definitely correct
to call from somewhere else (unless there is some other invariant in qemu about
coroutine context?).

> By removing this information those modifying the code now need to
> convert it back to coroutine_fn after auditing callers before they can
> use coroutine context.

I don't understand this. You mean someone who, later, would decide to make a
version of the function that yields? In that case, wouldn't it make sense to
introduce an alternative coroutine_fn counter-part? (I see, however, how line of
reasoning might have led to the "dynamic functions" maze).

> I'd compare this to a comment that says "lock foo is held across this
> function" but the function doesn't use anything that lock foo protects.
> Removing the comment isn't really helpful, you are throwing away
> information that can be useful when modifying the function.

Except that the function might also makes sense outside of coroutine context,
and you are forcing people to allocate a spurious coroutine if they want to
use it.

Note that I'm arguing for the sake of defining precisely what Qemu developpers
expect when they read or write "coroutine_fn". As long as we can agree on that
point, and get it documented in coroutine.h, I'm fine.  From a performance
point-of-view, it matters only for the CPC backend and there aren't that many
spuriously-annotated functions anyway.  We could even benchmark the overhead at
the end of the GSoC, but I don't expect it to be significant (if there were
dozens of them, it would be a different story).

Best,
-- 
Gabriel