All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-15 10:22 ` Vasilis Liaskovitis
  0 siblings, 0 replies; 36+ messages in thread
From: Vasilis Liaskovitis @ 2012-11-15 10:22 UTC (permalink / raw)
  To: linux-acpi, isimatu.yasuaki, wency
  Cc: rjw, lenb, toshi.kani, linux-kernel, linux-mm, Vasilis Liaskovitis

As discussed in https://patchwork.kernel.org/patch/1581581/
the driver core remove function needs to always succeed. This means we need
to know that the device can be successfully removed before acpi_bus_trim / 
acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
eject or driver unbind of memory devices fails e.g with:

echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind

since the ACPI core goes ahead and ejects the device regardless of whether the
the memory is still in use or not.

For this reason a new acpi_device operation called prepare_remove is introduced.
This operation should be registered for acpi devices whose removal (from kernel
perspective) can fail.  Memory devices fall in this category.
A similar operation is introduced in bus_type to safely handle driver unbind
from the device driver core.

acpi_bus_hot_remove_device and driver_unbind are changed to handle removal in 2
steps:
- preparation for removal i.e. perform part of removal that can fail. Should
  succeed for device and all its children.
- if above step was successfull, proceed to actual device removal

With this patchset, only acpi memory devices use the new prepare_remove
device operation. The actual memory removal (VM-related offline and other memory
cleanups) is moved to prepare_remove. The old remove operation just cleans up
the acpi structures. Directly ejecting PNP0C80 memory devices works safely. I
haven't tested yet with an ACPI container which contains memory devices.

v1->v2:
- new patch to introduce bus_type prepare_remove callback. Needed to prepare
removal on driver unbinding from device-driver core.
- v1 patches 1 and 2 simplified and merged in one. acpi_bus_trim does not require
argument changes.

Comments welcome.

Vasilis Liaskovitis (3):
  driver core: Introduce prepare_remove in bus_type
  acpi: Introduce prepare_remove operation in acpi_device_ops
  acpi_memhotplug: Add prepare_remove operation

 drivers/acpi/acpi_memhotplug.c |   22 ++++++++++++++++++++--
 drivers/acpi/scan.c            |   21 ++++++++++++++++++++-
 drivers/base/bus.c             |   36 ++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h        |    2 ++
 include/linux/device.h         |    2 ++
 5 files changed, 80 insertions(+), 3 deletions(-)

-- 
1.7.9

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-15 10:22 ` Vasilis Liaskovitis
  0 siblings, 0 replies; 36+ messages in thread
From: Vasilis Liaskovitis @ 2012-11-15 10:22 UTC (permalink / raw)
  To: linux-acpi, isimatu.yasuaki, wency
  Cc: rjw, lenb, toshi.kani, linux-kernel, linux-mm, Vasilis Liaskovitis

As discussed in https://patchwork.kernel.org/patch/1581581/
the driver core remove function needs to always succeed. This means we need
to know that the device can be successfully removed before acpi_bus_trim / 
acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
eject or driver unbind of memory devices fails e.g with:

echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind

since the ACPI core goes ahead and ejects the device regardless of whether the
the memory is still in use or not.

For this reason a new acpi_device operation called prepare_remove is introduced.
This operation should be registered for acpi devices whose removal (from kernel
perspective) can fail.  Memory devices fall in this category.
A similar operation is introduced in bus_type to safely handle driver unbind
from the device driver core.

acpi_bus_hot_remove_device and driver_unbind are changed to handle removal in 2
steps:
- preparation for removal i.e. perform part of removal that can fail. Should
  succeed for device and all its children.
- if above step was successfull, proceed to actual device removal

With this patchset, only acpi memory devices use the new prepare_remove
device operation. The actual memory removal (VM-related offline and other memory
cleanups) is moved to prepare_remove. The old remove operation just cleans up
the acpi structures. Directly ejecting PNP0C80 memory devices works safely. I
haven't tested yet with an ACPI container which contains memory devices.

v1->v2:
- new patch to introduce bus_type prepare_remove callback. Needed to prepare
removal on driver unbinding from device-driver core.
- v1 patches 1 and 2 simplified and merged in one. acpi_bus_trim does not require
argument changes.

Comments welcome.

Vasilis Liaskovitis (3):
  driver core: Introduce prepare_remove in bus_type
  acpi: Introduce prepare_remove operation in acpi_device_ops
  acpi_memhotplug: Add prepare_remove operation

 drivers/acpi/acpi_memhotplug.c |   22 ++++++++++++++++++++--
 drivers/acpi/scan.c            |   21 ++++++++++++++++++++-
 drivers/base/bus.c             |   36 ++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h        |    2 ++
 include/linux/device.h         |    2 ++
 5 files changed, 80 insertions(+), 3 deletions(-)

-- 
1.7.9


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

* [RFC PATCH v2 1/3] driver core: Introduce prepare_remove in bus_type
  2012-11-15 10:22 ` Vasilis Liaskovitis
@ 2012-11-15 10:22   ` Vasilis Liaskovitis
  -1 siblings, 0 replies; 36+ messages in thread
From: Vasilis Liaskovitis @ 2012-11-15 10:22 UTC (permalink / raw)
  To: linux-acpi, isimatu.yasuaki, wency
  Cc: rjw, lenb, toshi.kani, linux-kernel, linux-mm, Vasilis Liaskovitis

This function will call a bus-specific prepare_remove callback. If this call
is not successful, the device cannot be safely removed, or the driver cannot be
safely unbound.

This operation is needed to safely execute OSPM-induced unbind or rebind of ACPI
memory devices e.g.

echo "PNP0C80:00" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind

driver_unbind and device_reprobe will use the new callback before calling
device_release_driver()

PROBLEM: bus_remove_device and bus_remove_driver also call device_release_driver
but these functions always succeed under the core device-driver model i.e. there
is no possibility of failure. These functions do not call the prepare_remove
callback currently. This creates an unwanted assymetry between device/driver
removal and driver unbinding. Suggestions to fix welcome.

Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
---
 drivers/base/bus.c     |   36 ++++++++++++++++++++++++++++++++++++
 include/linux/device.h |    2 ++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 181ed26..c5dad55 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -34,6 +34,7 @@ static struct kset *system_kset;
 
 static int __must_check bus_rescan_devices_helper(struct device *dev,
 						void *data);
