Re: [Qemu-devel] [PULL 16/71] nbd: Increase bs->in_flight during AioContext switch

[Kevin Wolf]

Am 11.04.2019 um 19:20 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 11.04.2019 20:13, Vladimir Sementsov-Ogievskiy wrote:
> > 11.04.2019 19:48, Kevin Wolf wrote:
> >> Am 11.04.2019 um 16:48 hat Vladimir Sementsov-Ogievskiy geschrieben:
> >>> 11.04.2019 17:15, Kevin Wolf wrote:
> >>>> Am 11.04.2019 um 15:40 hat Vladimir Sementsov-Ogievskiy geschrieben:
> >>>>> 25.02.2019 18:19, Kevin Wolf wrote:
> >>>>>> bdrv_drain() must not leave connection_co scheduled, so bs->in_flight
> >>>>>> needs to be increased while the coroutine is waiting to be scheduled
> >>>>>> in the new AioContext after nbd_client_attach_aio_context().
> >>>>>
> >>>>> Hi!
> >>>>>
> >>>>> I have some questions, could you explain, please?
> >>>>>
> >>>>> "bdrv_drain() must not leave connection_co scheduled" - it's because we 
> >>>>> want to be
> >>>>> sure that connection_co yielded from nbd_read_eof, yes?
> >>>>>
> >>>>> But it is guaranteed by aio_wait_bh_oneshot.. Why do we need 
> >>>>> additioinally inc/dec
> >>>>> bs->in_flight ?
> >>>>
> >>>> Without incrementing bs->in_flight, nothing would guarantee that
> >>>> aio_poll() is called and the BH is actually executed before bdrv_drain()
> >>>> returns.
> >>>
> >>> Don't follow.. Don't we want exactly this, we want BH to be executed 
> >>> while node is still
> >>> drained, as you write in comment?
> >>
> >> Yes, exactly. But if bs->in_flight == 0, the AIO_WAIT_WHILE() condition
> >> in the drain code could become false, so aio_poll() would not be called
> >> again and drain would return even if the BH is still pending.
> >>
> > 
> > Ah, oops, sorry my English, I read it like "nothing would prevent". 
> > Understand now, thanks.
> 
> Or not again.. We will not return to drain code, as we will loop in 
> aio_wait_bh_oneshot,
> which will not return until BH handled

Oh, hm, good point, it's not just aio_bh_schedule_oneshot(), but the
waiting version.

I think I added the aio_wait_bh_oneshot() only later to address a review
comment, so maybe increasing bs->in_flight isn't needed any more now.

But it can't hurt at least...

Kevin