linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] acpi: drop support for force_remove
@ 2017-04-03  7:40 Michal Hocko
  2017-04-10 17:13 ` Michal Hocko
  0 siblings, 1 reply; 8+ messages in thread
From: Michal Hocko @ 2017-04-03  7:40 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Joey Lee, Jiri Kosina, Kani Toshimitsu, Len Brown, LKML,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Michal Hocko

From: Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org>

/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 <jlee-IBi9RG/b67k@public.gmane.org>
Signed-off-by: Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org>
---
Hi,
this has been previously discussed [1] without acpi mailing list CCed.
So I am resending it now with hopefully everybody involved.

[1] http://lkml.kernel.org/r/20170320192938.GA11363-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org

 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 <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
+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 @@ Contact:	Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
 		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 <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
-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 <lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org if you depend on this functionality\n");
+		return -EINVAL;
+	}
 	return size;
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] acpi: drop support for force_remove
  2017-04-03  7:40 [PATCH] acpi: drop support for force_remove Michal Hocko
@ 2017-04-10 17:13 ` Michal Hocko
  2017-04-10 22:15   ` Rafael J. Wysocki
  0 siblings, 1 reply; 8+ messages in thread
From: Michal Hocko @ 2017-04-10 17:13 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Joey Lee, Jiri Kosina, Kani Toshimitsu, Len Brown, LKML,
	linux-acpi, linux-api

On Mon 03-04-17 09:40:23, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> /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 <jlee@suse.com>
> Signed-off-by: Michal Hocko <mhocko@suse.com>

ping on this?

> ---
> Hi,
> this has been previously discussed [1] without acpi mailing list CCed.
> So I am resending it now with hopefully everybody involved.
> 
> [1] http://lkml.kernel.org/r/20170320192938.GA11363@dhcp22.suse.cz
> 
>  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 <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.
> 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 @@ Contact:	Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>  		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 <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/
>  Date:		February 2008
>  Contact:	Len Brown <lenb@kernel.org>
> 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;
>  }
>  
> -- 
> 2.11.0
> 

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] acpi: drop support for force_remove
  2017-04-10 17:13 ` Michal Hocko
@ 2017-04-10 22:15   ` Rafael J. Wysocki
  2017-04-11 12:20     ` Michal Hocko
  0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2017-04-10 22:15 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Joey Lee, Jiri Kosina, Kani Toshimitsu, Len Brown, LKML,
	linux-acpi, linux-api

On Monday, April 10, 2017 07:13:44 PM Michal Hocko wrote:
> On Mon 03-04-17 09:40:23, Michal Hocko wrote:
> > From: Michal Hocko <mhocko@suse.com>
> > 
> > /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 <jlee@suse.com>
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> 
> ping on this?

/shrug/

I'll apply it if nobody has any problems with it.

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] acpi: drop support for force_remove
  2017-04-10 22:15   ` Rafael J. Wysocki
@ 2017-04-11 12:20     ` Michal Hocko
  2017-04-11 13:48       ` Rafael J. Wysocki
  0 siblings, 1 reply; 8+ messages in thread
From: Michal Hocko @ 2017-04-11 12:20 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Joey Lee, Jiri Kosina, Kani Toshimitsu, Len Brown, LKML,
	linux-acpi, linux-api

On Tue 11-04-17 00:15:42, Rafael J. Wysocki wrote:
> On Monday, April 10, 2017 07:13:44 PM Michal Hocko wrote:
> > On Mon 03-04-17 09:40:23, Michal Hocko wrote:
> > > From: Michal Hocko <mhocko@suse.com>
> > > 
> > > /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 <jlee@suse.com>
> > > Signed-off-by: Michal Hocko <mhocko@suse.com>
> > 
> > ping on this?
> 
> /shrug/
> 
> I'll apply it if nobody has any problems with it.

It is definitely your call but how long are we going to wait
(considering that the current behavior is obviously broken wrt memory
hotplug)?
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] acpi: drop support for force_remove
  2017-04-11 12:20     ` Michal Hocko
@ 2017-04-11 13:48       ` Rafael J. Wysocki
       [not found]         ` <CAJZ5v0hhoej=FoqYaBOwy1-_AP35WyEYYUNkb3Pn22G0monanA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2017-04-11 13:48 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Rafael J. Wysocki, Joey Lee, Jiri Kosina, Kani Toshimitsu,
	Len Brown, LKML, ACPI Devel Maling List, linux-api

On Tue, Apr 11, 2017 at 2:20 PM, Michal Hocko <mhocko@kernel.org> wrote:
> On Tue 11-04-17 00:15:42, Rafael J. Wysocki wrote:
>> On Monday, April 10, 2017 07:13:44 PM Michal Hocko wrote:
>> > On Mon 03-04-17 09:40:23, Michal Hocko wrote:
>> > > From: Michal Hocko <mhocko@suse.com>
>> > >
>> > > /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 <jlee@suse.com>
>> > > Signed-off-by: Michal Hocko <mhocko@suse.com>
>> >
>> > ping on this?
>>
>> /shrug/
>>
>> I'll apply it if nobody has any problems with it.
>
> It is definitely your call but how long are we going to wait
> (considering that the current behavior is obviously broken wrt memory
> hotplug)?