+static int bus_prepare_remove_device(struct device *dev);
 
 static struct bus_type *bus_get(struct bus_type *bus)
 {
@@ -178,11 +179,18 @@ static ssize_t driver_unbind(struct device_driver *drv,
 	if (dev && dev->driver == drv) {
 		if (dev->parent)	/* Needed for USB */
 			device_lock(dev->parent);
+		err = bus_prepare_remove_device(dev);
+		if (err) {
+			if (dev->parent)
+				device_unlock(dev->parent);
+			goto out;
+		}
 		device_release_driver(dev);
 		if (dev->parent)
 			device_unlock(dev->parent);
 		err = count;
 	}
+out:
 	put_device(dev);
 	bus_put(bus);
 	return err;
@@ -587,6 +595,26 @@ void bus_remove_device(struct device *dev)
 	bus_put(dev->bus);
 }
 
+/**
+ * device_prepare_release_driver - call driver specific operations to prepare
+ * for manually detaching device from driver.
+ * @dev: device.
+ *
+ * Prepare for detaching device from driver.
+ * When called for a USB interface, @dev->parent lock must be held.
+ * This function returns 0 if preparation is successful, non-zero error value
+ * otherwise.
+ */
+static int bus_prepare_remove_device(struct device *dev)
+{
+	int ret = 0;
+	device_lock(dev);
+	if (dev->bus)
+		ret = dev->bus->prepare_remove(dev);
+	device_unlock(dev);
+	return ret;
+}
+
 static int driver_add_attrs(struct bus_type *bus, struct device_driver *drv)
 {
 	int error = 0;
@@ -820,9 +848,17 @@ EXPORT_SYMBOL_GPL(bus_rescan_devices);
  */
 int device_reprobe(struct device *dev)
 {
+	int ret;
+
 	if (dev->driver) {
 		if (dev->parent)        /* Needed for USB */
 			device_lock(dev->parent);
+		ret = bus_prepare_remove_device(dev);
+		if (ret) {
+			if (dev->parent)
+				device_unlock(dev->parent);
+			return ret;
+		}
 		device_release_driver(dev);
 		if (dev->parent)
 			device_unlock(dev->parent);
diff --git a/include/linux/device.h b/include/linux/device.h
index cc3aee5..8e7055b 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -104,6 +104,7 @@ struct bus_type {
 
 	int (*suspend)(struct device *dev, pm_message_t state);
 	int (*resume)(struct device *dev);
+	int (*prepare_remove) (struct device *dev);
 
 	const struct dev_pm_ops *pm;
 
@@ -853,6 +854,7 @@ extern void device_release_driver(struct device *dev);
 extern int  __must_check device_attach(struct device *dev);
 extern int __must_check driver_attach(struct device_driver *drv);
 extern int __must_check device_reprobe(struct device *dev);
+extern int device_prepare_release_driver(struct device *dev);
 
 /*
  * Easy functions for dynamically creating devices on the fly
-- 
1.7.9

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [RFC PATCH v2 1/3] driver core: Introduce prepare_remove in bus_type
@ 2012-11-15 10:22   ` Vasilis Liaskovitis
  0 siblings, 0 replies; 36+ messages in thread
From: Vasilis Liaskovitis @ 2012-11-15 10:22 UTC (permalink / raw)
  To: linux-acpi, isimatu.yasuaki, wency
  Cc: rjw, lenb, toshi.kani, linux-kernel, linux-mm, Vasilis Liaskovitis

This function will call a bus-specific prepare_remove callback. If this call
is not successful, the device cannot be safely removed, or the driver cannot be
safely unbound.

This operation is needed to safely execute OSPM-induced unbind or rebind of ACPI
memory devices e.g.

echo "PNP0C80:00" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind

driver_unbind and device_reprobe will use the new callback before calling
device_release_driver()

PROBLEM: bus_remove_device and bus_remove_driver also call device_release_driver
but these functions always succeed under the core device-driver model i.e. there
is no possibility of failure. These functions do not call the prepare_remove
callback currently. This creates an unwanted assymetry between device/driver
removal and driver unbinding. Suggestions to fix welcome.

Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
---
 drivers/base/bus.c     |   36 ++++++++++++++++++++++++++++++++++++
 include/linux/device.h |    2 ++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 181ed26..c5dad55 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -34,6 +34,7 @@ static struct kset *system_kset;
 
 static int __must_check bus_rescan_devices_helper(struct device *dev,
 						void *data);
+static int bus_prepare_remove_device(struct device *dev);
 
 static struct bus_type *bus_get(struct bus_type *bus)
 {
@@ -178,11 +179,18 @@ static ssize_t driver_unbind(struct device_driver *drv,
 	if (dev && dev->driver == drv) {
 		if (dev->parent)	/* Needed for USB */
 			device_lock(dev->parent);
+		err = bus_prepare_remove_device(dev);
+		if (err) {
+			if (dev->parent)
+				device_unlock(dev->parent);
+			goto out;
+		}
 		device_release_driver(dev);
 		if (dev->parent)
 			device_unlock(dev->parent);
 		err = count;
 	}
+out:
 	put_device(dev);
 	bus_put(bus);
 	return err;
@@ -587,6 +595,26 @@ void bus_remove_device(struct device *dev)
 	bus_put(dev->bus);
 }
 
+/**
+ * device_prepare_release_driver - call driver specific operations to prepare
+ * for manually detaching device from driver.
+ * @dev: device.
+ *
+ * Prepare for detaching device from driver.
+ * When called for a USB interface, @dev->parent lock must be held.
+ * This function returns 0 if preparation is successful, non-zero error value
+ * otherwise.
+ */
+static int bus_prepare_remove_device(struct device *dev)
+{
+	int ret = 0;
+	device_lock(dev);
+	if (dev->bus)
+		ret = dev->bus->prepare_remove(dev);
+	device_unlock(dev);
+	return ret;
+}
+
 static int driver_add_attrs(struct bus_type *bus, struct device_driver *drv)
 {
 	int error = 0;
@@ -820,9 +848,17 @@ EXPORT_SYMBOL_GPL(bus_rescan_devices);
  */
 int device_reprobe(struct device *dev)
 {
+	int ret;
+
 	if (dev->driver) {
 		if (dev->parent)        /* Needed for USB */
 			device_lock(dev->parent);
+		ret = bus_prepare_remove_device(dev);
+		if (ret) {
+			if (dev->parent)
+				device_unlock(dev->parent);
+			return ret;
+		}
 		device_release_driver(dev);
 		if (dev->parent)
 			device_unlock(dev->parent);
diff --git a/include/linux/device.h b/include/linux/device.h
index cc3aee5..8e7055b 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -104,6 +104,7 @@ struct bus_type {
 
 	int (*suspend)(struct device *dev, pm_message_t state);
 	int (*resume)(struct device *dev);
+	int (*prepare_remove) (struct device *dev);
 
 	const struct dev_pm_ops *pm;
 
@@ -853,6 +854,7 @@ extern void device_release_driver(struct device *dev);
 extern int  __must_check device_attach(struct device *dev);
 extern int __must_check driver_attach(struct device_driver *drv);
 extern int __must_check device_reprobe(struct device *dev);
+extern int device_prepare_release_driver(struct device *dev);
 
 /*
  * Easy functions for dynamically creating devices on the fly
-- 
1.7.9


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

* [RFC PATCH v2 2/3] acpi: Introduce prepare_remove operation in acpi_device_ops
  2012-11-15 10:22 ` Vasilis Liaskovitis
@ 2012-11-15 10:22   ` Vasilis Liaskovitis
  -1 siblings, 0 replies; 36+ messages in thread
From: Vasilis Liaskovitis @ 2012-11-15 10:22 UTC (permalink / raw)
  To: linux-acpi, isimatu.yasuaki, wency
  Cc: rjw, lenb, toshi.kani, linux-kernel, linux-mm, Vasilis Liaskovitis

This function should be registered for devices that need to execute some
non-driver core/acpi related action in order to be safely removed. If
the removal preparation is successful, the acpi/driver core can continue with
removing the device.

Make acpi_bus_remove call the device-specific prepare_remove callback before
removing the device. If prepare_remove fails, the removal is aborted.

Also introduce acpi_device_prepare_remove which will call the device-specific
prepare_remove callback on driver unbind or device reprobe requests from the
device-driver core.

Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
---
 drivers/acpi/scan.c     |   21 ++++++++++++++++++++-
 include/acpi/acpi_bus.h |    2 ++
 2 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 95ff1e8..725b012 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -582,11 +582,23 @@ static int acpi_device_remove(struct device * dev)
 	return 0;
 }
 
+static int acpi_device_prepare_remove(struct device *dev)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+	struct acpi_driver *acpi_drv = acpi_dev->driver;
+	int ret = 0;
+
+	if (acpi_drv && acpi_drv->ops.prepare_remove)
+		ret = acpi_drv->ops.prepare_remove(acpi_dev);
+	return ret;
+}
+
 struct bus_type acpi_bus_type = {
 	.name		= "acpi",
 	.match		= acpi_bus_match,
 	.probe		= acpi_device_probe,
 	.remove		= acpi_device_remove,
+	.prepare_remove	= acpi_device_prepare_remove,
 	.uevent		= acpi_device_uevent,
 };
 
@@ -1349,10 +1361,16 @@ static int acpi_device_set_context(struct acpi_device *device)
 
 static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
 {
+	int ret = 0;
 	if (!dev)
 		return -EINVAL;
 
 	dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
+
+	if (dev->driver && dev->driver->ops.prepare_remove)
+		ret = dev->driver->ops.prepare_remove(dev);
+	if (ret)
+		return ret;
 	device_release_driver(&dev->dev);
 
 	if (!rmdevice)
@@ -1671,7 +1689,8 @@ int acpi_bus_trim(struct acpi_device *start, int rmdevice)
 				err = acpi_bus_remove(child, rmdevice);
 			else
 				err = acpi_bus_remove(child, 1);
-
+			if (err)
+				return err;
 			continue;
 		}
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e04ce7b..1a13c82 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -94,6 +94,7 @@ typedef int (*acpi_op_start) (struct acpi_device * device);
 typedef int (*acpi_op_bind) (struct acpi_device * device);
 typedef int (*acpi_op_unbind) (struct acpi_device * device);
 typedef void (*acpi_op_notify) (struct acpi_device * device, u32 event);
+typedef int (*acpi_op_prepare_remove) (struct acpi_device *device);
 
 struct acpi_bus_ops {
 	u32 acpi_op_add:1;
@@ -107,6 +108,7 @@ struct acpi_device_ops {
 	acpi_op_bind bind;
 	acpi_op_unbind unbind;
 	acpi_op_notify notify;
+	acpi_op_prepare_remove prepare_remove;
 };
 
 #define ACPI_DRIVER_ALL_NOTIFY_EVENTS	0x1	/* system AND device events */
-- 
1.7.9

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [RFC PATCH v2 2/3] acpi: Introduce prepare_remove operation in acpi_device_ops
@ 2012-11-15 10:22   ` Vasilis Liaskovitis
  0 siblings, 0 replies; 36+ messages in thread
From: Vasilis Liaskovitis @ 2012-11-15 10:22 UTC (permalink / raw)
  To: linux-acpi, isimatu.yasuaki, wency
  Cc: rjw, lenb, toshi.kani, linux-kernel, linux-mm, Vasilis Liaskovitis

This function should be registered for devices that need to execute some
non-driver core/acpi related action in order to be safely removed. If
the removal preparation is successful, the acpi/driver core can continue with
removing the device.

Make acpi_bus_remove call the device-specific prepare_remove callback before
removing the device. If prepare_remove fails, the removal is aborted.

Also introduce acpi_device_prepare_remove which will call the device-specific
prepare_remove callback on driver unbind or device reprobe requests from the
device-driver core.

Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
---
 drivers/acpi/scan.c     |   21 ++++++++++++++++++++-
 include/acpi/acpi_bus.h |    2 ++
 2 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 95ff1e8..725b012 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -582,11 +582,23 @@ static int acpi_device_remove(struct device * dev)
 	return 0;
 }
 
+static int acpi_device_prepare_remove(struct device *dev)
+{
+	struct acpi_device *acpi_dev = to_acpi_device(dev);
+	struct acpi_driver *acpi_drv = acpi_dev->driver;
+	int ret = 0;
+
+	if (acpi_drv && acpi_drv->ops.prepare_remove)
+		ret = acpi_drv->ops.prepare_remove(acpi_dev);
+	return ret;
+}
+
 struct bus_type acpi_bus_type = {
 	.name		= "acpi",
 	.match		= acpi_bus_match,
 	.probe		= acpi_device_probe,
 	.remove		= acpi_device_remove,
+	.prepare_remove	= acpi_device_prepare_remove,
 	.uevent		= acpi_device_uevent,
 };
 
@@ -1349,10 +1361,16 @@ static int acpi_device_set_context(struct acpi_device *device)
 
 static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
 {
+	int ret = 0;
 	if (!dev)
 		return -EINVAL;
 
 	dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
+
+	if (dev->driver && dev->driver->ops.prepare_remove)
+		ret = dev->driver->ops.prepare_remove(dev);
+	if (ret)
+		return ret;
 	device_release_driver(&dev->dev);
 
 	if (!rmdevice)
@@ -1671,7 +1689,8 @@ int acpi_bus_trim(struct acpi_device *start, int rmdevice)
 				err = acpi_bus_remove(child, rmdevice);
 			else
 				err = acpi_bus_remove(child, 1);
-
+			if (err)
+				return err;
 			continue;
 		}
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e04ce7b..1a13c82 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -94,6 +94,7 @@ typedef int (*acpi_op_start) (struct acpi_device * device);
 typedef int (*acpi_op_bind) (struct acpi_device * device);
 typedef int (*acpi_op_unbind) (struct acpi_device * device);
 typedef void (*acpi_op_notify) (struct acpi_device * device, u32 event);
+typedef int (*acpi_op_prepare_remove) (struct acpi_device *device);
 
 struct acpi_bus_ops {
 	u32 acpi_op_add:1;
@@ -107,6 +108,7 @@ struct acpi_device_ops {
 	acpi_op_bind bind;
 	acpi_op_unbind unbind;
 	acpi_op_notify notify;
+	acpi_op_prepare_remove prepare_remove;
 };
 
 #define ACPI_DRIVER_ALL_NOTIFY_EVENTS	0x1	/* system AND device events */
-- 
1.7.9


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

* [RFC PATCH v2 3/3] acpi_memhotplug: Add prepare_remove operation
  2012-11-15 10:22 ` Vasilis Liaskovitis
@ 2012-11-15 10:22   ` Vasilis Liaskovitis
  -1 siblings, 0 replies; 36+ messages in thread
From: Vasilis Liaskovitis @ 2012-11-15 10:22 UTC (permalink / raw)
  To: linux-acpi, isimatu.yasuaki, wency
  Cc: rjw, lenb, toshi.kani, linux-kernel, linux-mm, Vasilis Liaskovitis

Offlining and removal of memory is now done in the prepare_remove callback,
not in the remove callback.

The prepare_remove callback will be called when trying to remove a memory device
with the following ways:

1. send eject request by SCI
2. echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
3. echo "PNP0C80:00" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind

Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
---
 drivers/acpi/acpi_memhotplug.c |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 92c973a..8615ff3 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -54,6 +54,7 @@ MODULE_LICENSE("GPL");
 
 static int acpi_memory_device_add(struct acpi_device *device);
 static int acpi_memory_device_remove(struct acpi_device *device, int type);
+static int acpi_memory_device_prepare_remove(struct acpi_device *device);
 
 static const struct acpi_device_id memory_device_ids[] = {
 	{ACPI_MEMORY_DEVICE_HID, 0},
@@ -68,6 +69,7 @@ static struct acpi_driver acpi_memory_device_driver = {
 	.ops = {
 		.add = acpi_memory_device_add,
 		.remove = acpi_memory_device_remove,
+		.prepare_remove = acpi_memory_device_prepare_remove,
 		},
 };
 
@@ -499,6 +501,20 @@ static int acpi_memory_device_add(struct acpi_device *device)
 static int acpi_memory_device_remove(struct acpi_device *device, int type)
 {
 	struct acpi_memory_device *mem_device = NULL;
+
+	if (!device || !acpi_driver_data(device))
+		return -EINVAL;
+
+	mem_device = acpi_driver_data(device);
+
+	kfree(mem_device);
+
+	return 0;
+}
+
+static int acpi_memory_device_prepare_remove(struct acpi_device *device)
+{
+	struct acpi_memory_device *mem_device = NULL;
 	int result;
 
 	if (!device || !acpi_driver_data(device))
@@ -506,12 +522,14 @@ static int acpi_memory_device_remove(struct acpi_device *device, int type)
 
 	mem_device = acpi_driver_data(device);
 
+	/*
+	 * offline and remove memory only when the memory device is
+	 * ejected.
+	 */
 	result = acpi_memory_remove_memory(mem_device);
 	if (result)
 		return result;
 
-	kfree(mem_device);
-
 	return 0;
 }
 
-- 
1.7.9

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [RFC PATCH v2 3/3] acpi_memhotplug: Add prepare_remove operation
@ 2012-11-15 10:22   ` Vasilis Liaskovitis
  0 siblings, 0 replies; 36+ messages in thread
From: Vasilis Liaskovitis @ 2012-11-15 10:22 UTC (permalink / raw)
  To: linux-acpi, isimatu.yasuaki, wency
  Cc: rjw, lenb, toshi.kani, linux-kernel, linux-mm, Vasilis Liaskovitis

Offlining and removal of memory is now done in the prepare_remove callback,
not in the remove callback.

The prepare_remove callback will be called when trying to remove a memory device
with the following ways:

1. send eject request by SCI
2. echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
3. echo "PNP0C80:00" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind

Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
---
 drivers/acpi/acpi_memhotplug.c |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 92c973a..8615ff3 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -54,6 +54,7 @@ MODULE_LICENSE("GPL");
 
 static int acpi_memory_device_add(struct acpi_device *device);
 static int acpi_memory_device_remove(struct acpi_device *device, int type);
+static int acpi_memory_device_prepare_remove(struct acpi_device *device);
 
 static const struct acpi_device_id memory_device_ids[] = {
 	{ACPI_MEMORY_DEVICE_HID, 0},
@@ -68,6 +69,7 @@ static struct acpi_driver acpi_memory_device_driver = {
 	.ops = {
 		.add = acpi_memory_device_add,
 		.remove = acpi_memory_device_remove,
+		.prepare_remove = acpi_memory_device_prepare_remove,
 		},
 };
 
@@ -499,6 +501,20 @@ static int acpi_memory_device_add(struct acpi_device *device)
 static int acpi_memory_device_remove(struct acpi_device *device, int type)
 {
 	struct acpi_memory_device *mem_device = NULL;
+
+	if (!device || !acpi_driver_data(device))
+		return -EINVAL;
+
+	mem_device = acpi_driver_data(device);
+
+	kfree(mem_device);
+
+	return 0;
+}
+
+static int acpi_memory_device_prepare_remove(struct acpi_device *device)
+{
+	struct acpi_memory_device *mem_device = NULL;
 	int result;
 
 	if (!device || !acpi_driver_data(device))
@@ -506,12 +522,14 @@ static int acpi_memory_device_remove(struct acpi_device *device, int type)
 
 	mem_device = acpi_driver_data(device);
 
+	/*
+	 * offline and remove memory only when the memory device is
+	 * ejected.
+	 */
 	result = acpi_memory_remove_memory(mem_device);
 	if (result)
 		return result;
 
-	kfree(mem_device);
-
 	return 0;
 }
 
-- 
1.7.9


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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-15 10:22 ` Vasilis Liaskovitis
@ 2012-11-16 21:17   ` Rafael J. Wysocki
  -1 siblings, 0 replies; 36+ messages in thread
From: Rafael J. Wysocki @ 2012-11-16 21:17 UTC (permalink / raw)
  To: Vasilis Liaskovitis
  Cc: linux-acpi, isimatu.yasuaki, wency, lenb, toshi.kani,
	linux-kernel, linux-mm, Greg Kroah-Hartman

On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> As discussed in https://patchwork.kernel.org/patch/1581581/
> the driver core remove function needs to always succeed. This means we need
> to know that the device can be successfully removed before acpi_bus_trim / 
> acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> eject or driver unbind of memory devices fails e.g with:
> 
> echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> 
> since the ACPI core goes ahead and ejects the device regardless of whether the
> the memory is still in use or not.
> 
> For this reason a new acpi_device operation called prepare_remove is introduced.
> This operation should be registered for acpi devices whose removal (from kernel
> perspective) can fail.  Memory devices fall in this category.
> A similar operation is introduced in bus_type to safely handle driver unbind
> from the device driver core.
> 
> acpi_bus_hot_remove_device and driver_unbind are changed to handle removal in 2
> steps:
> - preparation for removal i.e. perform part of removal that can fail. Should
>   succeed for device and all its children.
> - if above step was successfull, proceed to actual device removal
> 
> With this patchset, only acpi memory devices use the new prepare_remove
> device operation. The actual memory removal (VM-related offline and other memory
> cleanups) is moved to prepare_remove. The old remove operation just cleans up
> the acpi structures. Directly ejecting PNP0C80 memory devices works safely. I
> haven't tested yet with an ACPI container which contains memory devices.
> 
> v1->v2:
> - new patch to introduce bus_type prepare_remove callback. Needed to prepare
> removal on driver unbinding from device-driver core.
> - v1 patches 1 and 2 simplified and merged in one. acpi_bus_trim does not require
> argument changes.
> 
> Comments welcome.
> 
> Vasilis Liaskovitis (3):
>   driver core: Introduce prepare_remove in bus_type
>   acpi: Introduce prepare_remove operation in acpi_device_ops
>   acpi_memhotplug: Add prepare_remove operation
> 
>  drivers/acpi/acpi_memhotplug.c |   22 ++++++++++++++++++++--
>  drivers/acpi/scan.c            |   21 ++++++++++++++++++++-
>  drivers/base/bus.c             |   36 ++++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_bus.h        |    2 ++
>  include/linux/device.h         |    2 ++
>  5 files changed, 80 insertions(+), 3 deletions(-)

CCs of all driver core patches have to go to Greg Kroah-Hartman.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-16 21:17   ` Rafael J. Wysocki
  0 siblings, 0 replies; 36+ messages in thread
From: Rafael J. Wysocki @ 2012-11-16 21:17 UTC (permalink / raw)
  To: Vasilis Liaskovitis
  Cc: linux-acpi, isimatu.yasuaki, wency, lenb, toshi.kani,
	linux-kernel, linux-mm, Greg Kroah-Hartman

On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> As discussed in https://patchwork.kernel.org/patch/1581581/
> the driver core remove function needs to always succeed. This means we need
> to know that the device can be successfully removed before acpi_bus_trim / 
> acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> eject or driver unbind of memory devices fails e.g with:
> 
> echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> 
> since the ACPI core goes ahead and ejects the device regardless of whether the
> the memory is still in use or not.
> 
> For this reason a new acpi_device operation called prepare_remove is introduced.
> This operation should be registered for acpi devices whose removal (from kernel
> perspective) can fail.  Memory devices fall in this category.
> A similar operation is introduced in bus_type to safely handle driver unbind
> from the device driver core.
> 
> acpi_bus_hot_remove_device and driver_unbind are changed to handle removal in 2
> steps:
> - preparation for removal i.e. perform part of removal that can fail. Should
>   succeed for device and all its children.
> - if above step was successfull, proceed to actual device removal
> 
> With this patchset, only acpi memory devices use the new prepare_remove
> device operation. The actual memory removal (VM-related offline and other memory
> cleanups) is moved to prepare_remove. The old remove operation just cleans up
> the acpi structures. Directly ejecting PNP0C80 memory devices works safely. I
> haven't tested yet with an ACPI container which contains memory devices.
> 
> v1->v2:
> - new patch to introduce bus_type prepare_remove callback. Needed to prepare
> removal on driver unbinding from device-driver core.
> - v1 patches 1 and 2 simplified and merged in one. acpi_bus_trim does not require
> argument changes.
> 
> Comments welcome.
> 
> Vasilis Liaskovitis (3):
>   driver core: Introduce prepare_remove in bus_type
>   acpi: Introduce prepare_remove operation in acpi_device_ops
>   acpi_memhotplug: Add prepare_remove operation
> 
>  drivers/acpi/acpi_memhotplug.c |   22 ++++++++++++++++++++--
>  drivers/acpi/scan.c            |   21 ++++++++++++++++++++-
>  drivers/base/bus.c             |   36 ++++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_bus.h        |    2 ++
>  include/linux/device.h         |    2 ++
>  5 files changed, 80 insertions(+), 3 deletions(-)

CCs of all driver core patches have to go to Greg Kroah-Hartman.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-16 21:17   ` Rafael J. Wysocki
@ 2012-11-16 21:33     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-16 21:33 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Vasilis Liaskovitis, linux-acpi, isimatu.yasuaki, wency, lenb,
	toshi.kani, linux-kernel, linux-mm

On Fri, Nov 16, 2012 at 10:17:17PM +0100, Rafael J. Wysocki wrote:
> On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > As discussed in https://patchwork.kernel.org/patch/1581581/
> > the driver core remove function needs to always succeed. This means we need
> > to know that the device can be successfully removed before acpi_bus_trim / 
> > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > eject or driver unbind of memory devices fails e.g with:
> > 
> > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > 
> > since the ACPI core goes ahead and ejects the device regardless of whether the
> > the memory is still in use or not.
> > 
> > For this reason a new acpi_device operation called prepare_remove is introduced.
> > This operation should be registered for acpi devices whose removal (from kernel
> > perspective) can fail.  Memory devices fall in this category.
> > A similar operation is introduced in bus_type to safely handle driver unbind
> > from the device driver core.
> > 
> > acpi_bus_hot_remove_device and driver_unbind are changed to handle removal in 2
> > steps:
> > - preparation for removal i.e. perform part of removal that can fail. Should
> >   succeed for device and all its children.
> > - if above step was successfull, proceed to actual device removal
> > 
> > With this patchset, only acpi memory devices use the new prepare_remove
> > device operation. The actual memory removal (VM-related offline and other memory
> > cleanups) is moved to prepare_remove. The old remove operation just cleans up
> > the acpi structures. Directly ejecting PNP0C80 memory devices works safely. I
> > haven't tested yet with an ACPI container which contains memory devices.
> > 
> > v1->v2:
> > - new patch to introduce bus_type prepare_remove callback. Needed to prepare
> > removal on driver unbinding from device-driver core.
> > - v1 patches 1 and 2 simplified and merged in one. acpi_bus_trim does not require
> > argument changes.
> > 
> > Comments welcome.
> > 
> > Vasilis Liaskovitis (3):
> >   driver core: Introduce prepare_remove in bus_type
> >   acpi: Introduce prepare_remove operation in acpi_device_ops
> >   acpi_memhotplug: Add prepare_remove operation
> > 
> >  drivers/acpi/acpi_memhotplug.c |   22 ++++++++++++++++++++--
> >  drivers/acpi/scan.c            |   21 ++++++++++++++++++++-
> >  drivers/base/bus.c             |   36 ++++++++++++++++++++++++++++++++++++
> >  include/acpi/acpi_bus.h        |    2 ++
> >  include/linux/device.h         |    2 ++
> >  5 files changed, 80 insertions(+), 3 deletions(-)
> 
> CCs of all driver core patches have to go to Greg Kroah-Hartman.

I previously rejected this, so I don't see why I would take it this time
around :(

Please, no driver core changes for acpi, I don't see why it is suddenly
so special to need stuff like this that can't just be done in the ACPI
bus code itself.

thanks,

greg k-h

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-16 21:33     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-16 21:33 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Vasilis Liaskovitis, linux-acpi, isimatu.yasuaki, wency, lenb,
	toshi.kani, linux-kernel, linux-mm

On Fri, Nov 16, 2012 at 10:17:17PM +0100, Rafael J. Wysocki wrote:
> On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > As discussed in https://patchwork.kernel.org/patch/1581581/
> > the driver core remove function needs to always succeed. This means we need
> > to know that the device can be successfully removed before acpi_bus_trim / 
> > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > eject or driver unbind of memory devices fails e.g with:
> > 
> > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > 
> > since the ACPI core goes ahead and ejects the device regardless of whether the
> > the memory is still in use or not.
> > 
> > For this reason a new acpi_device operation called prepare_remove is introduced.
> > This operation should be registered for acpi devices whose removal (from kernel
> > perspective) can fail.  Memory devices fall in this category.
> > A similar operation is introduced in bus_type to safely handle driver unbind
> > from the device driver core.
> > 
> > acpi_bus_hot_remove_device and driver_unbind are changed to handle removal in 2
> > steps:
> > - preparation for removal i.e. perform part of removal that can fail. Should
> >   succeed for device and all its children.
> > - if above step was successfull, proceed to actual device removal
> > 
> > With this patchset, only acpi memory devices use the new prepare_remove
> > device operation. The actual memory removal (VM-related offline and other memory
> > cleanups) is moved to prepare_remove. The old remove operation just cleans up
> > the acpi structures. Directly ejecting PNP0C80 memory devices works safely. I
> > haven't tested yet with an ACPI container which contains memory devices.
> > 
> > v1->v2:
> > - new patch to introduce bus_type prepare_remove callback. Needed to prepare
> > removal on driver unbinding from device-driver core.
> > - v1 patches 1 and 2 simplified and merged in one. acpi_bus_trim does not require
> > argument changes.
> > 
> > Comments welcome.
> > 
> > Vasilis Liaskovitis (3):
> >   driver core: Introduce prepare_remove in bus_type
> >   acpi: Introduce prepare_remove operation in acpi_device_ops
> >   acpi_memhotplug: Add prepare_remove operation
> > 
> >  drivers/acpi/acpi_memhotplug.c |   22 ++++++++++++++++++++--
> >  drivers/acpi/scan.c            |   21 ++++++++++++++++++++-
> >  drivers/base/bus.c             |   36 ++++++++++++++++++++++++++++++++++++
> >  include/acpi/acpi_bus.h        |    2 ++
> >  include/linux/device.h         |    2 ++
> >  5 files changed, 80 insertions(+), 3 deletions(-)
> 
> CCs of all driver core patches have to go to Greg Kroah-Hartman.

I previously rejected this, so I don't see why I would take it this time
around :(

Please, no driver core changes for acpi, I don't see why it is suddenly
so special to need stuff like this that can't just be done in the ACPI
bus code itself.

thanks,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-16 21:33     ` Greg Kroah-Hartman
@ 2012-11-16 21:41       ` Rafael J. Wysocki
  -1 siblings, 0 replies; 36+ messages in thread
From: Rafael J. Wysocki @ 2012-11-16 21:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vasilis Liaskovitis, linux-acpi, isimatu.yasuaki, wency, lenb,
	toshi.kani, linux-kernel, linux-mm

On Friday, November 16, 2012 01:33:10 PM Greg Kroah-Hartman wrote:
> On Fri, Nov 16, 2012 at 10:17:17PM +0100, Rafael J. Wysocki wrote:
> > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > the driver core remove function needs to always succeed. This means we need
> > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > eject or driver unbind of memory devices fails e.g with:
> > > 
> > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > 
> > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > the memory is still in use or not.
> > > 
> > > For this reason a new acpi_device operation called prepare_remove is introduced.
> > > This operation should be registered for acpi devices whose removal (from kernel
> > > perspective) can fail.  Memory devices fall in this category.
> > > A similar operation is introduced in bus_type to safely handle driver unbind
> > > from the device driver core.
> > > 
> > > acpi_bus_hot_remove_device and driver_unbind are changed to handle removal in 2
> > > steps:
> > > - preparation for removal i.e. perform part of removal that can fail. Should
> > >   succeed for device and all its children.
> > > - if above step was successfull, proceed to actual device removal
> > > 
> > > With this patchset, only acpi memory devices use the new prepare_remove
> > > device operation. The actual memory removal (VM-related offline and other memory
> > > cleanups) is moved to prepare_remove. The old remove operation just cleans up
> > > the acpi structures. Directly ejecting PNP0C80 memory devices works safely. I
> > > haven't tested yet with an ACPI container which contains memory devices.
> > > 
> > > v1->v2:
> > > - new patch to introduce bus_type prepare_remove callback. Needed to prepare
> > > removal on driver unbinding from device-driver core.
> > > - v1 patches 1 and 2 simplified and merged in one. acpi_bus_trim does not require
> > > argument changes.
> > > 
> > > Comments welcome.
> > > 
> > > Vasilis Liaskovitis (3):
> > >   driver core: Introduce prepare_remove in bus_type
> > >   acpi: Introduce prepare_remove operation in acpi_device_ops
> > >   acpi_memhotplug: Add prepare_remove operation
> > > 
> > >  drivers/acpi/acpi_memhotplug.c |   22 ++++++++++++++++++++--
> > >  drivers/acpi/scan.c            |   21 ++++++++++++++++++++-
> > >  drivers/base/bus.c             |   36 ++++++++++++++++++++++++++++++++++++
> > >  include/acpi/acpi_bus.h        |    2 ++
> > >  include/linux/device.h         |    2 ++
> > >  5 files changed, 80 insertions(+), 3 deletions(-)
> > 
> > CCs of all driver core patches have to go to Greg Kroah-Hartman.
> 
> I previously rejected this, so I don't see why I would take it this time
> around :(
> 
> Please, no driver core changes for acpi, I don't see why it is suddenly
> so special to need stuff like this that can't just be done in the ACPI
> bus code itself.

OK, OK, that was just a notice to the author. :-)

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-16 21:41       ` Rafael J. Wysocki
  0 siblings, 0 replies; 36+ messages in thread
From: Rafael J. Wysocki @ 2012-11-16 21:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vasilis Liaskovitis, linux-acpi, isimatu.yasuaki, wency, lenb,
	toshi.kani, linux-kernel, linux-mm

On Friday, November 16, 2012 01:33:10 PM Greg Kroah-Hartman wrote:
> On Fri, Nov 16, 2012 at 10:17:17PM +0100, Rafael J. Wysocki wrote:
> > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > the driver core remove function needs to always succeed. This means we need
> > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > eject or driver unbind of memory devices fails e.g with:
> > > 
> > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > 
> > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > the memory is still in use or not.
> > > 
> > > For this reason a new acpi_device operation called prepare_remove is introduced.
> > > This operation should be registered for acpi devices whose removal (from kernel
> > > perspective) can fail.  Memory devices fall in this category.
> > > A similar operation is introduced in bus_type to safely handle driver unbind
> > > from the device driver core.
> > > 
> > > acpi_bus_hot_remove_device and driver_unbind are changed to handle removal in 2
> > > steps:
> > > - preparation for removal i.e. perform part of removal that can fail. Should
> > >   succeed for device and all its children.
> > > - if above step was successfull, proceed to actual device removal
> > > 
> > > With this patchset, only acpi memory devices use the new prepare_remove
> > > device operation. The actual memory removal (VM-related offline and other memory
> > > cleanups) is moved to prepare_remove. The old remove operation just cleans up
> > > the acpi structures. Directly ejecting PNP0C80 memory devices works safely. I
> > > haven't tested yet with an ACPI container which contains memory devices.
> > > 
> > > v1->v2:
> > > - new patch to introduce bus_type prepare_remove callback. Needed to prepare
> > > removal on driver unbinding from device-driver core.
> > > - v1 patches 1 and 2 simplified and merged in one. acpi_bus_trim does not require
> > > argument changes.
> > > 
> > > Comments welcome.
> > > 
> > > Vasilis Liaskovitis (3):
> > >   driver core: Introduce prepare_remove in bus_type
> > >   acpi: Introduce prepare_remove operation in acpi_device_ops
> > >   acpi_memhotplug: Add prepare_remove operation
> > > 
> > >  drivers/acpi/acpi_memhotplug.c |   22 ++++++++++++++++++++--
> > >  drivers/acpi/scan.c            |   21 ++++++++++++++++++++-
> > >  drivers/base/bus.c             |   36 ++++++++++++++++++++++++++++++++++++
> > >  include/acpi/acpi_bus.h        |    2 ++
> > >  include/linux/device.h         |    2 ++
> > >  5 files changed, 80 insertions(+), 3 deletions(-)
> > 
> > CCs of all driver core patches have to go to Greg Kroah-Hartman.
> 
> I previously rejected this, so I don't see why I would take it this time
> around :(
> 
> Please, no driver core changes for acpi, I don't see why it is suddenly
> so special to need stuff like this that can't just be done in the ACPI
> bus code itself.

OK, OK, that was just a notice to the author. :-)

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-15 10:22 ` Vasilis Liaskovitis
@ 2012-11-16 21:43   ` Rafael J. Wysocki
  -1 siblings, 0 replies; 36+ messages in thread
From: Rafael J. Wysocki @ 2012-11-16 21:43 UTC (permalink / raw)
  To: Vasilis Liaskovitis
  Cc: linux-acpi, isimatu.yasuaki, wency, lenb, toshi.kani,
	linux-kernel, linux-mm, Greg Kroah-Hartman

On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> As discussed in https://patchwork.kernel.org/patch/1581581/
> the driver core remove function needs to always succeed. This means we need
> to know that the device can be successfully removed before acpi_bus_trim / 
> acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> eject or driver unbind of memory devices fails e.g with:
> 
> echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> 
> since the ACPI core goes ahead and ejects the device regardless of whether the
> the memory is still in use or not.

So the question is, does the ACPI core have to do that and if so, then why?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-16 21:43   ` Rafael J. Wysocki
  0 siblings, 0 replies; 36+ messages in thread
From: Rafael J. Wysocki @ 2012-11-16 21:43 UTC (permalink / raw)
  To: Vasilis Liaskovitis
  Cc: linux-acpi, isimatu.yasuaki, wency, lenb, toshi.kani,
	linux-kernel, linux-mm, Greg Kroah-Hartman

On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> As discussed in https://patchwork.kernel.org/patch/1581581/
> the driver core remove function needs to always succeed. This means we need
> to know that the device can be successfully removed before acpi_bus_trim / 
> acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> eject or driver unbind of memory devices fails e.g with:
> 
> echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> 
> since the ACPI core goes ahead and ejects the device regardless of whether the
> the memory is still in use or not.

So the question is, does the ACPI core have to do that and if so, then why?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-16 21:43   ` Rafael J. Wysocki
@ 2012-11-16 22:45     ` Toshi Kani
  -1 siblings, 0 replies; 36+ messages in thread
From: Toshi Kani @ 2012-11-16 22:45 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Vasilis Liaskovitis, linux-acpi, isimatu.yasuaki, wency, lenb,
	linux-kernel, linux-mm, Greg Kroah-Hartman

On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > As discussed in https://patchwork.kernel.org/patch/1581581/
> > the driver core remove function needs to always succeed. This means we need
> > to know that the device can be successfully removed before acpi_bus_trim / 
> > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > eject or driver unbind of memory devices fails e.g with:
> > 
> > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > 
> > since the ACPI core goes ahead and ejects the device regardless of whether the
> > the memory is still in use or not.
> 
> So the question is, does the ACPI core have to do that and if so, then why?

The problem is that acpi_memory_devcie_remove() can fail.  However,
device_release_driver() is a void function, so it cannot report its
error.  Here are function flows for SCI, sysfs eject and unbind.

SCI & sysfs eject
===
acpi_bus_hot_remove_device()
  acpi_bus_trim()
    acpi_bus_remove()
      device_release_driver()  // Driver Core
        acpi_device_remove()
          acpi_memory_device_remove()  // ACPI Driver
  acpi_evaluate_object(handle, "_EJ0",,)  // Eject


sysfs unbind
===
driver_unbind()  // Driver Core
  device_release_driver()  // Driver Core
    acpi_device_remove()
      acpi_memory_device_remove()  // ACPI Driver
  put_device()
  bus_put()

Yasuaki's approach was to change device_release_driver() to report an
error so that acpi_bus_hot_remove_device() can fail without ejecting.
Vasilis's approach was to call ACPI driver via a new interface before
device_release_driver(), but still requires to change driver_unbind().
It looks to me that some changes to driver core is needed...

Thanks,
-Toshi


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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-16 22:45     ` Toshi Kani
  0 siblings, 0 replies; 36+ messages in thread
From: Toshi Kani @ 2012-11-16 22:45 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Vasilis Liaskovitis, linux-acpi, isimatu.yasuaki, wency, lenb,
	linux-kernel, linux-mm, Greg Kroah-Hartman

On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > As discussed in https://patchwork.kernel.org/patch/1581581/
> > the driver core remove function needs to always succeed. This means we need
> > to know that the device can be successfully removed before acpi_bus_trim / 
> > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > eject or driver unbind of memory devices fails e.g with:
> > 
> > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > 
> > since the ACPI core goes ahead and ejects the device regardless of whether the
> > the memory is still in use or not.
> 
> So the question is, does the ACPI core have to do that and if so, then why?

The problem is that acpi_memory_devcie_remove() can fail.  However,
device_release_driver() is a void function, so it cannot report its
error.  Here are function flows for SCI, sysfs eject and unbind.

SCI & sysfs eject
===
acpi_bus_hot_remove_device()
  acpi_bus_trim()
    acpi_bus_remove()
      device_release_driver()  // Driver Core
        acpi_device_remove()
          acpi_memory_device_remove()  // ACPI Driver
  acpi_evaluate_object(handle, "_EJ0",,)  // Eject


sysfs unbind
===
driver_unbind()  // Driver Core
  device_release_driver()  // Driver Core
    acpi_device_remove()
      acpi_memory_device_remove()  // ACPI Driver
  put_device()
  bus_put()

Yasuaki's approach was to change device_release_driver() to report an
error so that acpi_bus_hot_remove_device() can fail without ejecting.
Vasilis's approach was to call ACPI driver via a new interface before
device_release_driver(), but still requires to change driver_unbind().
It looks to me that some changes to driver core is needed...

Thanks,
-Toshi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-16 22:45     ` Toshi Kani
@ 2012-11-16 23:01       ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-16 23:01 UTC (permalink / raw)
  To: Toshi Kani
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, Nov 16, 2012 at 03:45:43PM -0700, Toshi Kani wrote:
> On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > the driver core remove function needs to always succeed. This means we need
> > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > eject or driver unbind of memory devices fails e.g with:
> > > 
> > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > 
> > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > the memory is still in use or not.
> > 
> > So the question is, does the ACPI core have to do that and if so, then why?
> 
> The problem is that acpi_memory_devcie_remove() can fail.  However,
> device_release_driver() is a void function, so it cannot report its
> error.  Here are function flows for SCI, sysfs eject and unbind.

Then don't ever let acpi_memory_device_remove() fail.  If the user wants
it gone, it needs to go away.  Just like any other device in the system
that can go away at any point in time, you can't "fail" that.

greg k-h

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-16 23:01       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-16 23:01 UTC (permalink / raw)
  To: Toshi Kani
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, Nov 16, 2012 at 03:45:43PM -0700, Toshi Kani wrote:
> On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > the driver core remove function needs to always succeed. This means we need
> > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > eject or driver unbind of memory devices fails e.g with:
> > > 
> > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > 
> > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > the memory is still in use or not.
> > 
> > So the question is, does the ACPI core have to do that and if so, then why?
> 
> The problem is that acpi_memory_devcie_remove() can fail.  However,
> device_release_driver() is a void function, so it cannot report its
> error.  Here are function flows for SCI, sysfs eject and unbind.

Then don't ever let acpi_memory_device_remove() fail.  If the user wants
it gone, it needs to go away.  Just like any other device in the system
that can go away at any point in time, you can't "fail" that.

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-16 23:01       ` Greg Kroah-Hartman
@ 2012-11-16 23:14         ` Toshi Kani
  -1 siblings, 0 replies; 36+ messages in thread
From: Toshi Kani @ 2012-11-16 23:14 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, 2012-11-16 at 15:01 -0800, Greg Kroah-Hartman wrote:
> On Fri, Nov 16, 2012 at 03:45:43PM -0700, Toshi Kani wrote:
> > On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> > > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > > the driver core remove function needs to always succeed. This means we need
> > > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > > eject or driver unbind of memory devices fails e.g with:
> > > > 
> > > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > > 
> > > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > > the memory is still in use or not.
> > > 
> > > So the question is, does the ACPI core have to do that and if so, then why?
> > 
> > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > device_release_driver() is a void function, so it cannot report its
> > error.  Here are function flows for SCI, sysfs eject and unbind.
> 
> Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> it gone, it needs to go away.  Just like any other device in the system
> that can go away at any point in time, you can't "fail" that.

That would be ideal, but we cannot delete a memory device that contains
kernel memory.  I am curious, how do you deal with a USB device that is
being mounted in this case?

Thanks,
-Toshi



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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-16 23:14         ` Toshi Kani
  0 siblings, 0 replies; 36+ messages in thread
From: Toshi Kani @ 2012-11-16 23:14 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, 2012-11-16 at 15:01 -0800, Greg Kroah-Hartman wrote:
> On Fri, Nov 16, 2012 at 03:45:43PM -0700, Toshi Kani wrote:
> > On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> > > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > > the driver core remove function needs to always succeed. This means we need
> > > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > > eject or driver unbind of memory devices fails e.g with:
> > > > 
> > > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > > 
> > > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > > the memory is still in use or not.
> > > 
> > > So the question is, does the ACPI core have to do that and if so, then why?
> > 
> > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > device_release_driver() is a void function, so it cannot report its
> > error.  Here are function flows for SCI, sysfs eject and unbind.
> 
> Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> it gone, it needs to go away.  Just like any other device in the system
> that can go away at any point in time, you can't "fail" that.

That would be ideal, but we cannot delete a memory device that contains
kernel memory.  I am curious, how do you deal with a USB device that is
being mounted in this case?

Thanks,
-Toshi


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-16 23:14         ` Toshi Kani
@ 2012-11-16 23:33           ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-16 23:33 UTC (permalink / raw)
  To: Toshi Kani
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, Nov 16, 2012 at 04:14:44PM -0700, Toshi Kani wrote:
> On Fri, 2012-11-16 at 15:01 -0800, Greg Kroah-Hartman wrote:
> > On Fri, Nov 16, 2012 at 03:45:43PM -0700, Toshi Kani wrote:
> > > On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> > > > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > > > the driver core remove function needs to always succeed. This means we need
> > > > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > > > eject or driver unbind of memory devices fails e.g with:
> > > > > 
> > > > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > > > 
> > > > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > > > the memory is still in use or not.
> > > > 
> > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > 
> > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > device_release_driver() is a void function, so it cannot report its
> > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > 
> > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > it gone, it needs to go away.  Just like any other device in the system
> > that can go away at any point in time, you can't "fail" that.
> 
> That would be ideal, but we cannot delete a memory device that contains
> kernel memory.  I am curious, how do you deal with a USB device that is
> being mounted in this case?

As the device is physically gone now, we deal with it and clean up
properly.

And that's the point here, what happens if the memory really is gone?
You will still have to handle it now being removed, you can't "fail" a
physical removal of a device.

If you remove a memory device that has kernel memory on it, well, you
better be able to somehow remap it before the kernel needs it :)

sorry,

greg k-h

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-16 23:33           ` Greg Kroah-Hartman
  0 siblings, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-16 23:33 UTC (permalink / raw)
  To: Toshi Kani
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, Nov 16, 2012 at 04:14:44PM -0700, Toshi Kani wrote:
> On Fri, 2012-11-16 at 15:01 -0800, Greg Kroah-Hartman wrote:
> > On Fri, Nov 16, 2012 at 03:45:43PM -0700, Toshi Kani wrote:
> > > On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> > > > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > > > the driver core remove function needs to always succeed. This means we need
> > > > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > > > eject or driver unbind of memory devices fails e.g with:
> > > > > 
> > > > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > > > 
> > > > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > > > the memory is still in use or not.
> > > > 
> > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > 
> > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > device_release_driver() is a void function, so it cannot report its
> > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > 
> > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > it gone, it needs to go away.  Just like any other device in the system
> > that can go away at any point in time, you can't "fail" that.
> 
> That would be ideal, but we cannot delete a memory device that contains
> kernel memory.  I am curious, how do you deal with a USB device that is
> being mounted in this case?

As the device is physically gone now, we deal with it and clean up
properly.

And that's the point here, what happens if the memory really is gone?
You will still have to handle it now being removed, you can't "fail" a
physical removal of a device.

If you remove a memory device that has kernel memory on it, well, you
better be able to somehow remap it before the kernel needs it :)

sorry,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-16 23:33           ` Greg Kroah-Hartman
@ 2012-11-16 23:35             ` Toshi Kani
  -1 siblings, 0 replies; 36+ messages in thread
From: Toshi Kani @ 2012-11-16 23:35 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, 2012-11-16 at 15:33 -0800, Greg Kroah-Hartman wrote:
> On Fri, Nov 16, 2012 at 04:14:44PM -0700, Toshi Kani wrote:
> > On Fri, 2012-11-16 at 15:01 -0800, Greg Kroah-Hartman wrote:
> > > On Fri, Nov 16, 2012 at 03:45:43PM -0700, Toshi Kani wrote:
> > > > On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> > > > > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > > > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > > > > the driver core remove function needs to always succeed. This means we need
> > > > > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > > > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > > > > eject or driver unbind of memory devices fails e.g with:
> > > > > > 
> > > > > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > > > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > > > > 
> > > > > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > > > > the memory is still in use or not.
> > > > > 
> > > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > > 
> > > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > > device_release_driver() is a void function, so it cannot report its
> > > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > > 
> > > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > > it gone, it needs to go away.  Just like any other device in the system
> > > that can go away at any point in time, you can't "fail" that.
> > 
> > That would be ideal, but we cannot delete a memory device that contains
> > kernel memory.  I am curious, how do you deal with a USB device that is
> > being mounted in this case?
> 
> As the device is physically gone now, we deal with it and clean up
> properly.
> 
> And that's the point here, what happens if the memory really is gone?
> You will still have to handle it now being removed, you can't "fail" a
> physical removal of a device.
> 
> If you remove a memory device that has kernel memory on it, well, you
> better be able to somehow remap it before the kernel needs it :)

:)

Well, we are not trying to support surprise removal here.  All three
use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
they should fail if the removal operation cannot complete in graceful
way.

Thanks,
-Toshi





--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-16 23:35             ` Toshi Kani
  0 siblings, 0 replies; 36+ messages in thread
From: Toshi Kani @ 2012-11-16 23:35 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, 2012-11-16 at 15:33 -0800, Greg Kroah-Hartman wrote:
> On Fri, Nov 16, 2012 at 04:14:44PM -0700, Toshi Kani wrote:
> > On Fri, 2012-11-16 at 15:01 -0800, Greg Kroah-Hartman wrote:
> > > On Fri, Nov 16, 2012 at 03:45:43PM -0700, Toshi Kani wrote:
> > > > On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> > > > > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > > > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > > > > the driver core remove function needs to always succeed. This means we need
> > > > > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > > > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > > > > eject or driver unbind of memory devices fails e.g with:
> > > > > > 
> > > > > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > > > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > > > > 
> > > > > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > > > > the memory is still in use or not.
> > > > > 
> > > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > > 
> > > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > > device_release_driver() is a void function, so it cannot report its
> > > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > > 
> > > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > > it gone, it needs to go away.  Just like any other device in the system
> > > that can go away at any point in time, you can't "fail" that.
> > 
> > That would be ideal, but we cannot delete a memory device that contains
> > kernel memory.  I am curious, how do you deal with a USB device that is
> > being mounted in this case?
> 
> As the device is physically gone now, we deal with it and clean up
> properly.
> 
> And that's the point here, what happens if the memory really is gone?
> You will still have to handle it now being removed, you can't "fail" a
> physical removal of a device.
> 
> If you remove a memory device that has kernel memory on it, well, you
> better be able to somehow remap it before the kernel needs it :)

:)

Well, we are not trying to support surprise removal here.  All three
use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
they should fail if the removal operation cannot complete in graceful
way.

Thanks,
-Toshi






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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-16 23:35             ` Toshi Kani
@ 2012-11-17  0:02               ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-17  0:02 UTC (permalink / raw)
  To: Toshi Kani
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, Nov 16, 2012 at 04:35:06PM -0700, Toshi Kani wrote:
> On Fri, 2012-11-16 at 15:33 -0800, Greg Kroah-Hartman wrote:
> > On Fri, Nov 16, 2012 at 04:14:44PM -0700, Toshi Kani wrote:
> > > On Fri, 2012-11-16 at 15:01 -0800, Greg Kroah-Hartman wrote:
> > > > On Fri, Nov 16, 2012 at 03:45:43PM -0700, Toshi Kani wrote:
> > > > > On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> > > > > > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > > > > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > > > > > the driver core remove function needs to always succeed. This means we need
> > > > > > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > > > > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > > > > > eject or driver unbind of memory devices fails e.g with:
> > > > > > > 
> > > > > > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > > > > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > > > > > 
> > > > > > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > > > > > the memory is still in use or not.
> > > > > > 
> > > > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > > > 
> > > > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > > > device_release_driver() is a void function, so it cannot report its
> > > > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > > > 
> > > > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > > > it gone, it needs to go away.  Just like any other device in the system
> > > > that can go away at any point in time, you can't "fail" that.
> > > 
> > > That would be ideal, but we cannot delete a memory device that contains
> > > kernel memory.  I am curious, how do you deal with a USB device that is
> > > being mounted in this case?
> > 
> > As the device is physically gone now, we deal with it and clean up
> > properly.
> > 
> > And that's the point here, what happens if the memory really is gone?
> > You will still have to handle it now being removed, you can't "fail" a
> > physical removal of a device.
> > 
> > If you remove a memory device that has kernel memory on it, well, you
> > better be able to somehow remap it before the kernel needs it :)
> 
> :)
> 
> Well, we are not trying to support surprise removal here.  All three
> use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
> they should fail if the removal operation cannot complete in graceful
> way.

Then handle that in the ACPI bus code, it isn't anything that the driver
core should care about, right?

And odds are, eventually you will have to handle surprise removal, it's
only a matter of time :)

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-17  0:02               ` Greg Kroah-Hartman
  0 siblings, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-17  0:02 UTC (permalink / raw)
  To: Toshi Kani
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, Nov 16, 2012 at 04:35:06PM -0700, Toshi Kani wrote:
> On Fri, 2012-11-16 at 15:33 -0800, Greg Kroah-Hartman wrote:
> > On Fri, Nov 16, 2012 at 04:14:44PM -0700, Toshi Kani wrote:
> > > On Fri, 2012-11-16 at 15:01 -0800, Greg Kroah-Hartman wrote:
> > > > On Fri, Nov 16, 2012 at 03:45:43PM -0700, Toshi Kani wrote:
> > > > > On Fri, 2012-11-16 at 22:43 +0100, Rafael J. Wysocki wrote:
> > > > > > On Thursday, November 15, 2012 11:22:47 AM Vasilis Liaskovitis wrote:
> > > > > > > As discussed in https://patchwork.kernel.org/patch/1581581/
> > > > > > > the driver core remove function needs to always succeed. This means we need
> > > > > > > to know that the device can be successfully removed before acpi_bus_trim / 
> > > > > > > acpi_bus_hot_remove_device are called. This can cause panics when OSPM-initiated
> > > > > > > eject or driver unbind of memory devices fails e.g with:
> > > > > > > 
> > > > > > > echo 1 >/sys/bus/pci/devices/PNP0C80:XX/eject
> > > > > > > echo "PNP0C80:XX" > /sys/bus/acpi/drivers/acpi_memhotplug/unbind
> > > > > > > 
> > > > > > > since the ACPI core goes ahead and ejects the device regardless of whether the
> > > > > > > the memory is still in use or not.
> > > > > > 
> > > > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > > > 
> > > > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > > > device_release_driver() is a void function, so it cannot report its
> > > > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > > > 
> > > > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > > > it gone, it needs to go away.  Just like any other device in the system
> > > > that can go away at any point in time, you can't "fail" that.
> > > 
> > > That would be ideal, but we cannot delete a memory device that contains
> > > kernel memory.  I am curious, how do you deal with a USB device that is
> > > being mounted in this case?
> > 
> > As the device is physically gone now, we deal with it and clean up
> > properly.
> > 
> > And that's the point here, what happens if the memory really is gone?
> > You will still have to handle it now being removed, you can't "fail" a
> > physical removal of a device.
> > 
> > If you remove a memory device that has kernel memory on it, well, you
> > better be able to somehow remap it before the kernel needs it :)
> 
> :)
> 
> Well, we are not trying to support surprise removal here.  All three
> use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
> they should fail if the removal operation cannot complete in graceful
> way.

Then handle that in the ACPI bus code, it isn't anything that the driver
core should care about, right?

And odds are, eventually you will have to handle surprise removal, it's
only a matter of time :)

greg k-h

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-17  0:02               ` Greg Kroah-Hartman
@ 2012-11-17  0:08                 ` Toshi Kani
  -1 siblings, 0 replies; 36+ messages in thread
From: Toshi Kani @ 2012-11-17  0:08 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

> > > > > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > > > > 
> > > > > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > > > > device_release_driver() is a void function, so it cannot report its
> > > > > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > > > > 
> > > > > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > > > > it gone, it needs to go away.  Just like any other device in the system
> > > > > that can go away at any point in time, you can't "fail" that.
> > > > 
> > > > That would be ideal, but we cannot delete a memory device that contains
> > > > kernel memory.  I am curious, how do you deal with a USB device that is
> > > > being mounted in this case?
> > > 
> > > As the device is physically gone now, we deal with it and clean up
> > > properly.
> > > 
> > > And that's the point here, what happens if the memory really is gone?
> > > You will still have to handle it now being removed, you can't "fail" a
> > > physical removal of a device.
> > > 
> > > If you remove a memory device that has kernel memory on it, well, you
> > > better be able to somehow remap it before the kernel needs it :)
> > 
> > :)
> > 
> > Well, we are not trying to support surprise removal here.  All three
> > use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
> > they should fail if the removal operation cannot complete in graceful
> > way.
> 
> Then handle that in the ACPI bus code, it isn't anything that the driver
> core should care about, right?

Unfortunately not.  Please take a look at the function flow for the
unbind case in my first email.  This request directly goes to
driver_unbind(), which is a driver core function.

> And odds are, eventually you will have to handle surprise removal, it's
> only a matter of time :)

Hardware guys will have hard time to support it before software guys can
do something here...  Staff like cache coherency is a devil.

Thanks,
-Toshi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-17  0:08                 ` Toshi Kani
  0 siblings, 0 replies; 36+ messages in thread
From: Toshi Kani @ 2012-11-17  0:08 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

> > > > > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > > > > 
> > > > > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > > > > device_release_driver() is a void function, so it cannot report its
> > > > > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > > > > 
> > > > > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > > > > it gone, it needs to go away.  Just like any other device in the system
> > > > > that can go away at any point in time, you can't "fail" that.
> > > > 
> > > > That would be ideal, but we cannot delete a memory device that contains
> > > > kernel memory.  I am curious, how do you deal with a USB device that is
> > > > being mounted in this case?
> > > 
> > > As the device is physically gone now, we deal with it and clean up
> > > properly.
> > > 
> > > And that's the point here, what happens if the memory really is gone?
> > > You will still have to handle it now being removed, you can't "fail" a
> > > physical removal of a device.
> > > 
> > > If you remove a memory device that has kernel memory on it, well, you
> > > better be able to somehow remap it before the kernel needs it :)
> > 
> > :)
> > 
> > Well, we are not trying to support surprise removal here.  All three
> > use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
> > they should fail if the removal operation cannot complete in graceful
> > way.
> 
> Then handle that in the ACPI bus code, it isn't anything that the driver
> core should care about, right?

Unfortunately not.  Please take a look at the function flow for the
unbind case in my first email.  This request directly goes to
driver_unbind(), which is a driver core function.

> And odds are, eventually you will have to handle surprise removal, it's
> only a matter of time :)

Hardware guys will have hard time to support it before software guys can
do something here...  Staff like cache coherency is a devil.

Thanks,
-Toshi


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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-17  0:08                 ` Toshi Kani
@ 2012-11-17  0:22                   ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-17  0:22 UTC (permalink / raw)
  To: Toshi Kani
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, Nov 16, 2012 at 05:08:53PM -0700, Toshi Kani wrote:
> > > > > > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > > > > > 
> > > > > > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > > > > > device_release_driver() is a void function, so it cannot report its
> > > > > > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > > > > > 
> > > > > > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > > > > > it gone, it needs to go away.  Just like any other device in the system
> > > > > > that can go away at any point in time, you can't "fail" that.
> > > > > 
> > > > > That would be ideal, but we cannot delete a memory device that contains
> > > > > kernel memory.  I am curious, how do you deal with a USB device that is
> > > > > being mounted in this case?
> > > > 
> > > > As the device is physically gone now, we deal with it and clean up
> > > > properly.
> > > > 
> > > > And that's the point here, what happens if the memory really is gone?
> > > > You will still have to handle it now being removed, you can't "fail" a
> > > > physical removal of a device.
> > > > 
> > > > If you remove a memory device that has kernel memory on it, well, you
> > > > better be able to somehow remap it before the kernel needs it :)
> > > 
> > > :)
> > > 
> > > Well, we are not trying to support surprise removal here.  All three
> > > use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
> > > they should fail if the removal operation cannot complete in graceful
> > > way.
> > 
> > Then handle that in the ACPI bus code, it isn't anything that the driver
> > core should care about, right?
> 
> Unfortunately not.  Please take a look at the function flow for the
> unbind case in my first email.  This request directly goes to
> driver_unbind(), which is a driver core function.

