On 24/08/2017 19:37, Eric Blake wrote: > On 08/24/2017 11:21 AM, Paolo Bonzini wrote: >> On 24/08/2017 17:33, Stefan Hajnoczi wrote: >>> This patch enters read_reply_co directly in >>> nbd_client_attach_aio_context(). This is safe because new_context is >>> acquired by the caller. This ensures that read_reply_co reaches its >>> first yield point and its ctx is set up. >> >> I'm not very confident with this patch. aio_context_acquire/release is >> going to go away, and this then becomes possible >> >> main context new_context >> qemu_aio_coroutine_enter >> send request >> wait for reply >> read first reply >> wake coroutine >> >> where the "wake coroutine" part thinks it's running in new_context, and >> thus simply enters the coroutine instead of using the bottom half. >> >> But blk_co_preadv() should need the read_reply_co itself, in order to be >> woken up after reading the reply header. The core issue here is that >> nbd_co_receive_reply was never called, I suspect. And if it was never >> called, read_reply_co should not be woken up by nbd_coroutine_end. >> >> So the fix is: >> >> 1) assign NULL to s->recv_coroutine[i] when nbd_co_send_request fails >> >> 2) move this to nbd_co_receive_reply: >> >> s->recv_coroutine[i] = NULL; >> >> /* Kick the read_reply_co to get the next reply. */ >> if (s->read_reply_co) { >> aio_co_wake(s->read_reply_co); >> } >> >> Does this make sense? (Note that the read_reply_co idea actually came >> from you, or from my recollections of your proposed design :)). > > How much of this overlaps with Vladimir's proposal? > https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg00846.html
The above should be about 15 lines added, 10 removed. :) Paolo
signature.asc
Description: OpenPGP digital signature