[Skiboot] [PATCH v2 02/16] core/pci: Add missing lock in set_power_timer

Frederic Barrat fbarrat at linux.ibm.com
Thu Oct 10 06:37:57 AEDT 2019


set_power_timer() was not using any lock, though it alters the slot
state and devices found under it. There's a remote possibility that
set_power_timer() is called through check_timers() by a thread already
holding the phb lock, so we try to take the lock but yield and rearm
the timer if somebody else is already owning it. There really
shouldn't be any contention here.

Signed-off-by: Frederic Barrat <fbarrat at linux.ibm.com>
---
Changelog:
v2:
 - avoid possible deadlock when locking the PHB from timer callbacks (Oliver)


core/pci-opal.c | 7 +++++++
 include/pci.h   | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/core/pci-opal.c b/core/pci-opal.c
index d5209600..b63be61f 100644
--- a/core/pci-opal.c
+++ b/core/pci-opal.c
@@ -674,6 +674,12 @@ static void set_power_timer(struct timer *t __unused, void *data,
 	struct pci_device *pd = slot->pd;
 	struct dt_node *dn = pd->dn;
 	uint8_t link;
+	struct phb *phb = slot->phb;
+
+	if (!phb_try_lock(phb)) {
+		schedule_timer(&slot->timer, msecs_to_tb(10));
+		return;
+	}
 
 	switch (slot->state) {
 	case PCI_SLOT_STATE_SPOWER_START:
@@ -720,6 +726,7 @@ static void set_power_timer(struct timer *t __unused, void *data,
 		prlog(PR_ERR, "PCI SLOT %016llx: Unexpected state 0x%08x\n",
 		      slot->id, slot->state);
 	}
+	phb_unlock(phb);
 }
 
 static int64_t opal_pci_set_power_state(uint64_t async_token,
diff --git a/include/pci.h b/include/pci.h
index dcd354a7..8169fe6a 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -390,6 +390,11 @@ static inline void phb_lock(struct phb *phb)
 	lock(&phb->lock);
 }
 
+static inline bool phb_try_lock(struct phb *phb)
+{
+	return try_lock(&phb->lock);
+}
+
 static inline void phb_unlock(struct phb *phb)
 {
 	unlock(&phb->lock);
-- 
2.21.0



More information about the Skiboot mailing list