qemu-trivial
[Top][All Lists]
Advanced

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

Re: [Qemu-trivial] [Qemu-devel] [PATCH] coroutine: Fix documentation of


From: Paolo Bonzini
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH] coroutine: Fix documentation of co_aio_sleep_ns()
Date: Wed, 8 Nov 2017 18:44:09 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

On 08/11/2017 18:36, Stefan Hajnoczi wrote:
> On Wed, Nov 08, 2017 at 09:57:47AM -0600, Eric Blake wrote:
>> On 11/08/2017 09:47 AM, Paolo Bonzini wrote:
>>> On 08/11/2017 16:42, Stefan Hajnoczi wrote:
>>>>> In any case, co_aio_sleep_ns()
>>>>> + * does not affect the #AioContext where the current coroutine is 
>>>>> running,
>>>>> + * as the coroutine will restart on the same #AioContext that it is
>>>>> + * running on.
>>>> I cannot parse the second sentence.  What does "affecting" an AioContext
>>>> mean?  Does "where the current coroutine is running" simply mean "the
>>>> caller"?
>>>>
>>>> What is it trying to say?  My guess is: the caller will be resumed in
>>>> the current AioContext, not the timer's AioContext.
>>>
>>> Yes, that is the intended meaning.  Perhaps just s/current//.
>>
>> How about:
>>
>> This function uses timers and hence needs to know the event loop
>> (#AioContext) to place the timer on.  After the time elapses, the
>> current coroutine will restart with the same #AioContext it is currently
>> running in, even if that is different than the timer context passed to
>> co_aio_sleep_ns().
> 
> These complicated semantics are a clue that the API should be
> simplified.  QEMU has changed since this function was first introduced.
> Now we can do the following:
> 
>   void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
>   {
>       AioContext *ctx = qemu_get_current_aio_context();
>       CoSleepCB sleep_cb = {
>           .co = qemu_coroutine_self(),
>       };
>       sleep_cb.ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, 
> &sleep_cb);
>       timer_mod(sleep_cb.ts, qemu_clock_get_ns(type) + ns);
>       qemu_coroutine_yield();
>       timer_del(sleep_cb.ts);
>       timer_free(sleep_cb.ts);
>   }
> 
> I don't see a reason for the caller to pass in an AioContext.

That should work, yes.

Paolo

> 
> Any objections?  Will send a patch if this is okay.
> 
> Stefan
> 


Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

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