ACPI: convert acpi_device_lock spinlock to mutex

Convert acpi_device_lock to a mutex to avoid
a potential race upon access to /proc/acpi/wakeup

Delete the lock entirely in wakeup.c
since it is not necessary (and can not sleep)

Found-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
This commit is contained in:
Shaohua Li 2009-04-07 10:24:29 +08:00 committed by Len Brown
parent d508afb437
commit 9090589d87
4 changed files with 21 additions and 39 deletions

View file

@ -343,9 +343,6 @@ acpi_system_write_alarm(struct file *file,
} }
#endif /* HAVE_ACPI_LEGACY_ALARM */ #endif /* HAVE_ACPI_LEGACY_ALARM */
extern struct list_head acpi_wakeup_device_list;
extern spinlock_t acpi_device_lock;
static int static int
acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset) acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
{ {
@ -353,7 +350,7 @@ acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
seq_printf(seq, "Device\tS-state\t Status Sysfs node\n"); seq_printf(seq, "Device\tS-state\t Status Sysfs node\n");
spin_lock(&acpi_device_lock); mutex_lock(&acpi_device_lock);
list_for_each_safe(node, next, &acpi_wakeup_device_list) { list_for_each_safe(node, next, &acpi_wakeup_device_list) {
struct acpi_device *dev = struct acpi_device *dev =
container_of(node, struct acpi_device, wakeup_list); container_of(node, struct acpi_device, wakeup_list);
@ -361,7 +358,6 @@ acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
if (!dev->wakeup.flags.valid) if (!dev->wakeup.flags.valid)
continue; continue;
spin_unlock(&acpi_device_lock);
ldev = acpi_get_physical_device(dev->handle); ldev = acpi_get_physical_device(dev->handle);
seq_printf(seq, "%s\t S%d\t%c%-8s ", seq_printf(seq, "%s\t S%d\t%c%-8s ",
@ -376,9 +372,8 @@ acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
seq_printf(seq, "\n"); seq_printf(seq, "\n");
put_device(ldev); put_device(ldev);
spin_lock(&acpi_device_lock);
} }
spin_unlock(&acpi_device_lock); mutex_unlock(&acpi_device_lock);
return 0; return 0;
} }
@ -409,7 +404,7 @@ acpi_system_write_wakeup_device(struct file *file,
strbuf[len] = '\0'; strbuf[len] = '\0';
sscanf(strbuf, "%s", str); sscanf(strbuf, "%s", str);
spin_lock(&acpi_device_lock); mutex_lock(&acpi_device_lock);
list_for_each_safe(node, next, &acpi_wakeup_device_list) { list_for_each_safe(node, next, &acpi_wakeup_device_list) {
struct acpi_device *dev = struct acpi_device *dev =
container_of(node, struct acpi_device, wakeup_list); container_of(node, struct acpi_device, wakeup_list);
@ -446,7 +441,7 @@ acpi_system_write_wakeup_device(struct file *file,
} }
} }
} }
spin_unlock(&acpi_device_lock); mutex_unlock(&acpi_device_lock);
return count; return count;
} }

View file

