From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44232) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bNgDD-0001go-65 for qemu-devel@nongnu.org; Thu, 14 Jul 2016 08:57:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bNgD8-0008AO-13 for qemu-devel@nongnu.org; Thu, 14 Jul 2016 08:57:54 -0400 Received: from ozlabs.org ([103.22.144.67]:60919) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bNgD7-0008A5-FF for qemu-devel@nongnu.org; Thu, 14 Jul 2016 08:57:49 -0400 Date: Thu, 14 Jul 2016 22:02:36 +1000 From: David Gibson Message-ID: <20160714120236.GR14615@voom.fritz.box> References: <1468483025-1084-1-git-send-email-david@gibson.dropbear.id.au> <1468483025-1084-2-git-send-email-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="oN4OvwWIcd1E23D1" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [RFC 1/2] linux-user: Don't leak cpus on thread exit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Riku Voipio , Igor Mammedov , QEMU Developers --oN4OvwWIcd1E23D1 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 14, 2016 at 10:52:48AM +0100, Peter Maydell wrote: > On 14 July 2016 at 08:57, David Gibson wrot= e: > > Currently linux-user does not correctly clean up CPU instances properly > > when running a threaded binary. > > > > On thread exit, do_syscall() removes the thread's CPU from the cpus list > > and calls object_unref(). However, the CPU still is still referenced f= rom > > the QOM tree. To correctly clean up we need to object_unparent() to re= move > > the CPU from the QOM tree, then object_unref() to release the final > > reference we're holding. > > > > Once this is done, the explicit remove from the cpus list is no longer > > necessary, since that's done automatically in the CPU unrealize path. > > > > Signed-off-by: David Gibson > > --- > > linux-user/syscall.c | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > I believe most full system targets also "leak" cpus in the same way, > > except that since they don't support cpu hot unplug the cpus never > > would have been disposed anyway. I'll look into fixing that another > > time. > > > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > index 8bf6205..dd91791 100644 > > --- a/linux-user/syscall.c > > +++ b/linux-user/syscall.c > > @@ -6823,10 +6823,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_= long arg1, > > if (CPU_NEXT(first_cpu)) { > > TaskState *ts; > > > > - cpu_list_lock(); > > - /* Remove the CPU from the list. */ > > - QTAILQ_REMOVE(&cpus, cpu, node); > > - cpu_list_unlock(); > > + object_unparent(OBJECT(cpu)); /* Remove from QOM */ > > ts =3D cpu->opaque; > > if (ts->child_tidptr) { > > put_user_u32(0, ts->child_tidptr); > > @@ -6834,7 +6831,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_l= ong arg1, > > NULL, NULL, 0); > > } > > thread_cpu =3D NULL; > > - object_unref(OBJECT(cpu)); > > + object_unref(OBJECT(cpu)); /* Remove the last ref we're ho= lding */ >=20 > Is it OK to now be removing the CPU from the list after we've done > the futex to signal the child task rather than before? Ah.. not sure. I was thinking the object_unparent() would trigger an unrealize (which would do the list remove) even if there was a reference keeping the object in existence. I haven't confirmed that thought. It could obviously be fixed with an explicit unrealize before the futex op. >=20 > > g_free(ts); > > rcu_unregister_thread(); > > pthread_exit(NULL); >=20 > thanks > -- PMM >=20 --=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 --oN4OvwWIcd1E23D1 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXh39bAAoJEGw4ysog2bOSEcEQALbd8lcDH2h7d9/r+4eujfBx vstleyyBPW3SiUxnJAeXL+3L0w35cakEIh97vJJcBOEjhpFttQ2xn7OHTZy+8Ge0 pZXybHYG6FVR6whdmBhxwtVsoCQCUnAwqEXlLYlRhj6D2Y3vxwVQSoJiBpKSHO6l zJO5Nx/bEsEKX/XQHq1Dvsal8ocjjrGqF9LOweOeXhpKSWa9TiFt38FqUYKvbTlp CN947L2nP5rSxFfxMiLzYkrSyhEp2ygqqSJdp/a9NgIWVKU2PNqfFVSJVbptxHo+ 538ogE3lAdnusTZ7JFfa+7XmRDnSKrEt17FXcTkVdAgNBcI5J4RyMzgbEeDux7QL YKmf/8BJO17gnm1WVQfwFSVZH8vwBosCrVMiUT8YUX/WI3ZBNoO36KLLmrdxHe9c SCKuqnHYqkaUlNsTlrJKhvgIpar0ZWtNllhsDgNDcEBht16xX869E3e+SyoOnOQL WzU9XYaPs85heVjv7SmneY08TL/WjlS4rJAPsQok2L4/YvwMThOLBkMGJjutZ7rH JwWEJO3OE0xBlruX55CG+DtwfbCdzol2XZCVzd/sMzhF7QlEGbAL0lL/8EtBoHIZ 0h2zuXR8XCTKeaC4GhLFSebKnHEiO9zVYk3EoY9mJ1EZagnVKUPudc/logCVdOMu 1arwAXav6KXQwJ+cy1as =TlEb -----END PGP SIGNATURE----- --oN4OvwWIcd1E23D1--