Re: [PATCH v2 2/9] block-copy: add missing coroutine_fn annotations

[Alberto Faria]

On Wed, Nov 9, 2022 at 12:24 PM Emanuele Giuseppe Esposito
<eesposit@redhat.com> wrote:
> CCing also Alberto and Paolo
>
> So basically I think what we need is something that scans the whole
> block layer code and puts the right coroutine_fn annotations (or
> assertions, if you want) in the right places.
>
> The rule should be (anyone please correct me if I am wrong):
> - if a function is only called by coroutine_fn functions, then it's a
> coroutine_fn. Theoretically all these functions should eventually end up
> calling qemu_coroutine_yield or similar.
>
> - if it's called only from non-coroutine, then leave it as it is.
> Probably asserting is a good idea.
>
> - if it's used by both, then it's a case-by-case decision: I think for
> simple functions, we can use a special annotation and document that they
> should always consider that they could run in both cases.
> If it's a big function like the g_c_w, call only the _co_ version in
> coroutine_fn, and create a coroutine or call the non-coroutine
> counterpart if we are not in coroutine context.
> Which is also what I am trying to do with g_c_w_simple.
>
> However, it would be nice to assign this to someone and do this
> automatically, not doing it by hand. I am not sure if Alberto static
> analyzer is currently capable of doing that.
>
> What do you all think?

>From what I've seen so far of coroutine_fn, its intended semantics
seem to align completely with the `async` of many languages. The only
restriction is that a function that calls coroutine_fn functions
(a.k.a. coroutines) must itself be coroutine_fn. Marking other
functions as coroutine_fn, as you mention in your first bullet above,
just artificially restricts them to coroutine context. Similarly,
restricting functions to non-coroutine context may not generally be
useful, except when there is an alternative version of the function
that is optimized for coroutine context in some way (e.g., calling a
coroutine_fn instead of the corresponding generated_co_wrapper).

But maybe you're writing a function that you predict will eventually
need to call a coroutine, even though it doesn't today. In those cases
it could make sense to mark it coroutine_fn, to prevent non-coroutine
callers from appearing and later breaking, but this should probably be
the exception, not the rule.

My WIP static analyzer [1] should be able to find most cases where a
non-coroutine_fn function calls a coroutine (some complicated cases
involving function pointers and typedefs are not yet implemented). It
also complains about cases where a coroutine calls a
generated_co_wrapper (see the no_coroutine_fn annotation, which you
can also apply to functions other than generated_co_wrappers). You can
use it today: just merge [1] with your code and run (after building
QEMU):

    ./static-analyzer.py build block

Alberto

[1] https://gitlab.com/albertofaria/qemu/-/tree/static-analysis