qemu_co_queue_next() and qemu_co_queue_restart_all() just call
aio_co_wake() which works well in non-coroutine context. So these
functions can be called from non-coroutine context as well. And
actually qemu_co_queue_restart_all() is called from
nbd_cancel_in_flight(), which is called from non-coroutine context.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/qemu/coroutine.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 292e61aef0..4829ff373d 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -210,13 +210,15 @@ void coroutine_fn qemu_co_queue_wait_impl(CoQueue *queue, QemuLockable *lock);
 /**
  * Removes the next coroutine from the CoQueue, and wake it up.
  * Returns true if a coroutine was removed, false if the queue is empty.
+ * OK to run from coroutine and non-coroutine context.
  */
-bool coroutine_fn qemu_co_queue_next(CoQueue *queue);
+bool qemu_co_queue_next(CoQueue *queue);
 
 /**
  * Empties the CoQueue; all coroutines are woken up.
+ * OK to run from coroutine and non-coroutine context.
  */
-void coroutine_fn qemu_co_queue_restart_all(CoQueue *queue);
+void qemu_co_queue_restart_all(CoQueue *queue);
 
 /**
  * Removes the next coroutine from the CoQueue, and wake it up.  Unlike
-- 
2.29.2

On Thu, Jun 10, 2021 at 01:07:31PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> qemu_co_queue_next() and qemu_co_queue_restart_all() just call
> aio_co_wake() which works well in non-coroutine context. So these
> functions can be called from non-coroutine context as well. And
> actually qemu_co_queue_restart_all() is called from
> nbd_cancel_in_flight(), which is called from non-coroutine context.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/qemu/coroutine.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

In the back of my mind, I have to repeatedly question my faulty memory
on whether:

absence of coroutine_fn means this function is unsafe to call from a
coroutine context (that is, coroutines can only call functions tagged
with coroutine_fn)

vs.

presence of coroutine_fn means this function must only use
coroutine-safe actions, but not all coroutine-safe functions have this
tag (in particular, functions which are designed to work both in or
out of coroutine context do not have the tag, but coroutines can call
functions without the tag)

But thankfully, rereading include/qemu/coroutine.h clears it up and
both of my initial thoughts are wrong although the second is closer;
it is yet another definition:

presence of coroutine_fn means this function must NOT be called except
from a coroutine context.  coroutines can call functions with or
without the tag, but if they lack the tag, the coroutine must ensure
the function won't block.

Thus, adding the tag is something we do when writing a function that
obeys coroutine rules and requires a coroutine context to already be
present, and once a function is then relaxed enough to work from both
regular and coroutine functions, we drop the marker.  But we keep the
_co_ in the function name to remind ourselves that it is
coroutine-safe, in addition to normal-safe.

And your patch is doing just that - we have functions that work from
both normal and coroutine context, so we can drop the marker but keep
_co_ in the name.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

10.06.2021 20:22, Eric Blake wrote:
> On Thu, Jun 10, 2021 at 01:07:31PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> qemu_co_queue_next() and qemu_co_queue_restart_all() just call
>> aio_co_wake() which works well in non-coroutine context. So these
>> functions can be called from non-coroutine context as well. And
>> actually qemu_co_queue_restart_all() is called from
>> nbd_cancel_in_flight(), which is called from non-coroutine context.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   include/qemu/coroutine.h | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> In the back of my mind, I have to repeatedly question my faulty memory
> on whether:
> 
> absence of coroutine_fn means this function is unsafe to call from a
> coroutine context (that is, coroutines can only call functions tagged
> with coroutine_fn)
> 
> vs.
> 
> presence of coroutine_fn means this function must only use
> coroutine-safe actions, but not all coroutine-safe functions have this
> tag (in particular, functions which are designed to work both in or
> out of coroutine context do not have the tag, but coroutines can call
> functions without the tag)
> 
> But thankfully, rereading include/qemu/coroutine.h clears it up and
> both of my initial thoughts are wrong although the second is closer;
> it is yet another definition:
> 
> presence of coroutine_fn means this function must NOT be called except
> from a coroutine context.  coroutines can call functions with or
> without the tag, but if they lack the tag, the coroutine must ensure
> the function won't block.
> 
> Thus, adding the tag is something we do when writing a function that
> obeys coroutine rules and requires a coroutine context to already be
> present, and once a function is then relaxed enough to work from both
> regular and coroutine functions, we drop the marker.  But we keep the
> _co_ in the function name to remind ourselves that it is
> coroutine-safe, in addition to normal-safe.
> 
> And your patch is doing just that - we have functions that work from
> both normal and coroutine context, so we can drop the marker but keep
> _co_ in the name.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Actually, usually _co_ == coroutine_fn. I don't drop it here because it's the name of the object: CoQueue, so qemu_co_queue_ refers to it..

We also have for example aio_co_wake, which is safe to call from non-coroutine context. And _co_ refers to what function do: wake a coroutine.

However it would be strange to have a function with _co_ in the name (in any meaning) which is unsafe to call from coroutine context.

-- 
Best regards,
Vladimir