From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51711) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YTByk-0000l9-Hv for qemu-devel@nongnu.org; Wed, 04 Mar 2015 11:17:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YTByg-00036c-IW for qemu-devel@nongnu.org; Wed, 04 Mar 2015 11:16:58 -0500 Received: from smtp3.mundo-r.com ([212.51.32.191]:59342 helo=smtp4.mundo-r.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YTByg-00036D-BP for qemu-devel@nongnu.org; Wed, 04 Mar 2015 11:16:54 -0500 Date: Wed, 4 Mar 2015 17:16:51 +0100 From: Alberto Garcia Message-ID: <20150304161651.GA2872@igalia.com> References: <142929b6b5adbf371309043d23864c22b0afdec4.1423842044.git.berto@igalia.com> <20150303210006.GG15846@stefanha-thinkpad.redhat.com> <20150304135341.GA11084@igalia.com> <20150304160427.GC15637@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20150304160427.GC15637@stefanha-thinkpad.redhat.com> Subject: Re: [Qemu-devel] [PATCH 7/9] throttle: Add throttle group support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Stefan Hajnoczi , qemu-devel@nongnu.org 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