linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Ashok Raj <ashok.raj@intel.com>,
	Keith Busch <keith.busch@intel.com>,
	"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
	Lukas Wunner <lukas@wunner.de>,
	Michael Jamet <michael.jamet@intel.com>,
	Yehezkel Bernat <yehezkel.bernat@intel.com>,
	Mario.Limonciello@dell.com,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2 6/8] PCI: pciehp: Fix race condition handling surprise link down
Date: Fri, 13 Oct 2017 21:35:46 +0300	[thread overview]
Message-ID: <20171013183548.68283-7-mika.westerberg@linux.intel.com> (raw)
In-Reply-To: <20171013183548.68283-1-mika.westerberg@linux.intel.com>

A surprise link down may retrain very quickly causing the same slot
generate a link up event before handling the link down event completes.

Since the link is active, the power off work queued from the first link
down will cause a second down event when power is disabled. However, the
link up event sets the slot state to POWERON_STATE before the event to
handle this is enqueued, making the second down event believe it needs
to do something.

This creates constant link up and down event cycle.

To prevent this it is better to handle each event at the time in order
it occurred, so change the driver to use ordered workqueue instead.

A normal device hotplug triggers two events (presense detect and link
up) that are already handled properly in the driver but we currently log
an error if we find an existing device in the slot. Since this is not an
error change the log level to be debug instead to avoid scaring users.

This is based on the original work by Ashok Raj.

Link: https://patchwork.kernel.org/patch/9469023
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/hotplug/pciehp_ctrl.c | 7 ++++---
 drivers/pci/hotplug/pciehp_hpc.c  | 2 +-
 drivers/pci/hotplug/pciehp_pci.c  | 6 +++++-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
index ec0b4c11ccd9..83f3d4af3677 100644
--- a/drivers/pci/hotplug/pciehp_ctrl.c
+++ b/drivers/pci/hotplug/pciehp_ctrl.c
@@ -113,10 +113,11 @@ static int board_added(struct slot *p_slot)
 
 	retval = pciehp_configure_device(p_slot);
 	if (retval) {
-		ctrl_err(ctrl, "Cannot add device at %04x:%02x:00\n",
-			 pci_domain_nr(parent), parent->number);
-		if (retval != -EEXIST)
+		if (retval != -EEXIST) {
+			ctrl_err(ctrl, "Cannot add device at %04x:%02x:00\n",
+				 pci_domain_nr(parent), parent->number);
 			goto err_exit;
+		}
 	}
 
 	pciehp_green_led_on(p_slot);
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index e5d5ce9e3010..83c93f9da65a 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -795,7 +795,7 @@ static int pcie_init_slot(struct controller *ctrl)
 	if (!slot)
 		return -ENOMEM;
 
-	slot->wq = alloc_workqueue("pciehp-%u", 0, 0, PSN(ctrl));
+	slot->wq = alloc_ordered_workqueue("pciehp-%u", 0, PSN(ctrl));
 	if (!slot->wq)
 		goto abort;
 
diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index c3af027ee1a6..2a1ca020cf5a 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -46,7 +46,11 @@ int pciehp_configure_device(struct slot *p_slot)
 
 	dev = pci_get_slot(parent, PCI_DEVFN(0, 0));
 	if (dev) {
-		ctrl_err(ctrl, "Device %s already exists at %04x:%02x:00, cannot hot-add\n",
+		/*
+		 * The device is already there. Either configured by the
+		 * boot firmware or a previous hotplug event.
+		 */
+		ctrl_dbg(ctrl, "Device %s already exists at %04x:%02x:00, skipping hot-add\n",
 			 pci_name(dev), pci_domain_nr(parent), parent->number);
 		pci_dev_put(dev);
 		ret = -EEXIST;
-- 
2.14.2

  parent reply	other threads:[~2017-10-13 18:35 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-13 18:35 [PATCH v2 0/8] PCI: Improvements for native PCIe hotplug Mika Westerberg
2017-10-13 18:35 ` [PATCH v2 1/8] PCI: Move pci_hp_add_bridge() to drivers/pci/probe.c Mika Westerberg
2017-10-13 18:35 ` [PATCH v2 2/8] PCI: Open-code the two pass loop when scanning bridges Mika Westerberg
2017-10-13 18:35 ` [PATCH v2 3/8] PCI: Do not allocate more buses than available in parent Mika Westerberg
2017-10-13 18:35 ` [PATCH v2 4/8] PCI: Distribute available buses to hotplug capable bridges Mika Westerberg
2017-10-13 18:35 ` [PATCH v2 5/8] PCI: Distribute available resources " Mika Westerberg
2017-10-13 18:35 ` Mika Westerberg [this message]
2017-10-13 18:35 ` [PATCH v2 7/8] PCI: pciehp: Do not clear Presence Detect Changed during initialization Mika Westerberg
2017-10-13 18:35 ` [PATCH v2 8/8] PCI: pciehp: Check that the device is really present before touching it Mika Westerberg
2017-10-20 21:15   ` Bjorn Helgaas
2017-10-23 11:04     ` Mika Westerberg
2017-10-20 21:37 ` [PATCH v2 0/8] PCI: Improvements for native PCIe hotplug Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171013183548.68283-7-mika.westerberg@linux.intel.com \
    --to=mika.westerberg@linux.intel.com \
    --cc=Mario.Limonciello@dell.com \
    --cc=ashok.raj@intel.com \
    --cc=bhelgaas@google.com \
    --cc=keith.busch@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=michael.jamet@intel.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=yehezkel.bernat@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).