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