From patchwork Sun Jul 3 22:28:23 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rafael Wysocki X-Patchwork-Id: 941172 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p63MRv3U028576 for ; Sun, 3 Jul 2011 22:27:58 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751878Ab1GCW1r (ORCPT ); Sun, 3 Jul 2011 18:27:47 -0400 Received: from ogre.sisk.pl ([217.79.144.158]:51824 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751610Ab1GCW1r (ORCPT ); Sun, 3 Jul 2011 18:27:47 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by ogre.sisk.pl (Postfix) with ESMTP id 512691B450D; Mon, 4 Jul 2011 00:03:05 +0200 (CEST) Received: from ogre.sisk.pl ([127.0.0.1]) by localhost (ogre.sisk.pl [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 08172-08; Mon, 4 Jul 2011 00:02:42 +0200 (CEST) Received: from ferrari.rjw.lan (220-bem-13.acn.waw.pl [82.210.184.220]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ogre.sisk.pl (Postfix) with ESMTP id B9C371B44A9; Mon, 4 Jul 2011 00:02:42 +0200 (CEST) From: "Rafael J. Wysocki" To: Andrea Righi Subject: Re: [PATCH v2] ACPI: fix acpi_power_off lockdep splat Date: Mon, 4 Jul 2011 00:28:23 +0200 User-Agent: KMail/1.13.6 (Linux/3.0.0-rc5+; KDE/4.6.0; x86_64; ; ) Cc: Len Brown , Peter Zijlstra , Ingo Molnar , Borislav Petkov , Maciej Rutecki , Florian Mickler , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org References: <1309684450-24549-1-git-send-email-andrea@betterlinux.com> <201107032259.05722.rjw@sisk.pl> <201107040011.32254.rjw@sisk.pl> In-Reply-To: <201107040011.32254.rjw@sisk.pl> MIME-Version: 1.0 Message-Id: <201107040028.23569.rjw@sisk.pl> X-Virus-Scanned: amavisd-new at ogre.sisk.pl using MkS_Vir for Linux Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Sun, 03 Jul 2011 22:27:58 +0000 (UTC) On Monday, July 04, 2011, Rafael J. Wysocki wrote: > On Sunday, July 03, 2011, Rafael J. Wysocki wrote: > > On Sunday, July 03, 2011, Andrea Righi wrote: > > > Implement acpi_os_create_lock() as a C-preprocessor macro to suppress > > > lockdep false positive. > > > > > > When lockdep is enabled the spin_lock_init macro stringifies it's > > > argument and uses that as a name for the lock in the debugging. > > > > > > By executing spin_lock_init in a macro the key changes from "lock" for > > > all locks to the actual argument of acpi_os_create_lock() > > > ("&acpi_gbl_global_lock_pending_lock", "&acpi_gbl_gpe_lock" or > > > "&acpi_gbl_hardware_lock" for now). > > > > > > This fixes: > > > https://bugzilla.kernel.org/show_bug.cgi?id=38152 > > > > > > ChangeLog (v1 -> v2): > > > - avoid to call spin_lock_init multiple times on the same lock > > > - rewrite patch description (thanks to Florian for providing a better > > > description of the patch) > > > > > > Reported-by: Borislav Petkov > > > CC: Florian Mickler > > > Signed-off-by: Andrea Righi > > > --- > > > drivers/acpi/osl.c | 3 +-- > > > include/acpi/acpiosxf.h | 12 +++++++++++- > > > 2 files changed, 12 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c > > > index 52ca964..4c985d3 100644 > > > --- a/drivers/acpi/osl.c > > > +++ b/drivers/acpi/osl.c > > > @@ -1336,14 +1336,13 @@ EXPORT_SYMBOL(acpi_resources_are_enforced); > > > * Create and initialize a spinlock. > > > */ > > > acpi_status > > > -acpi_os_create_lock(acpi_spinlock *out_handle) > > > +__acpi_os_create_lock(acpi_spinlock *out_handle) > > > { > > > > I would rename this to acpi_os_allocate_lock() or acpi_os_alloc_lock(), > > so that it doesn't suggest the lock is initialized by this function. > > > > Hmm. There's one more thing we need to take into account here. Namely, > > include/acpi/acpiosxf.h is used by other OSes, so we shouldn't put > > Linux-specific stuff into it. > > > > I'm not sure how to work around that at the moment. > > OK, the patch below builds for me and seems to work even, although I haven't > tested it with lockdep on. Below is a cleaned-up version, still untested with lockdep on. Thanks, Rafael --- drivers/acpi/osl.c | 17 ----------------- include/acpi/acpiosxf.h | 3 +++ include/acpi/platform/aclinux.h | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 17 deletions(-) -- 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 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__ */