Yes, and as the user asked for the driver to be unbound from the device,
it can not fail.

And that is WAY different from removing the memory from the system
itself.  Don't think that this is the "normal" way that memory should be
removed, that is what stuff like "eject" was created for the PCI slots.

Don't confuse the two things here, unbinding a driver from a device
should not remove the memory from the system, it doesn't do that for any
other type of 'unbind' call for any other bus.  The device is still
present, just that specific driver isn't controlling it anymore.

In other words, you should NEVER have a normal userspace flow that is
trying to do unbind.  unbind is only for radical things like
disconnecting a driver from a device if a userspace driver wants to
control it, or a hacked up way to implement revoke() for a device.

Again, no driver core changes are needed here.

greg k-h

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-17  0:22                   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 36+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-17  0:22 UTC (permalink / raw)
  To: Toshi Kani
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, Nov 16, 2012 at 05:08:53PM -0700, Toshi Kani wrote:
> > > > > > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > > > > > 
> > > > > > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > > > > > device_release_driver() is a void function, so it cannot report its
> > > > > > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > > > > > 
> > > > > > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > > > > > it gone, it needs to go away.  Just like any other device in the system
> > > > > > that can go away at any point in time, you can't "fail" that.
> > > > > 
> > > > > That would be ideal, but we cannot delete a memory device that contains
> > > > > kernel memory.  I am curious, how do you deal with a USB device that is
> > > > > being mounted in this case?
> > > > 
> > > > As the device is physically gone now, we deal with it and clean up
> > > > properly.
> > > > 
> > > > And that's the point here, what happens if the memory really is gone?
> > > > You will still have to handle it now being removed, you can't "fail" a
> > > > physical removal of a device.
> > > > 
> > > > If you remove a memory device that has kernel memory on it, well, you
> > > > better be able to somehow remap it before the kernel needs it :)
> > > 
> > > :)
> > > 
> > > Well, we are not trying to support surprise removal here.  All three
> > > use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
> > > they should fail if the removal operation cannot complete in graceful
> > > way.
> > 
> > Then handle that in the ACPI bus code, it isn't anything that the driver
> > core should care about, right?
> 
> Unfortunately not.  Please take a look at the function flow for the
> unbind case in my first email.  This request directly goes to
> driver_unbind(), which is a driver core function.

