Attempt to make machine check handling a bit more robust:

- Don't malloc() new MCA records for machine checks logged due to a
  CMCI or MC# exception.  Instead, use a pre-allocated pool of records.
  When a CMCI or MC# exception fires, schedule a swi to refill the pool.
  The pool is sized to hold at least one record per available machine
  bank, and one record per CPU. This should handle the case of all CPUs
  triggering a single bank at once as well as the case a single CPU
  triggering all of its banks.  The periodic scans still use malloc()
  since they are run from a safe context.
- Since we have to create an swi to handle refills, make the periodic scan
  a second swi for the same thread instead of having a separate taskqueue
  thread for the scans.

Suggested by:	mdf (avoiding malloc())
MFC after:	2 weeks

1 files changed, 72 insertions, 28 deletions

diff --git a/sys/x86/x86/mca.c b/sys/x86/x86/mca.c
index 59de9d8..9352561 100644
--- a/sys/x86/x86/mca.c
+++ b/sys/x86/x86/mca.c
@@ -50,7 +50,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/smp.h>
 #include <sys/sysctl.h>
 #include <sys/systm.h>
-#include <sys/taskqueue.h>
 #include <machine/intr_machdep.h>
 #include <machine/apicvar.h>
 #include <machine/cputypes.h>
@@ -85,6 +84,7 @@ struct mca_internal {
 static MALLOC_DEFINE(M_MCA, "MCA", "Machine Check Architecture");
 
 static int mca_count;		/* Number of records stored. */
+static int mca_banks;		/* Number of per-CPU register banks. */
 
 static SYSCTL_NODE(_hw, OID_AUTO, mca, CTLFLAG_RD, NULL,
     "Machine Check Architecture");
@@ -103,16 +103,16 @@ int workaround_erratum383;
 SYSCTL_INT(_hw_mca, OID_AUTO, erratum383, CTLFLAG_RD, &workaround_erratum383, 0,
     "Is the workaround for Erratum 383 on AMD Family 10h processors enabled?");
 
+static STAILQ_HEAD(, mca_internal) mca_freelist;
+static int mca_freecount;
 static STAILQ_HEAD(, mca_internal) mca_records;
 static struct callout mca_timer;
 static int mca_ticks = 3600;	/* Check hourly by default. */
-static struct taskqueue *mca_tq;
-static struct task mca_task;
 static struct mtx mca_lock;
+static void *mca_refill_swi, *mca_scan_swi;
 
 #ifdef DEV_APIC
 static struct cmc_state **cmc_state;	/* Indexed by cpuid, bank */
-static int cmc_banks;
 static int cmc_throttle = 60;	/* Time in seconds to throttle CMCI. */
 #endif
 
@@ -416,21 +416,60 @@ mca_check_status(int bank, struct mca_record *rec)
 	return (1);
 }
 
-static void __nonnull(1)
-mca_record_entry(const struct mca_record *record)
+static void
+mca_fill_freelist(void)
 {
 	struct mca_internal *rec;
+	int desired;
 
-	rec = malloc(sizeof(*rec), M_MCA, M_NOWAIT);
-	if (rec == NULL) {
-		printf("MCA: Unable to allocate space for an event.\n");
-		mca_log(record);
-		return;
+	/*
+	 * Ensure we have at least one record for each bank and one
+	 * record per CPU.
+	 */
+	desired = imax(mp_ncpus, mca_banks);
+	mtx_lock_spin(&mca_lock);
+	while (mca_freecount < desired) {
+		mtx_unlock_spin(&mca_lock);
+		rec = malloc(sizeof(*rec), M_MCA, M_WAITOK);
+		mtx_lock_spin(&mca_lock);
+		STAILQ_INSERT_TAIL(&mca_freelist, rec, link);
+		mca_freecount++;
+	}
+	mtx_unlock_spin(&mca_lock);
+}
+
+static void
+mca_refill(void *arg)
+{
+
+	mca_fill_freelist();
+}
+
+static void __nonnull(2)
+mca_record_entry(enum scan_mode mode, const struct mca_record *record)
+{
+	struct mca_internal *rec;
+
+	if (mode == POLLED) {
+		rec = malloc(sizeof(*rec), M_MCA, M_WAITOK);
+		mtx_lock_spin(&mca_lock);
+	} else {
+		mtx_lock_spin(&mca_lock);
+		rec = STAILQ_FIRST(&mca_freelist);
+		if (rec == NULL) {
+			mtx_unlock_spin(&mca_lock);
+			printf("MCA: Unable to allocate space for an event.\n");
+			mca_log(record);
+			return;
+		}
+		STAILQ_REMOVE_HEAD(&mca_freelist, link);
+		mca_freecount--;
+		if (mca_refill_swi != NULL)
+			swi_sched(mca_refill_swi, 0);
 	}
 
 	rec->rec = *record;
 	rec->logged = 0;
-	mtx_lock_spin(&mca_lock);
 	STAILQ_INSERT_TAIL(&mca_records, rec, link);
 	mca_count++;
 	mtx_unlock_spin(&mca_lock);
@@ -552,7 +591,7 @@ mca_scan(enum scan_mode mode)
 				recoverable = 0;
 				mca_log(&rec);
 			}
-			mca_record_entry(&rec);
+			mca_record_entry(mode, &rec);
 		}
 	
 #ifdef DEV_APIC
@@ -564,6 +603,8 @@ mca_scan(enum scan_mode mode)
 			cmci_update(mode, i, valid, &rec);
 #endif
 	}
