From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37702) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQR3P-0006sR-UI for qemu-devel@nongnu.org; Thu, 21 Jul 2016 23:23:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQR3K-0003sp-T1 for qemu-devel@nongnu.org; Thu, 21 Jul 2016 23:23:10 -0400 Date: Fri, 22 Jul 2016 11:32:48 +1000 From: David Gibson Message-ID: <20160722013248.GC15941@voom.fritz.box> References: <1469116479-233280-1-git-send-email-imammedo@redhat.com> <1469116479-233280-3-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ghzN8eJ9Qlbqn3iT" Content-Disposition: inline In-Reply-To: <1469116479-233280-3-git-send-email-imammedo@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/8] exec: don't use cpu_index to detect if cpu_exec_init()'s been called for cpu List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org, Paolo Bonzini , Peter Crosthwaite , Richard Henderson , Eduardo Habkost , "Michael S. Tsirkin" , Alexander Graf , Riku Voipio , Bharata B Rao , qemu-ppc@nongnu.org --ghzN8eJ9Qlbqn3iT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 21, 2016 at 05:54:33PM +0200, Igor Mammedov wrote: > Instead use QTAIL's tqe_prev field to detect if cpu's been > placed in list by cpu_exec_init() which is always set if > QTAIL element is in list. >=20 > Fixes SIGSEGV on failure path in case cpu_index is assigned > by board and cpu.relalize() fails before cpu_exec_init() is called. >=20 > In follow up patches, cpu_index will be assigned by boards that > support cpu hot(un)plug and need stable cpu_index that doesn't > depend on order cpus are created/removed. >=20 > Signed-off-by: Igor Mammedov > Reported-by: David Gibson Looks correct, although I wonder a bit about changing QTAILQ_REMOVE() for everyone for the sake of this one use case. > --- > include/qemu/queue.h | 2 ++ > exec.c | 4 ++-- > 2 files changed, 4 insertions(+), 2 deletions(-) >=20 > diff --git a/include/qemu/queue.h b/include/qemu/queue.h > index c2b6c81..2c2c74b 100644 > --- a/include/qemu/queue.h > +++ b/include/qemu/queue.h > @@ -407,6 +407,7 @@ struct { = \ > else \ > (head)->tqh_last =3D (elm)->field.tqe_prev; = \ > *(elm)->field.tqe_prev =3D (elm)->field.tqe_next; = \ > + (elm)->field.tqe_prev =3D NULL; = \ > } while (/*CONSTCOND*/0) > =20 > #define QTAILQ_FOREACH(var, head, field) \ > @@ -430,6 +431,7 @@ struct { = \ > #define QTAILQ_EMPTY(head) ((head)->tqh_first =3D=3D NULL) > #define QTAILQ_FIRST(head) ((head)->tqh_first) > #define QTAILQ_NEXT(elm, field) ((elm)->field.tqe_next) > +#define QTAILQ_IN_USE(elm, field) ((elm)->field.tqe_prev) > =20 > #define QTAILQ_LAST(head, headname) \ > (*(((struct headname *)((head)->tqh_last))->tqh_last)) > diff --git a/exec.c b/exec.c > index 2f57c62..8c5da32 100644 > --- a/exec.c > +++ b/exec.c > @@ -643,8 +643,8 @@ void cpu_exec_exit(CPUState *cpu) > CPUClass *cc =3D CPU_GET_CLASS(cpu); > =20 > cpu_list_lock(); > - if (cpu->cpu_index =3D=3D -1) { > - /* cpu_index was never allocated by this @cpu or was already fre= ed. */ > + if (!QTAILQ_IN_USE(cpu, node)) { > + /* there is nothing to undo since cpu_exec_init() hasn't been ca= lled */ > cpu_list_unlock(); > return; > } --=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 --ghzN8eJ9Qlbqn3iT Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXkXe/AAoJEGw4ysog2bOSNaAQAIAqIc13QVw1nvmPS5HcmgBy 0Oy7k0oBNFg4OzVMCt/k2JzEvnWi7i8mB6sLo5JEfudmWRy89eLYgAXW/ByjWh2l MrUJSApriPslHTyp4xliFtdggOrFq43LtECyZCc+TjSl5SXbduT1i54Su3QCL4Kx y6/N++aiYRry4wYSIU8LCLDFNbBuo0BcyTS9547Sx53+fOc7pqsTcZDQtoc1ZMT9 Is5mE6z6akCPYipwjXS0zmjrGzwcrjqNXMcZG2jbNf0BESycqBYaH0w6Gy957ywU C6ZdBE9i/QI2Y7U/MMSWjiSWwC2iVs5951Wx5mPqlvG3ACa9zxfMjIpjs1ocJ5Kr FrjmPcLL0WtFaHhhJI5/9i1MYsWwlxdKFzZ2Inu8dFWWGWvxYCTiKLSKHNRdCfql +7BPO5CR5Zlzd50dxl3XsiVBf+HGcZIGKhpfyETCHmPKBxVZBSfVV/zUyFcmKdUN Vfj7OOoFsgodT5eAy8zMnj6IJZ/qIvI3w3LQi4PTKIWEuWmb1rmu+40rFLi18Jqu dGnlCG37IyqIE6KGhdlnWLuPRGga/B16MFTdfdaLC2AKtmFSloNTaroVGHx0lCW8 6XkF9iOuiL2n4ECHtCJSqF1ExAZq6Wvw3IQFNZf7JrmavpFxlx0vafhGMKoRjEqB UWnRBhIboTLBGMFhIOOL =5lJL -----END PGP SIGNATURE----- --ghzN8eJ9Qlbqn3iT--