17.07.2021 00:25, Eric Blake wrote:
On Wed, Jul 14, 2021 at 07:59:14PM +0300, Vladimir Sementsov-Ogievskiy wrote:
Split out nbd_recv_coroutine_wake(), as it will be used in separate.
s/in separate/separately/
Also add a possibility to wake only first found sleeping coroutine.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
block/nbd.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
+
+static void nbd_recv_coroutines_wake_all(BDRVNBDState *s, bool only_one)
Without reading docs (including the parameter name), I would have guessed:
wake_all(s, true) - wakes all
wake_all(s, false) - wakes one
but your code does:
wake_all(s, true) - wakes one
wake_all(s, false) - wakes all
Maybe that means this function and/or its parameter is now misnamed.
Having the _all in the name with true to NOT be all is what threw me.
Would the following be any better:
nbd_recv_coroutines_wake(BDRVNBDState *s, bool all)
where
wake(s, true) - wakes all
wake(s, false) - wakes one
and where your helper function needs to be renamed, and callers
updated to match those semantics?
No objections, will do)
Actually in a previous commit message I promise to rename the function, but
actually don't do it.
{
int i;
for (i = 0; i < MAX_NBD_REQUESTS; i++) {
- NBDClientRequest *req = &s->requests[i];
-
- if (req->coroutine && req->receiving) {
- req->receiving = false;
- aio_co_wake(req->coroutine);
+ if (nbd_recv_coroutine_wake(&s->requests[i]) && only_one) {
+ return;
But while I'm not sold on the naming, the change in logic (to be able
to wake any one waiter but not the whole list) looks useful for future
patches.
--
Best regards,
Vladimir
