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

Re: [Xen-devel] [PATCH 3/4] xen: sched: reorganize cpu_disable_scheduler()



On 07/09/2015 12:24 PM, Dario Faggioli wrote:
Hey,

1 thing...

On Wed, 2015-07-08 at 17:13 +0200, Dario Faggioli wrote:
On Tue, 2015-07-07 at 13:16 +0200, Juergen Gross wrote:

@@ -645,25 +675,72 @@ int cpu_disable_scheduler(unsigned int cpu)
                   cpumask_setall(v->cpu_hard_affinity);
               }

-            if ( v->processor == cpu )
+            if ( v->processor != cpu )
               {
-                set_bit(_VPF_migrating, &v->pause_flags);
+                /* This vcpu is not on cpu, so we can move on. */
                   vcpu_schedule_unlock_irqrestore(lock, flags, v);
-                vcpu_sleep_nosync(v);
-                vcpu_migrate(v);
+                continue;
               }
-            else
-                vcpu_schedule_unlock_irqrestore(lock, flags, v);

               /*
-             * A vcpu active in the hypervisor will not be migratable.
-             * The caller should try again after releasing and reaquiring
-             * all locks.
+             * If we're here, it means that the vcpu is on cpu. Let's see how
+             * it's best to send it away, depending on whether we are shutting
+             * down/suspending, or doing cpupool manipulations.
                */
-            if ( v->processor == cpu )
-                ret = -EAGAIN;
-        }
+            set_bit(_VPF_migrating, &v->pause_flags);
+            vcpu_schedule_unlock_irqrestore(lock, flags, v);
+            vcpu_sleep_nosync(v);
+
+            /*
+             * In case of shutdown/suspend, it is not necessary to ask the
+             * scheduler to chime in. In fact:
+             *  * there is no reason for it: the end result we are after is
+             *    just 'all the vcpus on the boot pcpu, and no vcpu anywhere
+             *    else', so let's just go for it;
+             *  * it's wrong, when dealing a cpupool with only non-boot pcpus,
+             *    as the scheduler will always fail to send the vcpus away
+             *    from the last online (non boot) pcpu!

I'd add a comment that in shutdown/suspend case all domains are being
paused, so we can be active in dom0/Pool-0 only.

Sure, I'll add this.

...while putting such a comment together, I'm realizing that I'm not
sure about what you meant, or what you wanted the comment itself to
express.

I mean, it is certainly true that all domains are being paused (they've
been paused already, actually), but that include Dom0 too. Also, we are
in Xen, in stop_machine context, so I'm not sure what you meant either
with "we can be active in dom0/Pool-0 only".

We are running on the vcpu which issued the hypercall resulting in
pausing the domains. A vcpu can't pause itself.

So, I'm adding a line about things being paused. If you think I should
say anything more than that, let me know.

I think dom0/Pool-0 should be mentioned. The coding is written with this
assumption so it should be documented.


Juergen


_______________________________________________
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®.