cpupool: correct error handling when removing cpu from cpupool

When schedule_cpu_switch() called from cpupool_unassign_cpu_helper()
returns an error, the domlist_read_lock isn't released again.

As cpu_disable_scheduler() might have changed affinity of some
domains domain_update_node_affinity() must be called for all domains
in the cpupool even in error case.

Even if looking weird it is okay to let the to be removed cpu set in
cpupool_free_cpus in case of an error returned by
cpu_disable_scheduler(). Add a comment explaining the reason for this.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Dario Faggioli <dario.faggioli@citrix.com>

diff --git a/xen/common/cpupool.c b/xen/common/cpupool.c
index 8e7b7234d631a650240673457416c39f2bf26a69..d0189f8cc39b10cadc172196a894a05e38f337b1 100644 (file)
--- a/xen/common/cpupool.c
+++ b/xen/common/cpupool.c
@@ -312,17 +312,25 @@ static long cpupool_unassign_cpu_helper(void *info)
     rcu_read_lock(&domlist_read_lock);
     ret = cpu_disable_scheduler(cpu);
     cpumask_set_cpu(cpu, &cpupool_free_cpus);
+
+    /*
+     * cpu_disable_scheduler() returning an error doesn't require resetting
+     * cpupool_free_cpus' cpu bit. All error cases should be of temporary
+     * nature and tools will retry the operation. Even if the number of
+     * retries may be limited, the in-between state can easily be repaired
+     * by adding the cpu to the cpupool again.
+     */
     if ( !ret )
     {
         ret = schedule_cpu_switch(cpu, NULL);
         if ( ret )
-        {
             cpumask_clear_cpu(cpu, &cpupool_free_cpus);
-            goto out;
+        else
+        {
+            cpupool_moving_cpu = -1;
+            cpupool_put(cpupool_cpu_moving);
+            cpupool_cpu_moving = NULL;
         }
-        cpupool_moving_cpu = -1;
-        cpupool_put(cpupool_cpu_moving);
-        cpupool_cpu_moving = NULL;
     }
 
     for_each_domain_in_cpupool(d, c)