From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vasilis Liaskovitis Subject: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Thu, 15 Nov 2012 11:22:47 +0100 Message-ID: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Return-path: Sender: owner-linux-mm@kvack.org To: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com Cc: rjw@sisk.pl, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Vasilis Liaskovitis List-Id: linux-acpi@vger.kernel.org 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vasilis Liaskovitis Subject: [RFC PATCH v2 1/3] driver core: Introduce prepare_remove in bus_type Date: Thu, 15 Nov 2012 11:22:48 +0100 Message-ID: <1352974970-6643-2-git-send-email-vasilis.liaskovitis@profitbricks.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Return-path: In-Reply-To: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Sender: owner-linux-mm@kvack.org To: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com Cc: rjw@sisk.pl, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Vasilis Liaskovitis List-Id: linux-acpi@vger.kernel.org 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 --- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vasilis Liaskovitis Subject: [RFC PATCH v2 2/3] acpi: Introduce prepare_remove operation in acpi_device_ops Date: Thu, 15 Nov 2012 11:22:49 +0100 Message-ID: <1352974970-6643-3-git-send-email-vasilis.liaskovitis@profitbricks.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Return-path: In-Reply-To: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Sender: owner-linux-mm@kvack.org To: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com Cc: rjw@sisk.pl, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Vasilis Liaskovitis List-Id: linux-acpi@vger.kernel.org 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 --- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vasilis Liaskovitis Subject: [RFC PATCH v2 3/3] acpi_memhotplug: Add prepare_remove operation Date: Thu, 15 Nov 2012 11:22:50 +0100 Message-ID: <1352974970-6643-4-git-send-email-vasilis.liaskovitis@profitbricks.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Return-path: In-Reply-To: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Sender: owner-linux-mm@kvack.org To: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com Cc: rjw@sisk.pl, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Vasilis Liaskovitis List-Id: linux-acpi@vger.kernel.org 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 --- 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 22:17:17 +0100 Message-ID: <11897248.2VNutIHaJi@vostro.rjw.lan> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7Bit Return-path: In-Reply-To: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Sender: owner-linux-mm@kvack.org To: Vasilis Liaskovitis Cc: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Greg Kroah-Hartman List-Id: linux-acpi@vger.kernel.org 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 13:33:10 -0800 Message-ID: <20121116213310.GA12925@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <11897248.2VNutIHaJi@vostro.rjw.lan> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-pa0-f46.google.com ([209.85.220.46]:43791 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753639Ab2KPVdP (ORCPT ); Fri, 16 Nov 2012 16:33:15 -0500 Received: by mail-pa0-f46.google.com with SMTP id hz1so2127841pad.19 for ; Fri, 16 Nov 2012 13:33:14 -0800 (PST) Content-Disposition: inline In-Reply-To: <11897248.2VNutIHaJi@vostro.rjw.lan> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: "Rafael J. Wysocki" Cc: Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 22:41:52 +0100 Message-ID: <6896483.i6Vm9YII9b@vostro.rjw.lan> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <11897248.2VNutIHaJi@vostro.rjw.lan> <20121116213310.GA12925@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7Bit Return-path: Received: from ogre.sisk.pl ([193.178.161.156]:49654 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753639Ab2KPVhb (ORCPT ); Fri, 16 Nov 2012 16:37:31 -0500 In-Reply-To: <20121116213310.GA12925@kroah.com> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Greg Kroah-Hartman Cc: Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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. From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 22:43:59 +0100 Message-ID: <1446291.TgLDtXqY7q@vostro.rjw.lan> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7Bit Return-path: Received: from ogre.sisk.pl ([193.178.161.156]:49667 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753674Ab2KPVjh (ORCPT ); Fri, 16 Nov 2012 16:39:37 -0500 In-Reply-To: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Vasilis Liaskovitis Cc: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, 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. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Toshi Kani Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 15:45:43 -0700 Message-ID: <1353105943.12509.60.camel@misato.fc.hp.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: Received: from g5t0008.atlanta.hp.com ([15.192.0.45]:41774 "EHLO g5t0008.atlanta.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753797Ab2KPWx6 (ORCPT ); Fri, 16 Nov 2012 17:53:58 -0500 In-Reply-To: <1446291.TgLDtXqY7q@vostro.rjw.lan> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: "Rafael J. Wysocki" Cc: Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 15:01:43 -0800 Message-ID: <20121116230143.GA15338@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-pa0-f46.google.com ([209.85.220.46]:51707 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753454Ab2KPXBr (ORCPT ); Fri, 16 Nov 2012 18:01:47 -0500 Received: by mail-pa0-f46.google.com with SMTP id hz1so2165611pad.19 for ; Fri, 16 Nov 2012 15:01:47 -0800 (PST) Content-Disposition: inline In-Reply-To: <1353105943.12509.60.camel@misato.fc.hp.com> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Toshi Kani Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Toshi Kani Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 16:14:44 -0700 Message-ID: <1353107684.12509.65.camel@misato.fc.hp.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: Received: from g6t0184.atlanta.hp.com ([15.193.32.61]:26876 "EHLO g6t0184.atlanta.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753666Ab2KPXW5 (ORCPT ); Fri, 16 Nov 2012 18:22:57 -0500 In-Reply-To: <20121116230143.GA15338@kroah.com> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 15:33:55 -0800 Message-ID: <20121116233355.GA21144@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-pb0-f46.google.com ([209.85.160.46]:60115 "EHLO mail-pb0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753813Ab2KPXd7 (ORCPT ); Fri, 16 Nov 2012 18:33:59 -0500 Received: by mail-pb0-f46.google.com with SMTP id wy7so2252704pbc.19 for ; Fri, 16 Nov 2012 15:33:58 -0800 (PST) Content-Disposition: inline In-Reply-To: <1353107684.12509.65.camel@misato.fc.hp.com> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Toshi Kani Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Toshi Kani Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 16:35:06 -0700 Message-ID: <1353108906.10624.5.camel@misato.fc.hp.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20121116233355.GA21144@kroah.com> Sender: owner-linux-mm@kvack.org To: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org List-Id: linux-acpi@vger.kernel.org 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 16:02:50 -0800 Message-ID: <20121117000250.GA4425@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> <1353108906.10624.5.camel@misato.fc.hp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1353108906.10624.5.camel@misato.fc.hp.com> Sender: owner-linux-mm@kvack.org To: Toshi Kani Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org List-Id: linux-acpi@vger.kernel.org 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Toshi Kani Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 17:08:53 -0700 Message-ID: <1353110933.10939.6.camel@misato.fc.hp.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> <1353108906.10624.5.camel@misato.fc.hp.com> <20121117000250.GA4425@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20121117000250.GA4425@kroah.com> Sender: owner-linux-mm@kvack.org To: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org List-Id: linux-acpi@vger.kernel.org > > > > > > > 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 16:22:32 -0800 Message-ID: <20121117002232.GA22543@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> <1353108906.10624.5.camel@misato.fc.hp.com> <20121117000250.GA4425@kroah.com> <1353110933.10939.6.camel@misato.fc.hp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from mail-da0-f46.google.com ([209.85.210.46]:63062 "EHLO mail-da0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753687Ab2KQAWg (ORCPT ); Fri, 16 Nov 2012 19:22:36 -0500 Received: by mail-da0-f46.google.com with SMTP id n41so1350971dak.19 for ; Fri, 16 Nov 2012 16:22:36 -0800 (PST) Content-Disposition: inline In-Reply-To: <1353110933.10939.6.camel@misato.fc.hp.com> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Toshi Kani Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Toshi Kani Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 17:25:05 -0700 Message-ID: <1353111905.10939.12.camel@misato.fc.hp.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> <1353108906.10624.5.camel@misato.fc.hp.com> <20121117000250.GA4425@kroah.com> <1353110933.10939.6.camel@misato.fc.hp.com> <20121117002232.GA22543@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: Received: from g1t0027.austin.hp.com ([15.216.28.34]:17906 "EHLO g1t0027.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753832Ab2KQAdS (ORCPT ); Fri, 16 Nov 2012 19:33:18 -0500 In-Reply-To: <20121117002232.GA22543@kroah.com> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiang Liu Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Mon, 19 Nov 2012 00:16:33 +0800 Message-ID: <50A909E1.9030708@gmail.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> <1353108906.10624.5.camel@misato.fc.hp.com> <20121117000250.GA4425@kroah.com> <1353110933.10939.6.camel@misato.fc.hp.com> <20121117002232.GA22543@kroah.com> <1353111905.10939.12.camel@misato.fc.hp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1353111905.10939.12.camel@misato.fc.hp.com> Sender: owner-linux-mm@kvack.org To: Toshi Kani Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org List-Id: linux-acpi@vger.kernel.org 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2993079Ab2KOKXA (ORCPT ); Thu, 15 Nov 2012 05:23:00 -0500 Received: from mail-bk0-f46.google.com ([209.85.214.46]:37216 "EHLO mail-bk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2992441Ab2KOKW6 (ORCPT ); Thu, 15 Nov 2012 05:22:58 -0500 From: Vasilis Liaskovitis To: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com Cc: rjw@sisk.pl, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Vasilis Liaskovitis Subject: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Thu, 15 Nov 2012 11:22:47 +0100 Message-Id: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> X-Mailer: git-send-email 1.7.9 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2993172Ab2KOKXE (ORCPT ); Thu, 15 Nov 2012 05:23:04 -0500 Received: from mail-bk0-f46.google.com ([209.85.214.46]:45269 "EHLO mail-bk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2992947Ab2KOKXA (ORCPT ); Thu, 15 Nov 2012 05:23:00 -0500 From: Vasilis Liaskovitis To: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com Cc: rjw@sisk.pl, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Vasilis Liaskovitis Subject: [RFC PATCH v2 1/3] driver core: Introduce prepare_remove in bus_type Date: Thu, 15 Nov 2012 11:22:48 +0100 Message-Id: <1352974970-6643-2-git-send-email-vasilis.liaskovitis@profitbricks.com> X-Mailer: git-send-email 1.7.9 In-Reply-To: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2993305Ab2KOKY4 (ORCPT ); Thu, 15 Nov 2012 05:24:56 -0500 Received: from mail-bk0-f46.google.com ([209.85.214.46]:63424 "EHLO mail-bk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2993088Ab2KOKXB (ORCPT ); Thu, 15 Nov 2012 05:23:01 -0500 From: Vasilis Liaskovitis To: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com Cc: rjw@sisk.pl, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Vasilis Liaskovitis Subject: [RFC PATCH v2 2/3] acpi: Introduce prepare_remove operation in acpi_device_ops Date: Thu, 15 Nov 2012 11:22:49 +0100 Message-Id: <1352974970-6643-3-git-send-email-vasilis.liaskovitis@profitbricks.com> X-Mailer: git-send-email 1.7.9 In-Reply-To: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2993271Ab2KOKYz (ORCPT ); Thu, 15 Nov 2012 05:24:55 -0500 Received: from mail-bk0-f46.google.com ([209.85.214.46]:52807 "EHLO mail-bk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2992441Ab2KOKXC (ORCPT ); Thu, 15 Nov 2012 05:23:02 -0500 From: Vasilis Liaskovitis To: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com Cc: rjw@sisk.pl, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Vasilis Liaskovitis Subject: [RFC PATCH v2 3/3] acpi_memhotplug: Add prepare_remove operation Date: Thu, 15 Nov 2012 11:22:50 +0100 Message-Id: <1352974970-6643-4-git-send-email-vasilis.liaskovitis@profitbricks.com> X-Mailer: git-send-email 1.7.9 In-Reply-To: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753568Ab2KPVNA (ORCPT ); Fri, 16 Nov 2012 16:13:00 -0500 Received: from ogre.sisk.pl ([193.178.161.156]:49608 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751712Ab2KPVM7 (ORCPT ); Fri, 16 Nov 2012 16:12:59 -0500 From: "Rafael J. Wysocki" To: Vasilis Liaskovitis Cc: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Greg Kroah-Hartman Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 22:17:17 +0100 Message-ID: <11897248.2VNutIHaJi@vostro.rjw.lan> User-Agent: KMail/4.8.5 (Linux/3.7.0-rc5; KDE/4.8.5; x86_64; ; ) In-Reply-To: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753713Ab2KPXnU (ORCPT ); Fri, 16 Nov 2012 18:43:20 -0500 Received: from g5t0008.atlanta.hp.com ([15.192.0.45]:8622 "EHLO g5t0008.atlanta.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753584Ab2KPXnS (ORCPT ); Fri, 16 Nov 2012 18:43:18 -0500 Message-ID: <1353108906.10624.5.camel@misato.fc.hp.com> Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation From: Toshi Kani To: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Date: Fri, 16 Nov 2012 16:35:06 -0700 In-Reply-To: <20121116233355.GA21144@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.4.4 (3.4.4-2.fc17) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753837Ab2KQACA (ORCPT ); Fri, 16 Nov 2012 19:02:00 -0500 Received: from mail.kernel.org ([198.145.19.201]:37517 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753671Ab2KQAB7 (ORCPT ); Fri, 16 Nov 2012 19:01:59 -0500 Date: Fri, 16 Nov 2012 16:02:50 -0800 From: Greg Kroah-Hartman To: Toshi Kani Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Message-ID: <20121117000250.GA4425@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> <1353108906.10624.5.camel@misato.fc.hp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1353108906.10624.5.camel@misato.fc.hp.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753865Ab2KQARL (ORCPT ); Fri, 16 Nov 2012 19:17:11 -0500 Received: from g1t0029.austin.hp.com ([15.216.28.36]:5171 "EHLO g1t0029.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753839Ab2KQARH (ORCPT ); Fri, 16 Nov 2012 19:17:07 -0500 Message-ID: <1353110933.10939.6.camel@misato.fc.hp.com> Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation From: Toshi Kani To: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Date: Fri, 16 Nov 2012 17:08:53 -0700 In-Reply-To: <20121117000250.GA4425@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> <1353108906.10624.5.camel@misato.fc.hp.com> <20121117000250.GA4425@kroah.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.4.4 (3.4.4-2.fc17) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > > > > > > > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752513Ab2KRQQx (ORCPT ); Sun, 18 Nov 2012 11:16:53 -0500 Received: from mail-pa0-f46.google.com ([209.85.220.46]:39772 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752383Ab2KRQQv (ORCPT ); Sun, 18 Nov 2012 11:16:51 -0500 Message-ID: <50A909E1.9030708@gmail.com> Date: Mon, 19 Nov 2012 00:16:33 +0800 From: Jiang Liu User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121028 Thunderbird/16.0.2 MIME-Version: 1.0 To: Toshi Kani CC: Greg Kroah-Hartman , "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> <1353108906.10624.5.camel@misato.fc.hp.com> <20121117000250.GA4425@kroah.com> <1353110933.10939.6.camel@misato.fc.hp.com> <20121117002232.GA22543@kroah.com> <1353111905.10939.12.camel@misato.fc.hp.com> In-Reply-To: <1353111905.10939.12.camel@misato.fc.hp.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx179.postini.com [74.125.245.179]) by kanga.kvack.org (Postfix) with SMTP id 146BC6B005A for ; Fri, 16 Nov 2012 16:33:15 -0500 (EST) Received: by mail-pb0-f41.google.com with SMTP id xa7so2405498pbc.14 for ; Fri, 16 Nov 2012 13:33:14 -0800 (PST) Date: Fri, 16 Nov 2012 13:33:10 -0800 From: Greg Kroah-Hartman Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Message-ID: <20121116213310.GA12925@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <11897248.2VNutIHaJi@vostro.rjw.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <11897248.2VNutIHaJi@vostro.rjw.lan> Sender: owner-linux-mm@kvack.org List-ID: To: "Rafael J. Wysocki" Cc: Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx155.postini.com [74.125.245.155]) by kanga.kvack.org (Postfix) with SMTP id 4DE546B005A for ; Fri, 16 Nov 2012 16:37:31 -0500 (EST) From: "Rafael J. Wysocki" Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 22:41:52 +0100 Message-ID: <6896483.i6Vm9YII9b@vostro.rjw.lan> In-Reply-To: <20121116213310.GA12925@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <11897248.2VNutIHaJi@vostro.rjw.lan> <20121116213310.GA12925@kroah.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" Sender: owner-linux-mm@kvack.org List-ID: To: Greg Kroah-Hartman Cc: Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx199.postini.com [74.125.245.199]) by kanga.kvack.org (Postfix) with SMTP id A51626B005A for ; Fri, 16 Nov 2012 16:39:37 -0500 (EST) From: "Rafael J. Wysocki" Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Date: Fri, 16 Nov 2012 22:43:59 +0100 Message-ID: <1446291.TgLDtXqY7q@vostro.rjw.lan> In-Reply-To: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" Sender: owner-linux-mm@kvack.org List-ID: To: Vasilis Liaskovitis Cc: linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, toshi.kani@hp.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx143.postini.com [74.125.245.143]) by kanga.kvack.org (Postfix) with SMTP id C27CC6B0068 for ; Fri, 16 Nov 2012 17:53:58 -0500 (EST) Message-ID: <1353105943.12509.60.camel@misato.fc.hp.com> Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation From: Toshi Kani Date: Fri, 16 Nov 2012 15:45:43 -0700 In-Reply-To: <1446291.TgLDtXqY7q@vostro.rjw.lan> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: "Rafael J. Wysocki" Cc: Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx108.postini.com [74.125.245.108]) by kanga.kvack.org (Postfix) with SMTP id F3F5D6B002B for ; Fri, 16 Nov 2012 18:01:47 -0500 (EST) Received: by mail-pb0-f41.google.com with SMTP id xa7so2443806pbc.14 for ; Fri, 16 Nov 2012 15:01:47 -0800 (PST) Date: Fri, 16 Nov 2012 15:01:43 -0800 From: Greg Kroah-Hartman Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Message-ID: <20121116230143.GA15338@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1353105943.12509.60.camel@misato.fc.hp.com> Sender: owner-linux-mm@kvack.org List-ID: To: Toshi Kani Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx130.postini.com [74.125.245.130]) by kanga.kvack.org (Postfix) with SMTP id 158CA6B002B for ; Fri, 16 Nov 2012 18:22:58 -0500 (EST) Message-ID: <1353107684.12509.65.camel@misato.fc.hp.com> Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation From: Toshi Kani Date: Fri, 16 Nov 2012 16:14:44 -0700 In-Reply-To: <20121116230143.GA15338@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx151.postini.com [74.125.245.151]) by kanga.kvack.org (Postfix) with SMTP id 810446B004D for ; Fri, 16 Nov 2012 18:33:59 -0500 (EST) Received: by mail-da0-f41.google.com with SMTP id i14so1463739dad.14 for ; Fri, 16 Nov 2012 15:33:58 -0800 (PST) Date: Fri, 16 Nov 2012 15:33:55 -0800 From: Greg Kroah-Hartman Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Message-ID: <20121116233355.GA21144@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1353107684.12509.65.camel@misato.fc.hp.com> Sender: owner-linux-mm@kvack.org List-ID: To: Toshi Kani Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx164.postini.com [74.125.245.164]) by kanga.kvack.org (Postfix) with SMTP id D7D266B0071 for ; Fri, 16 Nov 2012 19:22:36 -0500 (EST) Received: by mail-pa0-f41.google.com with SMTP id fa10so2392931pad.14 for ; Fri, 16 Nov 2012 16:22:36 -0800 (PST) Date: Fri, 16 Nov 2012 16:22:32 -0800 From: Greg Kroah-Hartman Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation Message-ID: <20121117002232.GA22543@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> <1353108906.10624.5.camel@misato.fc.hp.com> <20121117000250.GA4425@kroah.com> <1353110933.10939.6.camel@misato.fc.hp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1353110933.10939.6.camel@misato.fc.hp.com> Sender: owner-linux-mm@kvack.org List-ID: To: Toshi Kani Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx174.postini.com [74.125.245.174]) by kanga.kvack.org (Postfix) with SMTP id 96BC26B0081 for ; Fri, 16 Nov 2012 19:33:18 -0500 (EST) Message-ID: <1353111905.10939.12.camel@misato.fc.hp.com> Subject: Re: [RFC PATCH v2 0/3] acpi: Introduce prepare_remove device operation From: Toshi Kani Date: Fri, 16 Nov 2012 17:25:05 -0700 In-Reply-To: <20121117002232.GA22543@kroah.com> References: <1352974970-6643-1-git-send-email-vasilis.liaskovitis@profitbricks.com> <1446291.TgLDtXqY7q@vostro.rjw.lan> <1353105943.12509.60.camel@misato.fc.hp.com> <20121116230143.GA15338@kroah.com> <1353107684.12509.65.camel@misato.fc.hp.com> <20121116233355.GA21144@kroah.com> <1353108906.10624.5.camel@misato.fc.hp.com> <20121117000250.GA4425@kroah.com> <1353110933.10939.6.camel@misato.fc.hp.com> <20121117002232.GA22543@kroah.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Greg Kroah-Hartman Cc: "Rafael J. Wysocki" , Vasilis Liaskovitis , linux-acpi@vger.kernel.org, isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com, lenb@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org 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: email@kvack.org