From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joshua Whitehead Subject: Re: [PATCH 4/4] xen: sched: get rid of cpupool_scheduler_cpumask() Date: Fri, 26 Jun 2015 10:57:40 -0400 Message-ID: <558D6864.1090409@dornerworks.com> References: <20150625103457.3353.39292.stgit@Solace.station> <20150625121534.3353.10992.stgit@Solace.station> <558D5CC3.80209@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" 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 1Z8V6r-0000EE-Pf for xen-devel@lists.xenproject.org; Fri, 26 Jun 2015 15:00:06 +0000 In-Reply-To: <558D5CC3.80209@suse.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: Juergen Gross , Dario Faggioli , xen-devel@lists.xenproject.org Cc: George Dunlap , Josh Whitehead , Robert VanVossen , Sisu Xi , Meng Xu List-Id: xen-devel@lists.xenproject.org On 6/26/2015 10:08 AM, Juergen Gross wrote: > On 06/25/2015 02:15 PM, Dario Faggioli wrote: >> and of (almost every) direct use of cpupool_online_cpumask(). >> >> In fact, what we really want for the most of the times, >> is the set of valid pCPUs of the cpupool a certain domain >> is part of. Furthermore, in case it's called with a NULL >> pool as argument, cpupool_scheduler_cpumask() does more >> harm than good, by returning the bitmask of free pCPUs! >> >> This commit, therefore: >> * gets rid of cpupool_scheduler_cpumask(), in favour of >> cpupool_domain_cpumask(), which makes it more evident >> what we are after, and accommodates some sanity checking; >> * replaces some of the calls to cpupool_online_cpumask() >> with calls to the new functions too. >> >> Signed-off-by: Dario Faggioli > > Acked-by: Juergen Gross > Acked-by: Joshua Whitehead >> --- >> Cc: George Dunlap >> Cc: Juergen Gross >> Cc: Robert VanVossen >> Cc: Josh Whitehead >> Cc: Meng Xu >> Cc: Sisu Xi >> --- >> xen/common/domain.c | 5 +++-- >> xen/common/domctl.c | 4 ++-- >> xen/common/sched_arinc653.c | 2 +- >> xen/common/sched_credit.c | 6 +++--- >> xen/common/sched_rt.c | 12 ++++++------ >> xen/common/sched_sedf.c | 2 +- >> xen/common/schedule.c | 2 +- >> xen/include/xen/sched-if.h | 12 ++++++++++-- >> 8 files changed, 27 insertions(+), 18 deletions(-) >> >> diff --git a/xen/common/domain.c b/xen/common/domain.c >> index 3bc52e6..c20accb 100644 >> --- a/xen/common/domain.c >> +++ b/xen/common/domain.c >> @@ -184,7 +184,8 @@ struct vcpu *alloc_vcpu( >> /* Must be called after making new vcpu visible to >> for_each_vcpu(). */ >> vcpu_check_shutdown(v); >> >> - domain_update_node_affinity(d); >> + if ( !is_idle_domain(d) ) >> + domain_update_node_affinity(d); >> >> return v; >> } >> @@ -437,7 +438,7 @@ void domain_update_node_affinity(struct domain *d) >> return; >> } >> >> - online = cpupool_online_cpumask(d->cpupool); >> + online = cpupool_domain_cpumask(d); >> >> spin_lock(&d->node_affinity_lock); >> >> diff --git a/xen/common/domctl.c b/xen/common/domctl.c >> index 2a2d203..a399aa6 100644 >> --- a/xen/common/domctl.c >> +++ b/xen/common/domctl.c >> @@ -664,7 +664,7 @@ long >> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) >> goto maxvcpu_out; >> >> ret = -ENOMEM; >> - online = cpupool_online_cpumask(d->cpupool); >> + online = cpupool_domain_cpumask(d); >> if ( max > d->max_vcpus ) >> { >> struct vcpu **vcpus; >> @@ -748,7 +748,7 @@ long >> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) >> if ( op->cmd == XEN_DOMCTL_setvcpuaffinity ) >> { >> cpumask_var_t new_affinity, old_affinity; >> - cpumask_t *online = >> cpupool_online_cpumask(v->domain->cpupool);; >> + cpumask_t *online = cpupool_domain_cpumask(v->domain);; >> >> /* >> * We want to be able to restore hard affinity if we are >> trying >> diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c >> index cff5da9..dbe02ed 100644 >> --- a/xen/common/sched_arinc653.c >> +++ b/xen/common/sched_arinc653.c >> @@ -667,7 +667,7 @@ a653sched_pick_cpu(const struct scheduler *ops, >> struct vcpu *vc) >> * If present, prefer vc's current processor, else >> * just find the first valid vcpu . >> */ >> - online = cpupool_scheduler_cpumask(vc->domain->cpupool); >> + online = cpupool_domain_cpumask(vc->domain); >> >> cpu = cpumask_first(online); >> >> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c >> index a1945ac..8c36635 100644 >> --- a/xen/common/sched_credit.c >> +++ b/xen/common/sched_credit.c >> @@ -309,7 +309,7 @@ __runq_remove(struct csched_vcpu *svc) >> static inline int __vcpu_has_soft_affinity(const struct vcpu *vc, >> const cpumask_t *mask) >> { >> - return !cpumask_subset(cpupool_online_cpumask(vc->domain->cpupool), >> + return !cpumask_subset(cpupool_domain_cpumask(vc->domain), >> vc->cpu_soft_affinity) && >> !cpumask_subset(vc->cpu_hard_affinity, >> vc->cpu_soft_affinity) && >> cpumask_intersects(vc->cpu_soft_affinity, mask); >> @@ -374,7 +374,7 @@ __runq_tickle(unsigned int cpu, struct csched_vcpu >> *new) >> >> /* cpu is vc->processor, so it must be in a cpupool. */ >> ASSERT(per_cpu(cpupool, cpu) != NULL); >> - online = cpupool_online_cpumask(per_cpu(cpupool, cpu)); >> + online = cpupool_domain_cpumask(new->sdom->dom); >> cpumask_and(&idle_mask, prv->idlers, online); >> idlers_empty = cpumask_empty(&idle_mask); >> >> @@ -641,7 +641,7 @@ _csched_cpu_pick(const struct scheduler *ops, >> struct vcpu *vc, bool_t commit) >> int balance_step; >> >> /* Store in cpus the mask of online cpus on which the domain can >> run */ >> - online = cpupool_scheduler_cpumask(vc->domain->cpupool); >> + online = cpupool_domain_cpumask(vc->domain); >> cpumask_and(&cpus, vc->cpu_hard_affinity, online); >> >> for_each_csched_balance_step( balance_step ) >> diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c >> index 4372486..08611c8 100644 >> --- a/xen/common/sched_rt.c >> +++ b/xen/common/sched_rt.c >> @@ -256,7 +256,7 @@ rt_dump_vcpu(const struct scheduler *ops, const >> struct rt_vcpu *svc) >> */ >> mask = _cpumask_scratch[svc->vcpu->processor]; >> >> - cpupool_mask = >> cpupool_scheduler_cpumask(svc->vcpu->domain->cpupool); >> + cpupool_mask = cpupool_domain_cpumask(svc->vcpu->domain); >> cpumask_and(mask, cpupool_mask, svc->vcpu->cpu_hard_affinity); >> cpulist_scnprintf(keyhandler_scratch, >> sizeof(keyhandler_scratch), mask); >> printk("[%5d.%-2u] cpu %u, (%"PRI_stime", %"PRI_stime")," >> @@ -673,7 +673,7 @@ rt_cpu_pick(const struct scheduler *ops, struct >> vcpu *vc) >> cpumask_t *online; >> int cpu; >> >> - online = cpupool_scheduler_cpumask(vc->domain->cpupool); >> + online = cpupool_domain_cpumask(vc->domain); >> cpumask_and(&cpus, online, vc->cpu_hard_affinity); >> >> cpu = cpumask_test_cpu(vc->processor, &cpus) >> @@ -753,7 +753,7 @@ __runq_pick(const struct scheduler *ops, const >> cpumask_t *mask) >> iter_svc = __q_elem(iter); >> >> /* mask cpu_hard_affinity & cpupool & mask */ >> - online = >> cpupool_scheduler_cpumask(iter_svc->vcpu->domain->cpupool); >> + online = cpupool_domain_cpumask(iter_svc->vcpu->domain); >> cpumask_and(&cpu_common, online, >> iter_svc->vcpu->cpu_hard_affinity); >> cpumask_and(&cpu_common, mask, &cpu_common); >> if ( cpumask_empty(&cpu_common) ) >> @@ -965,7 +965,7 @@ runq_tickle(const struct scheduler *ops, struct >> rt_vcpu *new) >> if ( new == NULL || is_idle_vcpu(new->vcpu) ) >> return; >> >> - online = cpupool_scheduler_cpumask(new->vcpu->domain->cpupool); >> + online = cpupool_domain_cpumask(new->vcpu->domain); >> cpumask_and(¬_tickled, online, new->vcpu->cpu_hard_affinity); >> cpumask_andnot(¬_tickled, ¬_tickled, &prv->tickled); >> >> @@ -1078,7 +1078,7 @@ rt_vcpu_wake(const struct scheduler *ops, struct >> vcpu *vc) >> >> ASSERT(!list_empty(&prv->sdom)); >> sdom = list_entry(prv->sdom.next, struct rt_dom, sdom_elem); >> - online = cpupool_scheduler_cpumask(sdom->dom->cpupool); >> + online = cpupool_domain_cpumask(sdom->dom); >> snext = __runq_pick(ops, online); /* pick snext from ALL valid >> cpus */ >> >> runq_tickle(ops, snext); >> @@ -1113,7 +1113,7 @@ rt_context_saved(const struct scheduler *ops, >> struct vcpu *vc) >> >> ASSERT(!list_empty(&prv->sdom)); >> sdom = list_entry(prv->sdom.next, struct rt_dom, sdom_elem); >> - online = cpupool_scheduler_cpumask(sdom->dom->cpupool); >> + online = cpupool_domain_cpumask(sdom->dom); >> snext = __runq_pick(ops, online); /* pick snext from ALL >> cpus */ >> >> runq_tickle(ops, snext); >> diff --git a/xen/common/sched_sedf.c b/xen/common/sched_sedf.c >> index cad5b39..396b651 100644 >> --- a/xen/common/sched_sedf.c >> +++ b/xen/common/sched_sedf.c >> @@ -383,7 +383,7 @@ static int sedf_pick_cpu(const struct scheduler >> *ops, struct vcpu *v) >> cpumask_t online_affinity; >> cpumask_t *online; >> >> - online = cpupool_scheduler_cpumask(v->domain->cpupool); >> + online = cpupool_domain_cpumask(v->domain); >> cpumask_and(&online_affinity, v->cpu_hard_affinity, online); >> return cpumask_cycle(v->vcpu_id % >> cpumask_weight(&online_affinity) - 1, >> &online_affinity); >> diff --git a/xen/common/schedule.c b/xen/common/schedule.c >> index 4ffcd98..7ad298a 100644 >> --- a/xen/common/schedule.c >> +++ b/xen/common/schedule.c >> @@ -80,7 +80,7 @@ static struct scheduler __read_mostly ops; >> >> #define DOM2OP(_d) (((_d)->cpupool == NULL) ? &ops : >> ((_d)->cpupool->sched)) >> #define VCPU2OP(_v) (DOM2OP((_v)->domain)) >> -#define VCPU2ONLINE(_v) cpupool_online_cpumask((_v)->domain->cpupool) >> +#define VCPU2ONLINE(_v) cpupool_domain_cpumask((_v)->domain) >> >> static inline void trace_runstate_change(struct vcpu *v, int new_state) >> { >> diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h >> index 7cc25c6..20af791 100644 >> --- a/xen/include/xen/sched-if.h >> +++ b/xen/include/xen/sched-if.h >> @@ -183,9 +183,17 @@ struct cpupool >> atomic_t refcnt; >> }; >> >> -#define cpupool_scheduler_cpumask(_pool) \ >> - (((_pool) == NULL) ? &cpupool_free_cpus : (_pool)->cpu_valid) >> #define cpupool_online_cpumask(_pool) \ >> (((_pool) == NULL) ? &cpu_online_map : (_pool)->cpu_valid) >> >> +static inline cpumask_t* cpupool_domain_cpumask(struct domain *d) >> +{ >> + /* >> + * d->cpupool == NULL only for the idle domain, which should >> + * have no interest in calling into this. >> + */ >> + ASSERT(d->cpupool != NULL); >> + return cpupool_online_cpumask(d->cpupool); >> +} >> + >> #endif /* __XEN_SCHED_IF_H__ */ >> >> >