From patchwork Fri May 31 01:04:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Williams X-Patchwork-Id: 13681016 X-Patchwork-Delegate: bhelgaas@google.com Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E4B525C89; Fri, 31 May 2024 01:04:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.9 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717117466; cv=none; b=g8zEjIj8xSgdNOjD30Vzizr1RYGP28cjxkiXJOtpTeY4Nv39M7phjSg/5fmRRmSXMqg6T6PMDD98xEvCI2oLzrCk7HkAVaxn8I+aO2nIgUW6VRij7n1HPFoWbRIkoNn6DAxpquR3IoZ3rNx/TS8UIjBx8bWfW5CsJCPbKGnCgp8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717117466; c=relaxed/simple; bh=+LdGt45zsVS7JdFsVl1ozP8KsH9eR/q0DhdaeOHz2DU=; h=Subject:From:To:Cc:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=FsBvvRBs4VTpA6oO8FYa5pZPwlvUnNyjX7x+1SYNtn3x/akviHt0vmET/v8gK1Nm1hvx/UHAlFkO9iTWGR7jMKsr/YHzdcVBBvff9Ra+ALmExJg13UwbutkFFbLEWN3TpKgsMkeE7eTXQoXUuck6kMu4DzD///EMNZqh4l3ARIY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=T5TsMOT/; arc=none smtp.client-ip=192.198.163.9 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="T5TsMOT/" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1717117465; x=1748653465; h=subject:from:to:cc:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=+LdGt45zsVS7JdFsVl1ozP8KsH9eR/q0DhdaeOHz2DU=; b=T5TsMOT/pHD3Wx/Emnh0Zta+X3QcKM6rk/h8oV+MdHMjwHzUgFoRpMlO iUnLJMvzAIHyFSzp3+M2bsO5lZ48wN91Yraok7C5XOzgwMpNEvtep3qYJ Lirjf572gM4yLYHH0uMjW1zNfTyaMu7TFooo/berlwfXuZW+jAajjnp0A MfblRBZw8Ewkz2xt9qhagLC7zCMiwgH7n+B5nBEpjJeldQmWK1l23Zj9U GSKoykLaq4RiqIFWPn+IyqvZBPfh+NJ8/V2115hkKZhv21R9SpQau7qkL LeD0rIsfaSYAXexUNSsmxv/HZOcIBMe+3CDp6mmIPOPFIWAE1UGsnq0sN Q==; X-CSE-ConnectionGUID: qnwNw1f4SuKutl1jh3Biyw== X-CSE-MsgGUID: hUWHyUzeQwCUX/m949/hVQ== X-IronPort-AV: E=McAfee;i="6600,9927,11088"; a="24308759" X-IronPort-AV: E=Sophos;i="6.08,202,1712646000"; d="scan'208";a="24308759" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 May 2024 18:04:24 -0700 X-CSE-ConnectionGUID: vHfGvFrBT8KZkuWYRpOVow== X-CSE-MsgGUID: malWm3zjSGC8uDcUI7wIJg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,202,1712646000"; d="scan'208";a="36624081" Received: from spittala-mobl3.amr.corp.intel.com (HELO dwillia2-xfh.jf.intel.com) ([10.209.84.244]) by orviesa007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 May 2024 18:04:25 -0700 Subject: [PATCH v2 1/3] PCI: Revert the cfg_access_lock lockdep mechanism From: Dan Williams To: bhelgaas@google.com Cc: Imre Deak , Jani Saarinen , Dave Jiang , linux-pci@vger.kernel.org, linux-cxl@vger.kernel.org Date: Thu, 30 May 2024 18:04:24 -0700 Message-ID: <171711746402.1628941.14575335981264103013.stgit@dwillia2-xfh.jf.intel.com> In-Reply-To: <171711745834.1628941.5259278474013108507.stgit@dwillia2-xfh.jf.intel.com> References: <171711745834.1628941.5259278474013108507.stgit@dwillia2-xfh.jf.intel.com> User-Agent: StGit/0.18-3-g996c Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 While the experiment did reveal that there are additional places that are missing the lock during secondary bus reset, one of the places that needs to take cfg_access_lock (pci_bus_lock()) is not prepared for lockdep annotation. Specifically, pci_bus_lock() takes pci_dev_lock() recursively and is currently dependent on the fact that the device_lock() is marked lockdep_set_novalidate_class(&dev->mutex). Otherwise, without that annotation, pci_bus_lock() would need to use something like a new pci_dev_lock_nested() helper, a scheme to track a PCI device's depth in the topology, and a hope that the depth of a PCI tree never exceeds the max value for a lockdep subclass. The alternative to ripping out the lockdep coverage would be to deploy a dynamic lock key for every PCI device. Unfortunately, there is evidence that increasing the number of keys that lockdep needs to track to be per-PCI-device is prohibitively expensive for something like the cfg_access_lock. The main motivation for adding the annotation in the first place was to catch unlocked secondary bus resets, not necessarily catch lock ordering problems between cfg_access_lock and other locks. Solve that narrower problem with follow-on patches, and just due to targeted revert for now. Fixes: 7e89efc6e9e4 ("PCI: Lock upstream bridge for pci_reset_function()") Reported-by: Imre Deak Closes: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_134186v1/shard-dg2-1/igt@device_reset@unbind-reset-rebind.html Cc: Jani Saarinen Cc: Dave Jiang Cc: Bjorn Helgaas Signed-off-by: Dan Williams Reviewed-by: Dave Jiang Tested-by: Kalle Valo Tested-by: Leon Romanovsky --- drivers/pci/access.c | 4 ---- drivers/pci/pci.c | 1 - drivers/pci/probe.c | 3 --- include/linux/lockdep.h | 5 ----- include/linux/pci.h | 2 -- 5 files changed, 15 deletions(-) diff --git a/drivers/pci/access.c b/drivers/pci/access.c index 30f031de9cfe..b123da16b63b 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -289,8 +289,6 @@ void pci_cfg_access_lock(struct pci_dev *dev) { might_sleep(); - lock_map_acquire(&dev->cfg_access_lock); - raw_spin_lock_irq(&pci_lock); if (dev->block_cfg_access) pci_wait_cfg(dev); @@ -345,8 +343,6 @@ void pci_cfg_access_unlock(struct pci_dev *dev) raw_spin_unlock_irqrestore(&pci_lock, flags); wake_up_all(&pci_cfg_wait); - - lock_map_release(&dev->cfg_access_lock); } EXPORT_SYMBOL_GPL(pci_cfg_access_unlock); diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 59e0949fb079..35fb1f17a589 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4883,7 +4883,6 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev) */ int pci_bridge_secondary_bus_reset(struct pci_dev *dev) { - lock_map_assert_held(&dev->cfg_access_lock); pcibios_reset_secondary_bus(dev); return pci_bridge_wait_for_secondary_bus(dev, "bus reset"); diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 8e696e547565..5fbabb4e3425 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2546,9 +2546,6 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) dev->dev.dma_mask = &dev->dma_mask; dev->dev.dma_parms = &dev->dma_parms; dev->dev.coherent_dma_mask = 0xffffffffull; - lockdep_register_key(&dev->cfg_access_key); - lockdep_init_map(&dev->cfg_access_lock, dev_name(&dev->dev), - &dev->cfg_access_key, 0); dma_set_max_seg_size(&dev->dev, 65536); dma_set_seg_boundary(&dev->dev, 0xffffffff); diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index 5e51b0de4c4b..08b0d1d9d78b 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -297,9 +297,6 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie); .wait_type_inner = _wait_type, \ .lock_type = LD_LOCK_WAIT_OVERRIDE, } -#define lock_map_assert_held(l) \ - lockdep_assert(lock_is_held(l) != LOCK_STATE_NOT_HELD) - #else /* !CONFIG_LOCKDEP */ static inline void lockdep_init_task(struct task_struct *task) @@ -391,8 +388,6 @@ extern int lockdep_is_held(const void *); #define DEFINE_WAIT_OVERRIDE_MAP(_name, _wait_type) \ struct lockdep_map __maybe_unused _name = {} -#define lock_map_assert_held(l) do { (void)(l); } while (0) - #endif /* !LOCKDEP */ #ifdef CONFIG_PROVE_LOCKING diff --git a/include/linux/pci.h b/include/linux/pci.h index fb004fd4e889..cafc5ab1cbcb 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -413,8 +413,6 @@ struct pci_dev { struct resource driver_exclusive_resource; /* driver exclusive resource ranges */ bool match_driver; /* Skip attaching driver */ - struct lock_class_key cfg_access_key; - struct lockdep_map cfg_access_lock; unsigned int transparent:1; /* Subtractive decode bridge */ unsigned int io_window:1; /* Bridge has I/O window */ From patchwork Fri May 31 01:04:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Williams X-Patchwork-Id: 13681017 X-Patchwork-Delegate: bhelgaas@google.com Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 46CE979CD; Fri, 31 May 2024 01:04:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.9 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717117471; cv=none; b=Ek0ZABiwYemX9irvEqfALR1+xghB2w+841AqFPrjT1gwnF6JqJRCtVOk57Br0tmxGbyfpHXLhzUUNixb4JukUnCWlNEW6D2kv7jlK3uuMQTDsRZg3tkChXtomvPF3buVl2Xtl5S2fcJmrmzEQujfeKAWcxd0kf6ov74hn2uW/0s= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717117471; c=relaxed/simple; bh=QUxh+Y5AFjiZYURoqwYt2w5Ltl8jSAgH+F/Cz4IcRPE=; h=Subject:From:To:Cc:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=TfvLiF/lSFKp140w8Myi3llVGiRAddtmTJOrc3r6+hzFBGTRCAq/XFdpmQGNhbu72RVp5W9aoXoyzfPHdtUdMvS4XRC4PQDtvuJj1a/suv5NLPY+HRCuT2rxp2qWjYAGrarfw0ZUtBDhWEoGxN/GoRkuJ7TjTmFKGMEwq+4/DtE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=K/pSjJn3; arc=none smtp.client-ip=192.198.163.9 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="K/pSjJn3" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1717117470; x=1748653470; h=subject:from:to:cc:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=QUxh+Y5AFjiZYURoqwYt2w5Ltl8jSAgH+F/Cz4IcRPE=; b=K/pSjJn3uBpqyL6kfGHMawHLDH315LR7O1Nq6Lbv3Zduod0v8eaa4Xc1 gvmQivKN5CnQHOKK8b2gO3LGJatAaHNZzW12XI/0ykc5jedaOXwlbqDg4 oBx0zhMQdbngqLG6iba0z0Pet3UMSemOqua8Jli4Cpz2M1nQnwcY90zHv 75/6vexIQF+3rAfqHVnDFxJbiKf5+8Jmheugmn484U2IHEtICgEall+cT OWGMSV+AfCaSR2nad3nITsLD+hEZX3LFRNzeBMZkbpxqThxNejJil1Dr8 7k5ySWbPOEP87aq92ne+esIx2Sg4h7zcQAreKXGJ7t0CX8VtMpHb88CKP w==; X-CSE-ConnectionGUID: VaxeBnHcQ3ixKEKtCmV0QQ== X-CSE-MsgGUID: bPg2inw3SG2rf455mR4gZQ== X-IronPort-AV: E=McAfee;i="6600,9927,11088"; a="24308769" X-IronPort-AV: E=Sophos;i="6.08,202,1712646000"; d="scan'208";a="24308769" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 May 2024 18:04:30 -0700 X-CSE-ConnectionGUID: 0z2OUCOARz6sgdU8XcG67w== X-CSE-MsgGUID: 3dPOpK8dTx6kYd7zXeDecQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,202,1712646000"; d="scan'208";a="36624104" Received: from spittala-mobl3.amr.corp.intel.com (HELO dwillia2-xfh.jf.intel.com) ([10.209.84.244]) by orviesa007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 May 2024 18:04:30 -0700 Subject: [PATCH v2 2/3] PCI: Warn on missing cfg_access_lock during secondary bus reset From: Dan Williams To: bhelgaas@google.com Cc: Dave Jiang , linux-pci@vger.kernel.org, linux-cxl@vger.kernel.org Date: Thu, 30 May 2024 18:04:29 -0700 Message-ID: <171711746953.1628941.4692125082286867825.stgit@dwillia2-xfh.jf.intel.com> In-Reply-To: <171711745834.1628941.5259278474013108507.stgit@dwillia2-xfh.jf.intel.com> References: <171711745834.1628941.5259278474013108507.stgit@dwillia2-xfh.jf.intel.com> User-Agent: StGit/0.18-3-g996c Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 The recent adventure with adding lockdep tracking for cfg_access_lock, while it yielded many false positives [1], it did catch a true positive in the pci_reset_bus() path [2]. So, while lockdep is difficult to deploy, open coding a check that cfg_access_lock is held during the reset is feasible. While this does not offer a full backtrace, it should be sufficient to implicate the caller of pci_bridge_secondary_bus_reset() as a path that needs investigation. Link: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_134186v1/shard-dg2-1/igt@device_reset@unbind-reset-rebind.html [1] Link: http://lore.kernel.org/r/cfb50601-5d2a-4676-a958-1bd3f1b06654@intel.com [2] Cc: Dave Jiang Cc: Bjorn Helgaas Signed-off-by: Dan Williams Reviewed-by: Dave Jiang --- drivers/pci/pci.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 35fb1f17a589..8df32a3a0979 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4883,6 +4883,9 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev) */ int pci_bridge_secondary_bus_reset(struct pci_dev *dev) { + if (!dev->block_cfg_access) + pci_warn_once(dev, "unlocked secondary bus reset via: %pS\n", + __builtin_return_address(0)); pcibios_reset_secondary_bus(dev); return pci_bridge_wait_for_secondary_bus(dev, "bus reset"); From patchwork Fri May 31 01:04:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Williams X-Patchwork-Id: 13681018 X-Patchwork-Delegate: bhelgaas@google.com Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D4A11D29E; Fri, 31 May 2024 01:04:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.9 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717117477; cv=none; b=sek4ggtSby2/YJc8WdDdDEjQVcvWLZJOyrtHWXxxZah1Rk9ILdszcqlmwy5L1zlXDFfpt10uWFZtPdDW/uQt7mTYVqxRJqVhGlBJgx1gQpPdPGAqmn5vvRWpIqzOrp8fqzYOFHSl6mQTYrI9LG/ZjaMQMm9jChJa+QE2PbzrJuc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717117477; c=relaxed/simple; bh=nCcgVAgKWEB3T5hFSMRFjSNKAig4iLQCOjbRl2/uRHA=; h=Subject:From:To:Cc:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=JMSEcBCAoo75nWwQ+Ie4KymF0u90aAwrvn02KlJpc+RHCv963TX7MGBSwX0HAHrjDJzNnvN37kkuk4cnuDy5s8NQxrBL+aZmOQLfU2+bJhWv9a2BLZvea8MRv7zdNyRFqI3TDyvRxEZb/zT68GTw0iLx+suQoTHkoirpTNpffdU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=gDBEGZ6R; arc=none smtp.client-ip=192.198.163.9 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="gDBEGZ6R" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1717117475; x=1748653475; h=subject:from:to:cc:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=nCcgVAgKWEB3T5hFSMRFjSNKAig4iLQCOjbRl2/uRHA=; b=gDBEGZ6RejLeQxU63ZfoBRXC0P4mh7tn4ycqpZNh5OgOsFxuEMsgqQM2 tv6V2lhbTCWAVU5MUCSDIrFr4ePX5rMfJ2W/CaDSTwdQ41jNtVdE89p6+ V04SpRtyQtmaUUXmDnS+ucdEQ/23QQ+cAKStrm1IXm3I5E6thDItINi1V IC5ccNOUI5x8OSNECR/3JQzhhtN8My2QeeQd0FkAeVoPKy8dp8ZFIbYIh qEJwYu13P+E3vJMVDpdArqX+byVuq+sj+tF4/ynN5UXHqqfaJZ9MlKcEV YEyFYuI2rFATadmZxX3oUP8z9T3YCt+lGdF1HNoIHKp5YGF/QfxVRdncn A==; X-CSE-ConnectionGUID: oqBSrPFJSnG26iIcF7PewA== X-CSE-MsgGUID: 7miudoaXQ9mLPjRWuBGnew== X-IronPort-AV: E=McAfee;i="6600,9927,11088"; a="24308775" X-IronPort-AV: E=Sophos;i="6.08,202,1712646000"; d="scan'208";a="24308775" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by fmvoesa103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 May 2024 18:04:35 -0700 X-CSE-ConnectionGUID: +AhVYoxFSIyVylr3XtmN5Q== X-CSE-MsgGUID: UfXOXKRGRNyW4yDlUh0kIA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.08,202,1712646000"; d="scan'208";a="36624122" Received: from spittala-mobl3.amr.corp.intel.com (HELO dwillia2-xfh.jf.intel.com) ([10.209.84.244]) by orviesa007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 May 2024 18:04:36 -0700 Subject: [PATCH v2 3/3] PCI: Add missing bridge lock to pci_bus_lock() From: Dan Williams To: bhelgaas@google.com Cc: Imre Deak , Dave Jiang , linux-pci@vger.kernel.org, linux-cxl@vger.kernel.org Date: Thu, 30 May 2024 18:04:35 -0700 Message-ID: <171711747501.1628941.15217746952476635316.stgit@dwillia2-xfh.jf.intel.com> In-Reply-To: <171711745834.1628941.5259278474013108507.stgit@dwillia2-xfh.jf.intel.com> References: <171711745834.1628941.5259278474013108507.stgit@dwillia2-xfh.jf.intel.com> User-Agent: StGit/0.18-3-g996c Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 One of the true positives that the cfg_access_lock lockdep effort identified is this sequence: WARNING: CPU: 14 PID: 1 at drivers/pci/pci.c:4886 pci_bridge_secondary_bus_reset+0x5d/0x70 RIP: 0010:pci_bridge_secondary_bus_reset+0x5d/0x70 Call Trace: ? __warn+0x8c/0x190 ? pci_bridge_secondary_bus_reset+0x5d/0x70 ? report_bug+0x1f8/0x200 ? handle_bug+0x3c/0x70 ? exc_invalid_op+0x18/0x70 ? asm_exc_invalid_op+0x1a/0x20 ? pci_bridge_secondary_bus_reset+0x5d/0x70 pci_reset_bus+0x1d8/0x270 vmd_probe+0x778/0xa10 pci_device_probe+0x95/0x120 Where pci_reset_bus() users are triggering unlocked secondary bus resets. Ironically pci_bus_reset(), several calls down from pci_reset_bus(), uses pci_bus_lock() before issuing the reset which locks everything *but* the bridge itself. For the same motivation as adding: bridge = pci_upstream_bridge(dev); if (bridge) pci_dev_lock(bridge); ...to pci_reset_function() for the "bus" and "cxl_bus" reset cases, add pci_dev_lock() for @bus->self to pci_bus_lock(). Reported-by: Imre Deak Closes: http://lore.kernel.org/r/6657833b3b5ae_14984b29437@dwillia2-xfh.jf.intel.com.notmuch Cc: Dave Jiang Cc: Bjorn Helgaas Signed-off-by: Dan Williams Reviewed-by: Dave Jiang --- drivers/pci/pci.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 8df32a3a0979..aac5daad3188 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5444,6 +5444,7 @@ static void pci_bus_lock(struct pci_bus *bus) { struct pci_dev *dev; + pci_dev_lock(bus->self); list_for_each_entry(dev, &bus->devices, bus_list) { pci_dev_lock(dev); if (dev->subordinate) @@ -5461,6 +5462,7 @@ static void pci_bus_unlock(struct pci_bus *bus) pci_bus_unlock(dev->subordinate); pci_dev_unlock(dev); } + pci_dev_unlock(bus->self); } /* Return 1 on successful lock, 0 on contention */ @@ -5468,6 +5470,7 @@ static int pci_bus_trylock(struct pci_bus *bus) { struct pci_dev *dev; + pci_dev_lock(bus->self); list_for_each_entry(dev, &bus->devices, bus_list) { if (!pci_dev_trylock(dev)) goto unlock; @@ -5486,6 +5489,7 @@ static int pci_bus_trylock(struct pci_bus *bus) pci_bus_unlock(dev->subordinate); pci_dev_unlock(dev); } + pci_dev_unlock(bus->self); return 0; }