From patchwork Wed Feb 6 10:16:58 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gu Zheng X-Patchwork-Id: 218530 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id D80952C029C for ; Wed, 6 Feb 2013 21:17:19 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751370Ab3BFKRR (ORCPT ); Wed, 6 Feb 2013 05:17:17 -0500 Received: from cn.fujitsu.com ([222.73.24.84]:65324 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1750772Ab3BFKRP (ORCPT ); Wed, 6 Feb 2013 05:17:15 -0500 X-IronPort-AV: E=Sophos;i="4.84,614,1355068800"; d="scan'208";a="6696552" Received: from unknown (HELO tang.cn.fujitsu.com) ([10.167.250.3]) by song.cn.fujitsu.com with ESMTP; 06 Feb 2013 18:14:59 +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 r16AHB7s016366; Wed, 6 Feb 2013 18:17:12 +0800 Received: from [10.167.225.44] ([10.167.225.44]) by fnstmail02.fnst.cn.fujitsu.com (Lotus Domino Release 8.5.3) with ESMTP id 2013020618155381-12723 ; Wed, 6 Feb 2013 18:15:53 +0800 Message-ID: <51122D9A.6040002@cn.fujitsu.com> Date: Wed, 06 Feb 2013 18:16:58 +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: Bjorn Helgaas , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org CC: Yinghai Lu , "'Lin Feng'" , tangchen , izumi.taku@jp.fujitsu.com, Jiang Liu , Lai Jiangshan , Wen Congyang Subject: pci-sysfs: queue sysfs rescan routine into workqueue to avoid potential deadlock situation X-MIMETrack: Itemize by SMTP Server on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/02/06 18:15:53, Serialize by Router on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/02/06 18:15:54, Serialize complete at 2013/02/06 18:15:54 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org There is a potential deadlock situation when we manipulate the pci-sysfs user interfaces from different bus hierarchy simultaneously, described as following: path1: sysfs remove device: | path2: sysfs rescan device: sysfs_schedule_callback_work() | sysfs_write_file() remove_callback() | flush_write_buffer() *1* mutex_lock(&pci_remove_rescan_mutex)|*2* sysfs_get_active(attr_sd) ... | dev_attr_store() device_remove_file() | dev_rescan_store() ... |*4* mutex_lock(&pci_remove_rescan_mutex) *3* sysfs_deactivate(sd) | ... wait_for_completion() |*5* sysfs_put_active(attr_sd) *6* mutex_unlock(&pci_remove_rescan_mutex) If path1 first holds the pci_remove_rescan_mutex at *1*, then another path called path2 actived and runs to *2* before path1 runs to *3*, we now runs to a deadlock situation: Path1 holds the mutex waiting path2 to decrease sysfs_dirent's s_active counter at *5*, but path2 is blocked at *4* when trying to get the pci_remove_rescan_mutex. The mutex won't be put by path1 until it reach *6*, but it's now blocked at *3*. Under the suggestion of Bjorn, and base on Yinghai Lu's patch: http://git.kernel.org/?p=linux/kernel/git/yinghai/linux-yinghai.git;a=commitdiff;h=277df390baeab7ba6aa136356b677a096c890c0c The circumvent approach is queuing the sysfs rescan routine into workqueue just like removal to avoid manipulating(remove/scan) the pci-tree at the same time. *dmesg ifno*: (snip) 1000e 0000:1c:00.0: eth9: Intel(R) PRO/1000 Network Connection sd 13:2:0:0: [sdb] Attached SCSI disk e1000e 0000:1c:00.0: eth9: MAC: 0, PHY: 4, PBA No: D50228-005 e1000e 0000:1c:00.1: Disabling ASPM L1 e1000e 0000:1c:00.1: Interrupt Throttling Rate (ints/sec) set to dynamic conservative mode e1000e 0000:1c:00.1: irq 143 for MSI/MSI-X e1000e 0000:1c:00.1: eth10: (PCI Express:2.5GT/s:Width x4) 00:15:17:cd:96:bf e1000e 0000:1c:00.1: eth10: Intel(R) PRO/1000 Network Connection e1000e 0000:1c:00.1: eth10: MAC: 0, PHY: 4, PBA No: D50228-005 INFO: task bash:62982 blocked for more than 120 seconds. "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. bash D 0000000000000000 0 62982 62978 0x00000080 ffff88038b277db8 0000000000000082 ffff88038b277fd8 0000000000013940 ffff88038b276010 0000000000013940 0000000000013940 0000000000013940 ffff88038b277fd8 0000000000013940 ffff880377449e30 ffff8806e822c670 Call Trace: [] schedule+0x29/0x70 [] schedule_preempt_disabled+0xe/0x10 [] __mutex_lock_slowpath+0xd3/0x150 [] mutex_lock+0x2b/0x50 [] dev_rescan_store+0x5c/0x80 [] dev_attr_store+0x20/0x30 [] sysfs_write_file+0xef/0x170 [] vfs_write+0xc8/0x190 [] sys_write+0x51/0x90 [] system_call_fastpath+0x16/0x1b INFO: task bash:64141 blocked for more than 120 seconds. "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. bash D ffffffff81610460 0 64141 64136 0x00000080 ffff8803540e9db8 0000000000000086 ffff8803540e9fd8 0000000000013940 ffff8803540e8010 0000000000013940 0000000000013940 0000000000013940 ffff8803540e9fd8 0000000000013940 ffff8807db338a10 ffff8806f09abc60 Call Trace: [] schedule+0x29/0x70 [] schedule_preempt_disabled+0xe/0x10 [] __mutex_lock_slowpath+0xd3/0x150 [] mutex_lock+0x2b/0x50 [] dev_rescan_store+0x5c/0x80 [] dev_attr_store+0x20/0x30 [] sysfs_write_file+0xef/0x170 [] vfs_write+0xc8/0x190 [] sys_write+0x51/0x90 [] system_call_fastpath+0x16/0x1b INFO: task kworker/u:3:64451 blocked for more than 120 seconds. "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. kworker/u:3 D ffffffff81610460 0 64451 2 0x00000080 ffff8807d51b7a30 0000000000000046 ffff8807d51b7fd8 0000000000013940 ffff8807d51b6010 0000000000013940 0000000000013940 0000000000013940 ffff8807d51b7fd8 0000000000013940 ffff8807db339420 ffff88037744b250 Call Trace: [] schedule+0x29/0x70 [] schedule_timeout+0x19d/0x220 [] ? __slab_free+0x1f2/0x2f0 [] wait_for_common+0x11e/0x190 [] ? try_to_wake_up+0x2c0/0x2c0 [] wait_for_completion+0x1d/0x20 [] sysfs_addrm_finish+0xb8/0xd0 [] ? sysfs_schedule_callback+0x1e0/0x1e0 [] sysfs_hash_and_remove+0x60/0xb0 [] sysfs_remove_file+0x39/0x50 [] device_remove_file+0x17/0x20 [] bus_remove_device+0xdc/0x180 [] device_del+0x120/0x1d0 [] device_unregister+0x22/0x60 [] pci_stop_bus_device+0x94/0xa0 [] pci_stop_bus_device+0x40/0xa0 [] pci_stop_bus_device+0x40/0xa0 [] pci_stop_bus_device+0x40/0xa0 [] pci_stop_and_remove_bus_device+0x16/0x30 [] remove_callback+0x29/0x40 [] sysfs_schedule_callback_work+0x24/0x70 [] process_one_work+0x179/0x4b0 [] worker_thread+0x12e/0x330 [] ? manage_workers+0x110/0x110 [] kthread+0x9e/0xb0 [] kernel_thread_helper+0x4/0x10 [] ? kthread_freezable_should_stop+0x70/0x70 [] ? gs_change+0x13/0x13 Signed-off-by: Yinghai Lu Signed-off-by: Gu Zheng Signed-off-by: Lin Feng --- drivers/pci/pci-sysfs.c | 92 +++++++++++++++++++++++++++++++++-------------- 1 files changed, 65 insertions(+), 27 deletions(-) @@ -308,21 +321,32 @@ struct bus_attribute pci_bus_attrs[] = { __ATTR_NULL }; +static void dev_rescan_callback(struct device *dev) +{ + struct pci_dev *pdev = to_pci_dev(dev); + + if (pdev->is_added) { + mutex_lock(&pci_remove_rescan_mutex); + pci_rescan_bus(pdev->bus); + mutex_unlock(&pci_remove_rescan_mutex); + } +} + static ssize_t dev_rescan_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { + int err; unsigned long val; - struct pci_dev *pdev = to_pci_dev(dev); if (strict_strtoul(buf, 0, &val) < 0) return -EINVAL; - if (val) { - mutex_lock(&pci_remove_rescan_mutex); - pci_rescan_bus(pdev->bus); - mutex_unlock(&pci_remove_rescan_mutex); - } + if (!val) + return count; + err = device_schedule_callback(dev, dev_rescan_callback); + if (err) + return err; return count; } @@ -339,7 +363,7 @@ static ssize_t remove_store(struct device *dev, struct device_attribute *dummy, const char *buf, size_t count) { - int ret = 0; + int err; unsigned long val; if (strict_strtoul(buf, 0, &val) < 0) @@ -348,31 +372,45 @@ remove_store(struct device *dev, struct device_attribute *dummy, /* An attribute cannot be unregistered by one of its own methods, * so we have to use this roundabout approach. */ - if (val) - ret = device_schedule_callback(dev, remove_callback); - if (ret) - count = ret; + if (!val) + return count; + + err = device_schedule_callback(dev, remove_callback); + if (err) + return err; + return count; } +static void dev_bus_rescan_callback(struct device *dev) +{ + struct pci_bus *bus = to_pci_bus(dev); + + mutex_lock(&pci_remove_rescan_mutex); + if (!pci_is_root_bus(bus) && list_empty(&bus->devices)) + pci_rescan_bus_bridge_resize(bus->self); + else + pci_rescan_bus(bus); + mutex_unlock(&pci_remove_rescan_mutex); +} + static ssize_t dev_bus_rescan_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { + int err; unsigned long val; - struct pci_bus *bus = to_pci_bus(dev); if (strict_strtoul(buf, 0, &val) < 0) return -EINVAL; - if (val) { - mutex_lock(&pci_remove_rescan_mutex); - if (!pci_is_root_bus(bus) && list_empty(&bus->devices)) - pci_rescan_bus_bridge_resize(bus->self); - else - pci_rescan_bus(bus); - mutex_unlock(&pci_remove_rescan_mutex); - } + if (!val) + return count; + + err = device_schedule_callback(dev, dev_bus_rescan_callback); + if (err) + return err; + return count; } -- 1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-pci" 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/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index 9c6e9bb..e66b498 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -285,21 +285,34 @@ msi_bus_store(struct device *dev, struct device_attribute *attr, } static DEFINE_MUTEX(pci_remove_rescan_mutex); + +static void bus_rescan_callback(struct device *dev) +{ + struct pci_bus *b = NULL; + + mutex_lock(&pci_remove_rescan_mutex); + while ((b = pci_find_next_bus(b)) != NULL) + pci_rescan_bus(b); + mutex_unlock(&pci_remove_rescan_mutex); +} + static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf, size_t count) { + int err; unsigned long val; - struct pci_bus *b = NULL; + struct device *dev = bus->dev_root; if (strict_strtoul(buf, 0, &val) < 0) return -EINVAL; - if (val) { - mutex_lock(&pci_remove_rescan_mutex); - while ((b = pci_find_next_bus(b)) != NULL) - pci_rescan_bus(b); - mutex_unlock(&pci_remove_rescan_mutex); - } + if (!val) + return count; + + err = device_schedule_callback(dev, bus_rescan_callback); + if (err) + return err; + return count; }