On Tue, Mar 24, 2015 at 04:31:45PM +0000, Stefan Hajnoczi wrote:
> > + /* get next bs round in round robin style */
> > + token = throttle_group_next_bs(token);
> > + while (token != start &&
> > + qemu_co_queue_empty(&token->throttled_reqs[is_write])) {
>
> It's not safe to access bs->throttled_reqs[]. There are many of
> other places that access bs->throttled_reqs[] without holding
> tg->lock (e.g. block.c).
>
> throttled_reqs needs to be protected by tg->lock in order for this
> to work.
Good catch, but I don't think that's the solution. throttled_reqs
cannot be protected by that lock because it must release it before
calling qemu_co_queue_wait(&bs->throttled_reqs[is_write]). Otherwise
it will cause a deadlock.
My assumption was that throttled_reqs would be protected by the BDS's
AioContext lock, but I overlooked the fact that this part of the
algorithm needs to access other BDSs' queues so we indeed have a
problem.
I will give it a thought to see what I can come up with. Since we only
need to check whether other BDSs have pending requests maybe I can
implement this with a flag.
Thanks!
Berto
