On Wed, Mar 04, 2015 at 05:16:51PM +0100, Alberto Garcia wrote: > On Wed, Mar 04, 2015 at 10:04:27AM -0600, Stefan Hajnoczi wrote: > > > > > 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.) > > No other code in ThrottleGroup takes the lock directly, so making this > an exception could be confusing.
It shouldn't be an exception. I'm proposing that the ThrottleGroup API should handle locking internally instead of requiring callers to do it. Stefan
pgpD9CYefgzce.pgp
Description: PGP signature