applied
thanks,
-- Len Brown, Intel Open Source Technology Center
On Tue, 9 Dec 2008, Shaohua Li wrote:
> Change acpi_device_lock to a mutex to avoid race in wakeup device
> read/write. Found by Linus.
>
> Signed-off-by: Shaohua Li <shaohua.li@xxxxxxxxx>
> ---
> drivers/acpi/scan.c | 14 +++++++-------
> drivers/acpi/sleep/proc.c | 13 ++++---------
> drivers/acpi/sleep/sleep.h | 3 +++
> drivers/acpi/sleep/wakeup.c | 31 ++++++++-----------------------
> 4 files changed, 22 insertions(+), 39 deletions(-)
>
> Index: linux/drivers/acpi/scan.c
> ===================================================================
> --- linux.orig/drivers/acpi/scan.c 2008-12-09 10:34:00.000000000 +0800
> +++ linux/drivers/acpi/scan.c 2008-12-09 10:36:07.000000000 +0800
> @@ -23,7 +23,7 @@ extern struct acpi_device *acpi_root;
>
> static LIST_HEAD(acpi_device_list);
> static LIST_HEAD(acpi_bus_id_list);
> -DEFINE_SPINLOCK(acpi_device_lock);
> +DEFINE_MUTEX(acpi_device_lock);
> LIST_HEAD(acpi_wakeup_device_list);
>
> struct acpi_device_bus_id{
> @@ -440,7 +440,7 @@ static int acpi_device_register(struct a
> return -ENOMEM;
> }
>
> - spin_lock(&acpi_device_lock);
> + mutex_lock(&acpi_device_lock);
> /*
> * Find suitable bus_id and instance number in acpi_bus_id_list
> * If failed, create one and link it into acpi_bus_id_list
> @@ -468,7 +468,7 @@ static int acpi_device_register(struct a
> list_add_tail(&device->g_list, &acpi_device_list);
> if (device->wakeup.flags.valid)
> list_add_tail(&device->wakeup_list, &acpi_wakeup_device_list);
> - spin_unlock(&acpi_device_lock);
> + mutex_unlock(&acpi_device_lock);
>
> if (device->parent)
> device->dev.parent = &parent->dev;
> @@ -489,20 +489,20 @@ static int acpi_device_register(struct a
> device->removal_type = ACPI_BUS_REMOVAL_NORMAL;
> return 0;
> end:
> - spin_lock(&acpi_device_lock);
> + mutex_lock(&acpi_device_lock);
> if (device->parent) {
> list_del(&device->node);
> list_del(&device->g_list);
> } else
> list_del(&device->g_list);
> list_del(&device->wakeup_list);
> - spin_unlock(&acpi_device_lock);
> + mutex_unlock(&acpi_device_lock);
> return result;
> }
>
> static void acpi_device_unregister(struct acpi_device *device, int type)
> {
> - spin_lock(&acpi_device_lock);
> + mutex_lock(&acpi_device_lock);
> if (device->parent) {
> list_del(&device->node);
> list_del(&device->g_list);
> @@ -510,7 +510,7 @@ static void acpi_device_unregister(struc
> list_del(&device->g_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);
>
> Index: linux/drivers/acpi/sleep/proc.c
> ===================================================================
> --- linux.orig/drivers/acpi/sleep/proc.c 2008-12-09 10:34:18.000000000 +0800
> +++ linux/drivers/acpi/sleep/proc.c 2008-12-09 10:40:10.000000000 +0800
> @@ -338,9 +338,6 @@ acpi_system_write_alarm(struct file *fil
> }
> #endif /* HAVE_ACPI_LEGACY_ALARM */
>
> -extern struct list_head acpi_wakeup_device_list;
> -extern spinlock_t acpi_device_lock;
> -
> static int
> acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
> {
> @@ -348,7 +345,7 @@ acpi_system_wakeup_device_seq_show(struc
>
> 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) {
> struct acpi_device *dev =
> container_of(node, struct acpi_device, wakeup_list);
> @@ -356,7 +353,6 @@ acpi_system_wakeup_device_seq_show(struc
>
> if (!dev->wakeup.flags.valid)
> continue;
> - spin_unlock(&acpi_device_lock);
>
> ldev = acpi_get_physical_device(dev->handle);
> seq_printf(seq, "%s\t S%d\t%c%-8s ",
> @@ -371,9 +367,8 @@ acpi_system_wakeup_device_seq_show(struc
> seq_printf(seq, "\n");
> put_device(ldev);
>
> - spin_lock(&acpi_device_lock);
> }
> - spin_unlock(&acpi_device_lock);
> + mutex_unlock(&acpi_device_lock);
> return 0;
> }
>
> @@ -404,7 +399,7 @@ acpi_system_write_wakeup_device(struct f
> strbuf[len] = '\0';
> sscanf(strbuf, "%s", str);
>
> - spin_lock(&acpi_device_lock);
> + mutex_lock(&acpi_device_lock);
> list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> struct acpi_device *dev =
> container_of(node, struct acpi_device, wakeup_list);
> @@ -441,7 +436,7 @@ acpi_system_write_wakeup_device(struct f
> }
> }
> }
> - spin_unlock(&acpi_device_lock);
> + mutex_unlock(&acpi_device_lock);
> return count;
> }
>
> Index: linux/drivers/acpi/sleep/sleep.h
> ===================================================================
> --- linux.orig/drivers/acpi/sleep/sleep.h 2008-12-09 10:37:22.000000000 +0800
> +++ linux/drivers/acpi/sleep/sleep.h 2008-12-09 10:37:50.000000000 +0800
> @@ -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(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;
> Index: linux/drivers/acpi/sleep/wakeup.c
> ===================================================================
> --- linux.orig/drivers/acpi/sleep/wakeup.c 2008-12-09 10:34:08.000000000 +0800
> +++ linux/drivers/acpi/sleep/wakeup.c 2008-12-09 10:39:18.000000000 +0800
> @@ -14,9 +14,6 @@
> #define _COMPONENT ACPI_SYSTEM_COMPONENT
> 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
> * @sleep_state: ACPI state
> @@ -30,7 +27,7 @@ void acpi_enable_wakeup_device_prep(u8 s
>
> ACPI_FUNCTION_TRACE("acpi_enable_wakeup_device_prep");
>
> - spin_lock(&acpi_device_lock);
> + mutex_lock(&acpi_device_lock);
> list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> struct acpi_device *dev = container_of(node,
> struct acpi_device,
> @@ -41,11 +38,9 @@ void acpi_enable_wakeup_device_prep(u8 s
> (sleep_state > (u32) dev->wakeup.sleep_state))
> continue;
>
> - spin_unlock(&acpi_device_lock);
> acpi_enable_wakeup_device_power(dev, sleep_state);
> - spin_lock(&acpi_device_lock);
> }
> - spin_unlock(&acpi_device_lock);
> + mutex_unlock(&acpi_device_lock);
> }
>
> /**
> @@ -62,7 +57,7 @@ void acpi_enable_wakeup_device(u8 sleep_
> * Refer ACPI2.0: P212
> */
> ACPI_FUNCTION_TRACE("acpi_enable_wakeup_device");
> - spin_lock(&acpi_device_lock);
> + mutex_lock(&acpi_device_lock);
> list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> struct acpi_device *dev =
> container_of(node, struct acpi_device, wakeup_list);
> @@ -76,22 +71,18 @@ void acpi_enable_wakeup_device(u8 sleep_
> if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
> || sleep_state > (u32) dev->wakeup.sleep_state) {
> if (dev->wakeup.flags.run_wake) {
> - spin_unlock(&acpi_device_lock);
> /* set_gpe_type will disable GPE, leave it like that */
> acpi_set_gpe_type(dev->wakeup.gpe_device,
> dev->wakeup.gpe_number,
> ACPI_GPE_TYPE_RUNTIME);
> - spin_lock(&acpi_device_lock);
> }
> continue;
> }
> - spin_unlock(&acpi_device_lock);
> if (!dev->wakeup.flags.run_wake)
> acpi_enable_gpe(dev->wakeup.gpe_device,
> dev->wakeup.gpe_number);
> - spin_lock(&acpi_device_lock);
> }
> - spin_unlock(&acpi_device_lock);
> + mutex_unlock(&acpi_device_lock);
> }
>
> /**
> @@ -105,7 +96,7 @@ void acpi_disable_wakeup_device(u8 sleep
>
> ACPI_FUNCTION_TRACE("acpi_disable_wakeup_device");
>
> - spin_lock(&acpi_device_lock);
> + mutex_lock(&acpi_device_lock);
> list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> struct acpi_device *dev =
> container_of(node, struct acpi_device, wakeup_list);
> @@ -116,19 +107,16 @@ void acpi_disable_wakeup_device(u8 sleep
> if ((!dev->wakeup.state.enabled && !dev->wakeup.flags.prepared)
> || sleep_state > (u32) dev->wakeup.sleep_state) {
> if (dev->wakeup.flags.run_wake) {
> - spin_unlock(&acpi_device_lock);
> acpi_set_gpe_type(dev->wakeup.gpe_device,
> dev->wakeup.gpe_number,
> ACPI_GPE_TYPE_WAKE_RUN);
> /* Re-enable it, since set_gpe_type will disable it */
> acpi_enable_gpe(dev->wakeup.gpe_device,
> dev->wakeup.gpe_number);
> - spin_lock(&acpi_device_lock);
> }
> continue;
> }
>
> - spin_unlock(&acpi_device_lock);
> acpi_disable_wakeup_device_power(dev);
> /* Never disable run-wake GPE */
> if (!dev->wakeup.flags.run_wake) {
> @@ -137,9 +125,8 @@ void acpi_disable_wakeup_device(u8 sleep
> acpi_clear_gpe(dev->wakeup.gpe_device,
> dev->wakeup.gpe_number, ACPI_NOT_ISR);
> }
> - spin_lock(&acpi_device_lock);
> }
> - spin_unlock(&acpi_device_lock);
> + mutex_unlock(&acpi_device_lock);
> }
>
> static int __init acpi_wakeup_device_init(void)
> @@ -149,7 +136,7 @@ static int __init acpi_wakeup_device_ini
> if (acpi_disabled)
> return 0;
>
> - spin_lock(&acpi_device_lock);
> + mutex_lock(&acpi_device_lock);
> list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> struct acpi_device *dev = container_of(node,
> struct acpi_device,
> @@ -157,16 +144,14 @@ static int __init acpi_wakeup_device_ini
> /* In case user doesn't load button driver */
> if (!dev->wakeup.flags.run_wake || dev->wakeup.state.enabled)
> continue;
> - spin_unlock(&acpi_device_lock);
> acpi_set_gpe_type(dev->wakeup.gpe_device,
> dev->wakeup.gpe_number,
> ACPI_GPE_TYPE_WAKE_RUN);
> acpi_enable_gpe(dev->wakeup.gpe_device,
> dev->wakeup.gpe_number);
> dev->wakeup.state.enabled = 1;
> - spin_lock(&acpi_device_lock);
> }
> - spin_unlock(&acpi_device_lock);
> + mutex_unlock(&acpi_device_lock);
> return 0;
> }
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
