Even if acpi_processor_handle_eject() offlines cpu, there is a chance
to online the cpu after that. So the patch closes the window by using
get/put_online_cpus().

Why does the patch change _cpu_up() logic?

The patch cares the race of hot-remove cpu and _cpu_up(). If the patch
does not change it, there is the following race.

hot-remove cpu                         |  _cpu_up()
------------------------------------- ------------------------------------
call acpi_processor_handle_eject()     |
     call cpu_down()                   |
     call get_online_cpus()            |
                                       | call cpu_hotplug_begin() and stop here
     call arch_unregister_cpu()        |
     call acpi_unmap_lsapic()          |
     call put_online_cpus()            |
                                       | start and continue _cpu_up()
     return acpi_processor_remove()    |
continue hot-remove the cpu            |

So _cpu_up() can continue to itself. And hot-remove cpu can also continue
itself. If the patch changes _cpu_cp() logic, the race disappears as below:

hot-remove cpu                         | _cpu_up()
-----------------------------------------------------------------------
call acpi_processor_handle_eject()     |
     call cpu_down()                   |
     call get_online_cpus()            |
                                       | call cpu_hotplug_begin() and stop here
     call arch_unregister_cpu()        |
     call acpi_unmap_lsapic()          |
          cpu's cpu_present is set     |
          to false by set_cpu_present()|
     call put_online_cpus()            |
                                       | start _cpu_up()
                                       | check cpu_present() and return -EINVAL
     return acpi_processor_remove()    |
continue hot-remove the cpu            |

Signed-off-by: Yasuaki Ishimatsu <isimatu.yasu...@jp.fujitsu.com>

---
 drivers/acpi/processor_driver.c |   12 ++++++++++++
 kernel/cpu.c                    |    8 +++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

Index: linux-3.5-rc6/drivers/acpi/processor_driver.c
===================================================================
--- linux-3.5-rc6.orig/drivers/acpi/processor_driver.c  2012-07-10 
13:58:20.862931557 +0900
+++ linux-3.5-rc6/drivers/acpi/processor_driver.c       2012-07-10 
14:05:46.318365136 +0900
@@ -850,8 +850,18 @@ static int acpi_processor_handle_eject(s
                        return ret;
        }

+       get_online_cpus();
+       /*
+        * Someone might online the cpu again at this point. So we check that
+        * cpu has been onlined or not. If cpu is online, it returns -EBUSY.
+        */
+       if (unlikely(cpu_online(pr->id))) {
+               put_online_cpus();
+               return -EBUSY;
+       }
        arch_unregister_cpu(pr->id);
        acpi_unmap_lsapic(pr->id);
+       put_online_cpus();
        return ret;
 }
 #else
Index: linux-3.5-rc6/kernel/cpu.c
===================================================================
--- linux-3.5-rc6.orig/kernel/cpu.c     2012-07-08 09:23:56.000000000 +0900
+++ linux-3.5-rc6/kernel/cpu.c  2012-07-10 14:15:26.065113805 +0900
@@ -343,11 +343,14 @@ static int __cpuinit _cpu_up(unsigned in
        unsigned long mod = tasks_frozen ? CPU_TASKS_FROZEN : 0;
        struct task_struct *idle;

-       if (cpu_online(cpu) || !cpu_present(cpu))
-               return -EINVAL;
-
        cpu_hotplug_begin();

+       if (cpu_online(cpu) || !cpu_present(cpu)) {
+               ret =  -EINVAL;
+               goto out;
+       }
+
+
        idle = idle_thread_get(cpu);
        if (IS_ERR(idle)) {
                ret = PTR_ERR(idle);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to