From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 0/3] VMX: Properly handle pi descriptor and per-cpu blocking list Date: Thu, 23 Jun 2016 17:11:43 +0200 Message-ID: <1466694703.18398.69.camel@citrix.com> References: <1463734431-22353-1-git-send-email-feng.wu@intel.com> <573F02B102000078000ED304@prv-mh.provo.novell.com> <5742D6BB02000078000EDA57@prv-mh.provo.novell.com> <5742E0BE02000078000EDABD@prv-mh.provo.novell.com> <1464007152.21930.55.camel@citrix.com> <1464098547.21930.107.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4812473171741523463==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: "Wu, Feng" , Jan Beulich Cc: "george.dunlap@eu.citrix.com" , "andrew.cooper3@citrix.com" , "Tian, Kevin" , "keir@xen.org" , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org --===============4812473171741523463== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-vMlb6peoPjB95bFRRdcB" --=-vMlb6peoPjB95bFRRdcB Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2016-06-23 at 12:33 +0000, Wu, Feng wrote: > > -----Original Message----- > > From: Dario Faggioli [mailto:dario.faggioli@citrix.com] > >=C2=A0 > > It goes through all the vcpus of all domains, and does not check or > > care whether they are running, runnable or blocked. > >=20 > > Let's look at this in some more details. So, let's assume that > > processor 5 is going away, and that you have the following vcpus > > around: > >=20 > > =C2=A0d0v0 : v->processor =3D 5, running on cpu 5 > > =C2=A0d0v1 : v->processor =3D 4, running on cpu 4 > > =C2=A0d1v0 : v->processor =3D 5, runnable but not running > > =C2=A0d2v3 : v->processor =3D 5, blocked > >=20 > > for d0v0, we do: > > =C2=A0 cpu_disable_scheduler(5) > > =C2=A0 =C2=A0 set_bit(_VPF_migrating, d0v0->pause_flags); > > =C2=A0 =C2=A0 vcpu_sleep_nosync(d0v0); > > =C2=A0 =C2=A0 =C2=A0 SCHED_OP(sleep, d0v0); > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 csched_vcpu_sleep(d0v0) > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 cpu_raise_softirq(5, SCHEDULE_SOFTIR= Q); > > =C2=A0 =C2=A0 vcpu_migrate(d0v0); > > =C2=A0 =C2=A0 =C2=A0 if ( v->is_running || ...) // assume v->is_running= is true > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 return > Hi Dario, after read this mail again, I get another question, > could you please help me out? >=20 > In the above code flow, we return in vcpu_migrate(d0v0) because > v->is_running =3D=3D 1, after vcpu_migrate() return, we check: >=20 > =C2=A0=C2=A0=C2=A0=C2=A0if ( v->processor =3D=3D cpu ) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0ret =3D -EAGAIN;=C2=A0 >=20 > In my understand in the above case, 'v->processor' is likely equal to > 'cpu', hence return -EAGAIN. However, in __cpu_disable(), there is > some check as below: >=20 > =C2=A0=C2=A0=C2=A0=C2=A0if ( cpu_disable_scheduler(cpu) ) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0BUG(); >=20 Right. But, as the comment inside cpu_disable_scheduler() itself says, we only return -EAGAIN in case we are calling cpu_disable_scheduler for removing a pCPU from a cpupool. In that case, we do not use __cpu_disable(), and hence we can safely return an error value. In that case, in fact, the caller of cpu_disable_scheduler() is cpupool_unassign_cpu_helprer(), which does what's necessary to deal with such error. > Might we hit the BUG() in the above case?=20 > No, because we call cpu_disable_scheduler() from __cpu_disable(), only when system state is SYS_STATE_suspend already, and hence we take the then branch of the 'if', which does never return an error. Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-vMlb6peoPjB95bFRRdcB 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 iQIcBAABCAAGBQJXa/wvAAoJEBZCeImluHPu2j0QAJRtoJub6WILj4tO4z1n9gSb 6AdCbFyZLdQjT8vxY4X715q2UQLk8eE83I911H5npmZDkw446M9CsX0m5iIKjFbK Eb8KYcsUZ6xhVaBWS2F20ssWfKCQuVszR5rzjcLtBe6lOD8H6/hiMQd3ytYD4IJK Nq+dc+TA0HLXEyZpFLLb1Day0KhO83c0BnLkdRdtk4C53WI5oCK/OszHffRRTDTy CnYpPtUUUC4ej9vjVGsnAo9fMgt7t3FEmUVd8aHvTvDnNY5eI4VEmfkXJdFBKHo4 v1vo3Q4sPqkMJOQzlc4U+y8sANoqFeokKzHstK9t0D+EtDctExBkG/9nRa585j1b JlS7DUz3VFs6xiYMEnUUaSMRY8GV5F1pcFUG4X8XmeQVe1pMczfZ6CD7BNqeLMsF rgo2uISr5LUmnm+9WQjJ8cH8LrTIjh0fLYB+NVoqVsX/KOzf/KIto9GaHdQf3ckt irKyjx1LDdGGvxNmV4EJFVCRE3Qn1XjWb2ni9v//XQsBwMbZZmMfy/HiWgShQ9/x 0uclHFNCOkvkPvksGz4wslPe9w5h7wda9jN/dsY+fp9pm29FuWi7oI5i0w3qOZJ1 L9Oat4kiHsSEUz44e6ffLTEJ+Zdc9Sqe1l4ALDvPc4TRH04rPgSiHkDkfKykGy3h O3z60OVJA/0QbfEiM6Zu =BmvD -----END PGP SIGNATURE----- --=-vMlb6peoPjB95bFRRdcB-- --===============4812473171741523463== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============4812473171741523463==--