[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 7/9] throttle: Add throttle group support
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 7/9] throttle: Add throttle group support |
Date: |
Wed, 4 Mar 2015 10:04:27 -0600 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Wed, Mar 04, 2015 at 02:53:42PM +0100, Alberto Garcia wrote:
> On Tue, Mar 03, 2015 at 03:00:06PM -0600, Stefan Hajnoczi wrote:
>
> > > + throttle_group_lock(bs->throttle_state);
> > > + bdrv_throttle_group_remove(bs);
> > > + throttle_group_unlock(bs->throttle_state);
> > > +
> > > + throttle_group_unref(bs->throttle_state);
> > > + bs->throttle_state = NULL;
> > > +
> > > throttle_timers_destroy(&bs->throttle_timers);
> > > }
> > >
> > > static void bdrv_throttle_read_timer_cb(void *opaque)
> > > {
> > > BlockDriverState *bs = opaque;
> > > - throttle_timer_fired(&bs->throttle_state, false);
> > > +
> > > + throttle_group_lock(bs->throttle_state);
> > > + throttle_timer_fired(bs->throttle_state, false);
> > > + throttle_group_unlock(bs->throttle_state);
> >
> > This pattern suggests throttle_timer_fired() should acquire the lock
> > internally instead.
>
> The idea is that the ThrottleState code itself doesn't know anything
> about locks or groups. As I understood it BenoƮt designed the
> ThrottleState code to be independent from the block layer and reusable
> for other things (that's why it's in util/).
Then ThrottleGroup could offer an API throttle_group_timer_fired() that
does the locking.
The advantage of encapsulating locking in ThrottleGroup is that callers
don't have to remember the take the lock. (But they must still be
careful about sequences of calls which will not be atomic.)
Stefan
pgpzovM8WxBlC.pgp
Description: PGP signature