Yes, and as the user asked for the driver to be unbound from the device,
it can not fail.

And that is WAY different from removing the memory from the system
itself.  Don't think that this is the "normal" way that memory should be
removed, that is what stuff like "eject" was created for the PCI slots.

Don't confuse the two things here, unbinding a driver from a device
should not remove the memory from the system, it doesn't do that for any
other type of 'unbind' call for any other bus.  The device is still
present, just that specific driver isn't controlling it anymore.

In other words, you should NEVER have a normal userspace flow that is
trying to do unbind.  unbind is only for radical things like
disconnecting a driver from a device if a userspace driver wants to
control it, or a hacked up way to implement revoke() for a device.

Again, no driver core changes are needed here.

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-17  0:22                   ` Greg Kroah-Hartman
@ 2012-11-17  0:25                     ` Toshi Kani
  -1 siblings, 0 replies; 36+ messages in thread
From: Toshi Kani @ 2012-11-17  0:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, 2012-11-16 at 16:22 -0800, Greg Kroah-Hartman wrote:
> On Fri, Nov 16, 2012 at 05:08:53PM -0700, Toshi Kani wrote:
> > > > > > > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > > > > > > 
> > > > > > > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > > > > > > device_release_driver() is a void function, so it cannot report its
> > > > > > > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > > > > > > 
> > > > > > > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > > > > > > it gone, it needs to go away.  Just like any other device in the system
> > > > > > > that can go away at any point in time, you can't "fail" that.
> > > > > > 
> > > > > > That would be ideal, but we cannot delete a memory device that contains
> > > > > > kernel memory.  I am curious, how do you deal with a USB device that is
> > > > > > being mounted in this case?
> > > > > 
> > > > > As the device is physically gone now, we deal with it and clean up
> > > > > properly.
> > > > > 
> > > > > And that's the point here, what happens if the memory really is gone?
> > > > > You will still have to handle it now being removed, you can't "fail" a
> > > > > physical removal of a device.
> > > > > 
> > > > > If you remove a memory device that has kernel memory on it, well, you
> > > > > better be able to somehow remap it before the kernel needs it :)
> > > > 
> > > > :)
> > > > 
> > > > Well, we are not trying to support surprise removal here.  All three
> > > > use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
> > > > they should fail if the removal operation cannot complete in graceful
> > > > way.
> > > 
> > > Then handle that in the ACPI bus code, it isn't anything that the driver
> > > core should care about, right?
> > 
> > Unfortunately not.  Please take a look at the function flow for the
> > unbind case in my first email.  This request directly goes to
> > driver_unbind(), which is a driver core function.
> 
> Yes, and as the user asked for the driver to be unbound from the device,
> it can not fail.
> 
> And that is WAY different from removing the memory from the system
> itself.  Don't think that this is the "normal" way that memory should be
> removed, that is what stuff like "eject" was created for the PCI slots.
> 
> Don't confuse the two things here, unbinding a driver from a device
> should not remove the memory from the system, it doesn't do that for any
> other type of 'unbind' call for any other bus.  The device is still
> present, just that specific driver isn't controlling it anymore.
> 
> In other words, you should NEVER have a normal userspace flow that is
> trying to do unbind.  unbind is only for radical things like
> disconnecting a driver from a device if a userspace driver wants to
> control it, or a hacked up way to implement revoke() for a device.
> 
> Again, no driver core changes are needed here.

