linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Sam Bobroff <sbobroff@linux.ibm.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: oohall@gmail.com
Subject: [PATCH RFC 01/15] powerpc/eeh: Introduce refcounting for struct eeh_pe
Date: Wed,  2 Oct 2019 16:02:39 +1000	[thread overview]
Message-ID: <f4282c1f3bc4105ae990b930b4c11b318376cda0.1569996166.git.sbobroff@linux.ibm.com> (raw)
In-Reply-To: <cover.1569996166.git.sbobroff@linux.ibm.com>

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@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


  reply	other threads:[~2019-10-02  6:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02  6:02 [PATCH RFC 00/15] powerpc/eeh: Synchronize access to struct eeh_pe Sam Bobroff
2019-10-02  6:02 ` Sam Bobroff [this message]
2019-10-02  6:02 ` [PATCH RFC 02/15] powerpc/eeh: Rename eeh_pe_get() to eeh_pe_find() Sam Bobroff
2019-11-13  2:32   ` Oliver O'Halloran
2019-10-02  6:02 ` [PATCH RFC 03/15] powerpc/eeh: Track orphaned struct eeh_pe Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 04/15] powerpc/eeh: Sync eeh_pe_next(), eeh_pe_find() and early-out traversals Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 05/15] powerpc/eeh: Sync eeh_pe_get_parent() Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 06/15] powerpc/eeh: Sync eeh_phb_pe_get() Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 07/15] powerpc/eeh: Sync eeh_add_to_parent_pe() and eeh_rmv_from_parent_pe() Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 08/15] powerpc/eeh: Sync eeh_handle_normal_event() Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 09/15] powerpw/eeh: Sync eeh_handle_special_event(), pnv_eeh_get_pe(), pnv_eeh_next_error() Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 10/15] powerpc/eeh: Sync eeh_phb_check_failure() Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 11/15] powerpc/eeh: Sync eeh_dev_check_failure() Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 12/15] powerpc/eeh: Sync eeh_pe_get_state() Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 13/15] powerpc/eeh: Sync pnv_eeh_ei_write() Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 14/15] powerpc/eeh: Sync eeh_force_recover_write() Sam Bobroff
2019-10-02  6:02 ` [PATCH RFC 15/15] powerpc/eeh: Sync pcibios_set_pcie_reset_state() Sam Bobroff

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=f4282c1f3bc4105ae990b930b4c11b318376cda0.1569996166.git.sbobroff@linux.ibm.com \
    --to=sbobroff@linux.ibm.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=oohall@gmail.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).