From patchwork Tue Aug 27 09:21:44 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gu Zheng X-Patchwork-Id: 2850067 Return-Path: X-Original-To: patchwork-linux-acpi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 928EC9F313 for ; Tue, 27 Aug 2013 09:26:14 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0504C2041E for ; Tue, 27 Aug 2013 09:26:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D7A5E20412 for ; Tue, 27 Aug 2013 09:26:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752785Ab3H0J0G (ORCPT ); Tue, 27 Aug 2013 05:26:06 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:64032 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1752145Ab3H0J0F (ORCPT ); Tue, 27 Aug 2013 05:26:05 -0400 X-IronPort-AV: E=Sophos;i="4.89,967,1367942400"; d="scan'208,223";a="8316923" Received: from unknown (HELO tang.cn.fujitsu.com) ([10.167.250.3]) by song.cn.fujitsu.com with ESMTP; 27 Aug 2013 17:22:56 +0800 Received: from fnstmail02.fnst.cn.fujitsu.com (tang.cn.fujitsu.com [127.0.0.1]) by tang.cn.fujitsu.com (8.14.3/8.13.1) with ESMTP id r7R9Q1uA006884; Tue, 27 Aug 2013 17:26:01 +0800 Received: from [10.167.226.100] ([10.167.226.100]) by fnstmail02.fnst.cn.fujitsu.com (Lotus Domino Release 8.5.3) with ESMTP id 2013082717240634-987035 ; Tue, 27 Aug 2013 17:24:06 +0800 Message-ID: <521C6FA8.4070804@cn.fujitsu.com> Date: Tue, 27 Aug 2013 17:21:44 +0800 From: Gu Zheng User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0.1) Gecko/20110930 Thunderbird/7.0.1 MIME-Version: 1.0 To: "Rafael J. Wysocki" CC: ACPI Devel Maling List , Greg Kroah-Hartman , Toshi Kani , LKML , Yasuaki Ishimatsu , Tejun Heo Subject: Re: [PATCH] driver core / ACPI: Avoid device removal locking problems References: <1543475.L7gSB7lLAu@vostro.rjw.lan> <4785337.sqQqDSPcpL@vostro.rjw.lan> <3389643.Fdir3g6c6B@vostro.rjw.lan> <1430120.G7JdUKlgj6@vostro.rjw.lan> In-Reply-To: <1430120.G7JdUKlgj6@vostro.rjw.lan> X-MIMETrack: Itemize by SMTP Server on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/08/27 17:24:06, Serialize by Router on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/08/27 17:24:06 Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Rafael, On 08/26/2013 11:02 PM, Rafael J. Wysocki wrote: > On Monday, August 26, 2013 04:43:26 PM Rafael J. Wysocki wrote: >> On Monday, August 26, 2013 02:42:09 PM Rafael J. Wysocki wrote: >>> On Monday, August 26, 2013 11:13:13 AM Gu Zheng wrote: >>>> Hi Rafael, > > [...] > >> >> OK, so the patch below is quick and dirty and overkill, but it should make the >> splat go away at least. > > And if this patch does make the splat go away for you, please also test the > appended one (Tejun, thanks for the hint!). > > I'll address the ACPI part differently later. What about changing device_hotplug_lock and acpi_scan_lock to rwsem? like the attached one(With a preliminary test, it also can make the splat go away).:) Regards, Gu > [...] > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > From f1682ceaef4105f75f4d6a0bb8e77c8a5dde365b Mon Sep 17 00:00:00 2001 From: Gu Zheng Date: Tue, 27 Aug 2013 17:59:55 +0900 Subject: [PATCH] acpi: fix removal lock dep Signed-off-by: Gu Zheng --- drivers/acpi/scan.c | 43 ++++++++++++++++++++++--------------------- drivers/acpi/sysfs.c | 7 +++++-- drivers/base/core.c | 45 ++++++++++++++++++++++++++++++++++++--------- drivers/base/memory.c | 5 +++-- include/linux/device.h | 8 ++++++-- 5 files changed, 72 insertions(+), 36 deletions(-) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 8a46c92..bb41760 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -36,7 +36,7 @@ bool acpi_force_hot_remove; static const char *dummy_hid = "device"; static LIST_HEAD(acpi_bus_id_list); -static DEFINE_MUTEX(acpi_scan_lock); +static DECLARE_RWSEM(acpi_scan_rwsem); static LIST_HEAD(acpi_scan_handlers_list); DEFINE_MUTEX(acpi_device_lock); LIST_HEAD(acpi_wakeup_device_list); @@ -49,13 +49,13 @@ struct acpi_device_bus_id{ void acpi_scan_lock_acquire(void) { - mutex_lock(&acpi_scan_lock); + down_write(&acpi_scan_rwsem); } EXPORT_SYMBOL_GPL(acpi_scan_lock_acquire); void acpi_scan_lock_release(void) { - mutex_unlock(&acpi_scan_lock); + up_write(&acpi_scan_rwsem); } EXPORT_SYMBOL_GPL(acpi_scan_lock_release); @@ -207,7 +207,7 @@ static int acpi_scan_hot_remove(struct acpi_device *device) return -EINVAL; } - lock_device_hotplug(); + device_hotplug_begin(); /* * Carry out two passes here and ignore errors in the first pass, @@ -240,7 +240,7 @@ static int acpi_scan_hot_remove(struct acpi_device *device) acpi_bus_online_companions, NULL, NULL, NULL); - unlock_device_hotplug(); + device_hotplug_end(); put_device(&device->dev); return -EBUSY; @@ -252,7 +252,7 @@ static int acpi_scan_hot_remove(struct acpi_device *device) acpi_bus_trim(device); - unlock_device_hotplug(); + device_hotplug_end(); /* Device node has been unregistered. */ put_device(&device->dev); @@ -308,7 +308,7 @@ static void acpi_bus_device_eject(void *context) struct acpi_scan_handler *handler; u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; - mutex_lock(&acpi_scan_lock); + acpi_scan_lock_acquire(); acpi_bus_get_device(handle, &device); if (!device) @@ -334,7 +334,7 @@ static void acpi_bus_device_eject(void *context) } out: - mutex_unlock(&acpi_scan_lock); + acpi_scan_lock_release(); return; err_out: @@ -349,8 +349,8 @@ static void acpi_scan_bus_device_check(acpi_handle handle, u32 ost_source) u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; int error; - mutex_lock(&acpi_scan_lock); - lock_device_hotplug(); + acpi_scan_lock_acquire(); + device_hotplug_begin(); if (ost_source != ACPI_NOTIFY_BUS_CHECK) { acpi_bus_get_device(handle, &device); @@ -376,9 +376,9 @@ static void acpi_scan_bus_device_check(acpi_handle handle, u32 ost_source) kobject_uevent(&device->dev.kobj, KOBJ_ONLINE); out: - unlock_device_hotplug(); + device_hotplug_end(); acpi_evaluate_hotplug_ost(handle, ost_source, ost_code, NULL); - mutex_unlock(&acpi_scan_lock); + acpi_scan_lock_release(); } static void acpi_scan_bus_check(void *context) @@ -469,15 +469,14 @@ void acpi_bus_hot_remove_device(void *context) acpi_handle handle = device->handle; int error; - mutex_lock(&acpi_scan_lock); + acpi_scan_lock_acquire(); error = acpi_scan_hot_remove(device); if (error && handle) acpi_evaluate_hotplug_ost(handle, ej_event->event, ACPI_OST_SC_NON_SPECIFIC_FAILURE, NULL); - - mutex_unlock(&acpi_scan_lock); + acpi_scan_lock_release(); kfree(context); } EXPORT_SYMBOL(acpi_bus_hot_remove_device); @@ -530,7 +529,8 @@ acpi_eject_store(struct device *d, struct device_attribute *attr, if (ACPI_FAILURE(status) || !acpi_device->flags.ejectable) return -ENODEV; - mutex_lock(&acpi_scan_lock); + if (!down_write_trylock(&acpi_scan_rwsem)) + return -EBUSY; if (acpi_device->flags.eject_pending) { /* ACPI eject notification event. */ @@ -560,7 +560,7 @@ acpi_eject_store(struct device *d, struct device_attribute *attr, ret = count; out: - mutex_unlock(&acpi_scan_lock); + up_write(&acpi_scan_rwsem); return ret; err_out: @@ -1858,11 +1858,12 @@ void acpi_scan_hotplug_enabled(struct acpi_hotplug_profile *hotplug, bool val) if (!!hotplug->enabled == !!val) return; - mutex_lock(&acpi_scan_lock); + acpi_scan_lock_acquire(); hotplug->enabled = val; - mutex_unlock(&acpi_scan_lock); + acpi_scan_lock_release(); + } static void acpi_scan_init_hotplug(acpi_handle handle, int type) @@ -2141,7 +2142,7 @@ int __init acpi_scan_init(void) acpi_memory_hotplug_init(); acpi_dock_init(); - mutex_lock(&acpi_scan_lock); + acpi_scan_lock_acquire(); /* * Enumerate devices in the ACPI namespace. */ @@ -2164,6 +2165,6 @@ int __init acpi_scan_init(void) acpi_pci_root_hp_init(); out: - mutex_unlock(&acpi_scan_lock); + acpi_scan_lock_release(); return result; } diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c index 05306a5..6d8b54f 100644 --- a/drivers/acpi/sysfs.c +++ b/drivers/acpi/sysfs.c @@ -796,9 +796,12 @@ static ssize_t force_remove_store(struct kobject *kobj, if (ret < 0) return ret; - lock_device_hotplug(); + if (!write_lock_device_hotplug()) + return -EBUSY; + acpi_force_hot_remove = val; - unlock_device_hotplug(); + + write_unlock_device_hotplug(); return size; } diff --git a/drivers/base/core.c b/drivers/base/core.c index 8856d74..83c0f46 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -408,9 +408,13 @@ static ssize_t show_online(struct device *dev, struct device_attribute *attr, { bool val; - lock_device_hotplug(); + if (!read_lock_device_hotplug()) { + msleep(10); + return restart_syscall(); + } + val = !dev->offline; - unlock_device_hotplug(); + read_unlock_device_hotplug(); return sprintf(buf, "%u\n", val); } @@ -424,9 +428,12 @@ static ssize_t store_online(struct device *dev, struct device_attribute *attr, if (ret < 0) return ret; - lock_device_hotplug(); + if (!write_lock_device_hotplug()) { + msleep(10); + return restart_syscall(); + } ret = val ? device_online(dev) : device_offline(dev); - unlock_device_hotplug(); + write_unlock_device_hotplug(); return ret < 0 ? ret : count; } @@ -1479,16 +1486,36 @@ EXPORT_SYMBOL_GPL(put_device); EXPORT_SYMBOL_GPL(device_create_file); EXPORT_SYMBOL_GPL(device_remove_file); -static DEFINE_MUTEX(device_hotplug_lock); +static DECLARE_RWSEM(device_hotplug_rwsem); + +bool __must_check read_lock_device_hotplug(void) +{ + return down_read_trylock(&device_hotplug_rwsem); +} + +void read_unlock_device_hotplug(void) +{ + up_read(&device_hotplug_rwsem); +} + +bool __must_check write_lock_device_hotplug(void) +{ + return down_write_trylock(&device_hotplug_rwsem); +} + +void write_unlock_device_hotplug(void) +{ + up_write(&device_hotplug_rwsem); +} -void lock_device_hotplug(void) +void device_hotplug_begin(void) { - mutex_lock(&device_hotplug_lock); + down_write(&device_hotplug_rwsem); } -void unlock_device_hotplug(void) +void device_hotplug_end(void) { - mutex_unlock(&device_hotplug_lock); + up_write(&device_hotplug_rwsem); } static int device_check_offline(struct device *dev, void *not_used) diff --git a/drivers/base/memory.c b/drivers/base/memory.c index 2b7813e..71991b9 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -351,7 +351,8 @@ store_mem_state(struct device *dev, mem = container_of(dev, struct memory_block, dev); - lock_device_hotplug(); + if (!write_lock_device_hotplug()) + return -EBUSY; if (!strncmp(buf, "online_kernel", min_t(int, count, 13))) { offline = false; @@ -373,7 +374,7 @@ store_mem_state(struct device *dev, if (!ret) dev->offline = offline; - unlock_device_hotplug(); + write_unlock_device_hotplug(); if (ret) return ret; diff --git a/include/linux/device.h b/include/linux/device.h index 22b546a..08581f4 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -893,8 +893,12 @@ static inline bool device_supports_offline(struct device *dev) return dev->bus && dev->bus->offline && dev->bus->online; } -extern void lock_device_hotplug(void); -extern void unlock_device_hotplug(void); +extern bool read_lock_device_hotplug(void); +extern void read_unlock_device_hotplug(void); +extern bool write_lock_device_hotplug(void); +extern void write_unlock_device_hotplug(void); +extern void device_hotplug_begin(void); +extern void device_hotplug_end(void); extern int device_offline(struct device *dev); extern int device_online(struct device *dev); /*