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 2/2] acpi : prevent cpu from becoming online
Date: Fri, 28 Sep 2012 19:39:45 +0900 [thread overview]
Message-ID: <50657E71.4050001@jp.fujitsu.com> (raw)
In-Reply-To: <50657D92.3010106@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>
Reviewed-by: Toshi Kani <toshi.kani@hp.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.6-rc7/drivers/acpi/processor_driver.c
===================================================================
--- linux-3.6-rc7.orig/drivers/acpi/processor_driver.c 2012-09-28 19:16:33.207858261 +0900
+++ linux-3.6-rc7/drivers/acpi/processor_driver.c 2012-09-28 19:23:05.129858476 +0900
@@ -857,8 +857,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, because other task "
+ "brought the CPU back online\n", pr->id);
+ return -EAGAIN;
+ }
arch_unregister_cpu(pr->id);
acpi_unmap_lsapic(pr->id);
+ put_online_cpus();
return ret;
}
#else
Index: linux-3.6-rc7/kernel/cpu.c
===================================================================
--- linux-3.6-rc7.orig/kernel/cpu.c 2012-09-24 10:10:57.000000000 +0900
+++ linux-3.6-rc7/kernel/cpu.c 2012-09-28 19:19:32.321858402 +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 2/2] acpi : prevent cpu from becoming online
Date: Fri, 28 Sep 2012 19:39:45 +0900 [thread overview]
Message-ID: <50657E71.4050001@jp.fujitsu.com> (raw)
In-Reply-To: <50657D92.3010106@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>
Reviewed-by: Toshi Kani <toshi.kani@hp.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.6-rc7/drivers/acpi/processor_driver.c
===================================================================
--- linux-3.6-rc7.orig/drivers/acpi/processor_driver.c 2012-09-28 19:16:33.207858261 +0900
+++ linux-3.6-rc7/drivers/acpi/processor_driver.c 2012-09-28 19:23:05.129858476 +0900
@@ -857,8 +857,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, because other task "
+ "brought the CPU back online\n", pr->id);
+ return -EAGAIN;
+ }
arch_unregister_cpu(pr->id);
acpi_unmap_lsapic(pr->id);
+ put_online_cpus();
return ret;
}
#else
Index: linux-3.6-rc7/kernel/cpu.c
===================================================================
--- linux-3.6-rc7.orig/kernel/cpu.c 2012-09-24 10:10:57.000000000 +0900
+++ linux-3.6-rc7/kernel/cpu.c 2012-09-28 19:19:32.321858402 +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-09-28 10:40 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-28 10:36 [PATCH 1/2] acpi : cpu hot-remove returns error when cpu_down() fails Yasuaki Ishimatsu
2012-09-28 10:36 ` Yasuaki Ishimatsu
2012-09-28 10:39 ` Yasuaki Ishimatsu [this message]
2012-09-28 10:39 ` [PATCH 2/2] acpi : prevent cpu from becoming online Yasuaki Ishimatsu
2012-10-19 1:06 ` [PATCH 1/2] acpi : cpu hot-remove returns error when cpu_down() fails Rafael J. Wysocki
2012-10-19 4:40 ` Yasuaki Ishimatsu
2012-10-19 4:40 ` Yasuaki Ishimatsu
2012-10-26 9:25 ` [PATCH v2 " Yasuaki Ishimatsu
2012-10-26 9:25 ` Yasuaki Ishimatsu
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=50657E71.4050001@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: link
Be 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.