Okay, we might be able to make the eject case to fail if an ACPI driver
is not bound to a device.  This way, the unbind case may be harmless to
proceed.  Let us think about this further on this (but we may come up
again :). 

Thanks,
-Toshi 






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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-17  0:25                     ` Toshi Kani
  0 siblings, 0 replies; 36+ messages in thread
From: Toshi Kani @ 2012-11-17  0:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Rafael J. Wysocki, Vasilis Liaskovitis, linux-acpi,
	isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On Fri, 2012-11-16 at 16:22 -0800, Greg Kroah-Hartman wrote:
> On Fri, Nov 16, 2012 at 05:08:53PM -0700, Toshi Kani wrote:
> > > > > > > > > So the question is, does the ACPI core have to do that and if so, then why?
> > > > > > > > 
> > > > > > > > The problem is that acpi_memory_devcie_remove() can fail.  However,
> > > > > > > > device_release_driver() is a void function, so it cannot report its
> > > > > > > > error.  Here are function flows for SCI, sysfs eject and unbind.
> > > > > > > 
> > > > > > > Then don't ever let acpi_memory_device_remove() fail.  If the user wants
> > > > > > > it gone, it needs to go away.  Just like any other device in the system
> > > > > > > that can go away at any point in time, you can't "fail" that.
> > > > > > 
> > > > > > That would be ideal, but we cannot delete a memory device that contains
> > > > > > kernel memory.  I am curious, how do you deal with a USB device that is
> > > > > > being mounted in this case?
> > > > > 
> > > > > As the device is physically gone now, we deal with it and clean up
> > > > > properly.
> > > > > 
> > > > > And that's the point here, what happens if the memory really is gone?
> > > > > You will still have to handle it now being removed, you can't "fail" a
> > > > > physical removal of a device.
> > > > > 
> > > > > If you remove a memory device that has kernel memory on it, well, you
> > > > > better be able to somehow remap it before the kernel needs it :)
> > > > 
> > > > :)
> > > > 
> > > > Well, we are not trying to support surprise removal here.  All three
> > > > use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
> > > > they should fail if the removal operation cannot complete in graceful
> > > > way.
> > > 
> > > Then handle that in the ACPI bus code, it isn't anything that the driver
> > > core should care about, right?
> > 
> > Unfortunately not.  Please take a look at the function flow for the
> > unbind case in my first email.  This request directly goes to
> > driver_unbind(), which is a driver core function.
> 
> Yes, and as the user asked for the driver to be unbound from the device,
> it can not fail.
> 
> And that is WAY different from removing the memory from the system
> itself.  Don't think that this is the "normal" way that memory should be
> removed, that is what stuff like "eject" was created for the PCI slots.
> 
> Don't confuse the two things here, unbinding a driver from a device
> should not remove the memory from the system, it doesn't do that for any
> other type of 'unbind' call for any other bus.  The device is still
> present, just that specific driver isn't controlling it anymore.
> 
> In other words, you should NEVER have a normal userspace flow that is
> trying to do unbind.  unbind is only for radical things like
> disconnecting a driver from a device if a userspace driver wants to
> control it, or a hacked up way to implement revoke() for a device.
> 
> Again, no driver core changes are needed here.

Okay, we might be able to make the eject case to fail if an ACPI driver
is not bound to a device.  This way, the unbind case may be harmless to
proceed.  Let us think about this further on this (but we may come up
again :). 

Thanks,
-Toshi 





--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
  2012-11-17  0:25                     ` Toshi Kani
