Re: [Qemu-devel] [PATCH v2] throttle-groups: update tg->any_timer_armed[] on detach

[Manos Pitsidianakis <el13635@mail.ntua.gr>]

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

On Wed, Sep 20, 2017 at 01:08:52PM +0200, Alberto Garcia wrote:
>On Wed 20 Sep 2017 12:17:40 PM CEST, Stefan Hajnoczi wrote:
>> @@ -592,6 +592,17 @@ void throttle_group_attach_aio_context(ThrottleGroupMember *tgm,
>>  void throttle_group_detach_aio_context(ThrottleGroupMember *tgm)
>>  {
>>      ThrottleTimers *tt = &tgm->throttle_timers;
>> +    ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts);
>> +
>> +    qemu_mutex_lock(&tg->lock);
>> +    if (timer_pending(tt->timers[0])) {
>> +        tg->any_timer_armed[0] = false;
>> +    }
>> +    if (timer_pending(tt->timers[1])) {
>> +        tg->any_timer_armed[1] = false;
>> +    }
>> +    qemu_mutex_unlock(&tg->lock);
>> +
>>      throttle_timers_detach_aio_context(tt);
>>      tgm->aio_context = NULL;
>>  }
>
>I'm sorry that I didn't noticed this in my previous e-mail, but after
>this call you might have destroyed the timer that was set for that
>throttling group, so if there are pending requests waiting it can happen
>that no one wakes them up.
>
>I think that the queue needs to be restarted after this, probably after
>having reattached the context (or actually after detaching it already,
>but then what happens if you try to restart the queue while aio_context
>is NULL?).

aio_co_enter in the restart queue function requires that aio_context is 
non-NULL. Perhaps calling throttle_group_restart_tgm in 
throttle_group_attach_aio_context will suffice.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]