On Wed 20 Sep 2017 01:39:02 PM CEST, Manos Pitsidianakis wrote:
>>>  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.

But can we guarantee that everything is safe between the _detach() and
_attach() calls?

Berto

Reply via email to