From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH] xen: domain_update_node_affinity: Correct the ASSERT Date: Mon, 28 Jul 2014 19:23:10 +0200 Message-ID: <1406568190.4038.15.camel@Solace> References: <1406302204-13992-1-git-send-email-julien.grall@linaro.org> <53D296F002000078000261CD@mail.emea.novell.com> <53D27B49.70309@linaro.org> <53D29B15020000780002620F@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5927599261984157052==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XBody-0006f4-UF for xen-devel@lists.xenproject.org; Mon, 28 Jul 2014 17:23:27 +0000 In-Reply-To: <53D29B15020000780002620F@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Juergen Gross , Keir Fraser , ian.campbell@citrix.com, tim@xen.org, Julien Grall , Ian Jackson , George Dunlap , stefano.stabellini@citrix.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org --===============5927599261984157052== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-cYCurpCJajd2hda+YQ5k" --=-cYCurpCJajd2hda+YQ5k Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On ven, 2014-07-25 at 16:59 +0100, Jan Beulich wrote: > >>> On 25.07.14 at 17:44, wrote: > > Hi Jan, > >=20 > > On 07/25/2014 04:42 PM, Jan Beulich wrote: > >>>>> On 25.07.14 at 17:30, wrote: > >>> --- a/xen/common/domain.c > >>> +++ b/xen/common/domain.c > >>> @@ -449,7 +449,7 @@ void domain_update_node_affinity(struct domain *d= ) > >>> } > >>> /* Filter out non-online cpus */ > >>> cpumask_and(dom_cpumask, dom_cpumask, online); > >>> - ASSERT(!cpumask_empty(dom_cpumask)); > >>> + ASSERT( !d->vcpu || !d->vcpu[0] || !cpumask_empty(dom_cpumas= k)); > >>=20 > >> Wouldn't it be better for the function to bail early in that case? > >=20 > > I've no idea. I mostly followed the advice on this thread: > >=20 > > http://www.gossamer-threads.com/lists/xen/devel/340233=20 >=20 > Yeah, I recall that discussion. But looking at the function, nothing > useful will be done when the domain has no vCPU yet. Dario? >=20 For sure nothing useful happens if the for_each_vcpu() loop never executes, because of lack of vcpus. Functionally wise, bailing or going ahead, but avoiding the ASSERT to trigger (as this patch does) is exactly the same. I'm not sure which approach I personally prefer... If calling the function without any allocated vcpus is a "legitimate" and frequent enough use case, I'd say let's bail explicitly. If it's a corner case, then I think this patch is ok. It looks to me like we're in the latter situation (corner case), so I'm actually fine with this patch. That's why I'm acking it, but really, it's a matter of taste.. FWIW, I'd ack a version that bails too. Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-cYCurpCJajd2hda+YQ5k Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEABECAAYFAlPWhv4ACgkQk4XaBE3IOsTLrQCeLVjToBXBAZ2ZqieHvmyN8AK1 2LcAnj4IP3E0efPcqBhseCM2RfJBJLhk =7Kyg -----END PGP SIGNATURE----- --=-cYCurpCJajd2hda+YQ5k-- --===============5927599261984157052== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============5927599261984157052==--