@ -24,7 +24,7 @@ extern struct acpi_device *acpi_root;
static LIST_HEAD(acpi_device_list); static LIST_HEAD(acpi_device_list);
static LIST_HEAD(acpi_bus_id_list); static LIST_HEAD(acpi_bus_id_list);
DEFINE_SPINLOCK(acpi_device_lock); DEFINE_MUTEX(acpi_device_lock);
LIST_HEAD(acpi_wakeup_device_list); LIST_HEAD(acpi_wakeup_device_list);
struct acpi_device_bus_id{ struct acpi_device_bus_id{
@ -500,7 +500,7 @@ static int acpi_device_register(struct acpi_device *device,
return -ENOMEM; return -ENOMEM;
} }
spin_lock(&acpi_device_lock); mutex_lock(&acpi_device_lock);
/* /*
* Find suitable bus_id and instance number in acpi_bus_id_list * Find suitable bus_id and instance number in acpi_bus_id_list
* If failed, create one and link it into acpi_bus_id_list * If failed, create one and link it into acpi_bus_id_list
@ -528,7 +528,7 @@ static int acpi_device_register(struct acpi_device *device,
list_add_tail(&device->g_list, &acpi_device_list); list_add_tail(&device->g_list, &acpi_device_list);
if (device->wakeup.flags.valid) if (device->wakeup.flags.valid)
list_add_tail(&device->wakeup_list, &acpi_wakeup_device_list); list_add_tail(&device->wakeup_list, &acpi_wakeup_device_list);
spin_unlock(&acpi_device_lock); mutex_unlock(&acpi_device_lock);
if (device->parent) if (device->parent)
device->dev.parent = &parent->dev; device->dev.parent = &parent->dev;
@ -549,20 +549,20 @@ static int acpi_device_register(struct acpi_device *device,
device->removal_type = ACPI_BUS_REMOVAL_NORMAL; device->removal_type = ACPI_BUS_REMOVAL_NORMAL;
return 0; return 0;
end: end:
spin_lock(&acpi_device_lock); mutex_lock(&acpi_device_lock);
if (device->parent) { if (device->parent) {
list_del(&device->node); list_del(&device->node);
list_del(&device->g_list); list_del(&device->g_list);
} else } else
list_del(&device->g_list); list_del(&device->g_list);
list_del(&device->wakeup_list); list_del(&device->wakeup_list);
spin_unlock(&acpi_device_lock); mutex_unlock(&acpi_device_lock);
return result; return result;
} }
static void acpi_device_unregister(struct acpi_device *device, int type) static void acpi_device_unregister(struct acpi_device *device, int type)
{ {
spin_lock(&acpi_device_lock); mutex_lock(&acpi_device_lock);
if (device->parent) { if (device->parent) {
list_del(&device->node); list_del(&device->node);
list_del(&device->g_list); list_del(&device->g_list);
@ -570,7 +570,7 @@ static void acpi_device_unregister(struct acpi_device *device, int type)
list_del(&device->g_list); list_del(&device->g_list);
list_del(&device->wakeup_list); list_del(&device->wakeup_list);
spin_unlock(&acpi_device_lock); mutex_unlock(&acpi_device_lock);
acpi_detach_data(device->handle, acpi_bus_data_handler); acpi_detach_data(device->handle, acpi_bus_data_handler);

View file

@ -5,3 +5,6 @@ extern int acpi_suspend (u32 state);
extern void acpi_enable_wakeup_device_prep(u8 sleep_state); extern void acpi_enable_wakeup_device_prep(u8 sleep_state);
extern void acpi_enable_wakeup_device(u8 sleep_state); extern void acpi_enable_wakeup_device(u8 sleep_state);
extern void acpi_disable_wakeup_device(u8 sleep_state); extern void acpi_disable_wakeup_device(u8 sleep_state);
extern struct list_head acpi_wakeup_device_list;
extern struct mutex acpi_device_lock;

View file

@ -12,12 +12,14 @@
#include "internal.h" #include "internal.h"
#include "sleep.h" #include "sleep.h"
/*
* We didn't lock acpi_device_lock in the file, because it invokes oops in
* suspend/resume and isn't really required as this is called in S-state. At
* that time, there is no device hotplug
**/
#define _COMPONENT ACPI_SYSTEM_COMPONENT #define _COMPONENT ACPI_SYSTEM_COMPONENT
ACPI_MODULE_NAME("wakeup_devices") ACPI_MODULE_NAME("wakeup_devices")
extern struct list_head acpi_wakeup_device_list;
extern spinlock_t acpi_device_lock;
/** /**
* acpi_enable_wakeup_device_prep - prepare wakeup devices * acpi_enable_wakeup_device_prep - prepare wakeup devices
* @sleep_state: ACPI state * @sleep_state: ACPI state
@ -29,7 +31,6 @@ void acpi_enable_wakeup_device_prep(u8 sleep_state)
{ {
struct list_head *node, *next; struct list_head *node, *next;
spin_lock(&acpi_device_lock);
list_for_each_safe(node, next, &acpi_wakeup_device_list) { list_for_each_safe(node, next, &acpi_wakeup_device_list) {
struct acpi_device *dev = container_of(node, struct acpi_device *dev = container_of(node,
struct acpi_device, struct acpi_device,
@ -40,11 +41,8 @@ void acpi_enable_wakeup_device_prep(u8 sleep_state)
(sleep_state > (u32) dev->wakeup.sleep_state)) (sleep_state > (u32) dev->wakeup.sleep_state))
continue; continue;
spin_unlock(&acpi_device_lock);
acpi_enable_wakeup_device_power(dev, sleep_state); acpi_enable_wakeup_device_power(dev, sleep_state);
spin_lock(&acpi_device_lock);
} }
spin_unlock(&acpi_device_lock);
} }
/** /**
@ -60,7 +58,6 @@ void acpi_enable_wakeup_device(u8 sleep_state)
* Caution: this routine must be invoked when interrupt is disabled * Caution: this routine must be invoked when interrupt is disabled
* Refer ACPI2.0: P212 * Refer ACPI2.0: P212
*/ */
spin_lock(&acpi_device_lock);
list_for_each_safe(node, next, &acpi_wakeup_device_list) { list_for_each_safe(node, next, &acpi_wakeup_device_list) {
struct acpi_device *dev = struct acpi_device *dev =
container_of(node, struct acpi_device, wakeup_list); container_of(node, struct acpi_device, wakeup_list);
@ -74,22 +71,17 @@ void acpi_enable_wakeup_device(u8 sleep_state)
if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared) if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
|| sleep_state > (u32) dev->wakeup.sleep_state) { || sleep_state > (u32) dev->wakeup.sleep_state) {
if (dev->wakeup.flags.run_wake) { if (dev->wakeup.flags.run_wake) {
spin_unlock(&acpi_device_lock);
/* set_gpe_type will disable GPE, leave it like that */ /* set_gpe_type will disable GPE, leave it like that */
acpi_set_gpe_type(dev->wakeup.gpe_device, acpi_set_gpe_type(dev->wakeup.gpe_device,
dev->wakeup.gpe_number, dev->wakeup.gpe_number,
ACPI_GPE_TYPE_RUNTIME); ACPI_GPE_TYPE_RUNTIME);
spin_lock(&acpi_device_lock);
} }
continue; continue;
} }
spin_unlock(&acpi_device_lock);
if (!dev->wakeup.flags.run_wake) if (!dev->wakeup.flags.run_wake)
acpi_enable_gpe(dev->wakeup.gpe_device, acpi_enable_gpe(dev->wakeup.gpe_device,
dev->wakeup.gpe_number); dev->wakeup.gpe_number);
spin_lock(&acpi_device_lock);
} }
spin_unlock(&acpi_device_lock);
} }
/** /**
@ -101,7 +93,6 @@ void acpi_disable_wakeup_device(u8 sleep_state)
{ {
struct list_head *node, *next; struct list_head *node, *next;
spin_lock(&acpi_device_lock);
list_for_each_safe(node, next, &acpi_wakeup_device_list) { list_for_each_safe(node, next, &acpi_wakeup_device_list) {
struct acpi_device *dev = struct acpi_device *dev =
container_of(node, struct acpi_device, wakeup_list); container_of(node, struct acpi_device, wakeup_list);
@ -112,19 +103,16 @@ void acpi_disable_wakeup_device(u8 sleep_state)
if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared) if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
|| sleep_state > (u32) dev->wakeup.sleep_state) { || sleep_state > (u32) dev->wakeup.sleep_state) {
if (dev->wakeup.flags.run_wake) { if (dev->wakeup.flags.run_wake) {
spin_unlock(&acpi_device_lock);
acpi_set_gpe_type(dev->wakeup.gpe_device, acpi_set_gpe_type(dev->wakeup.gpe_device,
dev->wakeup.gpe_number, dev->wakeup.gpe_number,
ACPI_GPE_TYPE_WAKE_RUN); ACPI_GPE_TYPE_WAKE_RUN);
/* Re-enable it, since set_gpe_type will disable it */ /* Re-enable it, since set_gpe_type will disable it */
acpi_enable_gpe(dev->wakeup.gpe_device, acpi_enable_gpe(dev->wakeup.gpe_device,
dev->wakeup.gpe_number); dev->wakeup.gpe_number);
spin_lock(&acpi_device_lock);
} }
continue; continue;
} }
spin_unlock(&acpi_device_lock);
acpi_disable_wakeup_device_power(dev); acpi_disable_wakeup_device_power(dev);
/* Never disable run-wake GPE */ /* Never disable run-wake GPE */
if (!dev->wakeup.flags.run_wake) { if (!dev->wakeup.flags.run_wake) {
@ -133,16 +121,14 @@ void acpi_disable_wakeup_device(u8 sleep_state)
acpi_clear_gpe(dev->wakeup.gpe_device, acpi_clear_gpe(dev->wakeup.gpe_device,
dev->wakeup.gpe_number, ACPI_NOT_ISR); dev->wakeup.gpe_number, ACPI_NOT_ISR);
} }
spin_lock(&acpi_device_lock);
} }
spin_unlock(&acpi_device_lock);
} }
int __init acpi_wakeup_device_init(void) int __init acpi_wakeup_device_init(void)
{ {
struct list_head *node, *next; struct list_head *node, *next;
spin_lock(&acpi_device_lock); mutex_lock(&acpi_device_lock);
list_for_each_safe(node, next, &acpi_wakeup_device_list) { list_for_each_safe(node, next, &acpi_wakeup_device_list) {
struct acpi_device *dev = container_of(node, struct acpi_device *dev = container_of(node,
struct acpi_device, struct acpi_device,
@ -150,15 +136,13 @@ int __init acpi_wakeup_device_init(void)
/* In case user doesn't load button driver */ /* In case user doesn't load button driver */
if (!dev->wakeup.flags.run_wake || dev->wakeup.state.enabled) if (!dev->wakeup.flags.run_wake || dev->wakeup.state.enabled)
continue; continue;
spin_unlock(&acpi_device_lock);
acpi_set_gpe_type(dev->wakeup.gpe_device, acpi_set_gpe_type(dev->wakeup.gpe_device,
dev->wakeup.gpe_number, dev->wakeup.gpe_number,
ACPI_GPE_TYPE_WAKE_RUN); ACPI_GPE_TYPE_WAKE_RUN);
acpi_enable_gpe(dev->wakeup.gpe_device, acpi_enable_gpe(dev->wakeup.gpe_device,
dev->wakeup.gpe_number); dev->wakeup.gpe_number);
dev->wakeup.state.enabled = 1; dev->wakeup.state.enabled = 1;
spin_lock(&acpi_device_lock);
} }
spin_unlock(&acpi_device_lock); mutex_unlock(&acpi_device_lock);
return 0; return 0;
} }