qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 01/32] co-queue: drop extra coroutine_fn marks


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v4 01/32] co-queue: drop extra coroutine_fn marks
Date: Thu, 10 Jun 2021 20:37:31 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]