From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 1/4] xen: credit2: implement utilization cap Date: Wed, 28 Jun 2017 16:56:52 +0200 Message-ID: <1498661812.7288.8.camel@citrix.com> References: <149692186557.9605.11625777539060264052.stgit@Solace.fritz.box> <149692372627.9605.8252407697848997058.stgit@Solace.fritz.box> <2db5b8c2-eb6b-3926-806e-9bcf2e46b4a1@citrix.com> <1498234767.7405.46.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6960851906948763563==" Return-path: Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dQEPG-00072T-1a for xen-devel@lists.xenproject.org; Wed, 28 Jun 2017 14:57:26 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: George Dunlap Cc: Wei Liu , Andrew Cooper , Anshul Makkar , Ian Jackson , Jan Beulich , xen-devel List-Id: xen-devel@lists.xenproject.org --===============6960851906948763563== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-tUlEnAz7ldmG0pYLrGpN" --=-tUlEnAz7ldmG0pYLrGpN Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2017-06-28 at 15:28 +0100, George Dunlap wrote: > On Fri, Jun 23, 2017 at 5:19 PM, Dario Faggioli > wrote: > > > > +{ > > > > +=C2=A0=C2=A0=C2=A0=C2=A0struct csched2_dom *sdom =3D data; > > > > +=C2=A0=C2=A0=C2=A0=C2=A0unsigned long flags; > > > > +=C2=A0=C2=A0=C2=A0=C2=A0s_time_t now; > > > > +=C2=A0=C2=A0=C2=A0=C2=A0LIST_HEAD(parked); > > > > + > > > > +=C2=A0=C2=A0=C2=A0=C2=A0spin_lock_irqsave(&sdom->budget_lock, flag= s); > > > > + > > > > +=C2=A0=C2=A0=C2=A0=C2=A0/* > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* It is possible that the domain ove= rrun, and that the > > > > budget > > > > hence went > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* below 0 (reasons may be system ove= rbooking, issues in > > > > or > > > > too coarse > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* runtime accounting, etc.). In part= icular, if we overrun > > > > by > > > > more than > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* tot_budget, then budget+tot_budget= would still be < 0, > > > > which in turn > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* means that, despite replenishment,= there's still no > > > > budget > > > > for unarking > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* and running vCPUs. > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* It is also possible that we are ha= ndling the > > > > replenishment > > > > much later > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* than expected (reasons may again b= e overbooking, or > > > > issues > > > > with timers). > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* If we are more than CSCHED2_BDGT_R= EPL_PERIOD late, this > > > > means we have > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* basically skipped (at least) one r= eplenishment. > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* We deal with both the issues here,= by, basically, doing > > > > more than just > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* one replenishment. Note, however, = that every time we > > > > add > > > > tot_budget > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* to the budget, we also move next_r= epl away by > > > > CSCHED2_BDGT_REPL_PERIOD. > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* This guarantees we always respect = the cap. > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0*/ > > > > +=C2=A0=C2=A0=C2=A0=C2=A0now =3D NOW(); > > > > +=C2=A0=C2=A0=C2=A0=C2=A0do > > > > +=C2=A0=C2=A0=C2=A0=C2=A0{ > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0sdom->next_repl += =3D CSCHED2_BDGT_REPL_PERIOD; > > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0sdom->budget +=3D = sdom->tot_budget; > > > > +=C2=A0=C2=A0=C2=A0=C2=A0} > > > > +=C2=A0=C2=A0=C2=A0=C2=A0while ( sdom->next_repl <=3D now || sdom->= budget <=3D 0 ); > > > In presence of an accounting/enforcing error, it may happen that it > > executes for C between 0 and T, for 2C between T and 2T, for 0 > > between > > 2T and 3T, etc. So, after 3T, it will also have executed for 3C, as > > above. >=20 > Right, but is that what the loop actually does? >=20 It should be. Well, actually, it does not really do that, but it does something that I think is equivalent. I probably should have described it better... Sorry! > It looks like now if it executes for 2C between T and 2T, then when > the replenishment happens at 2T, the budget will be -C.=C2=A0=C2=A0So the= first > round of the loop will bring the budget to 0; since budget <=3D 0, then > it will add *another* C to the budget.=C2=A0=C2=A0So then it will be allo= wed to > execute for C again between 2T and 3T, giving it a total of 4C > executed over 3T, in violation of the cap. >=20 > Am I reading that wrong? >=20 So, the loop body indeed adds C, but it also moves the next replenishment time ahead by T. In the case you describe, at 2T, with budget -C, the first round of the loop will make the budget 0, and set the next replenishment to 3T. As you say, since budget is 0, and 0 is <=3D than 0, we stay in the loop for another round, which sets the budget to C, and the next replenishment to 4T. So, in summary, the domain have executed for 3C between 0 and 2T, and will be given only another C, until 4T, summing up to a total of 4C/4T, which should be fine. Or at least, that's what I had in mind... if the code does actually do something different, then it's a bug. ;-P > I thought you had intentionally decided to allow that to happen, to > avoid making the capped domain have to sit out for an entire budget > period. >=20 Ah, so I completely misunderstood your point, when replying. I thought *you* were arguing in favour of avoiding the capped domain have to sit out for an entire budget period! :-D Thanks and Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-tUlEnAz7ldmG0pYLrGpN 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 iQIcBAABCAAGBQJZU8O4AAoJEBZCeImluHPuAGMQAMA3bMzITd8JFctpeWF3s0PS JLK0lEL4CKJ31qS9zwRNGMkt63C0Eczn6zG6grQ1Xws9IODmb0TwrXEHu/nESnCm nHvRf3UsRG7KPFkw9wgZ2KCoTAQjQtL4qeWoKOIfKMbI6hxt7gOSMfd6fYxNfZXI R9XyjbNwBnKWQ4gjFzLf8FU+VEYSUrx5xlIFL62rTNnAMY/oPoKT76kga26dMiue +A+Kyu4kJwud6+1YoFu2T0GJSMs8Kpudcd75Aa8nIPRGVasrmJ6vmMe6PWMC7ofC fMt+kzOFp3HJujUcPEZB9/ohJhapgrkjyesrdzYT5ssm0YByBZx1uFnbgl/j4IRE UNmjf/PwSc1/zIWNSPiU8cyrrDoft3s7gatsPtzblIJtc/YJ1HLGZLO9hgs+1DeG Jzeop/kmiBF658Mcv9+ZBb9pDKNBUKGlGzhOTUwb7px/DLnbHcc7VxYxplHqTNLw t6pJ4ADgu1ho4y0RNJVG3zupMONnktmAtU41pWf/11WfBtf5JLS4U3n65Ckvfm7X SuobIcXzd4IcAvJLyCOK78Ce7+oED7Tlz2bON86Vdp3qivCZONDh692uDAPVv6xw JuhMXtOuJ6xfT65rx9RlzYeTBFhuiUT8CZQf4eotjS8NPgx8BB5T6gvlqB+ghaCW hONcIxhl7OCP/Rcu3+4R =6uoR -----END PGP SIGNATURE----- --=-tUlEnAz7ldmG0pYLrGpN-- --===============6960851906948763563== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============6960851906948763563==--