From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH -v3 1/1] ratelimit: Implement rate limit for credit2 scheduler Rate limit assures that a vcpu will execute for a minimum amount of time before being put at the back of a queue or being preempted by higher priority thread. Date: Tue, 26 Jul 2016 18:21:36 +0200 Message-ID: <1469550096.32102.53.camel@citrix.com> References: <1469463147-4798-1-git-send-email-anshul.makkar@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0775684989702562702==" Return-path: In-Reply-To: <1469463147-4798-1-git-send-email-anshul.makkar@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Anshul Makkar , xen-devel@lists.xen.org Cc: george.dunlap@eu.citrix.com List-Id: xen-devel@lists.xenproject.org --===============0775684989702562702== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-Pwr6VrgA8ooH2zm+sBZl" --=-Pwr6VrgA8ooH2zm+sBZl Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2016-07-25 at 17:12 +0100, Anshul Makkar wrote: > It introduces context-switch rate-limiting. The patch enables the VM > to batch > its work and prevents the system from spending most of its time in > context > switches because of a VM that is waking/sleeping at high rate. >=20 > ratelimit can be disabled by setting it to 0. >=20 Thanks Anshul. I've looked at the patch, and it seemed all right to me. I decided to build and test it, and I've seem something that I found weird. But first of all, one thing that I'm sorry I'm mentioning now (especially considering it was quite evident! :-/). The subject, which will become the "title" of the commit, is way too long. That must be a very concise headline of what the patch is about, and should also have tags, specifying what areas of the codebase are affected. So what do you think of this: =C2=A0 xen: credit2: implement context switch rate-limiting. On a more technical side, I think that... > @@ -2116,9 +2147,22 @@ csched2_runtime(const struct scheduler *ops, > int cpu, struct csched2_vcpu *snext > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* 1) Run until snext's credit will be= 0 > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* 2) But if someone is waiting, run u= ntil snext's credit is > equal > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* to his > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* 3) But never run longer than MAX_TIMER o= r shorter than > MIN_TIMER. > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* 3) But never run longer than MAX_TIMER o= r shorter than > MIN_TIMER or > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* the ratelimit time. > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0*/ > =C2=A0 > +=C2=A0=C2=A0=C2=A0=C2=A0/* Calculate mintime */ > +=C2=A0=C2=A0=C2=A0=C2=A0min_time =3D CSCHED2_MIN_TIMER; > +=C2=A0=C2=A0=C2=A0=C2=A0if ( prv->ratelimit_us ) > +=C2=A0=C2=A0=C2=A0=C2=A0{ > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0s_time_t ratelimit_min = =3D prv->ratelimit_us; > ... this should be: =C2=A0s_time_t ratelimit_min =3D MICROSECS(prv->ratelimit_us); I realized that by looking at traces and seeing entries for which CSCHED2_MIN_TIMER was being returned, even if I had set sched_ratelimit_us to a value greater than that. I think the rest of the code is ok, and this is the only issue... but I'll make the change above here locally and continue my testing, and let you know if I find anything else. 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) --=-Pwr6VrgA8ooH2zm+sBZl 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 iQIcBAABCAAGBQJXl44RAAoJEBZCeImluHPuJuYP/2UXby/rARNHPw4t/OUnhw5n Hmgzgq1gFQVG62Rk0NSMkEU8VGkJRhUoefVcIS2lRMBRlmSv47RCEHlE9nanmuVA lWsQHfZ9qLE98cYo6NlG7ePn8+Ks2lQfSyyrMPJBOxE4IRd3TZJookW6jwzOXCee kWowghGO0ChRFPwIkvunulA2AN4YWPZpVnfXKHxQCMN/u3TOFkAxwg4VszZc3YTh T9FCNm1BSh2YRAuml4LBI78nilWoUDNa8tq4mDMN7/EA6+ujLQ9Dh2RI0iOVk46I 2a0FgLQAOGPXbjZNDfZ8B/ttzEjxMGtdjhSUTe3v4gPbCXylZ0VJjDRMV7P3j9v/ x/d7X8xu9fbz41e5pclRyyJmRxaYRL/tXo4mFzO9YTbGZ8aHtHjjB2gERJ7GUguu cWSQcRyZea9afLE9ejSAJcHgRuCOEBssTwYr1WWlYlBPlK9VCfiD/GiB67q/Tbli iDArlOnNj6Tou+fS5atox7HyvBY/kbjCko89dQaPopSiVSHRT+vBUAbKXtrcGRwL ftRvlSzGAW/UNHgIKt9j1U8AZ0HTuIRNA2VtZD2v6I3E1C4JdyuzoXJXUUUnRYb6 I3Nm3e9PTbez86JpuH5YWiw2ia5tXgojA6XuQ8LkHCHiahQPoAZ3f+Mw9Muz0ItC hTnqH/obebYhEfGOtlL7 =xt6L -----END PGP SIGNATURE----- --=-Pwr6VrgA8ooH2zm+sBZl-- --===============0775684989702562702== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============0775684989702562702==--