From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 05/10] xen: credit2: implement yield() Date: Fri, 30 Sep 2016 16:01:22 +0200 Message-ID: <1475244082.5315.123.camel@citrix.com> References: <147520253247.22544.10673844222866363947.stgit@Solace.fritz.box> <147520403328.22544.3265744862320473651.stgit@Solace.fritz.box> <19039c3f-8e75-5e07-bd92-2e98c9b8dc9f@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5452908073434894583==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bpyNa-00017A-34 for xen-devel@lists.xenproject.org; Fri, 30 Sep 2016 14:01:34 +0000 In-Reply-To: <19039c3f-8e75-5e07-bd92-2e98c9b8dc9f@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: George Dunlap , xen-devel@lists.xenproject.org Cc: George Dunlap , Andrew Cooper , Anshul Makkar , Jan Beulich List-Id: xen-devel@lists.xenproject.org --===============5452908073434894583== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-QJf1Mz/mkrSCKs/c9VIS" --=-QJf1Mz/mkrSCKs/c9VIS Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2016-09-30 at 13:52 +0100, George Dunlap wrote: > On 30/09/16 03:53, Dario Faggioli wrote: > >=20 > > When a vcpu explicitly yields it is usually giving > > us an advice of "let someone else run and come back > > to me in a bit." > >=20 > > Credit2 isn't, so far, doing anything when a vcpu > > yields, which means an yield is basically a NOP (well, > > actually, it's pure overhead, as it causes the scheduler > > kick in, but the result is --at least 99% of the time-- > > that the very same vcpu that yielded continues to run). > >=20 > > Implement a "preempt bias", to be applied to yielding > > vcpus. Basically when evaluating what vcpu to run next, > > if a vcpu that has just yielded is encountered, we give > > it a credit penalty, and check whether there is anyone > > else that would better take over the cpu (of course, > > if there isn't the yielding vcpu will continue). > >=20 > > The value of this bias can be configured with a boot > > time parameter, and the default is set to 1 ms. > >=20 > > Also, add an yield performance counter, and fix the > > style of a couple of comments. > >=20 > > Signed-off-by: Dario Faggioli >=20 > Hmm, I'm sorry for not asking this earlier -- but what's the > rationale > for having a "yield bias", rather than just choosing the next > runnable > guy in the queue on yield, regardless of what his credits are? >=20 Flexibility, I'd say. Flexibility of deciding 'how strong' an yield would be and, e.g., have two different values for two cpupools (not possible with just this patch, but not a big deal to do in future). Sure, if we only think at the spinlock case, that's not really important (if good at all). OTOH, if you think at yielding as a way of saying: <>. Well, this implementation gives you a way of quantifying that "while". But of course, more flexibility often means more complexity. And in this case, rather than complexity in the code, what would be hard is to come up with a good value for a specific workload. IOW, right now we have no yield. Instead of adding a "yield switch", it's implemented as a "yield knob", which has its up and down sides. I personally like knobs a lot more than switches... But I see the risk of people starting to turn the knob, expecting wonders, and being disappointed (and complaining!) if things don't improve for them! :-P > If snext has 9ms of credit and the top runnable guy on the runqueue > has > 7.8ms of credit, doesn't it make sense to run him instead of making > snext run again and burn his credit? >=20 Again, in the one use case for which yield is most popular (the spinlock one) this that you say totally makes sense. Which makes me think that, even if we were to keep (or go back to) using the bias, I'd probably go with a default value higher than 1ms worth of credits. *ANYWAY*, you asked for a rationale, this is mine. But all this being said, though, I honestly think the simple solution you're hinting at is better, at least for now. Also, I've just tried to implement it, and yes, it works by doing as you suggest here, and the code is simpler. Therefore, I'm going for that. :-) I've just seen you have applied most of the series already. I'll send a v3 consisting only of the remaining patches, with this one modified as suggested. 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) --=-QJf1Mz/mkrSCKs/c9VIS 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 iQIcBAABCAAGBQJX7nAzAAoJEBZCeImluHPudF0P/A389KZP22rM290j8a1x38x1 tkFwY/eTtQmiApbrNEa+Cx+dfqhstZwl0/k6+hiE/1lCDHzid/ZctCMhNOkqv+EH Lt4MBDsTMv6RTXW4SebFQKpD3b0K4lQ6v1+dB/rHIATVh8uckXHbbQeGU3L89sSQ cgdGLzmmziEIiUO+qX7AewJsfbYTOV4+N45THyJwkfovblCJsN92CGdfK3xaESP/ du8pidx89GMCAov6+t4ushPIDAy4XFx8PGGAxNRuliexSxBLPgnbDHbDjB1GL9vW UYoVsHLdDpivGCv5sMw/Et+Sob5fHhS80ERr9jJmbPBgcCkbbNFNz09WYKnCYNdh jxgN+72Xmy83zPXxdWIwcq1euBGpmr7cNoG6siE4/PW8PXdccCFTbLbycpuuUK+v ic959SIwBN+KOVbE9Zzho7vtys3ImfmRlDTfFvMDguUJEskaQjjYwg9dQq7pvfNK F+2zvlRVxLRnXUeVQRjLrDAQeONj0ju6JBiEvyyGw8I2B6DInvmD4dtj4LJjtul5 Etm8IRSw7nO3iNbaFEM1uRGcc4x+IEo92gjI+FjUaLwOIm2MsbOljOupJGdfsN9l up+BuPnoPJF3W7OpImwxcGF1dJniKgL7S2WbtbMJT8Y+OSkyAhKYwXLN3i9g6B1A 7OkF+pV0djgv8jqW7UDi =j6Bj -----END PGP SIGNATURE----- --=-QJf1Mz/mkrSCKs/c9VIS-- --===============5452908073434894583== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============5452908073434894583==--