[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake |
Date: |
Fri, 7 Jun 2019 11:18:15 +0000 |
07.06.2019 10:57, Kevin Wolf wrote:
> Am 11.04.2019 um 19:27 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> Introduce a function to gracefully wake-up a coroutine, sleeping in
>> qemu_co_sleep_ns() sleep.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>
> You can simply reenter the coroutine while it has yielded in
> qemu_co_sleep_ns(). This is supported.
No it doesn't. qemu_aio_coroutine_enter checks for scheduled field, and aborts
if it is set.
If I just use aio_co_enter instead of my new function, I get into
#1 0x00007f5d2514f8f8 in __GI_abort () at abort.c:90
#2 0x000055e9c8145278 in qemu_aio_coroutine_enter (ctx=0x55e9c9b12300,
co=0x55e9c9b23cb0)
at util/qemu-coroutine.c:132
#3 0x000055e9c8124f6d in aio_co_enter (ctx=0x55e9c9b12300, co=0x55e9c9b23cb0)
at util/async.c:494
#4 0x000055e9c8124eb1 in aio_co_wake (co=0x55e9c9b23cb0) at util/async.c:478
#5 0x000055e9c808d5d4 in nbd_teardown_connection (bs=0x55e9c9b1bc50) at
block/nbd-client.c:88
#6 0x000055e9c8090673 in nbd_client_close (bs=0x55e9c9b1bc50) at
block/nbd-client.c:1289
#7 0x000055e9c808ca3f in nbd_close (bs=0x55e9c9b1bc50) at block/nbd.c:486
#8 0x000055e9c8006cd6 in bdrv_close (bs=0x55e9c9b1bc50) at block.c:3841
(gdb) fr 2
#2 0x000055e9c8145278 in qemu_aio_coroutine_enter (ctx=0x55e9c9b12300,
co=0x55e9c9b23cb0)
at util/qemu-coroutine.c:132
132 abort();
(gdb) list
127 * been deleted */
128 if (scheduled) {
129 fprintf(stderr,
130 "%s: Co-routine was already scheduled in '%s'\n",
131 __func__, scheduled);
132 abort();
133 }
134
135 if (to->caller) {
136 fprintf(stderr, "Co-routine re-entered recursively\n");
(gdb) p scheduled
$1 = 0x55e9c818e990 "qemu_co_sleep_ns"
>
> I think what you add here is just the condition that you wake up the
> coroutine only if it's currently sleeping, but not when it has yielded
> for other reasons. This suggests that you're trying to reenter a
> coroutine of which you don't know where exactly in its code it currently
> is. This is wrong.
>
> Just knowing that it's sleeping doesn't tell you where the coroutine is.
> It could have called a function that sleeps internally and must not be
> woken up early. If you reenter a coroutine, you always must know the
> exact point where it yielded (or in exceptional cases, the exact points
> (plural)). Just reentering because it sleeps will wake it up in
> unexpected places, generally speaking.
>
> So I don't think this function is a good idea. It's too easy to misuse,
> and if you don't misuse it, you can directly call aio_co_wake().
>
> Kevin
>
--
Best regards,
Vladimir
- Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake, Eric Blake, 2019/06/06
- Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake, Kevin Wolf, 2019/06/07
- Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake,
Vladimir Sementsov-Ogievskiy <=
- Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake, Kevin Wolf, 2019/06/07
- Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake, Vladimir Sementsov-Ogievskiy, 2019/06/07
- Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake, Vladimir Sementsov-Ogievskiy, 2019/06/07
- Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake, Vladimir Sementsov-Ogievskiy, 2019/06/07
- Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake, Kevin Wolf, 2019/06/11
- Re: [Qemu-devel] [PATCH v6 5/7] qemu-coroutine-sleep: introduce qemu_co_sleep_wake, Vladimir Sementsov-Ogievskiy, 2019/06/11