From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 EDB0A13FD83; Sun, 28 Jul 2024 16:06:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722182809; cv=none; b=h9ABxMCV5AciEkbg5rHz3jq99yXkvBZ0kK9cyZPsnatWLyuCiDAwDjub2MumDvAqlnYSPfSDK0O+5z1X06Fy37knKEdww1y9Bor9M1toiqXJUvR5Vkv1jBZkwkK9NsrseZSvuTPAtkWik4mgMoIQOSlsa0b90YbjF9TFamc7OJI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722182809; c=relaxed/simple; bh=/lGRAITXtSpzwUJ7q32wIkYJ92q3Oyo0JBm3X1tuKws=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Zv4Kuo9YqW6M5qLijB6gH1UatysxpvbkzkN374larfCBdUGcl/RFPk1iFhk8pBpd0qLk+oWXvoG9XwloNOhJ6nKamzeAwt23FMp2dMoQ6/fZHFoH/3s6GW2eubFa3K/E3x8lvMT2ATjtS1bv0HLqMAxZhT4+0FjXH+l4Gb2V6Qs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=aFBMRjEU; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="aFBMRjEU" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 72C83C4AF15; Sun, 28 Jul 2024 16:06:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1722182808; bh=/lGRAITXtSpzwUJ7q32wIkYJ92q3Oyo0JBm3X1tuKws=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=aFBMRjEUSLNYG02sVuiIQXKDsUdP5IwLbMWYgkNhsyx1XGLaz/VRGICcjECKD8Txn vq5R9ZqWEQEjp3oFsNHhrepdBly1iMZ4IJA6RIV4xptX8NEylF877YA2P9WYQMg0YJ /T1lRLnKy/GTs5s0pLHMFfwqBBK6o+3VLxhwbKc6va334O3YUgNqKTAgPWU2Ccld4v uMn2BWMvQjAqKs2Bcx7OK4y1ngyj3VhkHtksoEnN4ebB26tJmCXkV9vKgERCqTl9m6 mid2nprYsMJd9p1kAJcRSgDd8LyrKJNEJwtbss5Iu5kLYkkAGH14hUq0pFs5Ap1Aso zKfz/jSY1ZUEg== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Dan Williams , Imre Deak , Keith Busch , Bjorn Helgaas , Hans de Goede , Kalle Valo , Dave Jiang , Sasha Levin , linux-pci@vger.kernel.org Subject: [PATCH AUTOSEL 6.10 22/23] PCI: Add missing bridge lock to pci_bus_lock() Date: Sun, 28 Jul 2024 12:05:03 -0400 Message-ID: <20240728160538.2051879-22-sashal@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240728160538.2051879-1-sashal@kernel.org> References: <20240728160538.2051879-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.10.2 Content-Transfer-Encoding: 8bit From: Dan Williams [ Upstream commit a4e772898f8bf2e7e1cf661a12c60a5612c4afab ] 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(). Link: https://lore.kernel.org/r/171711747501.1628941.15217746952476635316.stgit@dwillia2-xfh.jf.intel.com Reported-by: Imre Deak Closes: http://lore.kernel.org/r/6657833b3b5ae_14984b29437@dwillia2-xfh.jf.intel.com.notmuch Signed-off-by: Dan Williams Signed-off-by: Keith Busch [bhelgaas: squash in recursive locking deadlock fix from Keith Busch: https://lore.kernel.org/r/20240711193650.701834-1-kbusch@meta.com] Signed-off-by: Bjorn Helgaas Tested-by: Hans de Goede Tested-by: Kalle Valo Reviewed-by: Dave Jiang Signed-off-by: Sasha Levin --- drivers/pci/pci.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 35fb1f17a589c..bd3818c72ba29 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5441,10 +5441,12 @@ 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) pci_bus_lock(dev->subordinate); + else + pci_dev_lock(dev); } } @@ -5456,8 +5458,10 @@ static void pci_bus_unlock(struct pci_bus *bus) list_for_each_entry(dev, &bus->devices, bus_list) { if (dev->subordinate) pci_bus_unlock(dev->subordinate); - pci_dev_unlock(dev); + else + pci_dev_unlock(dev); } + pci_dev_unlock(bus->self); } /* Return 1 on successful lock, 0 on contention */ @@ -5465,15 +5469,15 @@ static int pci_bus_trylock(struct pci_bus *bus) { struct pci_dev *dev; + if (!pci_dev_trylock(bus->self)) + return 0; + list_for_each_entry(dev, &bus->devices, bus_list) { - if (!pci_dev_trylock(dev)) - goto unlock; if (dev->subordinate) { - if (!pci_bus_trylock(dev->subordinate)) { - pci_dev_unlock(dev); + if (!pci_bus_trylock(dev->subordinate)) goto unlock; - } - } + } else if (!pci_dev_trylock(dev)) + goto unlock; } return 1; @@ -5481,8 +5485,10 @@ static int pci_bus_trylock(struct pci_bus *bus) list_for_each_entry_continue_reverse(dev, &bus->devices, bus_list) { if (dev->subordinate) pci_bus_unlock(dev->subordinate); - pci_dev_unlock(dev); + else + pci_dev_unlock(dev); } + pci_dev_unlock(bus->self); return 0; } @@ -5514,9 +5520,10 @@ static void pci_slot_lock(struct pci_slot *slot) list_for_each_entry(dev, &slot->bus->devices, bus_list) { if (!dev->slot || dev->slot != slot) continue; - pci_dev_lock(dev); if (dev->subordinate) pci_bus_lock(dev->subordinate); + else + pci_dev_lock(dev); } } @@ -5542,14 +5549,13 @@ static int pci_slot_trylock(struct pci_slot *slot) list_for_each_entry(dev, &slot->bus->devices, bus_list) { if (!dev->slot || dev->slot != slot) continue; - if (!pci_dev_trylock(dev)) - goto unlock; if (dev->subordinate) { if (!pci_bus_trylock(dev->subordinate)) { pci_dev_unlock(dev); goto unlock; } - } + } else if (!pci_dev_trylock(dev)) + goto unlock; } return 1; @@ -5560,7 +5566,8 @@ static int pci_slot_trylock(struct pci_slot *slot) continue; if (dev->subordinate) pci_bus_unlock(dev->subordinate); - pci_dev_unlock(dev); + else + pci_dev_unlock(dev); } return 0; } -- 2.43.0