qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PULL 16/71] nbd: Increase bs->in_flight d


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [Qemu-devel] [PULL 16/71] nbd: Increase bs->in_flight during AioContext switch
Date: Thu, 11 Apr 2019 17:20:26 +0000

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

> 
>>>>
>>>>>>
>>>>>> Signed-off-by: Kevin Wolf <address@hidden>
>>>>>> ---
>>>>>>     block/nbd-client.c | 20 ++++++++++++++++++--
>>>>>>     1 file changed, 18 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/block/nbd-client.c b/block/nbd-client.c
>>>>>> index 60f38f0320..bfbaf7ebe9 100644
>>>>>> --- a/block/nbd-client.c
>>>>>> +++ b/block/nbd-client.c
>>>>>> @@ -977,14 +977,30 @@ void 
>>>>>> nbd_client_detach_aio_context(BlockDriverState *bs)
>>>>>>         qio_channel_detach_aio_context(QIO_CHANNEL(client->ioc));
>>>>>>     }
>>>>>> +static void nbd_client_attach_aio_context_bh(void *opaque)
>>>>>> +{
>>>>>> +    BlockDriverState *bs = opaque;
>>>>>> +    NBDClientSession *client = nbd_get_client_session(bs);
>>>>>> +
>>>>>> +    /* The node is still drained, so we know the coroutine has yielded 
>>>>>> in
>>>>>> +     * nbd_read_eof(), the only place where bs->in_flight can reach 0, 
>>>>>> or it is
>>>>>> +     * entered for the first time. Both places are safe for entering the
>>>>>> +     * coroutine.*/
>>>>>> +    qemu_aio_coroutine_enter(bs->aio_context, client->connection_co);
>>>>>> +    bdrv_dec_in_flight(bs);
>>>>>> +}
>>>>>> +
>>>>>>     void nbd_client_attach_aio_context(BlockDriverState *bs,
>>>>>>                                        AioContext *new_context)
>>>>>>     {
>>>>>>         NBDClientSession *client = nbd_get_client_session(bs);
>>>>>>         qio_channel_attach_aio_context(QIO_CHANNEL(client->ioc), 
>>>>>> new_context);
>>>>>> -    /* FIXME Really need a bdrv_inc_in_flight() here */
>>>>>> -    aio_co_schedule(new_context, client->connection_co);
>>>>>> +    bdrv_inc_in_flight(bs);
>>>>>> +
>>>>>> +    /* Need to wait here for the BH to run because the BH must run 
>>>>>> while the
>>>>>> +     * node is still drained. */
>>>>>> +    aio_wait_bh_oneshot(new_context, nbd_client_attach_aio_context_bh, 
>>>>>> bs);
>>>>>>     }
>>>>>>     void nbd_client_close(BlockDriverState *bs)
>>>>>>
>>>>>
>>>>>
>>>>> -- 
>>>>> Best regards,
>>>>> Vladimir
>>>
>>>
>>> -- 
>>> Best regards,
>>> Vladimir
> 
> 


-- 
Best regards,
Vladimir

reply via email to

[Prev in Thread] Current Thread [Next in Thread]