From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44077) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yak0A-0006sQ-5t for qemu-devel@nongnu.org; Wed, 25 Mar 2015 08:01:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yak05-00030E-F1 for qemu-devel@nongnu.org; Wed, 25 Mar 2015 08:01:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38158) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yak05-00030A-9s for qemu-devel@nongnu.org; Wed, 25 Mar 2015 08:01:33 -0400 Date: Wed, 25 Mar 2015 12:01:30 +0000 From: Stefan Hajnoczi Message-ID: <20150325120130.GA22940@stefanha-thinkpad.redhat.com> References: <3c960428503cfd363a55f72678a2f6a710d94a3e.1426000801.git.berto@igalia.com> <20150324150307.GD1493@stefanha-thinkpad.redhat.com> <20150324153348.GA20218@igalia.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ew6BAiZeqk4r7MaW" Content-Disposition: inline In-Reply-To: <20150324153348.GA20218@igalia.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: Alberto Garcia Cc: Kevin Wolf , Stefan Hajnoczi , qemu-devel@nongnu.org --ew6BAiZeqk4r7MaW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 24, 2015 at 04:33:48PM +0100, Alberto Garcia wrote: > On Tue, Mar 24, 2015 at 03:03:07PM +0000, Stefan Hajnoczi wrote: >=20 > > > +typedef struct ThrottleGroup { > > > + char *name; /* This is constant during the lifetime of the group= */ > >=20 > > Is this also protected by throttle_groups_lock? > >=20 > > 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. >=20 > The creation and destruction of ThrottleGroup objects are protected by > throttle_groups_lock. That includes handling the memory for that field > and its contents. >=20 > Once the ThrottleGroup is created the 'name' field doesn't need any > additional locking since it remains constant during the lifetime of > the group. >=20 > 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). Right, the race condition is when the group is released. Looking at this again, the assumption isn't that throttle_groups_lock is held. The AioContext lock is held by throttle_group_get_name() users and that's why there is no race when releasing the reference. If someone adds a throttle_group_get_name() caller in the future without holding AioContext, then we'd be in trouble. That is why documenting the locking constraints is useful. Stefan --ew6BAiZeqk4r7MaW Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVEqOaAAoJEJykq7OBq3PIy+EH/R9JQltj30zNf09FUxcnOwYS bCQ8xRGUh8Hdj7sLk4CtK+j0NDEaxupOJQbxVxuJNrQDWzy2tZU0N1hT7OJ1H7ea NGhbPOJ6cFtSfgHHVfeLnTa2q0pgeKZtUzjvvwHvjQfsx6J811VLqC+3FWb3STyW jZCx+y9y2/US2JDIXaZ8NWcadiGIiekMyuRvRvrECiq9uhIWbQKiiUy0Io5ohUUJ B/NBJW91TAGA5mIRqbZ1UEgwKK/mwLdTkcqHtbUNG5QExLJcDIqCgr4Z6I7a1FL1 GzNzQv4Wh9KPicFXhs6r5yTH9ZTdoxpjnFmVyqKWfKhIPbKr7rHFA/FsYUaeQE4= =fKSy -----END PGP SIGNATURE----- --ew6BAiZeqk4r7MaW--