From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH] sched: fix race between sched_move_domain() and vcpu_wake() Date: Fri, 11 Oct 2013 13:15:32 +0200 Message-ID: <1381490132.5006.33.camel@Abyss.lan> References: <1381426196-11392-1-git-send-email-david.vrabel@citrix.com> <5257D3D2.4020907@eu.citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4181003530853624945==" Return-path: In-Reply-To: <5257D3D2.4020907@eu.citrix.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: George Dunlap Cc: Andrew Cooper , Juergen Gross , David Vrabel , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org --===============4181003530853624945== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-SOnkCnqoWcprCd3idTTE" --=-SOnkCnqoWcprCd3idTTE Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On ven, 2013-10-11 at 11:32 +0100, George Dunlap wrote: > So going through the code and trying to reconstruct all the state in my= =20 > head... >=20 > If you look at vcpu_migrate(), it grabs both locks. But it looks like= =20 > the main purpose for that is so that we can call the migrate SCHED_OP(),= =20 > which for credit2 needs to do some mucking about with runqueues, and=20 > thus needs both locks. > Just to make sure I understood what's going on, the SCHED_OP you're referring is SCHED_OP(..,migrate,..) here, right? > In the case of move_domain, this is unnecessary,=20 > since it is removed from the old scheduler and then added to the new one. >=20 I think that too, and that's why I wouldn't take both locks: it'd actually be misleading rather than enlightening for people reading the code, at least that's how I see it. Perhaps, we can put a comment somewhere (as George is also saying). Regarding the patch, I personally like Jan's idea. > But I think this patch is still not quite right: both v->processor and= =20 > per_cpu(schedule_data, ...).schedule_lock may change under your feet; so= =20 > you always need to do the lock in a loop, checking to make sure that you= =20 > *still* have the right lock after you have actually grabbed it. >=20 Which, if I'm not mistaken, we sort of get for free it we go Jan's way, don't we? Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-SOnkCnqoWcprCd3idTTE 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 v1.4.15 (GNU/Linux) iEYEABECAAYFAlJX3dQACgkQk4XaBE3IOsQr0wCeOj3WRfBVn4+RpZqqM9jW6Q0T NuYAn2LIqS4h2JUJoc7GkAXlXRTmemaa =6lct -----END PGP SIGNATURE----- --=-SOnkCnqoWcprCd3idTTE-- --===============4181003530853624945== 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 --===============4181003530853624945==--