A couple of days more?  Surely not going to push it into 4.11 at this
stage as it is an ABI change.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] acpi: drop support for force_remove
       [not found]         ` <CAJZ5v0hhoej=FoqYaBOwy1-_AP35WyEYYUNkb3Pn22G0monanA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-04-11 13:52           ` Michal Hocko
  2017-04-11 13:54             ` Rafael J. Wysocki
  0 siblings, 1 reply; 8+ messages in thread
From: Michal Hocko @ 2017-04-11 13:52 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Joey Lee, Jiri Kosina, Kani Toshimitsu,
	Len Brown, LKML, ACPI Devel Maling List,
	linux-api-u79uwXL29TY76Z2rM5mHXA

On Tue 11-04-17 15:48:04, Rafael J. Wysocki wrote:
> On Tue, Apr 11, 2017 at 2:20 PM, Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Tue 11-04-17 00:15:42, Rafael J. Wysocki wrote:
[...]
> >> I'll apply it if nobody has any problems with it.
> >
> > It is definitely your call but how long are we going to wait
> > (considering that the current behavior is obviously broken wrt memory
> > hotplug)?
> 
> A couple of days more?  Surely not going to push it into 4.11 at this
> stage as it is an ABI change.

Ohh, I didn't want to push it to 4.11 this late but having it in
linux-next might help to catch some users who are not following the
mailing list. Going to 4.12 would be really great but I wouldn't really
object even 4.13...
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] acpi: drop support for force_remove
  2017-04-11 13:52           ` Michal Hocko
@ 2017-04-11 13:54             ` Rafael J. Wysocki
       [not found]               ` <CAJZ5v0hzv1oxpNJvSc2omqaOFRgiBB8UKCbRKGkgzHbEtqosbQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2017-04-11 13:54 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Rafael J. Wysocki, Rafael J. Wysocki, Joey Lee, Jiri Kosina,
	Kani Toshimitsu, Len Brown, LKML, ACPI Devel Maling List,
	linux-api

On Tue, Apr 11, 2017 at 3:52 PM, Michal Hocko <mhocko@kernel.org> wrote:
> On Tue 11-04-17 15:48:04, Rafael J. Wysocki wrote:
>> On Tue, Apr 11, 2017 at 2:20 PM, Michal Hocko <mhocko@kernel.org> wrote:
>> > On Tue 11-04-17 00:15:42, Rafael J. Wysocki wrote:
> [...]
>> >> I'll apply it if nobody has any problems with it.
>> >
>> > It is definitely your call but how long are we going to wait
>> > (considering that the current behavior is obviously broken wrt memory
>> > hotplug)?
>>
>> A couple of days more?  Surely not going to push it into 4.11 at this
>> stage as it is an ABI change.
>
> Ohh, I didn't want to push it to 4.11 this late but having it in
> linux-next might help to catch some users who are not following the
> mailing list. Going to 4.12 would be really great but I wouldn't really
> object even 4.13...

4.12 is probably fine.  I don't see a compelling reason to sit on this
for a whole cycle, FWIW.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] acpi: drop support for force_remove
       [not found]               ` <CAJZ5v0hzv1oxpNJvSc2omqaOFRgiBB8UKCbRKGkgzHbEtqosbQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-04-14 22:33                 ` Rafael J. Wysocki
  0 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki @ 2017-04-14 22:33 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Michal Hocko, Joey Lee, Jiri Kosina, Kani Toshimitsu, Len Brown,
	LKML, ACPI Devel Maling List, linux-api-u79uwXL29TY76Z2rM5mHXA

On Tuesday, April 11, 2017 03:54:43 PM Rafael J. Wysocki wrote:
> On Tue, Apr 11, 2017 at 3:52 PM, Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Tue 11-04-17 15:48:04, Rafael J. Wysocki wrote:
> >> On Tue, Apr 11, 2017 at 2:20 PM, Michal Hocko <mhocko-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >> > On Tue 11-04-17 00:15:42, Rafael J. Wysocki wrote:
> > [...]
> >> >> I'll apply it if nobody has any problems with it.
> >> >
> >> > It is definitely your call but how long are we going to wait
> >> > (considering that the current behavior is obviously broken wrt memory
> >> > hotplug)?
> >>
> >> A couple of days more?  Surely not going to push it into 4.11 at this
> >> stage as it is an ABI change.
> >
> > Ohh, I didn't want to push it to 4.11 this late but having it in
> > linux-next might help to catch some users who are not following the
> > mailing list. Going to 4.12 would be really great but I wouldn't really
> > object even 4.13...
> 
> 4.12 is probably fine.  I don't see a compelling reason to sit on this
> for a whole cycle, FWIW.

It should hit linux-next next week.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-04-14 22:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-03  7:40 [PATCH] acpi: drop support for force_remove Michal Hocko
2017-04-10 17:13 ` Michal Hocko
2017-04-10 22:15   ` Rafael J. Wysocki
2017-04-11 12:20     ` Michal Hocko
2017-04-11 13:48       ` Rafael J. Wysocki
     [not found]         ` <CAJZ5v0hhoej=FoqYaBOwy1-_AP35WyEYYUNkb3Pn22G0monanA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-04-11 13:52           ` Michal Hocko
2017-04-11 13:54             ` Rafael J. Wysocki
     [not found]               ` <CAJZ5v0hzv1oxpNJvSc2omqaOFRgiBB8UKCbRKGkgzHbEtqosbQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-04-14 22:33                 ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).