[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] throttle-groups: update tg->any_timer_armed[
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH v2] throttle-groups: update tg->any_timer_armed[] on detach |
Date: |
Wed, 20 Sep 2017 14:17:51 +0200 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
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