From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52568) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaQqc-0007k7-Jq for qemu-devel@nongnu.org; Tue, 24 Mar 2015 11:34:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaQqZ-00086w-0S for qemu-devel@nongnu.org; Tue, 24 Mar 2015 11:34:30 -0400 Received: from smtp3.mundo-r.com ([212.51.32.191]:42322 helo=smtp4.mundo-r.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaQqY-0007v6-Or for qemu-devel@nongnu.org; Tue, 24 Mar 2015 11:34:26 -0400 Date: Tue, 24 Mar 2015 16:33:48 +0100 From: Alberto Garcia Message-ID: <20150324153348.GA20218@igalia.com> References: <3c960428503cfd363a55f72678a2f6a710d94a3e.1426000801.git.berto@igalia.com> <20150324150307.GD1493@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150324150307.GD1493@stefanha-thinkpad.redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/6] throttle: Add throttle group infrastructure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On Tue, Mar 24, 2015 at 03:03:07PM +0000, Stefan Hajnoczi wrote: > > +typedef struct ThrottleGroup { > > + char *name; /* This is constant during the lifetime of the group */ > > Is this also protected by throttle_groups_lock? > > I guess throttle_groups_lock must be held in order to read this > field - otherwise there is a risk that the object is freed unless > you have already incremented the refcount. The creation and destruction of ThrottleGroup objects are protected by throttle_groups_lock. That includes handling the memory for that field and its contents. Once the ThrottleGroup is created the 'name' field doesn't need any additional locking since it remains constant during the lifetime of the group. The only way to read it from the outside is throttle_group_get_name() and that's safe (until you release the reference to the group, that is). > > + QemuMutex lock; > > What does this lock protect? I guess 'ts', 'head', and 'tokens' > fields. Please document it. I thought it was obvious in the code but since you ask I guess it's not :) I can add an additional comment. Berto