Re: [PATCH] ACPI: Fix lockdep false positives in acpi_power_off()

[Florian Mickler]

On Tue, 5 Jul 2011 01:32:11 +0200
"Rafael J. Wysocki" <rjw@xxxxxxx> wrote:
> 
> OK, below is an official version.
> 
> Thanks,
> Rafael
> 
> ---
> From: Rafael J. Wysocki <rjw@xxxxxxx>
> Subject: ACPI: Fix lockdep false positives in acpi_power_off()
> 
> All ACPICA locks are allocated and initialized by the same function,
> acpi_os_create_lock(), with the help of a local variable called
> "lock".  Thus, when lockdep is enabled, it uses "lock" as the
> name of all those locks and regards them as instances of the same
> lock, which causes it to report possible locking problems with them
> when there aren't any.
> 
> To work around this problem, define acpi_os_create_lock() as a macro
> and make it pass its argument to spin_lock_init(), so that lockdep
> uses it as the name of the new lock.  Define this macron in a
> Linux-specific file to minimize the resulting modifications of
> the OS-independent ACPICA parts.
> 
> This change is based on an earlier patch from Andrea Righi and it
> addresses a regression from 2.6.39 tracked as
> https://bugzilla.kernel.org/show_bug.cgi?id=38152
> 
> Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx>
> Reported-by: Borislav Petkov <bp@xxxxxxxxx>
> Tested-by: Andrea Righi <andrea@xxxxxxxxxxxxxxx>
> ---
>  drivers/acpi/osl.c              |   17 -----------------
>  include/acpi/acpiosxf.h         |    3 +++
>  include/acpi/platform/aclinux.h |   18 ++++++++++++++++++
>  3 files changed, 21 insertions(+), 17 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/osl.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/osl.c
> +++ linux-2.6/drivers/acpi/osl.c
> @@ -1333,23 +1333,6 @@ int acpi_resources_are_enforced(void)
>  EXPORT_SYMBOL(acpi_resources_are_enforced);
>  
>  /*
> - * Create and initialize a spinlock.
> - */
> -acpi_status
> -acpi_os_create_lock(acpi_spinlock *out_handle)
> -{
> -	spinlock_t *lock;
> -
> -	lock = ACPI_ALLOCATE(sizeof(spinlock_t));
> -	if (!lock)
> -		return AE_NO_MEMORY;
> -	spin_lock_init(lock);
> -	*out_handle = lock;
> -
> -	return AE_OK;
> -}
> -
> -/*
>   * Deallocate the memory for a spinlock.
>   */
>  void acpi_os_delete_lock(acpi_spinlock handle)
> Index: linux-2.6/include/acpi/acpiosxf.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/acpiosxf.h
> +++ linux-2.6/include/acpi/acpiosxf.h
> @@ -98,8 +98,11 @@ acpi_os_table_override(struct acpi_table
>  /*
>   * Spinlock primitives
>   */
> +
> +#ifndef acpi_os_create_lock
>  acpi_status
>  acpi_os_create_lock(acpi_spinlock *out_handle);
> +#endif
>  
>  void acpi_os_delete_lock(acpi_spinlock handle);
>  
> Index: linux-2.6/include/acpi/platform/aclinux.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/platform/aclinux.h
> +++ linux-2.6/include/acpi/platform/aclinux.h
> @@ -159,6 +159,24 @@ static inline void *acpi_os_acquire_obje
>  	} while (0)
>  #endif
>  
> +/*
> + * When lockdep is enabled, the spin_lock_init() macro stringifies it's
> + * argument and uses that as a name for the lock in debugging.
> + * By executing spin_lock_init() in a macro the key changes from "lock" for
> + * all locks to the name of the argument of acpi_os_create_lock(), which
> + * prevents lockdep from reporting false positives for ACPICA locks.
> + */
> +#define acpi_os_create_lock(__handle)				\
> +({								\
> +	spinlock_t *lock = ACPI_ALLOCATE(sizeof(*lock));	\
> +								\
> +	if (lock) {						\
> +		*(__handle) = lock;				\
> +		spin_lock_init(*(__handle));			\
> +	}							\
> +	lock ? AE_OK : AE_NO_MEMORY;				\
> +})
> +
>  #endif /* __KERNEL__ */
>  
>  #endif /* __ACLINUX_H__ */

Since I made the effort of digging into the lockdep code, you can
actually add my 
 Reviewed-by: Florian Mickler <florian@xxxxxxxxxxx>

Regards,
Flo

--
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/