* [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail
@ 2013-01-24 22:56 Rafael J. Wysocki
2013-01-25 0:12 ` Yinghai Lu
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2013-01-24 22:56 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: LKML, Bjorn Helgaas, Yinghai Lu, Toshi Kani, Jiang Liu
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Since acpi_bus_trim() cannot fail, change its definition to a void
function, so that its callers don't check the return value in vain
and update the callers.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
On top of current linux-pm.git/linux-next.
Thanks,
Rafael
---
drivers/acpi/dock.c | 8 ++------
drivers/acpi/scan.c | 12 +++---------
drivers/pci/hotplug/acpiphp_glue.c | 10 +++-------
include/acpi/acpi_bus.h | 2 +-
4 files changed, 9 insertions(+), 23 deletions(-)
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -127,13 +127,8 @@ void acpi_bus_hot_remove_device(void *co
ACPI_DEBUG_PRINT((ACPI_DB_INFO,
"Hot-removing device %s...\n", dev_name(&device->dev)));
- if (acpi_bus_trim(device)) {
- printk(KERN_ERR PREFIX
- "Removing device failed\n");
- goto err_out;
- }
-
- /* device has been freed */
+ acpi_bus_trim(device);
+ /* Device node has been released. */
device = NULL;
/* power off device */
@@ -1656,7 +1651,7 @@ static acpi_status acpi_bus_remove(acpi_
return AE_OK;
}
-int acpi_bus_trim(struct acpi_device *start)
+void acpi_bus_trim(struct acpi_device *start)
{
/*
* Execute acpi_bus_device_detach() as a post-order callback to detach
@@ -1672,7 +1667,6 @@ int acpi_bus_trim(struct acpi_device *st
acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
acpi_bus_remove, NULL, NULL);
acpi_bus_remove(start->handle, 0, NULL, NULL);
- return 0;
}
EXPORT_SYMBOL_GPL(acpi_bus_trim);
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -386,7 +386,7 @@ int acpi_bus_register_driver(struct acpi
void acpi_bus_unregister_driver(struct acpi_driver *driver);
int acpi_bus_scan(acpi_handle handle);
void acpi_bus_hot_remove_device(void *context);
-int acpi_bus_trim(struct acpi_device *start);
+void acpi_bus_trim(struct acpi_device *start);
acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
int acpi_match_device_ids(struct acpi_device *device,
const struct acpi_device_id *ids);
Index: linux-pm/drivers/acpi/dock.c
===================================================================
--- linux-pm.orig/drivers/acpi/dock.c
+++ linux-pm/drivers/acpi/dock.c
@@ -336,13 +336,9 @@ static struct acpi_device * dock_create_
static void dock_remove_acpi_device(acpi_handle handle)
{
struct acpi_device *device;
- int ret;
- if (!acpi_bus_get_device(handle, &device)) {
- ret = acpi_bus_trim(device);
- if (ret)
- pr_debug("error removing bus, %x\n", -ret);
- }
+ if (!acpi_bus_get_device(handle, &device))
+ acpi_bus_trim(device);
}
/**
Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -742,8 +742,7 @@ static int acpiphp_bus_add(struct acpiph
/* this shouldn't be in here, so remove
* the bus then re-add it...
*/
- ret_val = acpi_bus_trim(device);
- dbg("acpi_bus_trim return %x\n", ret_val);
+ acpi_bus_trim(device);
}
ret_val = acpi_bus_scan(func->handle);
@@ -772,11 +771,8 @@ static int acpiphp_bus_trim(acpi_handle
return retval;
}
- retval = acpi_bus_trim(device);
- if (retval)
- err("cannot remove from acpi list\n");
-
- return retval;
+ acpi_bus_trim(device);
+ return 0;
}
static void acpiphp_set_acpi_region(struct acpiphp_slot *slot)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail
2013-01-24 22:56 [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail Rafael J. Wysocki
@ 2013-01-25 0:12 ` Yinghai Lu
2013-01-25 0:43 ` Yasuaki Ishimatsu
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Yinghai Lu @ 2013-01-25 0:12 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, Toshi Kani, Jiang Liu
On Thu, Jan 24, 2013 at 2:56 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Since acpi_bus_trim() cannot fail, change its definition to a void
> function, so that its callers don't check the return value in vain
> and update the callers.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
Acked-by: Yinghai Lu <yinghai@kernel.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail
2013-01-24 22:56 [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail Rafael J. Wysocki
2013-01-25 0:12 ` Yinghai Lu
@ 2013-01-25 0:43 ` Yasuaki Ishimatsu
2013-01-25 15:19 ` Toshi Kani
2013-02-18 12:25 ` Vasilis Liaskovitis
3 siblings, 0 replies; 7+ messages in thread
From: Yasuaki Ishimatsu @ 2013-01-25 0:43 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, Yinghai Lu,
Toshi Kani, Jiang Liu
2013/01/25 7:56, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Since acpi_bus_trim() cannot fail, change its definition to a void
> function, so that its callers don't check the return value in vain
> and update the callers.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
Acked-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
>
> On top of current linux-pm.git/linux-next.
>
> Thanks,
> Rafael
>
> ---
> drivers/acpi/dock.c | 8 ++------
> drivers/acpi/scan.c | 12 +++---------
> drivers/pci/hotplug/acpiphp_glue.c | 10 +++-------
> include/acpi/acpi_bus.h | 2 +-
> 4 files changed, 9 insertions(+), 23 deletions(-)
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -127,13 +127,8 @@ void acpi_bus_hot_remove_device(void *co
> ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> "Hot-removing device %s...\n", dev_name(&device->dev)));
>
> - if (acpi_bus_trim(device)) {
> - printk(KERN_ERR PREFIX
> - "Removing device failed\n");
> - goto err_out;
> - }
> -
> - /* device has been freed */
> + acpi_bus_trim(device);
> + /* Device node has been released. */
> device = NULL;
>
> /* power off device */
> @@ -1656,7 +1651,7 @@ static acpi_status acpi_bus_remove(acpi_
> return AE_OK;
> }
>
> -int acpi_bus_trim(struct acpi_device *start)
> +void acpi_bus_trim(struct acpi_device *start)
> {
> /*
> * Execute acpi_bus_device_detach() as a post-order callback to detach
> @@ -1672,7 +1667,6 @@ int acpi_bus_trim(struct acpi_device *st
> acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
> acpi_bus_remove, NULL, NULL);
> acpi_bus_remove(start->handle, 0, NULL, NULL);
> - return 0;
> }
> EXPORT_SYMBOL_GPL(acpi_bus_trim);
>
> Index: linux-pm/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ linux-pm/include/acpi/acpi_bus.h
> @@ -386,7 +386,7 @@ int acpi_bus_register_driver(struct acpi
> void acpi_bus_unregister_driver(struct acpi_driver *driver);
> int acpi_bus_scan(acpi_handle handle);
> void acpi_bus_hot_remove_device(void *context);
> -int acpi_bus_trim(struct acpi_device *start);
> +void acpi_bus_trim(struct acpi_device *start);
> acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
> int acpi_match_device_ids(struct acpi_device *device,
> const struct acpi_device_id *ids);
> Index: linux-pm/drivers/acpi/dock.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/dock.c
> +++ linux-pm/drivers/acpi/dock.c
> @@ -336,13 +336,9 @@ static struct acpi_device * dock_create_
> static void dock_remove_acpi_device(acpi_handle handle)
> {
> struct acpi_device *device;
> - int ret;
>
> - if (!acpi_bus_get_device(handle, &device)) {
> - ret = acpi_bus_trim(device);
> - if (ret)
> - pr_debug("error removing bus, %x\n", -ret);
> - }
> + if (!acpi_bus_get_device(handle, &device))
> + acpi_bus_trim(device);
> }
>
> /**
> Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
> +++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
> @@ -742,8 +742,7 @@ static int acpiphp_bus_add(struct acpiph
> /* this shouldn't be in here, so remove
> * the bus then re-add it...
> */
> - ret_val = acpi_bus_trim(device);
> - dbg("acpi_bus_trim return %x\n", ret_val);
> + acpi_bus_trim(device);
> }
>
> ret_val = acpi_bus_scan(func->handle);
> @@ -772,11 +771,8 @@ static int acpiphp_bus_trim(acpi_handle
> return retval;
> }
>
> - retval = acpi_bus_trim(device);
> - if (retval)
> - err("cannot remove from acpi list\n");
> -
> - return retval;
> + acpi_bus_trim(device);
> + return 0;
> }
>
> static void acpiphp_set_acpi_region(struct acpiphp_slot *slot)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail
2013-01-24 22:56 [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail Rafael J. Wysocki
2013-01-25 0:12 ` Yinghai Lu
2013-01-25 0:43 ` Yasuaki Ishimatsu
@ 2013-01-25 15:19 ` Toshi Kani
2013-02-18 12:25 ` Vasilis Liaskovitis
3 siblings, 0 replies; 7+ messages in thread
From: Toshi Kani @ 2013-01-25 15:19 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, Yinghai Lu, Jiang Liu
On Thu, 2013-01-24 at 23:56 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Since acpi_bus_trim() cannot fail, change its definition to a void
> function, so that its callers don't check the return value in vain
> and update the callers.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Toshi Kani <toshi.kani@hp.com>
Thanks,
-Toshi
> ---
>
> On top of current linux-pm.git/linux-next.
>
> Thanks,
> Rafael
>
> ---
> drivers/acpi/dock.c | 8 ++------
> drivers/acpi/scan.c | 12 +++---------
> drivers/pci/hotplug/acpiphp_glue.c | 10 +++-------
> include/acpi/acpi_bus.h | 2 +-
> 4 files changed, 9 insertions(+), 23 deletions(-)
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -127,13 +127,8 @@ void acpi_bus_hot_remove_device(void *co
> ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> "Hot-removing device %s...\n", dev_name(&device->dev)));
>
> - if (acpi_bus_trim(device)) {
> - printk(KERN_ERR PREFIX
> - "Removing device failed\n");
> - goto err_out;
> - }
> -
> - /* device has been freed */
> + acpi_bus_trim(device);
> + /* Device node has been released. */
> device = NULL;
>
> /* power off device */
> @@ -1656,7 +1651,7 @@ static acpi_status acpi_bus_remove(acpi_
> return AE_OK;
> }
>
> -int acpi_bus_trim(struct acpi_device *start)
> +void acpi_bus_trim(struct acpi_device *start)
> {
> /*
> * Execute acpi_bus_device_detach() as a post-order callback to detach
> @@ -1672,7 +1667,6 @@ int acpi_bus_trim(struct acpi_device *st
> acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
> acpi_bus_remove, NULL, NULL);
> acpi_bus_remove(start->handle, 0, NULL, NULL);
> - return 0;
> }
> EXPORT_SYMBOL_GPL(acpi_bus_trim);
>
> Index: linux-pm/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ linux-pm/include/acpi/acpi_bus.h
> @@ -386,7 +386,7 @@ int acpi_bus_register_driver(struct acpi
> void acpi_bus_unregister_driver(struct acpi_driver *driver);
> int acpi_bus_scan(acpi_handle handle);
> void acpi_bus_hot_remove_device(void *context);
> -int acpi_bus_trim(struct acpi_device *start);
> +void acpi_bus_trim(struct acpi_device *start);
> acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
> int acpi_match_device_ids(struct acpi_device *device,
> const struct acpi_device_id *ids);
> Index: linux-pm/drivers/acpi/dock.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/dock.c
> +++ linux-pm/drivers/acpi/dock.c
> @@ -336,13 +336,9 @@ static struct acpi_device * dock_create_
> static void dock_remove_acpi_device(acpi_handle handle)
> {
> struct acpi_device *device;
> - int ret;
>
> - if (!acpi_bus_get_device(handle, &device)) {
> - ret = acpi_bus_trim(device);
> - if (ret)
> - pr_debug("error removing bus, %x\n", -ret);
> - }
> + if (!acpi_bus_get_device(handle, &device))
> + acpi_bus_trim(device);
> }
>
> /**
> Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
> +++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
> @@ -742,8 +742,7 @@ static int acpiphp_bus_add(struct acpiph
> /* this shouldn't be in here, so remove
> * the bus then re-add it...
> */
> - ret_val = acpi_bus_trim(device);
> - dbg("acpi_bus_trim return %x\n", ret_val);
> + acpi_bus_trim(device);
> }
>
> ret_val = acpi_bus_scan(func->handle);
> @@ -772,11 +771,8 @@ static int acpiphp_bus_trim(acpi_handle
> return retval;
> }
>
> - retval = acpi_bus_trim(device);
> - if (retval)
> - err("cannot remove from acpi list\n");
> -
> - return retval;
> + acpi_bus_trim(device);
> + return 0;
> }
>
> static void acpiphp_set_acpi_region(struct acpiphp_slot *slot)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail
2013-01-24 22:56 [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail Rafael J. Wysocki
` (2 preceding siblings ...)
2013-01-25 15:19 ` Toshi Kani
@ 2013-02-18 12:25 ` Vasilis Liaskovitis
2013-02-18 13:17 ` Rafael J. Wysocki
3 siblings, 1 reply; 7+ messages in thread
From: Vasilis Liaskovitis @ 2013-02-18 12:25 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, Yinghai Lu,
Toshi Kani, Jiang Liu, wency
Hi,
On Thu, Jan 24, 2013 at 11:56:50PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Since acpi_bus_trim() cannot fail, change its definition to a void
> function, so that its callers don't check the return value in vain
> and update the callers.
I have missed a few patchsets/discussions in the last month and wanted to
ask a question related to this: Does the new always-succeed 2-pass
trim_device design guarantee safe memory hot-remove operations?
Afaict if memory offline fails now, the device is ejected (_EJ0) anyways
causing a panic. Tested in a VM with linux-next-20130207 and
linux-next-20130218 by doing an SCI-eject request on a hot-plugged dimm.
Are there more patches in development for safe memory hot-remove?
thanks,
- Vasilis
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail
2013-02-18 12:25 ` Vasilis Liaskovitis
@ 2013-02-18 13:17 ` Rafael J. Wysocki
2013-02-18 14:53 ` Vasilis Liaskovitis
0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2013-02-18 13:17 UTC (permalink / raw)
To: Vasilis Liaskovitis
Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, Yinghai Lu,
Toshi Kani, Jiang Liu, wency
On Monday, February 18, 2013 01:25:49 PM Vasilis Liaskovitis wrote:
> Hi,
>
> On Thu, Jan 24, 2013 at 11:56:50PM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Since acpi_bus_trim() cannot fail, change its definition to a void
> > function, so that its callers don't check the return value in vain
> > and update the callers.
>
> I have missed a few patchsets/discussions in the last month and wanted to
> ask a question related to this: Does the new always-succeed 2-pass
> trim_device design guarantee safe memory hot-remove operations?
I doesn't by itself. Nor it really can, because the .remove() callbacks of
device drivers are not allowed to fail.
> Afaict if memory offline fails now, the device is ejected (_EJ0) anyways
> causing a panic. Tested in a VM with linux-next-20130207 and
> linux-next-20130218 by doing an SCI-eject request on a hot-plugged dimm.
>
> Are there more patches in development for safe memory hot-remove?
Yes, there are. I sent a patch series yesterday introducing some safety
measures (you can disable memory hotplug from user space or disable
automatic ejection). There's more to come still.
The plan is to introduce offline/online operations for memory modules (in
analogy with CPU core online/offline) that can be started by user space and
memory eject will only be possible after offline (i.e. when the memory module
is known to be not in use).
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail
2013-02-18 13:17 ` Rafael J. Wysocki
@ 2013-02-18 14:53 ` Vasilis Liaskovitis
0 siblings, 0 replies; 7+ messages in thread
From: Vasilis Liaskovitis @ 2013-02-18 14:53 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: ACPI Devel Maling List, LKML, Bjorn Helgaas, Yinghai Lu,
Toshi Kani, Jiang Liu, wency
On Mon, Feb 18, 2013 at 02:17:02PM +0100, Rafael J. Wysocki wrote:
> On Monday, February 18, 2013 01:25:49 PM Vasilis Liaskovitis wrote:
> > Hi,
> >
> > On Thu, Jan 24, 2013 at 11:56:50PM +0100, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > Since acpi_bus_trim() cannot fail, change its definition to a void
> > > function, so that its callers don't check the return value in vain
> > > and update the callers.
> >
> > I have missed a few patchsets/discussions in the last month and wanted to
> > ask a question related to this: Does the new always-succeed 2-pass
> > trim_device design guarantee safe memory hot-remove operations?
>
> I doesn't by itself. Nor it really can, because the .remove() callbacks of
> device drivers are not allowed to fail.
right.
>
> > Afaict if memory offline fails now, the device is ejected (_EJ0) anyways
> > causing a panic. Tested in a VM with linux-next-20130207 and
> > linux-next-20130218 by doing an SCI-eject request on a hot-plugged dimm.
> >
> > Are there more patches in development for safe memory hot-remove?
>
> Yes, there are. I sent a patch series yesterday introducing some safety
> measures (you can disable memory hotplug from user space or disable
> automatic ejection). There's more to come still.
>
> The plan is to introduce offline/online operations for memory modules (in
> analogy with CPU core online/offline) that can be started by user space and
> memory eject will only be possible after offline (i.e. when the memory module
> is known to be not in use).
ok, thanks for the info. I 'll test these and the follow ups.
thanks again,
- Vasilis
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-02-18 14:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-24 22:56 [PATCH] ACPI / scan: Make it clear that acpi_bus_trim() cannot fail Rafael J. Wysocki
2013-01-25 0:12 ` Yinghai Lu
2013-01-25 0:43 ` Yasuaki Ishimatsu
2013-01-25 15:19 ` Toshi Kani
2013-02-18 12:25 ` Vasilis Liaskovitis
2013-02-18 13:17 ` Rafael J. Wysocki
2013-02-18 14:53 ` Vasilis Liaskovitis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.