Commit ffc10d82 authored by Michal Hocko's avatar Michal Hocko Committed by Rafael J. Wysocki

ACPI / scan: Drop support for force_remove

/sys/firmware/acpi/hotplug/force_remove was presumably added to support
auto offlining in the past. This is, however, inherently dangerous for
some hotplugable resources like memory. The memory offlining fails when
the memory is still in use and cannot be dropped or migrated. If we
ignore the failure we are basically allowing for subtle memory
corruption or a crash.

We have actually noticed the later while hitting BUG() during the memory
hotremove (remove_memory):
	ret = walk_memory_range(PFN_DOWN(start), PFN_UP(start + size - 1), NULL,
			check_memblock_offlined_cb);
	if (ret)
		BUG();

it took us quite non-trivial time realize that the customer had
force_remove enabled. Even if the BUG was removed here and we could
propagate the error up the call chain it wouldn't help at all because
then we would hit a crash or a memory corruption later and harder to
debug. So force_remove is unfixable for the memory hotremove. We haven't
checked other hotplugable resources to be prone to a similar problems.

Remove the force_remove functionality because it is not fixable currently.
Keep the sysfs file and report an error if somebody tries to enable it.
Encourage users to report about the missing functionality and work with
them with an alternative solution.
Reviewed-by: default avatarLee, Chun-Yi <jlee@suse.com>
Signed-off-by: default avatarMichal Hocko <mhocko@suse.com>
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
parent 39da7c50
What: /sys/firmware/acpi/hotplug/force_remove
Date: Mar 2017
Contact: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Description:
Since the force_remove is inherently broken and dangerous to
use for some hotplugable resources like memory (because ignoring
the offline failure might lead to memory corruption and crashes)
enabling this knob is not safe and thus unsupported.
...@@ -44,16 +44,6 @@ Description: ...@@ -44,16 +44,6 @@ Description:
or 0 (unset). Attempts to write any other values to it will or 0 (unset). Attempts to write any other values to it will
cause -EINVAL to be returned. cause -EINVAL to be returned.
What: /sys/firmware/acpi/hotplug/force_remove
Date: May 2013
Contact: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Description:
The number in this file (0 or 1) determines whether (1) or not
(0) the ACPI subsystem will allow devices to be hot-removed even
if they cannot be put offline gracefully (from the kernel's
viewpoint). That number can be changed by writing a boolean
value to this file.
What: /sys/firmware/acpi/interrupts/ What: /sys/firmware/acpi/interrupts/
Date: February 2008 Date: February 2008
Contact: Len Brown <lenb@kernel.org> Contact: Len Brown <lenb@kernel.org>
......
...@@ -65,8 +65,6 @@ static inline void acpi_cmos_rtc_init(void) {} ...@@ -65,8 +65,6 @@ static inline void acpi_cmos_rtc_init(void) {}
#endif #endif
int acpi_rev_override_setup(char *str); int acpi_rev_override_setup(char *str);
extern bool acpi_force_hot_remove;
void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug, void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug,
const char *name); const char *name);
int acpi_scan_add_handler_with_hotplug(struct acpi_scan_handler *handler, int acpi_scan_add_handler_with_hotplug(struct acpi_scan_handler *handler,
......
...@@ -30,12 +30,6 @@ extern struct acpi_device *acpi_root; ...@@ -30,12 +30,6 @@ extern struct acpi_device *acpi_root;
#define INVALID_ACPI_HANDLE ((acpi_handle)empty_zero_page) #define INVALID_ACPI_HANDLE ((acpi_handle)empty_zero_page)
/*
* If set, devices will be hot-removed even if they cannot be put offline
* gracefully (from the kernel's standpoint).
*/
bool acpi_force_hot_remove;
static const char *dummy_hid = "device"; static const char *dummy_hid = "device";
static LIST_HEAD(acpi_dep_list); static LIST_HEAD(acpi_dep_list);
...@@ -170,9 +164,6 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data, ...@@ -170,9 +164,6 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data,
pn->put_online = false; pn->put_online = false;
} }
ret = device_offline(pn->dev); ret = device_offline(pn->dev);
if (acpi_force_hot_remove)
continue;
if (ret >= 0) { if (ret >= 0) {
pn->put_online = !ret; pn->put_online = !ret;
} else { } else {
...@@ -241,11 +232,11 @@ static int acpi_scan_try_to_offline(struct acpi_device *device) ...@@ -241,11 +232,11 @@ static int acpi_scan_try_to_offline(struct acpi_device *device)
acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
NULL, acpi_bus_offline, (void *)true, NULL, acpi_bus_offline, (void *)true,
(void **)&errdev); (void **)&errdev);
if (!errdev || acpi_force_hot_remove) if (!errdev)
acpi_bus_offline(handle, 0, (void *)true, acpi_bus_offline(handle, 0, (void *)true,
(void **)&errdev); (void **)&errdev);
if (errdev && !acpi_force_hot_remove) { if (errdev) {
dev_warn(errdev, "Offline failed.\n"); dev_warn(errdev, "Offline failed.\n");
acpi_bus_online(handle, 0, NULL, NULL); acpi_bus_online(handle, 0, NULL, NULL);
acpi_walk_namespace(ACPI_TYPE_ANY, handle, acpi_walk_namespace(ACPI_TYPE_ANY, handle,
...@@ -263,8 +254,7 @@ static int acpi_scan_hot_remove(struct acpi_device *device) ...@@ -263,8 +254,7 @@ static int acpi_scan_hot_remove(struct acpi_device *device)
unsigned long long sta; unsigned long long sta;
acpi_status status; acpi_status status;
if (device->handler && device->handler->hotplug.demand_offline if (device->handler && device->handler->hotplug.demand_offline) {
&& !acpi_force_hot_remove) {
if (!acpi_scan_is_offline(device, true)) if (!acpi_scan_is_offline(device, true))
return -EBUSY; return -EBUSY;
} else { } else {
......
...@@ -921,7 +921,7 @@ void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug, ...@@ -921,7 +921,7 @@ void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug,
static ssize_t force_remove_show(struct kobject *kobj, static ssize_t force_remove_show(struct kobject *kobj,
struct kobj_attribute *attr, char *buf) struct kobj_attribute *attr, char *buf)
{ {
return sprintf(buf, "%d\n", !!acpi_force_hot_remove); return sprintf(buf, "%d\n", 0);
} }
static ssize_t force_remove_store(struct kobject *kobj, static ssize_t force_remove_store(struct kobject *kobj,
...@@ -935,9 +935,10 @@ static ssize_t force_remove_store(struct kobject *kobj, ...@@ -935,9 +935,10 @@ static ssize_t force_remove_store(struct kobject *kobj,
if (ret < 0) if (ret < 0)
return ret; return ret;
lock_device_hotplug(); if (val) {
acpi_force_hot_remove = val; pr_err("Enabling force_remove is not supported anymore. Please report to linux-acpi@vger.kernel.org if you depend on this functionality\n");
unlock_device_hotplug(); return -EINVAL;
}
return size; return size;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment