From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v2] credit: generalize __vcpu_has_soft_affinity() Date: Fri, 6 Mar 2015 11:32:05 +0000 Message-ID: <54F99035.8000804@eu.citrix.com> References: <54F9672B0200007800066D4A@mail.emea.novell.com> <54F9792A.7030605@eu.citrix.com> <54F98C920200007800066EC5@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YTqUY-00042u-Lg for xen-devel@lists.xenproject.org; Fri, 06 Mar 2015 11:32:30 +0000 In-Reply-To: <54F98C920200007800066EC5@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Dario Faggioli List-Id: xen-devel@lists.xenproject.org On 03/06/2015 10:16 AM, Jan Beulich wrote: >>>> On 06.03.15 at 10:53, wrote: >> On 03/06/2015 07:36 AM, Jan Beulich wrote: >>> As pointed out in the discussion of the patch at >>> http://lists.xenproject.org/archives/html/xen-devel/2015-02/msg03256.html >>> generalizing the conditions here means code elsewhere doesn't need to >>> take into consideration internals of how load balancing in the credit >>> scheduler works. >>> >>> Signed-off-by: Jan Beulich >>> --- >>> v2: Use VCPU2ONLINE(vc) (or really an open coded variant thereof) >>> instead of cpu_online_map (suggested by Dario). >>> >>> --- a/xen/common/sched_credit.c >>> +++ b/xen/common/sched_credit.c >>> @@ -292,11 +292,10 @@ __runq_remove(struct csched_vcpu *svc) >>> static inline int __vcpu_has_soft_affinity(const struct vcpu *vc, >>> const cpumask_t *mask) >>> { >>> - if ( cpumask_full(vc->cpu_soft_affinity) >>> - || !cpumask_intersects(vc->cpu_soft_affinity, mask) ) >>> - return 0; >>> - >>> - return 1; >>> + return !cpumask_subset(cpupool_online_cpumask(vc->domain->cpupool), >>> + vc->cpu_soft_affinity) && >>> + !cpumask_subset(vc->cpu_soft_affinity, vc->cpu_hard_affinity) && >>> + cpumask_intersects(vc->cpu_soft_affinity, mask); >> >> It looks like the comment above this line could use changing too; perhaps: >> >> --- >> Hard affinity balancing is always necessary and must never be skipped. >> But soft affinity need only be considered when it has a functionally >> different effect than other constraints (such as hard affinity, cpus >> online, or cpupools). >> >> Soft affinity only needs to be considered if: >> * The cpus in the cpupool are not a subset of soft affinity >> * The hard affinity is not a subset of soft affinity > > "hard" and "soft" appear to be swapped here. I corrected this, > please let me know if you disagree (in which case the patch would > need changing too). Uum -- I think my comment is right. If the soft affinity is a subset of hard affinity, then there are some cpus in the hard affinity which are "preferred" (soft affine) and some that are "not preferred" (non-soft-affine). Whereas, if hard affinity is a subset of soft affinity, then all cpus in the hard affinity are "preffered" (soft affine), and so there's no sense in doing the soft affinity step. In which case, yes, I think the patch needs to be adjusted. Dario, am I crazy? -George