[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] nbd-client: avoid spurious qio_channel_yield()
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry |
Date: |
Wed, 23 Aug 2017 18:17:19 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 23/08/2017 16:51, Eric Blake wrote:
> On 08/22/2017 07:51 AM, Stefan Hajnoczi wrote:
>> The following scenario leads to an assertion failure in
>> qio_channel_yield():
>>
>> 1. Request coroutine calls qio_channel_yield() successfully when sending
>> would block on the socket. It is now yielded.
>> 2. nbd_read_reply_entry() calls nbd_recv_coroutines_enter_all() because
>> nbd_receive_reply() failed.
>> 3. Request coroutine is entered and returns from qio_channel_yield().
>> Note that the socket fd handler has not fired yet so
>> ioc->write_coroutine is still set.
>> 4. Request coroutine attempts to send the request body with nbd_rwv()
>> but the socket would still block. qio_channel_yield() is called
>> again and assert(!ioc->write_coroutine) is hit.
>>
>> The problem is that nbd_read_reply_entry() does not distinguish between
>> request coroutines that are waiting to receive a reply and those that
>> are not.
>>
>> This patch adds a per-request bool receiving flag so
>> nbd_read_reply_entry() can avoid spurious aio_wake() calls.
>>
>> Reported-by: Dr. David Alan Gilbert <address@hidden>
>> Signed-off-by: Stefan Hajnoczi <address@hidden>
>> ---
>> This should fix the issue that Dave is seeing but I'm concerned that
>> there are more problems in nbd-client.c. We don't have good
>> abstractions for writing coroutine socket I/O code. Something like Go's
>> channels would avoid manual low-level coroutine calls. There is
>> currently no way to cancel qio_channel_yield() so requests doing I/O may
>> remain in-flight indefinitely and nbd-client.c doesn't join them...
>
> Vladimir has some cleanups that rewrite the NBD coroutines to be more
> legible, but it is invasive enough to be 2.11 material. I think that
> for a stop-gap of getting 2.10 out the door, we may be better off
> including this patch - but I would still like some positive review from
> more than just me. There's not much time left before I need to send the
> -rc4 NBD pull request, though.
>
Reviewed-by: Paolo Bonzini <address@hidden>
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry, Stefan Hajnoczi, 2017/08/22
- Re: [Qemu-devel] [PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry, Paolo Bonzini, 2017/08/22
- Re: [Qemu-devel] [PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry, Dr. David Alan Gilbert, 2017/08/22
- Re: [Qemu-devel] [PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry, Eric Blake, 2017/08/23
- Re: [Qemu-devel] [PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry, Eric Blake, 2017/08/23
- Re: [Qemu-devel] [PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry, Vladimir Sementsov-Ogievskiy, 2017/08/23
- Re: [Qemu-devel] [PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry, Eric Blake, 2017/08/23