[RFC PATCH v2 04/16] ACPI: introduce interfaces to manage ACPI device reference count

From: Jiang Liu
Date: Sat Aug 04 2012 - 08:20:19 EST


From: Jiang Liu <jiang.liu@xxxxxxxxxx>

Function acpi_bus_get_device() return an ACPI device without holding a
reference count the device, which is not safe to ACPI hotplug operations.

So change acpi_bus_get_device() to hold a reference count on the returned
ACPI device and introduces acpi_get_device()/acpi_put_device() to manage
ACPI device reference count.

This patch is still incomplete, need to check and modify all callers of
acpi_bus_get_device().

Signed-off-by: Jiang Liu <liuj97@xxxxxxxxx>
---
drivers/acpi/bus.c | 22 +++++++++++++++++++---
drivers/acpi/internal.h | 3 +++
drivers/acpi/scan.c | 6 ++++++
drivers/gpu/drm/i915/intel_opregion.c | 2 ++
drivers/gpu/drm/nouveau/nouveau_acpi.c | 1 +
drivers/pci/hotplug/acpiphp_glue.c | 9 +++++++--
drivers/pci/hotplug/acpiphp_ibm.c | 5 ++++-
drivers/pci/hotplug/sgi_hotplug.c | 6 +++++-
drivers/platform/x86/thinkpad_acpi.c | 2 ++
drivers/pnp/pnpacpi/core.c | 23 ++++++++++-------------
include/acpi/acpi_bus.h | 16 +++++++++++++++-
11 files changed, 74 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index adceafd..e13dcbc 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -89,26 +89,42 @@ static struct dmi_system_id dsdt_dmi_table[] __initdata = {
Device Management
-------------------------------------------------------------------------- */

-int acpi_bus_get_device(acpi_handle handle, struct acpi_device **device)
+bool acpi_bus_has_device(acpi_handle handle)
{
- acpi_status status = AE_OK;
+ acpi_status status;
+ struct acpi_device *device = NULL;
+
+ down_read(&acpi_device_data_handler_sem);
+ status = acpi_get_data(handle, acpi_bus_data_handler, (void **)&device);
+ up_read(&acpi_device_data_handler_sem);
+
+ return ACPI_SUCCESS(status) && device;
+}
+EXPORT_SYMBOL(acpi_bus_has_device);

+int acpi_bus_get_device(acpi_handle handle, struct acpi_device **device)
+{
+ acpi_status status;

if (!device)
return -EINVAL;

/* TBD: Support fixed-feature devices */

+ down_read(&acpi_device_data_handler_sem);
status = acpi_get_data(handle, acpi_bus_data_handler, (void **)device);
if (ACPI_FAILURE(status) || !*device) {
+ up_read(&acpi_device_data_handler_sem);
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No context for object [%p]\n",
handle));
return -ENODEV;
+ } else {
+ get_device(&(*device)->dev);
+ up_read(&acpi_device_data_handler_sem);
}

return 0;
}
-
EXPORT_SYMBOL(acpi_bus_get_device);

acpi_status acpi_bus_get_status_handle(acpi_handle handle,
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index ca75b9c..70843c0 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -93,4 +93,7 @@ static inline int suspend_nvs_save(void) { return 0; }
static inline void suspend_nvs_restore(void) {}
#endif

+extern struct rw_semaphore acpi_device_data_handler_sem;
+void acpi_bus_data_handler(acpi_handle handle, void *context);
+
#endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 459593a..28408af 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -10,6 +10,7 @@
#include <linux/signal.h>
#include <linux/kthread.h>
#include <linux/dmi.h>
+#include <linux/sem.h>

#include <acpi/acpi_drivers.h>
#include <linux/idr.h>
@@ -33,6 +34,7 @@ static LIST_HEAD(acpi_device_list);
static LIST_HEAD(acpi_bus_id_list);
DEFINE_MUTEX(acpi_device_lock);
LIST_HEAD(acpi_wakeup_device_list);
+DECLARE_RWSEM(acpi_device_data_handler_sem);

struct acpi_device_bus_id{
char bus_id[15];
@@ -564,7 +566,9 @@ static void acpi_device_unregister(struct acpi_device *device, int type)
acpi_device_release_id(device);
mutex_unlock(&acpi_device_lock);

+ down_write(&acpi_device_data_handler_sem);
acpi_detach_data(device->handle, acpi_bus_data_handler);
+ up_write(&acpi_device_data_handler_sem);

acpi_device_remove_files(device);
device_unregister(&device->dev);
@@ -1227,8 +1231,10 @@ static int acpi_device_set_context(struct acpi_device *device)
if (!device->handle)
return 0;

+ down_write(&acpi_device_data_handler_sem);
status = acpi_attach_data(device->handle,
acpi_bus_data_handler, device);
+ up_write(&acpi_device_data_handler_sem);
if (ACPI_SUCCESS(status))
return 0;

diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 18bd0af..da7dd48 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -359,6 +359,8 @@ static void intel_didl_outputs(struct drm_device *dev)
}
}

