opal: poller re-entrancy try open-power#3

So my great attempt at avoiding all re-entencies fails due to HBRT... at
least until we have some kind of way to thread things, it will have to
re-enter so let's bite the bullet, make the poller list walking lockless
(we'll handle removal when we have to, ie, not yet) and slightly extend
the coverage of the PSI lock while at it. All the other pollers already
have their own locks anyway so we are actually removing some overhead.

Signed-off-by: Benjamin Herrenschmidt <[email protected]>

Author: ozbenh

core/opal.c

@@ -201,50 +201,33 @@ void opal_del_poller(void (*poller)(void *data))
 {
 	struct opal_poll_entry *ent;
 
+	/* XXX This is currently unused. To solve various "interesting"
+	 * locking issues, the pollers are run locklessly, so if we were
+	 * to free them, we would have to be careful, using something
+	 * akin to RCU to synchronize with other OPAL entries. For now
+	 * if anybody uses it, print a warning and leak the entry, don't
+	 * free it.
+	 */
+	prerror("WARNING: Unsupported opal_del_poller\n");
+
 	lock(&opal_poll_lock);
 	list_for_each(&opal_pollers, ent, link) {
 		if (ent->poller == poller) {
 			list_del(&ent->link);
-			free(ent);
+			/* free(ent); */
 			break;
 		}
 	}
 	unlock(&opal_poll_lock);
 }
 
-bool __opal_check_poll_recursion(const char *caller)
-{
-	if (!lock_held_by_me(&opal_poll_lock))
-		return false;
-	prerror("OPAL: poller recursion caught in %s !\n", caller);
-	backtrace();
-
-	return true;
-}
-
-void __opal_run_pollers(const char *caller)
+void opal_run_pollers(void)
 {
 	struct opal_poll_entry *poll_ent;
 
-	/* Debug path. Warn if we recursed */
-	if (__opal_check_poll_recursion(caller)) {
-		/* This shouldn't happen. However, if it does, we are goin
-		 * to end up warning a *LOT* so let's introduce an arbitrary
-		 * delay here.
-		 */
-		time_wait_ms_nopoll(10);
-		return;
-	}
-
-	/*
-	 * Only run the pollers if they aren't already running
-	 * on another CPU and we aren't re-entering.
-	 */
-	if (try_lock(&opal_poll_lock)) {
-		list_for_each(&opal_pollers, poll_ent, link)
-			poll_ent->poller(poll_ent->data);
-		unlock(&opal_poll_lock);
-	}
+	/* The pollers are run lokelessly, see comment in opal_del_poller */
+	list_for_each(&opal_pollers, poll_ent, link)
+		poll_ent->poller(poll_ent->data);
 }
 
 static int64_t opal_poll_events(uint64_t *outstanding_event_mask)

hw/fsp/fsp.c

@@ -1544,22 +1544,8 @@ int fsp_sync_msg(struct fsp_msg *msg, bool autofree)
 	if (rc)
 		goto bail;
 
-	/* Debug .. make sure we aren't recursing on the poll lock
-	 * or the FSP lock
-	 */
-	if (opal_check_poll_recursion()) {
-		/* Until we're confident we found all culprits, fallback
-		 * to the old way of just polling the FSP
-		 */
-		while(fsp_msg_busy(msg)) {
-			lock(&fsp_lock);
-			__fsp_poll(false);
-			unlock(&fsp_lock);
-		}
-	} else {
-		while(fsp_msg_busy(msg))
-			opal_run_pollers();
-	}
+	while(fsp_msg_busy(msg))
+		opal_run_pollers();
 
 	switch(msg->state) {
 	case fsp_msg_done:

hw/psi.c

@@ -122,18 +122,14 @@ static void psi_link_poll(void *data __unused)
 		(tb_compare(now, psi_link_timer) == TB_AAFTERB) ||
 		(tb_compare(now, psi_link_timer) == TB_AEQUALB)) {
 
+		lock(&psi_lock);
+
 		list_for_each(&psis, psi, list) {
 			u64 val;
 
 			if (psi->active || !psi->working)
 				continue;
 
-			lock(&psi_lock);
-			if (psi->active || !psi->working) {
-				unlock(&psi_lock);
-				continue;
-			}
-
 			val = in_be64(psi->regs + PSIHB_CR);
 
 			printf("PSI[0x%03x]: Poll CR=0x%016llx\n",
@@ -150,9 +146,7 @@ static void psi_link_poll(void *data __unused)
 				fsp_reinit_fsp();
 				return;
 			}
-			unlock(&psi_lock);
 		}
-
 		if (!psi_link_timeout)
 			psi_link_timeout =
 				now + secs_to_tb(PSI_LINK_RECOVERY_TIMEOUT);
@@ -165,6 +159,8 @@ static void psi_link_poll(void *data __unused)
 
 		/* Poll every 10 seconds */
 		psi_link_timer = now + secs_to_tb(PSI_LINK_CHECK_INTERVAL);
+
+		unlock(&psi_lock);
 	}
 }
 

include/opal.h

@@ -900,11 +900,7 @@ extern void __opal_register(uint64_t token, void *func, unsigned num_args);
  */
 extern void opal_add_poller(void (*poller)(void *data), void *data);
 extern void opal_del_poller(void (*poller)(void *data));
-extern void __opal_run_pollers(const char *caller);
-extern bool __opal_check_poll_recursion(const char *caller);
-
-#define opal_run_pollers() __opal_run_pollers(__func__)
-#define opal_check_poll_recursion() __opal_check_poll_recursion(__func__)
+extern void opal_run_pollers(void);
 
 /*
  * Warning: no locking, only call that from the init processor