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. Truth to be told I'm not a very big fan of the timer callback having to deal with that in any case. The any_timer_armed flag should be something internal to the throttling code. I'll try to figure out a more elegant way to solve this. Berto
