From: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> To: srivatsa.bhat@linux.vnet.ibm.com, toshi.kani@hp.com, lenb@kernel.org, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 2/3] acpi : prevent cpu from becoming online Date: Fri, 13 Jul 2012 17:51:14 +0900 [thread overview] Message-ID: <4FFFE182.2090601@jp.fujitsu.com> (raw) In-Reply-To: <4FFFE0DA.3050806@jp.fujitsu.com> 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_up() 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 | Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> --- drivers/acpi/processor_driver.c | 14 ++++++++++++++ kernel/cpu.c | 8 +++++--- 2 files changed, 19 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-13 17:31:37.799130100 +0900 +++ linux-3.5-rc6/drivers/acpi/processor_driver.c 2012-07-13 17:39:47.727006338 +0900 @@ -850,8 +850,22 @@ static int acpi_processor_handle_eject(s return ret; } + get_online_cpus(); + /* + * The cpu might become online again at this point. So we check whether + * the cpu has been onlined or not. If the cpu became online, it means + * that someone wants to use the cpu. So acpi_processor_handle_eject() + * returns -EAGAIN. + */ + if (unlikely(cpu_online(pr->id))) { + put_online_cpus(); + pr_warn("Failed to remove CPU %d, ", pr->id); + pr_warn("because other task brought the CPU back online\n"); + return -EAGAIN; + } 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-13 17:31:37.800130087 +0900 +++ linux-3.5-rc6/kernel/cpu.c 2012-07-13 17:31:39.661106874 +0900 @@ -343,11 +343,13 @@ 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);
WARNING: multiple messages have this Message-ID (diff)
From: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> To: <srivatsa.bhat@linux.vnet.ibm.com>, <toshi.kani@hp.com>, <lenb@kernel.org>, <linux-acpi@vger.kernel.org>, <linux-kernel@vger.kernel.org> Subject: [PATCH v4 2/3] acpi : prevent cpu from becoming online Date: Fri, 13 Jul 2012 17:51:14 +0900 [thread overview] Message-ID: <4FFFE182.2090601@jp.fujitsu.com> (raw) In-Reply-To: <4FFFE0DA.3050806@jp.fujitsu.com> 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_up() 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 | Reviewed-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> --- drivers/acpi/processor_driver.c | 14 ++++++++++++++ kernel/cpu.c | 8 +++++--- 2 files changed, 19 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-13 17:31:37.799130100 +0900 +++ linux-3.5-rc6/drivers/acpi/processor_driver.c 2012-07-13 17:39:47.727006338 +0900 @@ -850,8 +850,22 @@ static int acpi_processor_handle_eject(s return ret; } + get_online_cpus(); + /* + * The cpu might become online again at this point. So we check whether + * the cpu has been onlined or not. If the cpu became online, it means + * that someone wants to use the cpu. So acpi_processor_handle_eject() + * returns -EAGAIN. + */ + if (unlikely(cpu_online(pr->id))) { + put_online_cpus(); + pr_warn("Failed to remove CPU %d, ", pr->id); + pr_warn("because other task brought the CPU back online\n"); + return -EAGAIN; + } 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-13 17:31:37.800130087 +0900 +++ linux-3.5-rc6/kernel/cpu.c 2012-07-13 17:31:39.661106874 +0900 @@ -343,11 +343,13 @@ 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);
next prev parent reply other threads:[~2012-07-13 8:51 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-07-13 8:48 [PATCH v4 1/3] acpi : cpu hot-remove returns error when cpu_down() fails Yasuaki Ishimatsu 2012-07-13 8:48 ` Yasuaki Ishimatsu 2012-07-13 8:51 ` Yasuaki Ishimatsu [this message] 2012-07-13 8:51 ` [PATCH v4 2/3] acpi : prevent cpu from becoming online Yasuaki Ishimatsu 2012-07-13 15:25 ` Toshi Kani 2012-07-13 8:53 ` [PATCH v4 3/3] acpi : acpi_bus_trim() stops removing devices when failing to remove the device Yasuaki Ishimatsu 2012-07-13 8:53 ` Yasuaki Ishimatsu 2012-07-13 15:54 ` Toshi Kani 2012-07-13 15:20 ` [PATCH v4 1/3] acpi : cpu hot-remove returns error when cpu_down() fails Toshi Kani
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4FFFE182.2090601@jp.fujitsu.com \ --to=isimatu.yasuaki@jp.fujitsu.com \ --cc=lenb@kernel.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=srivatsa.bhat@linux.vnet.ibm.com \ --cc=toshi.kani@hp.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.