+ acpi_device_put(acpi_dev);
+
if (!acpi_video_bus) {
pr_warn("No ACPI video bus found\n");
return;
diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index fc841e8..f83efe3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -423,6 +423,7 @@ nouveau_acpi_edid(struct drm_device *dev, struct drm_connector *connector)
return -ENODEV;

ret = acpi_video_get_edid(acpidev, type, -1, &edid);
+ acpi_device_put(acpidev);
if (ret < 0)
return ret;

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 806c44f..78cffba4 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -724,11 +724,13 @@ static int acpiphp_bus_add(struct acpiphp_func *func)
* the bus then re-add it...
*/
ret_val = acpi_bus_trim(device, 1);
+ acpi_device_put(device);
dbg("acpi_bus_trim return %x\n", ret_val);
}

ret_val = acpi_bus_add(&device, pdevice, func->handle,
ACPI_BUS_TYPE_DEVICE);
+ acpi_device_put(pdevice);
if (ret_val) {
dbg("error adding bus, %x\n",
-ret_val);
@@ -760,6 +762,8 @@ static int acpiphp_bus_trim(acpi_handle handle)
if (retval)
err("cannot remove from acpi list\n");

+ acpi_device_put(device);
+
return retval;
}

@@ -1114,8 +1118,10 @@ static void handle_bridge_insertion(acpi_handle handle, u32 type)
}
if (acpi_bus_add(&device, pdevice, handle, ACPI_BUS_TYPE_DEVICE)) {
err("cannot add bridge to acpi list\n");
+ acpi_device_put(pdevice);
return;
}
+ acpi_device_put(pdevice);
if (!acpiphp_configure_bridge(handle) &&
!acpi_bus_start(device))
add_bridge(handle);
@@ -1192,7 +1198,6 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
char objname[64];
struct acpi_buffer buffer = { .length = sizeof(objname),
.pointer = objname };
- struct acpi_device *device;
int num_sub_bridges = 0;
struct acpiphp_hp_work *hp_work;
acpi_handle handle;
@@ -1202,7 +1207,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
handle = hp_work->handle;
type = hp_work->type;

- if (acpi_bus_get_device(handle, &device)) {
+ if (!acpi_bus_has_device(handle)) {
/* This bridge must have just been physically inserted */
handle_bridge_insertion(handle, type);
goto out;
diff --git a/drivers/pci/hotplug/acpiphp_ibm.c b/drivers/pci/hotplug/acpiphp_ibm.c
index c35e8ad..ecd4b8d 100644
--- a/drivers/pci/hotplug/acpiphp_ibm.c
+++ b/drivers/pci/hotplug/acpiphp_ibm.c
@@ -450,7 +450,7 @@ static int __init ibm_acpiphp_init(void)
}
if (acpiphp_register_attention(&ibm_attention_info)) {
retval = -ENODEV;
- goto init_return;
+ goto put_device;
}

ibm_note.device = device;
@@ -471,6 +471,8 @@ static int __init ibm_acpiphp_init(void)

init_cleanup:
acpiphp_unregister_attention(&ibm_attention_info);
+put_device:
+ acpi_device_put(device);
init_return:
return retval;
}
@@ -493,6 +495,7 @@ static void __exit ibm_acpiphp_exit(void)
err("%s: Notification handler removal failed\n", __func__);
/* remove the /sys entries */
sysfs_remove_bin_file(sysdir, &ibm_apci_table_attr);
+ acpi_device_put(ibm_note.device);
}

module_init(ibm_acpiphp_init);
diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
index de57311..ad120f1 100644
--- a/drivers/pci/hotplug/sgi_hotplug.c
+++ b/drivers/pci/hotplug/sgi_hotplug.c
@@ -464,6 +464,8 @@ static int enable_slot(struct hotplug_slot *bss_hotplug_slot)
}
}
}
+
+ acpi_device_put(pdevice);
}

/* Call the driver for the new device */
@@ -540,8 +542,10 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)

