From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2] sched: credit2: respect per-vcpu hard affinity Date: Tue, 10 Mar 2015 13:23:05 +0000 Message-ID: <1425993783.2729.109.camel@citrix.com> References: <1423453550-3526-1-git-send-email-jtweaver@hawaii.edu> <1425352508.10194.241.camel@citrix.com> <54F9C55F.5070608@eu.citrix.com> <1425661338.16932.52.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3870230522721166654==" Return-path: In-Reply-To: Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "jtweaver@hawaii.edu" Cc: "xen-devel@lists.xen.org" , George Dunlap , "JBeulich@suse.com" , "henric@hawaii.edu" List-Id: xen-devel@lists.xenproject.org --===============3870230522721166654== Content-Language: en-US Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-ttYzi+tEH59ogxX3jIuv" --=-ttYzi+tEH59ogxX3jIuv Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, 2015-03-08 at 21:11 -1000, Justin Weaver wrote: > Thanks to all for the comments! I've implemented most of the changes > recommended here in the v2 review. I should have a new patch set ready > this week (with an updated soft affinity patch as well).=20 > Great! :-) > > Oh, and this is what was causing you troubles, in case source and > > destination runqueue were the same... Help me understand, which call to > > sched_move_irqs() in schedule.c were we missing? I'd say it is the one > > in vcpu_migrate(), but that does not seem to care about vc->processor > > (much rater about new_cpu)... what am I missing? > > > > However, if they are not the same, the call to migrate() will override > > this right away, won't it? What I mean to say is, if this is required > > only in case trqd and svc->rqd are equal, can we tweak this part of > > csched2_vcpu_migrate() as follows? > > > > if ( trqd !=3D svc->rqd ) > > migrate(ops, svc, trqd, NOW()); > > else > > vc->processor =3D new_cpu; >=20 > You are right; it does not have anything to do with sched_move_irqs() > not being called (like you said it doesn't care about vc->processor). > Ah, ok. :-) > You are never going to believe any of my explanations now! :)=20 > EhEh... If I'd do that to people who fail to understand how things works at the first or second attempt, I would stop believing myself! :-D :-D > Without the processor assignment > here the vcpu might go on being assigned to a processor it no longer > is allowed to run on.=20 > Ok. > In that case, function runq_candidate may only > get called for the vcpu's old processor, and runq_candidate will no > longer let a vcpu run on a processor that it's not allowed to run on > (because of the hard affinity check first introduced in v1 of this > patch).=20 > It mostly makes sense. Out of the top of my head, it still looks like there should be a pCPU that, when scheduling, would pick it up... I need to think more about this... > So in that condition the vcpu never gets to run. That's still > somewhat of a vague explanation, but I have observed that that is what > happens.=20 > Do you mean you _actually_ saw this, with some debugging printk-s, or tracing, or something like this? > Anyway I think everyone agrees that the processor assignment > needs to be here, and I did move it to an else block for v3 like you > recommended above. > Yes, that's the point, the assignement above is correct, IMO, so it should be there, whether or not it is the cause of the issue :-) > > I don't like the "_safe_" part, but that is not a big deal, I certainly > > can live with it. >=20 > I changed it to _choose_valid_pcpu ... discuss!=20 > I'm fine with what Goerge proposes in his email. > (Also, I can send out > a pre-patch to change the double underscores in the whole file) >=20 For static symbols, yes. As Jan says, it's George's call. If you're up for it, I think it would be an improvement. > >> > VCPU2ONLINE(svc->vcpu) would make the line shorter. >=20 > I agree. VCPU2ONLINE is defined in schedule.c ... do you want me to > move it to a common header along with the other parts we discussed > (__vcpu_has_soft_affinity, etc.)?=20 > Either that or, if you only need it once, just open code it. > Okay to move them to sched-if.h, or > should I put them in a new header file? >=20 sched-if.h is ok for the step-wise load balancing macros, and it would be the proper place where to move this too, if we go for moving it. Thanks and Regards, Dario --=-ttYzi+tEH59ogxX3jIuv 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 iEYEABECAAYFAlT+8DcACgkQk4XaBE3IOsSjwgCeIyKD2fTiyfvo8wQNqFE03w0w OeAAn1310M3K/YQlDr0S1/4b0/mGkuTD =u0au -----END PGP SIGNATURE----- --=-ttYzi+tEH59ogxX3jIuv-- --===============3870230522721166654== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============3870230522721166654==--