From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH 2 of 2] Avoid vcpu migration of paused vcpus Date: Thu, 22 Mar 2012 11:38:32 +0100 Message-ID: <4F6B0128.1060405@ts.fujitsu.com> References: <932dc3987e3dac816d51.1332404573@nehalem1> <4F6B0C58020000780007A2A8@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4F6B0C58020000780007A2A8@nat28.tlf.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 List-Id: xen-devel@lists.xenproject.org On 03/22/2012 11:26 AM, Jan Beulich wrote: >>>> On 22.03.12 at 09:22, Juergen Gross wrote: >> --- a/xen/common/schedule.c Thu Mar 22 09:21:44 2012 +0100 >> +++ b/xen/common/schedule.c Thu Mar 22 09:21:47 2012 +0100 >> @@ -525,6 +525,47 @@ void vcpu_force_reschedule(struct vcpu * >> } >> } >> >> +static int vcpu_chk_affinity(struct vcpu *v, unsigned int cpu) >> +{ >> + cpumask_t online_affinity; >> + struct cpupool *c; >> + >> + c = v->domain->cpupool; >> + cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid); >> + if ( cpumask_empty(&online_affinity)&& > There's no need for a local cpumask_t variable here - please use > !cpumask_intersects(v->cpu_affinity, c->cpu_valid) instead. Okay. >> + cpumask_test_cpu(cpu, v->cpu_affinity) ) >> + { >> + printk("Breaking vcpu affinity for domain %d vcpu %d\n", >> + v->domain->domain_id, v->vcpu_id); >> + cpumask_setall(v->cpu_affinity); >> + } >> + >> + return ( !cpumask_test_cpu(cpu, c->cpu_valid)&& (v->processor == cpu) ); > Please drop the extra outermost parentheses. Okay. >> +} >> + >> +void vcpu_arouse(struct vcpu *v) >> +{ >> + unsigned long flags; >> + >> + if ( atomic_read(&v->pause_count) || >> + atomic_read(&v->domain->pause_count) ) > Is it not possible (or even more correct) to use vcpu_runnable() here? No. There might be flags set in v->pause_flags which I don't want to test here. I'm only interested in "real" paused state. An old "migrating" case should be reconsidered here, e.g. >> + return; >> + >> + vcpu_schedule_lock_irqsave(v, flags); >> + >> + if ( unlikely(vcpu_chk_affinity(v, v->processor)&& (v != current)) ) > unlikely() is generally useful only around individual conditions (i.e. > not around&& or || expressions). Ah, I didn't know that. Will change it. >> + { >> + set_bit(_VPF_migrating,&v->pause_flags); >> + vcpu_schedule_unlock_irqrestore(v, flags); >> + vcpu_migrate(v); >> + return; >> + } >> + >> + vcpu_schedule_unlock_irqrestore(v, flags); >> + >> + vcpu_wake(v); >> +} >> + >> /* >> * This function is used by cpu_hotplug code from stop_machine context >> * and from cpupools to switch schedulers on a cpu. >> @@ -534,7 +575,6 @@ int cpu_disable_scheduler(unsigned int c >> struct domain *d; >> struct vcpu *v; >> struct cpupool *c; >> - cpumask_t online_affinity; >> int ret = 0; >> >> c = per_cpu(cpupool, cpu); >> @@ -547,34 +587,33 @@ int cpu_disable_scheduler(unsigned int c >> { >> vcpu_schedule_lock_irq(v); >> >> - cpumask_and(&online_affinity, v->cpu_affinity, c->cpu_valid); >> - if ( cpumask_empty(&online_affinity)&& >> - cpumask_test_cpu(cpu, v->cpu_affinity) ) >> + if ( likely(!atomic_read(&v->pause_count)&& >> + !atomic_read(&d->pause_count)) ) > Same question as above regarding vcpu_runnable(). Same answer :-) >> { >> - printk("Breaking vcpu affinity for domain %d vcpu %d\n", >> - v->domain->domain_id, v->vcpu_id); >> - cpumask_setall(v->cpu_affinity); >> - } >> + if ( vcpu_chk_affinity(v, cpu) ) >> + { >> + set_bit(_VPF_migrating,&v->pause_flags); >> + vcpu_schedule_unlock_irq(v); >> + vcpu_sleep_nosync(v); >> + vcpu_migrate(v); >> + } >> + else >> + { >> + vcpu_schedule_unlock_irq(v); >> + } > Please drop the unnecessary braces here, as per the recently posted > coding style draft. Okay. Juergen -- Juergen Gross Principal Developer Operating Systems PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html