From patchwork Fri Aug 5 22:34:08 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergey Senozhatsky X-Patchwork-Id: 1039492 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 p75MZKfa026685 for ; Fri, 5 Aug 2011 22:35:21 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756070Ab1HEWfU (ORCPT ); Fri, 5 Aug 2011 18:35:20 -0400 Received: from mail-pz0-f42.google.com ([209.85.210.42]:53941 "EHLO mail-pz0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752230Ab1HEWfS (ORCPT ); Fri, 5 Aug 2011 18:35:18 -0400 Received: by pzk37 with SMTP id 37so4594107pzk.1 for ; Fri, 05 Aug 2011 15:35:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=iX/muAvcBy+n8IqNJ+dUKOSrzNq6jm/b7EVpW7zQzpY=; b=YsXc4uUKjeb+ymVtThwOw2IVDTa7zR30p2I9XuERoXIQ8md0RLPXj2CXKaDXylVazg s8y2hachMyuP5jmby7RpyuIqwOrl9xA8j+J1qD72F4IbtmPsAzhrc7Y+vAF75LwrprO9 ClUJqpsZZBuQmkDoghv1qfpi89U9Ofmb3r90E= Received: by 10.142.151.6 with SMTP id y6mr2362522wfd.441.1312583716577; Fri, 05 Aug 2011 15:35:16 -0700 (PDT) Received: from localhost ([80.249.93.203]) by mx.google.com with ESMTPS id t7sm2001283wfl.2.2011.08.05.15.35.12 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 05 Aug 2011 15:35:14 -0700 (PDT) Date: Sat, 6 Aug 2011 01:34:08 +0300 From: Sergey Senozhatsky To: "Lan, Tianyu" Cc: Len Brown , "linux-acpi@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: [PATCH] Battery: sysfs_remove_battery(): possible circular locking Message-ID: <20110805223408.GA3363@swordfish.minsk.epam.com> References: <20110805003322.GA8311@swordfish> <1312521008.2096.173.camel@lantianyu-ws> <20110805163944.GA3132@swordfish.minsk.epam.com> <625BA99ED14B2D499DC4E29D8138F15062C4514167@shsmsx502.ccr.corp.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <625BA99ED14B2D499DC4E29D8138F15062C4514167@shsmsx502.ccr.corp.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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]); Fri, 05 Aug 2011 22:35:21 +0000 (UTC) Commit 9c921c22a7f33397a6774d7fa076db9b6a0fd669 Author: Lan Tianyu ACPI / Battery: Resolve the race condition in the sysfs_remove_battery() fixed BUG https://bugzilla.kernel.org/show_bug.cgi?id=35642 , but as a side effect made lockdep unhappy with sysfs_remove_battery(): [14818.477168] [14818.477170] ======================================================= [14818.477200] [ INFO: possible circular locking dependency detected ] [14818.477221] 3.1.0-dbg-07865-g1280ea8-dirty #668 [14818.477236] ------------------------------------------------------- [14818.477257] s2ram/1599 is trying to acquire lock: [14818.477276] (s_active#8){++++.+}, at: [] sysfs_addrm_finish+0x31/0x5a [14818.477323] [14818.477325] but task is already holding lock: [14818.477350] (&battery->lock){+.+.+.}, at: [] sysfs_remove_battery+0x10/0x4b [battery] [14818.477395] [14818.477397] which lock already depends on the new lock. [14818.477399] [..] [14818.479121] stack backtrace: [14818.479148] Pid: 1599, comm: s2ram Not tainted 3.1.0-dbg-07865-g1280ea8-dirty #668 [14818.479175] Call Trace: [14818.479198] [] print_circular_bug+0x293/0x2a4 [14818.479228] [] __lock_acquire+0xfe4/0x164b [14818.479260] [] ? sysfs_addrm_finish+0x31/0x5a [14818.479288] [] lock_acquire+0x138/0x1ac [14818.479316] [] ? sysfs_addrm_finish+0x31/0x5a [14818.479345] [] sysfs_deactivate+0x9b/0xec [14818.479373] [] ? sysfs_addrm_finish+0x31/0x5a [14818.479405] [] sysfs_addrm_finish+0x31/0x5a [14818.479433] [] sysfs_hash_and_remove+0x54/0x77 [14818.479461] [] sysfs_remove_file+0x12/0x14 [14818.479488] [] device_remove_file+0x12/0x14 [14818.479516] [] device_del+0x119/0x17c [14818.479542] [] device_unregister+0xe/0x1a [14818.479570] [] power_supply_unregister+0x23/0x27 [14818.479601] [] sysfs_remove_battery+0x34/0x4b [battery] [14818.479632] [] battery_notify+0x2c/0x3a [battery] [14818.479662] [] notifier_call_chain+0x74/0xa1 [14818.479692] [] __blocking_notifier_call_chain+0x6c/0x89 [14818.479722] [] blocking_notifier_call_chain+0xf/0x11 [14818.479751] [] pm_notifier_call_chain+0x15/0x27 [14818.479770] [] enter_state+0xa7/0xd5 [14818.479782] [] state_store+0xaa/0xc0 [14818.479795] [] ? pm_async_store+0x45/0x45 [14818.479807] [] kobj_attr_store+0x17/0x19 [14818.479820] [] sysfs_write_file+0x103/0x13f [14818.479834] [] vfs_write+0xad/0x13d [14818.479847] [] sys_write+0x45/0x6c [14818.479860] [] system_call_fastpath+0x16/0x1b This patch introduces separate lock to struct acpi_battery to grab in sysfs_remove_battery() instead of battery->lock. So fix by Lan Tianyu is still there, we just grab independent lock. Signed-off-by: Sergey Senozhatsky --- drivers/acpi/battery.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 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 diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 87c0a8d..7711d94 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -99,6 +99,7 @@ enum { struct acpi_battery { struct mutex lock; + struct mutex sysfs_lock; struct power_supply bat; struct acpi_device *device; struct notifier_block pm_nb; @@ -573,16 +574,16 @@ static int sysfs_add_battery(struct acpi_battery *battery) static void sysfs_remove_battery(struct acpi_battery *battery) { - mutex_lock(&battery->lock); + mutex_lock(&battery->sysfs_lock); if (!battery->bat.dev) { - mutex_unlock(&battery->lock); + mutex_unlock(&battery->sysfs_lock); return; } device_remove_file(battery->bat.dev, &alarm_attr); power_supply_unregister(&battery->bat); battery->bat.dev = NULL; - mutex_unlock(&battery->lock); + mutex_unlock(&battery->sysfs_lock); } /* @@ -982,6 +983,7 @@ static int acpi_battery_add(struct acpi_device *device) strcpy(acpi_device_class(device), ACPI_BATTERY_CLASS); device->driver_data = battery; mutex_init(&battery->lock); + mutex_init(&battery->sysfs_lock); if (ACPI_SUCCESS(acpi_get_handle(battery->device->handle, "_BIX", &handle))) set_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags); @@ -1010,6 +1012,7 @@ static int acpi_battery_add(struct acpi_device *device) fail: sysfs_remove_battery(battery); mutex_destroy(&battery->lock); + mutex_destroy(&battery->sysfs_lock); kfree(battery); return result; } @@ -1027,6 +1030,7 @@ static int acpi_battery_remove(struct acpi_device *device, int type) #endif sysfs_remove_battery(battery); mutex_destroy(&battery->lock); + mutex_destroy(&battery->sysfs_lock); kfree(battery); return 0; }