From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, mreitz@redhat.com,
kwolf@redhat.com, pbonzini@redhat.com,
Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [PATCH v4 01/32] co-queue: drop extra coroutine_fn marks
Date: Thu, 10 Jun 2021 20:37:31 +0300 [thread overview]
Message-ID: <1610543c-24bc-ebe5-8c93-60f609a17e1e@virtuozzo.com> (raw)
In-Reply-To: <20210610172204.w2gtdeoj3v72m5rg@redhat.com>
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
next prev parent reply other threads:[~2021-06-10 17:38 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 10:07 [PATCH v4 00/32] block/nbd: rework client connection Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 01/32] co-queue: drop extra coroutine_fn marks Vladimir Sementsov-Ogievskiy
2021-06-10 17:22 ` Eric Blake
2021-06-10 17:37 ` Vladimir Sementsov-Ogievskiy [this message]
2021-06-10 10:07 ` [PATCH v4 02/32] block/nbd: fix channel object leak Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 03/32] block/nbd: fix how state is cleared on nbd_open() failure paths Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 04/32] block/nbd: connect_thread_func(): do qio_channel_set_delay(false) Vladimir Sementsov-Ogievskiy
2021-06-10 18:37 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 05/32] qemu-sockets: introduce socket_address_parse_named_fd() Vladimir Sementsov-Ogievskiy
2021-06-11 13:22 ` Eric Blake
2021-06-11 14:10 ` Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 06/32] block/nbd: call socket_address_parse_named_fd() in advance Vladimir Sementsov-Ogievskiy
2021-06-11 13:54 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 07/32] block/nbd: ensure ->connection_thread is always valid Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 08/32] block/nbd: nbd_client_handshake(): fix leak of s->ioc Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 09/32] block/nbd: BDRVNBDState: drop unused connect_err and connect_status Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 10/32] block/nbd: simplify waking of nbd_co_establish_connection() Vladimir Sementsov-Ogievskiy
2021-06-11 14:06 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 11/32] block/nbd: drop thr->state Vladimir Sementsov-Ogievskiy
2021-06-11 14:25 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 12/32] block/nbd: bs-independent interface for nbd_co_establish_connection() Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 13/32] block/nbd: make nbd_co_establish_connection_cancel() bs-independent Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 14/32] block/nbd: rename NBDConnectThread to NBDClientConnection Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 15/32] block/nbd: introduce nbd_client_connection_new() Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 16/32] block/nbd: introduce nbd_client_connection_release() Vladimir Sementsov-Ogievskiy
2021-06-11 14:28 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 17/32] nbd: move connection code from block/nbd to nbd/client-connection Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 18/32] nbd/client-connection: use QEMU_LOCK_GUARD Vladimir Sementsov-Ogievskiy
2021-06-11 14:31 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 19/32] nbd/client-connection: add possibility of negotiation Vladimir Sementsov-Ogievskiy
2021-06-11 15:07 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 20/32] nbd/client-connection: implement connection retry Vladimir Sementsov-Ogievskiy
2021-06-11 15:12 ` Eric Blake
2021-11-22 16:30 ` Eric Blake
2021-11-22 17:17 ` Vladimir Sementsov-Ogievskiy
2021-11-22 21:51 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 21/32] nbd/client-connection: shutdown connection on release Vladimir Sementsov-Ogievskiy
2021-06-11 15:27 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 22/32] block/nbd: split nbd_handle_updated_info out of nbd_client_handshake() Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 23/32] block/nbd: use negotiation of NBDClientConnection Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 24/32] block/nbd: don't touch s->sioc in nbd_teardown_connection() Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 25/32] block/nbd: drop BDRVNBDState::sioc Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 26/32] nbd/client-connection: return only one io channel Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 27/32] block-coroutine-wrapper: allow non bdrv_ prefix Vladimir Sementsov-Ogievskiy
2021-06-10 10:07 ` [PATCH v4 28/32] block/nbd: split nbd_co_do_establish_connection out of nbd_reconnect_attempt Vladimir Sementsov-Ogievskiy
2021-06-11 15:29 ` Eric Blake
2021-06-10 10:07 ` [PATCH v4 29/32] nbd/client-connection: add option for non-blocking connection attempt Vladimir Sementsov-Ogievskiy
2021-06-10 10:08 ` [PATCH v4 30/32] block/nbd: reuse nbd_co_do_establish_connection() in nbd_open() Vladimir Sementsov-Ogievskiy
2021-06-10 10:08 ` [PATCH v4 31/32] block/nbd: add nbd_client_connected() helper Vladimir Sementsov-Ogievskiy
2021-06-10 10:08 ` [PATCH v4 32/32] block/nbd: safer transition to receiving request Vladimir Sementsov-Ogievskiy
2021-06-11 15:55 ` [PATCH v4 00/32] block/nbd: rework client connection Eric Blake
2021-06-11 17:23 ` Vladimir Sementsov-Ogievskiy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1610543c-24bc-ebe5-8c93-60f609a17e1e@virtuozzo.com \
--to=vsementsov@virtuozzo.com \
--cc=eblake@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).