ret = acpi_bus_get_device(chandle,
&device);
- if (ACPI_SUCCESS(ret))
+ if (ACPI_SUCCESS(ret)) {
acpi_bus_trim(device, 1);
+ acpi_device_put(device);
+ }
}
}

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 8b5610d..285eac5f 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -782,6 +782,7 @@ static int __init setup_acpi_notify(struct ibm_struct *ibm)
pr_err("acpi_install_notify_handler(%s) failed: %s\n",
ibm->name, acpi_format_exception(status));
}
+ acpi_device_put(ibm->acpi->device);
return -ENODEV;
}
ibm->flags.acpi_notify_installed = 1;
@@ -8461,6 +8462,7 @@ static void ibm_exit(struct ibm_struct *ibm)
acpi_remove_notify_handler(*ibm->acpi->handle,
ibm->acpi->type,
dispatch_acpi_notify);
+ acpi_device_put(ibm->acpi->device);
ibm->flags.acpi_notify_installed = 0;
}

diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index d21e8f5..684b462 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -82,7 +82,6 @@ static int pnpacpi_get_resources(struct pnp_dev *dev)

static int pnpacpi_set_resources(struct pnp_dev *dev)
{
- struct acpi_device *acpi_dev;
acpi_handle handle;
struct acpi_buffer buffer;
int ret;
@@ -90,7 +89,7 @@ static int pnpacpi_set_resources(struct pnp_dev *dev)
pnp_dbg(&dev->dev, "set resources\n");

handle = DEVICE_ACPI_HANDLE(&dev->dev);
- if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
+ if (!handle || !acpi_bus_has_device(handle)) {
dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
return -ENODEV;
}
@@ -113,14 +112,13 @@ static int pnpacpi_set_resources(struct pnp_dev *dev)

static int pnpacpi_disable_resources(struct pnp_dev *dev)
{
- struct acpi_device *acpi_dev;
acpi_handle handle;
int ret;

dev_dbg(&dev->dev, "disable resources\n");

handle = DEVICE_ACPI_HANDLE(&dev->dev);
- if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
+ if (!handle || !acpi_bus_has_device(handle)) {
dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
return 0;
}
@@ -138,11 +136,10 @@ static int pnpacpi_disable_resources(struct pnp_dev *dev)
#ifdef CONFIG_ACPI_SLEEP
static bool pnpacpi_can_wakeup(struct pnp_dev *dev)
{
- struct acpi_device *acpi_dev;
acpi_handle handle;

handle = DEVICE_ACPI_HANDLE(&dev->dev);
- if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
+ if (!handle || !acpi_bus_has_device(handle)) {
dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
return false;
}
@@ -152,12 +149,11 @@ static bool pnpacpi_can_wakeup(struct pnp_dev *dev)

static int pnpacpi_suspend(struct pnp_dev *dev, pm_message_t state)
{
- struct acpi_device *acpi_dev;
acpi_handle handle;
int error = 0;

handle = DEVICE_ACPI_HANDLE(&dev->dev);
- if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
+ if (!handle || !acpi_bus_has_device(handle)) {
dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
return 0;
}
@@ -190,11 +186,10 @@ static int pnpacpi_suspend(struct pnp_dev *dev, pm_message_t state)

static int pnpacpi_resume(struct pnp_dev *dev)
{
- struct acpi_device *acpi_dev;
acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
int error = 0;

- if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &acpi_dev))) {
+ if (!handle || !acpi_bus_has_device(handle)) {
dev_dbg(&dev->dev, "ACPI device not found in %s!\n", __func__);
return -ENODEV;
}
@@ -310,10 +305,12 @@ static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle,
{
struct acpi_device *device;

- if (!acpi_bus_get_device(handle, &device))
- pnpacpi_add_device(device);
- else
+ if (acpi_bus_get_device(handle, &device))
return AE_CTRL_DEPTH;
+
+ pnpacpi_add_device(device);
+ acpi_device_put(device);
+
return AE_OK;
}

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 9e6e1c6..e2a3eb0 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -345,8 +345,22 @@ extern void unregister_acpi_bus_notifier(struct notifier_block *nb);
* External Functions
*/

+static inline struct acpi_device *acpi_device_get(struct acpi_device *d)
+{
+ if (d)
+ get_device(&d->dev);
+
+ return d;
+}
+
+static inline void acpi_device_put(struct acpi_device *d)
+{
+ if (d)
+ put_device(&d->dev);
+}
+
+bool acpi_bus_has_device(acpi_handle handle);
int acpi_bus_get_device(acpi_handle handle, struct acpi_device **device);
-void acpi_bus_data_handler(acpi_handle handle, void *context);
acpi_status acpi_bus_get_status_handle(acpi_handle handle,
unsigned long long *sta);
int acpi_bus_get_status(struct acpi_device *device);
--
1.7.9.5


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/