From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v6][RFC]xen: sched: convert RTDS from time to event driven model Date: Fri, 4 Mar 2016 17:34:39 +0100 Message-ID: <1457109279.2959.553.camel@citrix.com> References: <1456430736-4606-1-git-send-email-tiche@seas.upenn.edu> <1456443078.2959.85.camel@citrix.com> <56CFDF85.504@seas.upenn.edu> <1456477891.2959.132.camel@citrix.com> <56D08B50.5090301@seas.upenn.edu> <1456510170.2959.210.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7641640250946652305==" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.84) (envelope-from ) id 1absgl-0001Fi-1n for xen-devel@lists.xenproject.org; Fri, 04 Mar 2016 16:34:51 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: "Chen, Tianyang" Cc: xen-devel@lists.xenproject.org, george.dunlap@citrix.com, Dagaen Golomb , Meng Xu List-Id: xen-devel@lists.xenproject.org --===============7641640250946652305== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-jrhCOHR/Z9z3VXRQLv02" --=-jrhCOHR/Z9z3VXRQLv02 Content-Type: multipart/mixed; boundary="=-eLoit6oicAGyBU0UvdYv" --=-eLoit6oicAGyBU0UvdYv Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2016-02-26 at 13:33 -0500, Chen, Tianyang wrote: > Attached. >=20 Hey, here I am... sorry it took a bit. I've had a look, and I've been able to come up with some code that I at least do not dislike too much! ;-P Have a look at the attached patch (you should apply it on top of the sched_rt.c file that you sent me in attachment). About what was happening... > On 2016-02-26 13:09, Dario Faggioli wrote: > > On Fri, 2016-02-26 at 12:28 -0500, Tianyang Chen wrote: > > >=C2=A0 > > > I added debug prints in all these functions and noticed that it > > > could > > > be=C2=A0 > > > caused by racing between spurious wakeups and context switching. > > >=20 It's not really spurious wakeup, it's regular wakeup happening immediately after the vcpu blocked, when we still haven't been able to execute rt_context_saved(). It's not really a race, and in fact we have the _RTDS_scheduled and _RTDS_delayed_runq_add flags that are meant to deal with exactly these situations. In fact (I added some more debug printk): > > > (XEN) vcpu1 wakes up nowhere > > > (XEN) cpu1 picked vcpu1=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0*** vcpu1 is on = a cpu > > > (XEN) cpu1 picked idle=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0*** vcpu1 i= s waiting to be context > > > switched > > > (XEN) vcpu2 wakes up nowhere > > > (XEN) cpu0 picked vcpu0 > > > (XEN) cpu2 picked vcpu2 > > > (XEN) cpu0 picked vcpu0 > > > (XEN) cpu0 picked vcpu0 > > > (XEN) d0 attempted to change d0v2's CR4 flags 00000620 -> > > > 00040660 > > > (XEN) cpu0 picked vcpu0 > > > (XEN) vcpu2 sleeps on cpu > > > (XEN) vcpu1 wakes up nowhere=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0*** v= cpu1 wakes up without > > > sleep? > > >=20 > > > (XEN) Assertion '!__vcpu_on_replq(svc)' failed at sched_rt.c:526 > > > (XEN) ----[ Xen-4.7-unstable=C2=A0=C2=A0x86_64=C2=A0=C2=A0debug=3Dy= =C2=A0=C2=A0Tainted:=C2=A0=C2=A0=C2=A0=C2=A0C ]--- > > > - > (XEN) [=C2=A0=C2=A0=C2=A056.113897] vcpu13 wakes up nowhere (XEN) [=C2=A0=C2=A0=C2=A056.113906] cpu4 picked vcpu13 (XEN) [=C2=A0=C2=A0=C2=A056.113962] vcpu13 blocks (XEN) [=C2=A0=C2=A0=C2=A056.113965] cpu4 picked idle (XEN) [=C2=A0=C2=A0=C2=A056.113969] vcpu13 unblocks (XEN) [=C2=A0=C2=A0=C2=A056.113972] vcpu13 wakes up nowhere (XEN) [=C2=A0=C2=A0=C2=A056.113980] vcpu13 woken while still in scheduling = tail (XEN) [=C2=A0=C2=A0=C2=A056.113985] vcpu13 context saved and added back to = queue (XEN) [=C2=A0=C2=A0=C2=A056.113992] cpu4 picked vcpu13 So, as you can see, at 56.113962 vcpu13 blocks (note, _blocks_ !=3D _sleeps_), and cpu4 goes idle. Ideally, rt_context_saved() would run and remove the replenishment event of vcpu13 from the replenishment queue. But, at 56.113969, vcpu13 wakes up already, before rt_context_saved() had a chance to run (which happens at 56.113985). It is not a spurious wakeup, as the vcpu was actually blocked and is being unblocked. Since rt_context_saved() hasn't run yet, the replenishment event is still in the queue, and hence any ASSERT asking for !__vcpu_on_replq(vcpu13) is doomed to making Xen explode! :-/ That does not happen in the execution trace above, because that was collected with my patch applied, where I either leave the replenishment event alone, if it still valid (i.e., no deadline miss happened during the blocked period) or, if a new one needs to be enqueued, I dequeue the old one first. The end result is not particularly pretty, but I am up for doing even worse, for the sake of keeping things like this: =C2=A0ASSERT( !__vcpu_on_replq(svc) ); at the beginning of replq_insert() (and its appropriate counterpart at the beginning of replq_remove()). In fact, I consider them really really helpful, when reasoning and trying to figure out how the code works... There is nothing that I hate more than an 'enqueue' function for which you don't know if it is ok to call it when the entity being enqueued is in the queue already (and what actually happens if it is). These ASSERTs, greatly help, from that point of view, clearly stating that, _no_, in this case it's absolutely not right to call the enqueue function if the event is in the queue already. :-) Hope I made myself clear. I gave some testing to the attached patch, and it seems to work to me, but I'd appreciate if you could do more of that. Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-eLoit6oicAGyBU0UvdYv Content-Disposition: attachment; filename="sched_rt_wakeup_schedtail.patch" Content-Type: text/x-patch; name="sched_rt_wakeup_schedtail.patch"; charset="UTF-8" Content-Transfer-Encoding: base64 LS0tIC9ob21lL2RhcmlvL0Rlc2t0b3Avc2NoZWRfcnQuYwkyMDE2LTAzLTA0IDE1OjQ3OjE0Ljkw MTc4NzEyMiArMDEwMAorKysgeGVuL2NvbW1vbi9zY2hlZF9ydC5jCTIwMTYtMDMtMDQgMTY6NTU6 MDYuNzQ2NjQxNDI0ICswMTAwCkBAIC00MzksNiArNDM5LDggQEAKICAgICBzdHJ1Y3QgbGlzdF9o ZWFkICpyZXBscSA9IHJ0X3JlcGxxKG9wcyk7CiAgICAgc3RydWN0IHRpbWVyKiByZXBsX3RpbWVy ID0gcHJ2LT5yZXBsX3RpbWVyOwogCisgICAgQVNTRVJUKCBfX3ZjcHVfb25fcmVwbHEoc3ZjKSAp OworCiAgICAgLyoKICAgICAgKiBEaXNhcm0gdGhlIHRpbWVyIGJlZm9yZSByZS1wcm9ncmFtbWlu ZyBpdC4KICAgICAgKiBJdCBkb2Vzbid0IG1hdHRlciBpZiB0aGUgdmNwdSB0byBiZSByZW1vdmVk CkBAIC01ODAsNyArNTgyLDcgQEAKIH0KIAogc3RhdGljIHZvaWQKLXJ0X2RlaW5pdChjb25zdCBz dHJ1Y3Qgc2NoZWR1bGVyICpvcHMpCitydF9kZWluaXQoc3RydWN0IHNjaGVkdWxlciAqb3BzKQog ewogICAgIHN0cnVjdCBydF9wcml2YXRlICpwcnYgPSBydF9wcml2KG9wcyk7CiAKQEAgLTExNTMs NiArMTE1NSw3IEBACiB7CiAgICAgc3RydWN0IHJ0X3ZjcHUgKiBjb25zdCBzdmMgPSBydF92Y3B1 KHZjKTsKICAgICBzX3RpbWVfdCBub3cgPSBOT1coKTsKKyAgICBib29sX3QgbWlzc2VkOwogCiAg ICAgQlVHX09OKCBpc19pZGxlX3ZjcHUodmMpICk7CiAKQEAgLTExODEsMjggKzExODQsNDIgQEAK IAogICAgIC8qCiAgICAgICogSWYgYSBkZWFkbGluZSBwYXNzZWQgd2hpbGUgc3ZjIHdhcyBhc2xl ZXAvYmxvY2tlZCwgd2UgbmVlZCBuZXcKLSAgICAgKiBzY2hlZHVsaW5nIHBhcmFtZXRlcnMgKCBh IG5ldyBkZWFkbGluZSBhbmQgZnVsbCBidWRnZXQpLCBhbmQKLSAgICAgKiBhbHNvIGEgbmV3IHJl cGxlbmlzaG1lbnQgZXZlbnQKKyAgICAgKiBzY2hlZHVsaW5nIHBhcmFtZXRlcnMgKGEgbmV3IGRl YWRsaW5lIGFuZCBmdWxsIGJ1ZGdldCkuCiAgICAgICovCi0gICAgaWYgKCBub3cgPj0gc3ZjLT5j dXJfZGVhZGxpbmUpCi0gICAgeyAgIAorICAgIGlmICggKG1pc3NlZCA9IG5vdyA+PSBzdmMtPmN1 cl9kZWFkbGluZSkgKQogICAgICAgICBydF91cGRhdGVfZGVhZGxpbmUobm93LCBzdmMpOwotICAg ICAgICBfX3JlcGxxX3JlbW92ZShvcHMsIHN2Yyk7Ci0gICAgfQotCi0gICAvLyBpZiggIV9fdmNw dV9vbl9yZXBscShzdmMpICkKLSAgICAgICAgX19yZXBscV9pbnNlcnQob3BzLCBzdmMpOwogCi0g ICAgLyogSWYgY29udGV4dCBoYXNuJ3QgYmVlbiBzYXZlZCBmb3IgdGhpcyB2Y3B1IHlldCwgd2Ug Y2FuJ3QgcHV0IGl0IG9uCi0gICAgICogdGhlIFJ1bnF1ZXVlL0RlcGxldGVkUS4gSW5zdGVhZCwg d2Ugc2V0IGEgZmxhZyBzbyB0aGF0IGl0IHdpbGwgYmUKLSAgICAgKiBwdXQgb24gdGhlIFJ1bnF1 ZXVlL0RlcGxldGVkUSBhZnRlciB0aGUgY29udGV4dCBoYXMgYmVlbiBzYXZlZC4KKyAgICAvKiAK KyAgICAgKiBJZiBjb250ZXh0IGhhc24ndCBiZWVuIHNhdmVkIGZvciB0aGlzIHZjcHUgeWV0LCB3 ZSBjYW4ndCBwdXQgaXQgb24KKyAgICAgKiB0aGUgcnVuLXF1ZXVlL2RlcGxldGVkLXF1ZXVlLiBJ bnN0ZWFkLCB3ZSBzZXQgdGhlIGFwcHJvcHJpYXRlIGZsYWcsCisgICAgICogaXQgdGhlIHZjcHUg d2lsbCBiZSBwdXQgYmFjayBvbiBxdWV1ZSBhZnRlciB0aGUgY29udGV4dCBoYXMgYmVlbiBzYXZl ZAorICAgICAqIChpbiBydF9jb250ZXh0X3NhdmUoKSkuCiAgICAgICovCiAgICAgaWYgKCB1bmxp a2VseShzdmMtPmZsYWdzICYgUlREU19zY2hlZHVsZWQpICkKICAgICB7CisgICAgICAgIHByaW50 aygidmNwdSVkIHdva2VuIHdoaWxlIHN0aWxsIGluIHNjaGVkdWxpbmcgdGFpbFxuIiwgdmMtPnZj cHVfaWQpOwogICAgICAgICBzZXRfYml0KF9fUlREU19kZWxheWVkX3J1bnFfYWRkLCAmc3ZjLT5m bGFncyk7CisgICAgICAgIC8qCisgICAgICAgICAqIFRoZSB2Y3B1IGlzIHdha2luZyB1cCBhbHJl YWR5LCBhbmQgd2UgZGlkbid0IGV2ZW4gaGFkIHRoZSB0aW1lIHRvCisgICAgICAgICAqIHJlbW92 ZSBpdHMgbmV4dCByZXBsZW5pc2htZW50IGV2ZW50IGZyb20gdGhlIHJlcGxlbmlzaG1lbnQgcXVl dWUKKyAgICAgICAgICogd2hlbiBoZSBibG9ja2VkISBObyBiaWcgZGVhbC4gSWYgd2UgZGlkIG5v dCBtaXNzIHRoZSBkZWFkbGluZSBpbgorICAgICAgICAgKiB0aGUgbWVhbnRpbWUsIGxldCdzIGp1 c3QgbGVhdmUgaXQgdGhlcmUuIElmIHdlIGRpZCwgbGV0J3MgcmVtb3ZlIGl0CisgICAgICAgICAq IGFuZCBxdWV1ZSBhIG5ldyBvbmUgKHRvIG9jY3VyIGF0IG91ciBuZXcgZGVhZGxpbmUpLgorICAg ICAgICAgKi8KKyAgICAgICAgaWYgKCBtaXNzZWQgKQorICAgICAgICB7CisgICAgICAgICAgIC8v IFhYWCBZb3UgbWF5IHdhbnQgdG8gaW1wbGVtZW50IHNvbWV0aGluZyBsaWtlIHJlcGxxX3JlaW5z ZXJ0KCksCisgICAgICAgICAgIC8vICAgICBmb3IgZGVhbGluZyB3aXRoIHRoaXMgY2FzZSwgYW5k IGNhbGxpbmcgdGhhdCBvbmUgZnJvbSBoZXJlCisgICAgICAgICAgIC8vICAgICAob2YgY291cnNl ISkuIEknZCBkbyB0aGF0IGJ5IG1lcmdpbmcgX3JlbW92ZSBhbmQgX2luc2VydAorICAgICAgICAg ICAvLyAgICAgYW5kIGtpbGxpbmcgdGhlIGR1cGxpY2F0ZWQgYml0cy4KKyAgICAgICAgICAgX19y ZXBscV9yZW1vdmUob3BzLCBzdmMpOyAKKyAgICAgICAgICAgX19yZXBscV9pbnNlcnQob3BzLCBz dmMpOyAKKyAgICAgICAgfQogICAgICAgICByZXR1cm47CiAgICAgfQogCisgICAgLyogUmVwbGVu aXNobWVudCBldmVudCBnb3QgY2FuY2VsbGVkIHdoZW4gd2UgYmxvY2tlZC4gQWRkIGl0IGJhY2su ICovCisgICAgX19yZXBscV9pbnNlcnQob3BzLCBzdmMpOyAKICAgICAvKiBpbnNlcnQgc3ZjIHRv IHJ1bnEvZGVwbGV0ZWRxIGJlY2F1c2Ugc3ZjIGlzIG5vdCBpbiBxdWV1ZSBub3cgKi8KICAgICBf X3J1bnFfaW5zZXJ0KG9wcywgc3ZjKTsKIAo= --=-eLoit6oicAGyBU0UvdYv-- --=-jrhCOHR/Z9z3VXRQLv02 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 iEYEABECAAYFAlbZuR8ACgkQk4XaBE3IOsQGcQCePT08Hq3mFq5EasP7pysOOBVr 0+MAn1Dlz8p/mZFJq4cHlQ+dgTdD7e1/ =zz8P -----END PGP SIGNATURE----- --=-jrhCOHR/Z9z3VXRQLv02-- --===============7641640250946652305== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============7641640250946652305==--