@ 2012-11-18 16:16                       ` Jiang Liu
  -1 siblings, 0 replies; 36+ messages in thread
From: Jiang Liu @ 2012-11-18 16:16 UTC (permalink / raw)
  To: Toshi Kani
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Vasilis Liaskovitis,
	linux-acpi, isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On 11/17/2012 08:25 AM, Toshi Kani wrote:
> On Fri, 2012-11-16 at 16:22 -0800, Greg Kroah-Hartman wrote:
>> On Fri, Nov 16, 2012 at 05:08:53PM -0700, Toshi Kani wrote:
>>>>>>>>>> So the question is, does the ACPI core have to do that and if so, then why?
>>>>>>>>>
>>>>>>>>> The problem is that acpi_memory_devcie_remove() can fail.  However,
>>>>>>>>> device_release_driver() is a void function, so it cannot report its
>>>>>>>>> error.  Here are function flows for SCI, sysfs eject and unbind.
>>>>>>>>
>>>>>>>> Then don't ever let acpi_memory_device_remove() fail.  If the user wants
>>>>>>>> it gone, it needs to go away.  Just like any other device in the system
>>>>>>>> that can go away at any point in time, you can't "fail" that.
>>>>>>>
>>>>>>> That would be ideal, but we cannot delete a memory device that contains
>>>>>>> kernel memory.  I am curious, how do you deal with a USB device that is
>>>>>>> being mounted in this case?
>>>>>>
>>>>>> As the device is physically gone now, we deal with it and clean up
>>>>>> properly.
>>>>>>
>>>>>> And that's the point here, what happens if the memory really is gone?
>>>>>> You will still have to handle it now being removed, you can't "fail" a
>>>>>> physical removal of a device.
>>>>>>
>>>>>> If you remove a memory device that has kernel memory on it, well, you
>>>>>> better be able to somehow remap it before the kernel needs it :)
>>>>>
>>>>> :)
>>>>>
>>>>> Well, we are not trying to support surprise removal here.  All three
>>>>> use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
>>>>> they should fail if the removal operation cannot complete in graceful
>>>>> way.
>>>>
>>>> Then handle that in the ACPI bus code, it isn't anything that the driver
>>>> core should care about, right?
>>>
>>> Unfortunately not.  Please take a look at the function flow for the
>>> unbind case in my first email.  This request directly goes to
>>> driver_unbind(), which is a driver core function.
>>
>> Yes, and as the user asked for the driver to be unbound from the device,
>> it can not fail.
>>
>> And that is WAY different from removing the memory from the system
>> itself.  Don't think that this is the "normal" way that memory should be
>> removed, that is what stuff like "eject" was created for the PCI slots.
>>
>> Don't confuse the two things here, unbinding a driver from a device
>> should not remove the memory from the system, it doesn't do that for any
>> other type of 'unbind' call for any other bus.  The device is still
>> present, just that specific driver isn't controlling it anymore.
>>
>> In other words, you should NEVER have a normal userspace flow that is
>> trying to do unbind.  unbind is only for radical things like
>> disconnecting a driver from a device if a userspace driver wants to
>> control it, or a hacked up way to implement revoke() for a device.
>>
>> Again, no driver core changes are needed here.
> 
> Okay, we might be able to make the eject case to fail if an ACPI driver
> is not bound to a device.  This way, the unbind case may be harmless to
> proceed.  Let us think about this further on this (but we may come up
> again :). 
Hi all,
	The ACPI based system device hotplug framework project I'm working
on has already provided a solution for this issue.
	We have added several callbacks to struct acpi_device_ops to support
ACPI system device hotplug. By that way, we could guarantee unbinding ACPI
CPU/memory/PCI host bridge drivers will always success. And we have a plan
to implement the existing "eject" interface with the new hotplug framework.

	For more information, please take a look at:
http://www.spinics.net/lists/linux-acpi/msg39487.html
http://www.spinics.net/lists/linux-acpi/msg39490.html

Thanks!
Gerry

> 
> Thanks,
> -Toshi 
> 
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation
@ 2012-11-18 16:16                       ` Jiang Liu
  0 siblings, 0 replies; 36+ messages in thread
