On Wed, Nov 9, 2022 at 1:24 PM Emanuele Giuseppe Esposito <eespo...@redhat.com> wrote: > > > What I do know is that it's extremely confusing to understand if a > > > function that is *not* marked as coroutine_fn is actually being called > > > also from coroutines or not.
Agreed. This is a huge point in favor of pushing coroutine wrappers as far up in the call stack as possible, because it means more coroutine_fns and fewer mixed functions. > > This is a lot better than our "coroutine_fn" sign, which actually do no > > check (and can't do). Don't you plan to swap a "coroutine_fn" noop > > marker with more meaningful IN_COROUTINE(); (or something like this, > > which just do assert(qemu_in_coroutine())) at start of the function? It > > would be a lot safer. > > 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. coroutine_fn markers are done by Alberto's static analyzer, which I used to add coroutine_fn pretty much everywhere in the code base where they are *needed*. My rules are simple: * there MUST be no calls from non-coroutine_fn to coroutine_fn, this is obvious * there MUST be no blocking in coroutine_fn * there SHOULD be no calls from coroutine_fn to generated_co_wrapper; use the wrapped *_co_* function directly instead. To catch the last one, or possibly the last two, Alberto added no_coroutine_fn. In a perfect world non-marked functions would be "valid either in coroutine or non-coroutine function": they would call neither coroutine_fns nor no_coroutine_fns. This is unfortunately easier said than done, but in order to move towards that case, I think we can look again at vrc and extend it with new commands. Alberto's work covers *local* tests, looking at one caller and one callee at a time. With vrc's knowledge of the global call graph, we can find *all* paths from a coroutine_fn to a generated_co_wrapper, including those that go through unmarked functions. Then there are two cases: * if the unmarked function is never called from outside a coroutine, call the wrapped function and change it to coroutine_fn * if the unmarked function can be called from outside a coroutine, change it to a coroutine_fn (renaming it) and add a generated_co_wrapper. Rinse and repeat. > 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. I think the first step has to be done by hand, because it entails creating new generated_co_wrappers. Checking for regressions can then be done automatically though. Paolo
