From ffc10d82ff5df7087a9b737de55a69ac4f89bf56 Mon Sep 17 00:00:00 2001 From: Michal Hocko Date: Mon, 3 Apr 2017 09:40:23 +0200 Subject: [PATCH 1/5] 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: Lee, Chun-Yi Signed-off-by: Michal Hocko Signed-off-by: Rafael J. Wysocki --- Documentation/ABI/obsolete/sysfs-firmware-acpi | 8 ++++++++ Documentation/ABI/testing/sysfs-firmware-acpi | 10 ---------- drivers/acpi/internal.h | 2 -- drivers/acpi/scan.c | 16 +++------------- drivers/acpi/sysfs.c | 9 +++++---- 5 files changed, 16 insertions(+), 29 deletions(-) create mode 100644 Documentation/ABI/obsolete/sysfs-firmware-acpi diff --git a/Documentation/ABI/obsolete/sysfs-firmware-acpi b/Documentation/ABI/obsolete/sysfs-firmware-acpi new file mode 100644 index 000000000000..6715a71bec3d --- /dev/null +++ b/Documentation/ABI/obsolete/sysfs-firmware-acpi @@ -0,0 +1,8 @@ +What: /sys/firmware/acpi/hotplug/force_remove +Date: Mar 2017 +Contact: Rafael J. Wysocki +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. diff --git a/Documentation/ABI/testing/sysfs-firmware-acpi b/Documentation/ABI/testing/sysfs-firmware-acpi index c7fc72d4495c..613f42a9d5cd 100644 --- a/Documentation/ABI/testing/sysfs-firmware-acpi +++ b/Documentation/ABI/testing/sysfs-firmware-acpi @@ -44,16 +44,6 @@ Description: or 0 (unset). Attempts to write any other values to it will cause -EINVAL to be returned. -What: /sys/firmware/acpi/hotplug/force_remove -Date: May 2013 -Contact: Rafael J. Wysocki -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/ Date: February 2008 Contact: Len Brown diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index f15900132912..66229ffa909b 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -65,8 +65,6 @@ static inline void acpi_cmos_rtc_init(void) {} #endif int acpi_rev_override_setup(char *str); -extern bool acpi_force_hot_remove; - void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug, const char *name); int acpi_scan_add_handler_with_hotplug(struct acpi_scan_handler *handler, diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 192691880d55..e2080b6e54aa 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -30,12 +30,6 @@ extern struct acpi_device *acpi_root; #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 LIST_HEAD(acpi_dep_list); @@ -170,9 +164,6 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data, pn->put_online = false; } ret = device_offline(pn->dev); - if (acpi_force_hot_remove) - continue; - if (ret >= 0) { pn->put_online = !ret; } else { @@ -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, NULL, acpi_bus_offline, (void *)true, (void **)&errdev); - if (!errdev || acpi_force_hot_remove) + if (!errdev) acpi_bus_offline(handle, 0, (void *)true, (void **)&errdev); - if (errdev && !acpi_force_hot_remove) { + if (errdev) { dev_warn(errdev, "Offline failed.\n"); acpi_bus_online(handle, 0, NULL, NULL); acpi_walk_namespace(ACPI_TYPE_ANY, handle, @@ -263,8 +254,7 @@ static int acpi_scan_hot_remove(struct acpi_device *device) unsigned long long sta; acpi_status status; - if (device->handler && device->handler->hotplug.demand_offline - && !acpi_force_hot_remove) { + if (device->handler && device->handler->hotplug.demand_offline) { if (!acpi_scan_is_offline(device, true)) return -EBUSY; } else { diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c index cf05ae973381..1b5ee1e0e5a3 100644 --- a/drivers/acpi/sysfs.c +++ b/drivers/acpi/sysfs.c @@ -921,7 +921,7 @@ void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug, static ssize_t force_remove_show(struct kobject *kobj, 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, @@ -935,9 +935,10 @@ static ssize_t force_remove_store(struct kobject *kobj, if (ret < 0) return ret; - lock_device_hotplug(); - acpi_force_hot_remove = val; - unlock_device_hotplug(); + if (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"); + return -EINVAL; + } return size; } From 79f97c3051e16b117b47e0586a368c4f54a82909 Mon Sep 17 00:00:00 2001 From: Shanker Donthineni Date: Wed, 29 Mar 2017 06:39:19 -0500 Subject: [PATCH 2/5] ACPI / platform: Update platform device NUMA node based on _PXM method The optional _PXM method evaluates to an integer that identifies the proximity domain of a device object. On ACPI based kernel boot, the field numa_node in 'struct device' is always set to -1 irrespective of _PXM value that is specified in the ACPI device object. But in case of device-tree based kernel boot the numa_node field is populated and reflects to a DT property that is specified in DTS according to the below document. https://www.kernel.org/doc/Documentation/devicetree/bindings/numa.txt http://lxr.free-electrons.com/source/drivers/of/device.c#L54 Without this patch dev_to_node() always returns -1 for all platform devices. This patch adds support for the ACPI _PXM method and updates the platform device NUMA node id using acpi_get_node() which provides the PXM-to-NUMA mapping information. The individual platform device drivers should be able to use the NUMA-aware memory allocation functions kmalloc_node() and alloc_pages_node() to improve performance. Signed-off-by: Shanker Donthineni Reviewed-by: Hanjun Guo Tested-by: Jiandi An [ rjw: Subject / changelog ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/acpi_platform.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 03250e1f1103..88cd949003f3 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -121,11 +121,14 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev, if (IS_ERR(pdev)) dev_err(&adev->dev, "platform device creation failed: %ld\n", PTR_ERR(pdev)); - else + else { + set_dev_node(&pdev->dev, acpi_get_node(adev->handle)); dev_dbg(&adev->dev, "created platform device %s\n", dev_name(&pdev->dev)); + } kfree(resources); + return pdev; } EXPORT_SYMBOL_GPL(acpi_create_platform_device); From f49c3f90a31f6e19ef3343dcc8809dac1019b59e Mon Sep 17 00:00:00 2001 From: Baoquan He Date: Fri, 7 Apr 2017 16:22:47 +0800 Subject: [PATCH 3/5] ACPI / tables: Drop acpi_parse_entries() which is not used Function acpi_parse_entries() is not used any more and if necessary, acpi_table_parse_entries() can be used instead of it, so drop it. Signed-off-by: Baoquan He [ rjw: Subject / changelog ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/tables.c | 16 ---------------- include/linux/acpi.h | 4 ---- 2 files changed, 20 deletions(-) diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index 2604189d6cd1..0dae722ab2ec 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -310,22 +310,6 @@ acpi_parse_entries_array(char *id, unsigned long table_size, return errs ? -EINVAL : count; } -int __init -acpi_parse_entries(char *id, - unsigned long table_size, - acpi_tbl_entry_handler handler, - struct acpi_table_header *table_header, - int entry_id, unsigned int max_entries) -{ - struct acpi_subtable_proc proc = { - .id = entry_id, - .handler = handler, - }; - - return acpi_parse_entries_array(id, table_size, table_header, - &proc, 1, max_entries); -} - int __init acpi_table_parse_entries_array(char *id, unsigned long table_size, diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 9b05886f9773..83abbfceabad 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -233,10 +233,6 @@ int acpi_numa_init (void); int acpi_table_init (void); int acpi_table_parse(char *id, acpi_tbl_table_handler handler); -int __init acpi_parse_entries(char *id, unsigned long table_size, - acpi_tbl_entry_handler handler, - struct acpi_table_header *table_header, - int entry_id, unsigned int max_entries); int __init acpi_table_parse_entries(char *id, unsigned long table_size, int entry_id, acpi_tbl_entry_handler handler, From f5beabfe61794d9a9d9549d387cda2bffd81e504 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 17 Apr 2017 01:19:50 +0200 Subject: [PATCH 4/5] ACPI / scan: Apply default enumeration to devices with ACPI drivers The current code in acpi_bus_attach() is inconsistent with respect to device objects with ACPI drivers bound to them, as it allows ACPI drivers to bind to device objects with existing "physical" device companions, but it doesn't allow "physical" device objects to be created for ACPI device objects with ACPI drivers bound to them. Thus, in some cases, the outcome depends on the ordering of events which is confusing at best. For this reason, modify acpi_bus_attach() to call acpi_default_enumeration() for device objects with the pnp.type.platform_id flag set regardless of whether or not any ACPI drivers are bound to them. Signed-off-by: Rafael J. Wysocki Reviewed-by: Mika Westerberg Reviewed-by: Joey Lee --- drivers/acpi/scan.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index acfa6c0831f2..f74bc0d28692 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1856,10 +1856,10 @@ static void acpi_bus_attach(struct acpi_device *device) if (ret < 0) return; - if (ret > 0 || !device->pnp.type.platform_id) - acpi_device_set_enumerated(device); - else + if (device->pnp.type.platform_id) acpi_default_enumeration(device); + else + acpi_device_set_enumerated(device); ok: list_for_each_entry(child, &device->children, node) From c381fc3a1bbfc9d780048a0b94afd162e4c3815b Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 17 Apr 2017 01:20:48 +0200 Subject: [PATCH 5/5] ACPI / scan: Avoid enumerating devices more than once acpi_bus_attach() does not check the visited flag for devices that have been enumerated already and some of them may be enumerated for multiple times as a result, because some callers of acpi_bus_scan() don't check the visited flag either. For this reason, modify acpi_bus_attach() to check the visited flag and avoid enumerating devices that have already been enumerated. Signed-off-by: Rafael J. Wysocki Reviewed-by: Mika Westerberg Reviewed-by: Joey Lee --- drivers/acpi/scan.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index f74bc0d28692..c26931067415 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1840,6 +1840,8 @@ static void acpi_bus_attach(struct acpi_device *device) device->flags.power_manageable = 0; device->flags.initialized = true; + } else if (device->flags.visited) { + goto ok; } ret = acpi_scan_attach_handler(device);