From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44216) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bNgD9-0001dT-BB for qemu-devel@nongnu.org; Thu, 14 Jul 2016 08:57:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bNgD8-0008AU-2u for qemu-devel@nongnu.org; Thu, 14 Jul 2016 08:57:51 -0400 Received: from ozlabs.org ([2401:3900:2:1::2]:44968) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bNgD7-0008A4-FA for qemu-devel@nongnu.org; Thu, 14 Jul 2016 08:57:50 -0400 Date: Thu, 14 Jul 2016 21:59:45 +1000 From: David Gibson Message-ID: <20160714115945.GQ14615@voom.fritz.box> References: <1468483025-1084-1-git-send-email-david@gibson.dropbear.id.au> <1468483025-1084-3-git-send-email-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="NDuspjMMC1Ui5ypn" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [RFC 2/2] linux-user: Fix cpu_index generation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: Peter Maydell , Igor Mammedov , Riku Voipio , QEMU Developers --NDuspjMMC1Ui5ypn Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 14, 2016 at 03:50:56PM +0530, Bharata B Rao wrote: > On Thu, Jul 14, 2016 at 3:24 PM, Peter Maydell = wrote: > > On 14 July 2016 at 08:57, David Gibson wr= ote: > >> With CONFIG_USER_ONLY, generation of cpu_index values is done differen= tly > >> than for full system targets. This method turns out to be broken, sin= ce > >> it can fairly easily result in duplicate cpu_index values for > >> simultaneously active cpus (i.e. threads in the emulated process). > >> > >> Consider this sequence: > >> Create thread 1 > >> Create thread 2 > >> Exit thread 1 > >> Create thread 3 > >> > >> With the current logic thread 1 will get cpu_index 1, thread 2 will get > >> cpu_index 2 and thread 3 will also get cpu_index 2 (because there are 2 > >> threads in the cpus list at the point of its creation). > >> > >> We mostly get away with this because cpu_index values aren't that impo= rtant > >> for userspace emulation. Still, it can't be good, so this patch fixes= it > >> by making CONFIG_USER_ONLY use the same bitmap based allocation that f= ull > >> system targets already use. > >> > >> Signed-off-by: David Gibson > >> --- > >> exec.c | 19 ------------------- > >> 1 file changed, 19 deletions(-) > >> > >> diff --git a/exec.c b/exec.c > >> index 011babd..e410dab 100644 > >> --- a/exec.c > >> +++ b/exec.c > >> @@ -596,7 +596,6 @@ AddressSpace *cpu_get_address_space(CPUState *cpu,= int asidx) > >> } > >> #endif > >> > >> -#ifndef CONFIG_USER_ONLY > >> static DECLARE_BITMAP(cpu_index_map, MAX_CPUMASK_BITS); > >> > >> static int cpu_get_free_index(Error **errp) > >> @@ -617,24 +616,6 @@ static void cpu_release_index(CPUState *cpu) > >> { > >> bitmap_clear(cpu_index_map, cpu->cpu_index, 1); > >> } > >> -#else > >> - > >> -static int cpu_get_free_index(Error **errp) > >> -{ > >> - CPUState *some_cpu; > >> - int cpu_index =3D 0; > >> - > >> - CPU_FOREACH(some_cpu) { > >> - cpu_index++; > >> - } > >> - return cpu_index; > >> -} > >> - > >> -static void cpu_release_index(CPUState *cpu) > >> -{ > >> - return; > >> -} > >> -#endif > > > > Won't this change impose a maximum limit of 256 simultaneous > > threads? That seems a little low for comfort. >=20 > This was the reason why the bitmap logic wasn't applied to > CONFIG_USER_ONLY when it was introduced. >=20 > https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg01980.html Ah.. good point. Hrm, ok, my next idea would be to just (globally) sequentially allocate cpu_index values for CONFIG_USER, and never try to re-use them. Does that seem reasonable? > But then we didn't have actual removal, but we do now. You mean patch 1/2 in this set? Or something else? Even so, 256 does seem a bit low for a number of simultaneously active threads - there are some bug hairy multi-threaded programs out there. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --NDuspjMMC1Ui5ypn Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXh36xAAoJEGw4ysog2bOSHR0P/RFu3iKPtIm9SsfcFYkaaHub mbwUwOH+tLgrENT8ZmfAgCJhPfIEu9vqTb9I8ISqhwRcLFIjYoPksjNQcLfpNCMI EFlUjH/RpA5i/1a2a25pZUhu9uBvmtpTsYFRYJ8TYKNuawkuy8U0rtkH3MHi7Ijl LQsmwruIPVokU7kxPM0gI38HS1E56KG0BaYYR6gF0QbQoLWTGVo4H9sy/XrBQEN6 1A2vl+/3oUyU0etNA+gFnVHCURTbpFAv15XLWUXnqvo8Z7/WFfOFl/f00qdJ3g4Y c52b/md69qzVXFYkP+rsDbj8N3VvS3vqoanUsrcLUpa3RSKlGycSh84ErfYtTyCe 2O7pUslgLwTfnBEPzf1wy9rKszjAAMpg6iOitUptcMLrFoSz4ghYYRauGDud9CCb 6zE7yWoOtR97S9sfSTBKqyqdIObd8/FXBw4ZZdkwuO51xpjQOyiKF6ruK1ZKnUl8 mzfvDt2/TvKs7H0crxvq+W1CIpEvVe1Jo/qCvVKz6KWjIKjBQSbqp7qx2yhkHw5i rkaow9lZLuyGHz4WSg7hRMc/lt11lsM6YLV5CPXAs7usv5tgoLyKgDUw/VAGJyMq /tf7o/Av/cZG8EZ8C/I9jzEmFcFQpUFE72DinvPwxRPQ6+yDDBPdlubQG4cF8GyN +sQzT9Sx7meISx080Fii =PpJr -----END PGP SIGNATURE----- --NDuspjMMC1Ui5ypn--