idle_loop: either deal with tasklets or go idle

In fact, there are two kinds of tasklets: vCPU and
softirq context. When we want to do vCPU context tasklet
work, we force the idle vCPU (of a particular pCPU) into
execution, and run it from there.

This means there are two possible reasons for choosing
to run the idle vCPU:
1) we want a pCPU to go idle,
2) we want to run some vCPU context tasklet work.

If we're in case 2), it does not make sense to even
try to go idle (as the check will _always_ fail).

This patch rearranges the code of the body of idle
vCPUs, so that we actually check whether we are in
case 1) or 2), and act accordingly.

As a matter of fact, this also means that we do not
check if there's any tasklet work to do after waking
up from idle. This is not a problem, because:
a) for softirq context tasklets, if any is queued
   "during" wakeup from idle, TASKLET_SOFTIRQ is
   raised, and the call to do_softirq() (which is still
   happening *after* the wakeup) will take care of it;
b) for vCPU context tasklets, if any is queued "during"
   wakeup from idle, SCHEDULE_SOFTIRQ is raised and
   do_softirq() (happening after the wakeup) calls
   the scheduler. The scheduler sees that there is
   tasklet work pending and confirms the idle vCPU
   in execution, which then will get to execute
   do_tasklet().

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 76310ed41d41a327cba41ad24fca2aa667df8243..2dc8b0ab5aa0071082692836bceb372e80b9dc9d 100644 (file)
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -41,20 +41,27 @@ DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
 
 void idle_loop(void)
 {
+    unsigned int cpu = smp_processor_id();
+
     for ( ; ; )
     {
-        if ( cpu_is_offline(smp_processor_id()) )
+        if ( cpu_is_offline(cpu) )
             stop_cpu();
 
-        local_irq_disable();
-        if ( cpu_is_haltable(smp_processor_id()) )
+        /* Are we here for running vcpu context tasklets, or for idling? */
+        if ( unlikely(tasklet_work_to_do(cpu)) )
+            do_tasklet();
+        else
         {
-            dsb(sy);
-            wfi();
+            local_irq_disable();
+            if ( cpu_is_haltable(cpu) )
+            {
+                dsb(sy);
+                wfi();
+            }
+            local_irq_enable();
         }
-        local_irq_enable();
 
-        do_tasklet();
         do_softirq();
         /*
          * We MUST be last (or before dsb, wfi). Otherwise after we get the

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 0162d7d816b2074e1c48ab884a1f66221bfefe9f..f7873da3230e126091fde00c97abce0b40ea948e 100644 (file)
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -112,12 +112,18 @@ static void play_dead(void)
 
 static void idle_loop(void)
 {
+    unsigned int cpu = smp_processor_id();
+
     for ( ; ; )
     {
-        if ( cpu_is_offline(smp_processor_id()) )
+        if ( cpu_is_offline(cpu) )
             play_dead();
-        (*pm_idle)();
-        do_tasklet();
+
+        /* Are we here for running vcpu context tasklets, or for idling? */
+        if ( unlikely(tasklet_work_to_do(cpu)) )
+            do_tasklet();
+        else
+            pm_idle();
         do_softirq();
         /*
          * We MUST be last (or before pm_idle). Otherwise after we get the

diff --git a/xen/common/tasklet.c b/xen/common/tasklet.c
index 365a7778c90e89eaf5f880098a3e125bd79e0f70..0f0a6f83652820ba3edc4ed2ba03c48d21af71a4 100644 (file)
--- a/xen/common/tasklet.c
+++ b/xen/common/tasklet.c
@@ -111,11 +111,15 @@ void do_tasklet(void)
     struct list_head *list = &per_cpu(tasklet_list, cpu);
 
     /*
-     * Work must be enqueued *and* scheduled. Otherwise there is no work to
-     * do, and/or scheduler needs to run to update idle vcpu priority.
+     * We want to be sure any caller has checked that a tasklet is both
+     * enqueued and scheduled, before calling this. And, if the caller has
+     * actually checked, it's not an issue that we are outside of the
+     * critical region, in fact:
+     * - TASKLET_enqueued is cleared only here,
+     * - TASKLET_scheduled is only cleared when schedule() find it set,
+     *   without TASKLET_enqueued being set as well.
      */
-    if ( likely(*work_to_do != (TASKLET_enqueued|TASKLET_scheduled)) )
-        return;
+    ASSERT(tasklet_work_to_do(cpu));
 
     spin_lock_irq(&tasklet_lock);
 

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 1127ca99bd307c62b2809f164dde186c504ba8fd..6673b27d88b7592ceb05a1755082bfdb5086e4c7 100644 (file)
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -843,6 +843,11 @@ uint64_t get_cpu_idle_time(unsigned int cpu);
 /*
  * Used by idle loop to decide whether there is work to do:
  *  (1) Run softirqs; or (2) Play dead; or (3) Run tasklets.
+ *
+ * About (3), if a tasklet is enqueued, it will be scheduled
+ * really really soon, and hence it's pointless to try to
+ * sleep between these two events (that's why we don't call
+ * the tasklet_work_to_do() helper).
  */
 #define cpu_is_haltable(cpu)                    \
     (!softirq_pending(cpu) &&                   \

diff --git a/xen/include/xen/tasklet.h b/xen/include/xen/tasklet.h
index 8c3de7e20e04deccbd885199241b601c4c81d9e0..23d69c738e320e3e8850ea83de33db57931c8391 100644 (file)
--- a/xen/include/xen/tasklet.h
+++ b/xen/include/xen/tasklet.h
@@ -40,6 +40,16 @@ DECLARE_PER_CPU(unsigned long, tasklet_work_to_do);
 #define TASKLET_enqueued   (1ul << _TASKLET_enqueued)
 #define TASKLET_scheduled  (1ul << _TASKLET_scheduled)
 
+static inline bool tasklet_work_to_do(unsigned int cpu)
+{
+    /*
+     * Work must be enqueued *and* scheduled. Otherwise there is no work to
+     * do, and/or scheduler needs to run to update idle vcpu priority.
+     */
+    return per_cpu(tasklet_work_to_do, cpu) == (TASKLET_enqueued|
+                                                TASKLET_scheduled);
+}
+
 void tasklet_schedule_on_cpu(struct tasklet *t, unsigned int cpu);
 void tasklet_schedule(struct tasklet *t);
 void do_tasklet(void);