From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Huth Subject: Re: [kvm-unit-tests PATCH 1/4] lib/alloc: improve robustness Date: Wed, 2 Nov 2016 07:08:51 +0100 Message-ID: <986a24ad-af96-5940-9268-4ce0560d8981@redhat.com> References: <1477939906-28802-1-git-send-email-drjones@redhat.com> <1477939906-28802-2-git-send-email-drjones@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="pEJE38cj9FwuEbt93oAP7Jrsk27bExQus" Cc: pbonzini@redhat.com, lvivier@redhat.com To: Andrew Jones , kvm@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:45574 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750818AbcKBGJA (ORCPT ); Wed, 2 Nov 2016 02:09:00 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8A1B54E4CF for ; Wed, 2 Nov 2016 06:08:59 +0000 (UTC) In-Reply-To: <1477939906-28802-2-git-send-email-drjones@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --pEJE38cj9FwuEbt93oAP7Jrsk27bExQus Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 31.10.2016 19:51, Andrew Jones wrote: > Add some asserts to make sure phys_alloc_init is called only once > and before any other alloc calls. >=20 > Signed-off-by: Andrew Jones > --- > lib/alloc.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) >=20 > diff --git a/lib/alloc.c b/lib/alloc.c > index e1d7b8a380ff..a83c8c5db850 100644 > --- a/lib/alloc.c > +++ b/lib/alloc.c > @@ -42,12 +42,13 @@ void phys_alloc_show(void) > =20 > void phys_alloc_init(phys_addr_t base_addr, phys_addr_t size) > { > - spin_lock(&lock); > + /* we should only be called once by one processor */ > + assert(!top); > + assert(size); If you remove the spinlock, isn't there a potential race between the assert(!top) and the "top =3D ..." below? E.g. if two CPUs enter this function at the same time, both could check and passs assert(!top) independently, and then do the "top =3D ..." in parallel? > base =3D base_addr; > top =3D base + size; > align_min =3D DEFAULT_MINIMUM_ALIGNMENT; > - nr_regions =3D 0; > - spin_unlock(&lock); > } > =20 > void phys_alloc_set_minimum_alignment(phys_addr_t align) > @@ -63,14 +64,14 @@ static phys_addr_t phys_alloc_aligned_safe(phys_add= r_t size, > { > static bool warned =3D false; > phys_addr_t addr, size_orig =3D size; > - u64 top_safe; > + u64 top_safe =3D top; > =20 > - spin_lock(&lock); > + if (safe && sizeof(long) =3D=3D 4) > + top_safe =3D MIN(top, 1ULL << 32); > =20 > - top_safe =3D top; > + assert(base < top_safe); > =20 > - if (safe && sizeof(long) =3D=3D 4) > - top_safe =3D MIN(top_safe, 1ULL << 32); > + spin_lock(&lock); > =20 > align =3D MAX(align, align_min); This hunk rather looks like a code re-arrangement to me which is not directly related to the patch description - maybe you should put this into a separate patch? Thomas --pEJE38cj9FwuEbt93oAP7Jrsk27bExQus Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJYGYL4AAoJEC7Z13T+cC21HesP/jaZdSG8noo2u0pXtEBA88cp TBe7n56gA+3TXeJyK2S1n7LL6XU1t8c+CnxcgTQ5mYC/ydQKab5QVVd1+UiqkS9g qFt+5/pNPuZZxLSaLYtDQjfyFiJE/AkQA717tAQ+itaTXRSNhTYuMdpuq9Qfivy/ i50UQH9rDqzOqyR2iGh4TfJs3nKIR20Q229wvNCmhayMemy/9HKnkgJ/hBmN3Cjn +REFsdKyu/WBZm0Akp0Is9bOyAjF1mVNNRqNx8rRakGDGYtvFg+wDKWJyU0wUcQO 15oRAOul4m/ur76MheGioaVMZHitUlrOg3eJufeAABgvYPBGXPgGoQ4DUzAzUZo+ NvTMWjLvbIX103Z2E//W5EpH9gZk5pIVdbcRNRg7hNU73/zBKe5utQ3JeXuVPfw9 1wrmE5emWDzW4S/29C/ME35rBZSEsO0g2IHvtb2yCQH+Bepy3oAbo9Qpq4c02Q/q z8REXwnFtBHKM+ZTwBoIEydpIsjh72cgHqFftFhSQzyUO/Q2/EHgjlz81vEYOiz7 ld5Q1y+MSQ5dtU4+6CEuLr/1MXSQLOcEW1ar0ySkU4SR+co/EWURj+bLsjBHz8u0 KwFPXAjs7YxqfzDnt+GDrDSVmmnga6qZDd9nWmbRcuivf6BV6apC7y859CsTUmq4 x4cEybnTdrqa+2Y9Ch8n =ek4n -----END PGP SIGNATURE----- --pEJE38cj9FwuEbt93oAP7Jrsk27bExQus--