From: Jiang Liu @ 2012-11-18 16:16 UTC (permalink / raw)
  To: Toshi Kani
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Vasilis Liaskovitis,
	linux-acpi, isimatu.yasuaki, wency, lenb, linux-kernel, linux-mm

On 11/17/2012 08:25 AM, Toshi Kani wrote:
> On Fri, 2012-11-16 at 16:22 -0800, Greg Kroah-Hartman wrote:
>> On Fri, Nov 16, 2012 at 05:08:53PM -0700, Toshi Kani wrote:
>>>>>>>>>> So the question is, does the ACPI core have to do that and if so, then why?
>>>>>>>>>
>>>>>>>>> The problem is that acpi_memory_devcie_remove() can fail.  However,
>>>>>>>>> device_release_driver() is a void function, so it cannot report its
>>>>>>>>> error.  Here are function flows for SCI, sysfs eject and unbind.
>>>>>>>>
>>>>>>>> Then don't ever let acpi_memory_device_remove() fail.  If the user wants
>>>>>>>> it gone, it needs to go away.  Just like any other device in the system
>>>>>>>> that can go away at any point in time, you can't "fail" that.
>>>>>>>
>>>>>>> That would be ideal, but we cannot delete a memory device that contains
>>>>>>> kernel memory.  I am curious, how do you deal with a USB device that is
>>>>>>> being mounted in this case?
>>>>>>
>>>>>> As the device is physically gone now, we deal with it and clean up
>>>>>> properly.
>>>>>>
>>>>>> And that's the point here, what happens if the memory really is gone?
>>>>>> You will still have to handle it now being removed, you can't "fail" a
>>>>>> physical removal of a device.
>>>>>>
>>>>>> If you remove a memory device that has kernel memory on it, well, you
>>>>>> better be able to somehow remap it before the kernel needs it :)
>>>>>
>>>>> :)
>>>>>
>>>>> Well, we are not trying to support surprise removal here.  All three
>>>>> use-cases (SCI, eject, and unbind) are for graceful removal.  Therefore
>>>>> they should fail if the removal operation cannot complete in graceful
>>>>> way.
>>>>
>>>> Then handle that in the ACPI bus code, it isn't anything that the driver
>>>> core should care about, right?
>>>
>>> Unfortunately not.  Please take a look at the function flow for the
>>> unbind case in my first email.  This request directly goes to
>>> driver_unbind(), which is a driver core function.
>>
>> Yes, and as the user asked for the driver to be unbound from the device,
>> it can not fail.
>>
>> And that is WAY different from removing the memory from the system
>> itself.  Don't think that this is the "normal" way that memory should be
>> removed, that is what stuff like "eject" was created for the PCI slots.
>>
>> Don't confuse the two things here, unbinding a driver from a device
>> should not remove the memory from the system, it doesn't do that for any
>> other type of 'unbind' call for any other bus.  The device is still
>> present, just that specific driver isn't controlling it anymore.
>>
>> In other words, you should NEVER have a normal userspace flow that is
>> trying to do unbind.  unbind is only for radical things like
>> disconnecting a driver from a device if a userspace driver wants to
>> control it, or a hacked up way to implement revoke() for a device.
>>
>> Again, no driver core changes are needed here.
> 
> Okay, we might be able to make the eject case to fail if an ACPI driver
> is not bound to a device.  This way, the unbind case may be harmless to
> proceed.  Let us think about this further on this (but we may come up
> again :). 
Hi all,
	The ACPI based system device hotplug framework project I'm working
