[PATCH RFC 01/15] powerpc/eeh: Introduce refcounting for struct eeh_pe

Sam Bobroff sbobroff at linux.ibm.com
Wed Oct 2 16:02:39 AEST 2019


Introduce infrastructure supporting refcounting for struct eeh_pe.

This will provide protection of the list members and struct memory so
that crashes related to accessing free memory or poisoned list
pointers can be avoided (for example, when EEH is triggered during
device removal).

While this provides no additional synchronization of the other EEH
state, it seems to be an effective way of providing the necessary
safety with a very low risk of introducing deadlocks.

Signed-off-by: Sam Bobroff <sbobroff at linux.ibm.com>
---
 arch/powerpc/include/asm/eeh.h |  7 ++++
 arch/powerpc/kernel/eeh_pe.c   | 70 +++++++++++++++++++++++++++++++++-
 2 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 5448b68ff260..54ba958760f2 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -13,6 +13,7 @@
 #include <linux/string.h>
 #include <linux/time.h>
 #include <linux/atomic.h>
+#include <linux/kref.h>
 
 #include <uapi/asm/eeh.h>
 
@@ -72,6 +73,7 @@ struct pci_dn;
 #define EEH_PE_PRI_BUS		(1 << 11)	/* Cached primary bus   */
 
 struct eeh_pe {
+	struct kref kref;		/* Reference count		*/
 	int type;			/* PE type: PHB/Bus/Device	*/
 	int state;			/* PE EEH dependent mode	*/
 	int config_addr;		/* Traditional PCI address	*/
@@ -276,7 +278,12 @@ static inline bool eeh_state_active(int state)
 
 typedef void (*eeh_edev_traverse_func)(struct eeh_dev *edev, void *flag);
 typedef void *(*eeh_pe_traverse_func)(struct eeh_pe *pe, void *flag);
+void eeh_lock_pes(unsigned long *flags);
+void eeh_unlock_pes(unsigned long flags);
 void eeh_set_pe_aux_size(int size);
+void eeh_get_pe(struct eeh_pe *pe);
+void eeh_put_pe(struct eeh_pe *pe);
+void eeh_pe_move_to_parent(struct eeh_pe **pe);
 int eeh_phb_pe_create(struct pci_controller *phb);
 int eeh_wait_state(struct eeh_pe *pe, int max_wait);
 struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb);
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 177852e39a25..d455df7b4928 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -22,6 +22,25 @@
 static int eeh_pe_aux_size = 0;
 static LIST_HEAD(eeh_phb_pe);
 
+/*
+ * Lock protecting the parent and child list fields of struct eeh_pe (this does
+ * not include the edev list).  The lock does not, in general, prevent struct
+ * eeh_pe objects from being freed, but it does provide that when held and a
+ * reference is held on a PE, that neither that PE or any other PE reachable
+ * via it's parent or child fields will be freed.
+ */
+static DEFINE_RAW_SPINLOCK(eeh_pe_lock);
+
+void eeh_lock_pes(unsigned long *flags)
+{
+	raw_spin_lock_irqsave(&eeh_pe_lock, *flags);
+}
+
+void eeh_unlock_pes(unsigned long flags)
+{
+	raw_spin_unlock_irqrestore(&eeh_pe_lock, flags);
+}
+
 /**
  * eeh_set_pe_aux_size - Set PE auxillary data size
  * @size: PE auxillary data size
@@ -59,6 +78,7 @@ static struct eeh_pe *eeh_pe_alloc(struct pci_controller *phb, int type)
 	if (!pe) return NULL;
 
 	/* Initialize PHB PE */
+	kref_init(&pe->kref); /* Acquire existence ref */
 	pe->type = type;
 	pe->phb = phb;
 	INIT_LIST_HEAD(&pe->child_list);
@@ -69,6 +89,51 @@ static struct eeh_pe *eeh_pe_alloc(struct pci_controller *phb, int type)
 	return pe;
 }
 
+static void eeh_pe_free(struct kref *kref)
+{
+	struct eeh_pe *pe = container_of(kref, struct eeh_pe, kref);
+
+	if (WARN_ONCE(pe->type & EEH_PE_PHB,
+		      "EEH: PHB#%x-PE#%x: Attempt to free PHB PE!\n",
+		      pe->phb->global_number, pe->addr))
+		return;
+
+	if (WARN_ONCE(pe->parent,
+		      "EEH: PHB#%x-PE#%x: Attempt to free in-use PE!\n",
+		      pe->phb->global_number, pe->addr))
+		return;
+
+	kfree(pe);
+}
+
+void eeh_get_pe(struct eeh_pe *pe)
+{
+	if (!pe)
+		return;
+	WARN_ONCE(!kref_get_unless_zero(&pe->kref),
+		  "EEH: PHB#%x-PE#%x: Attempt to get when refcount 0!\n",
+		  pe->phb->global_number, pe->addr);
+}
+
+void eeh_put_pe(struct eeh_pe *pe)
+{
+	if (!pe)
+		return;
+	kref_put(&pe->kref, eeh_pe_free);
+}
+
+void eeh_pe_move_to_parent(struct eeh_pe **pe)
+{
+	unsigned long flags;
+	struct eeh_pe *old_pe = *pe;
+
+	eeh_lock_pes(&flags);
+	*pe = (*pe)->parent;
+	eeh_get_pe(*pe);
+	eeh_put_pe(old_pe);
+	eeh_unlock_pes(flags);
+}
+
 /**
  * eeh_phb_pe_create - Create PHB PE
  * @phb: PCI controller
@@ -439,7 +504,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
 			pr_err("%s: No PHB PE is found (PHB Domain=%d)\n",
 				__func__, pdn->phb->global_number);
 			edev->pe = NULL;
-			kfree(pe);
+			eeh_put_pe(pe); /* Release existence ref */
 			return -EEXIST;
 		}
 	}
@@ -509,7 +574,8 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
 			if (list_empty(&pe->edevs) &&
 			    list_empty(&pe->child_list)) {
 				list_del(&pe->child);
-				kfree(pe);
+				pe->parent = NULL;
+				eeh_put_pe(pe); /* Release existence ref */
 			} else {
 				break;
 			}
-- 
2.22.0.216.g00a2a96fc9



More information about the Linuxppc-dev mailing list