Re: [Qemu-devel] [PATCH 4/6] throttle: Add throttle group support

[Alberto Garcia]

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