[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2 of 2] Avoid vcpu migration of paused vcpus


  • To: Jan Beulich <JBeulich@xxxxxxxx>
  • From: Juergen Gross <juergen.gross@xxxxxxxxxxxxxx>
  • Date: Thu, 22 Mar 2012 11:38:32 +0100
  • Cc: xen-devel <xen-devel@xxxxxxxxxxxxx>
  • Delivery-date: Thu, 22 Mar 2012 10:38:49 +0000
  • Domainkey-signature: s=s1536a; d=ts.fujitsu.com; c=nofws; q=dns; h=X-SBRSScore:X-IronPort-AV:Received:X-IronPort-AV: Received:Received:Message-ID:Date:From:Organization: User-Agent:MIME-Version:To:CC:Subject:References: In-Reply-To:Content-Type:Content-Transfer-Encoding; b=gg8zsAVaUJkr1MmOsozV0E0k+pLSDL4gUMyVC2E0hfzbPQlmzwS5rWTO 6L51avMXp3RcswV3FoNOtoyr1T1EPtZD6wQUmqrH94FysWATvra34IBbP rpgatlRpcbx7kDWlHBgOjkjymUWLWnW245u7YBSG5zJba66D1Rtj9UX5D y+/J1JBsGNEBySMKxhSp8Ti/fkkWVNDhM27jMRTEqYV6uh2MHsP8JJRdM ZvVYIjOJBJx4JTLSKLKEUXxvG294a;
  • List-id: Xen developer discussion <xen-devel.lists.xen.org>

On 03/22/2012 11:26 AM, Jan Beulich wrote:
On 22.03.12 at 09:22, Juergen Gross<juergen.gross@xxxxxxxxxxxxxx>  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@xxxxxxxxxxxxxx
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.