+	if (mode == POLLED)
+		mca_fill_freelist();
 	return (mode == MCE ? recoverable : count);
 }
 
@@ -573,7 +614,7 @@ mca_scan(enum scan_mode mode)
  * them to the console.
  */
 static void
-mca_scan_cpus(void *context, int pending)
+mca_scan_cpus(void *arg)
 {
 	struct mca_internal *mca;
 	struct thread *td;
@@ -608,7 +649,7 @@ static void
 mca_periodic_scan(void *arg)
 {
 
-	taskqueue_enqueue(mca_tq, &mca_task);
+	swi_sched(mca_scan_swi, 1);
 	callout_reset(&mca_timer, mca_ticks * hz, mca_periodic_scan, NULL);
 }
 
@@ -622,36 +663,37 @@ sysctl_mca_scan(SYSCTL_HANDLER_ARGS)
 	if (error)
 		return (error);
 	if (i)
-		taskqueue_enqueue(mca_tq, &mca_task);
+		swi_sched(mca_scan_swi, 1);
 	return (0);
 }
 
 static void
 mca_startup(void *dummy)
 {
+	struct intr_event *ie;
 
 	if (!mca_enabled || !(cpu_feature & CPUID_MCA))
 		return;
 
-	mca_tq = taskqueue_create("mca", M_WAITOK, taskqueue_thread_enqueue,
-	    &mca_tq);
-	taskqueue_start_threads(&mca_tq, 1, PI_SWI(SWI_TQ), "mca taskq");
-	callout_reset(&mca_timer, mca_ticks * hz, mca_periodic_scan,
-		    NULL);
+	ie = NULL;
+	swi_add(&ie, "mca:scan", mca_scan_cpus, NULL, SWI_TQ, INTR_MPSAFE,
+	    &mca_scan_swi);
+	swi_add(&ie, "mca:refill", mca_refill, NULL, SWI_TQ, INTR_MPSAFE,
+	    &mca_refill_swi);
+	callout_reset(&mca_timer, mca_ticks * hz, mca_periodic_scan, NULL);
 }
 SYSINIT(mca_startup, SI_SUB_SMP, SI_ORDER_ANY, mca_startup, NULL);
 
 #ifdef DEV_APIC
 static void
-cmci_setup(uint64_t mcg_cap)
+cmci_setup(void)
 {
 	int i;
 
 	cmc_state = malloc((mp_maxid + 1) * sizeof(struct cmc_state **),
 	    M_MCA, M_WAITOK);
-	cmc_banks = mcg_cap & MCG_CAP_COUNT;
 	for (i = 0; i <= mp_maxid; i++)
-		cmc_state[i] = malloc(sizeof(struct cmc_state) * cmc_banks,
+		cmc_state[i] = malloc(sizeof(struct cmc_state) * mca_banks,
 		    M_MCA, M_WAITOK | M_ZERO);
 	SYSCTL_ADD_PROC(NULL, SYSCTL_STATIC_CHILDREN(_hw_mca), OID_AUTO,
 	    "cmc_throttle", CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_MPSAFE,
@@ -673,10 +715,12 @@ mca_setup(uint64_t mcg_cap)
 	    CPUID_TO_FAMILY(cpu_id) == 0x10 && amd10h_L1TP)
 		workaround_erratum383 = 1;
 
+	mca_banks = mcg_cap & MCG_CAP_COUNT;
 	mtx_init(&mca_lock, "mca", NULL, MTX_SPIN);
 	STAILQ_INIT(&mca_records);
-	TASK_INIT(&mca_task, 0, mca_scan_cpus, NULL);
 	callout_init(&mca_timer, CALLOUT_MPSAFE);
+	STAILQ_INIT(&mca_freelist);
+	mca_fill_freelist();
 	SYSCTL_ADD_INT(NULL, SYSCTL_STATIC_CHILDREN(_hw_mca), OID_AUTO,
 	    "count", CTLFLAG_RD, &mca_count, 0, "Record count");
 	SYSCTL_ADD_PROC(NULL, SYSCTL_STATIC_CHILDREN(_hw_mca), OID_AUTO,
@@ -690,7 +734,7 @@ mca_setup(uint64_t mcg_cap)
 	    sysctl_mca_scan, "I", "Force an immediate scan for machine checks");
 #ifdef DEV_APIC
 	if (mcg_cap & MCG_CAP_CMCI_P)
-		cmci_setup(mcg_cap);
+		cmci_setup();
 #endif
 }
 
@@ -708,7 +752,7 @@ cmci_monitor(int i)
 	struct cmc_state *cc;
 	uint64_t ctl;
 
-	KASSERT(i < cmc_banks, ("CPU %d has more MC banks", PCPU_GET(cpuid)));
+	KASSERT(i < mca_banks, ("CPU %d has more MC banks", PCPU_GET(cpuid)));
 
 	ctl = rdmsr(MSR_MC_CTL2(i));
 	if (ctl & MC_CTL2_CMCI_EN)
@@ -752,7 +796,7 @@ cmci_resume(int i)
 	struct cmc_state *cc;
 	uint64_t ctl;
 
-	KASSERT(i < cmc_banks, ("CPU %d has more MC banks", PCPU_GET(cpuid)));
+	KASSERT(i < mca_banks, ("CPU %d has more MC banks", PCPU_GET(cpuid)));
 
 	/* Ignore banks not monitored by this CPU. */
 	if (!(PCPU_GET(cmci_mask) & 1 << i))