on has already provided a solution for this issue.
	We have added several callbacks to struct acpi_device_ops to support
ACPI system device hotplug. By that way, we could guarantee unbinding ACPI
CPU/memory/PCI host bridge drivers will always success. And we have a plan
to implement the existing "eject" interface with the new hotplug framework.

	For more information, please take a look at:
http://www.spinics.net/lists/linux-acpi/msg39487.html
http://www.spinics.net/lists/linux-acpi/msg39490.html

Thanks!
Gerry

> 
> Thanks,
> -Toshi 
> 
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


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

end of thread, other threads:[~2012-11-18 16:16 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-15 10:22 [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Vasilis Liaskovitis
2012-11-15 10:22 ` Vasilis Liaskovitis
2012-11-15 10:22 ` [RFC PATCH v2 1/3] driver core: Introduce prepare_remove in bus_type Vasilis Liaskovitis
2012-11-15 10:22   ` Vasilis Liaskovitis
2012-11-15 10:22 ` [RFC PATCH v2 2/3] acpi: Introduce prepare_remove operation in acpi_device_ops Vasilis Liaskovitis
2012-11-15 10:22   ` Vasilis Liaskovitis
2012-11-15 10:22 ` [RFC PATCH v2 3/3] acpi_memhotplug: Add prepare_remove operation Vasilis Liaskovitis
2012-11-15 10:22   ` Vasilis Liaskovitis
2012-11-16 21:17 ` [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Rafael J. Wysocki
2012-11-16 21:17   ` Rafael J. Wysocki
2012-11-16 21:33   ` Greg Kroah-Hartman
2012-11-16 21:33     ` Greg Kroah-Hartman
2012-11-16 21:41     ` Rafael J. Wysocki
2012-11-16 21:41       ` Rafael J. Wysocki
2012-11-16 21:43 ` Rafael J. Wysocki
2012-11-16 21:43   ` Rafael J. Wysocki
2012-11-16 22:45   ` Toshi Kani
2012-11-16 22:45     ` Toshi Kani
2012-11-16 23:01     ` Greg Kroah-Hartman
2012-11-16 23:01       ` Greg Kroah-Hartman
2012-11-16 23:14       ` Toshi Kani
2012-11-16 23:14         ` Toshi Kani
2012-11-16 23:33         ` Greg Kroah-Hartman
2012-11-16 23:33           ` Greg Kroah-Hartman
2012-11-16 23:35           ` Toshi Kani
2012-11-16 23:35             ` Toshi Kani
2012-11-17  0:02             ` Greg Kroah-Hartman
2012-11-17  0:02               ` Greg Kroah-Hartman
2012-11-17  0:08               ` Toshi Kani
2012-11-17  0:08                 ` Toshi Kani
2012-11-17  0:22                 ` Greg Kroah-Hartman
2012-11-17  0:22                   ` Greg Kroah-Hartman
2012-11-17  0:25                   ` Toshi Kani
2012-11-17  0:25                     ` Toshi Kani
2012-11-18 16:16                     ` Jiang Liu
2012-11-18 16:16                       ` Jiang Liu

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.