Subject: Re: wired entry count problem on sun3x pmap
To: None <thorpej@shagadelic.org>
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
List: port-sun3
Date: 09/13/2006 23:33:02
thorpej@shagadelic.org wrote:

> > These TAILQs are modified in pmap_enter(), pmap_unwire() and
> > pmap_remove(). In this case, should these functions be protected
> > by splvm()?
> 
> Nope, you should be OK without splvm() then.

Thanks. Actually sprinkled splvm() didn't help at all.

Now I realize we should handle unwiring page case not only
in pmap_remove_[abc]() but also free_[abc]_table() functions.
(patch attached)

I'll try to see how pkgsrc perl5 build goes..
(it will take 20~40 hours again ;-)
---
Izumi Tsutsui


Index: arch/sun3/sun3x/pmap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/sun3/sun3x/pmap.c,v
retrieving revision 1.90
diff -u -r1.90 pmap.c
--- arch/sun3/sun3x/pmap.c	10 May 2006 06:24:03 -0000	1.90
+++ arch/sun3/sun3x/pmap.c	13 Sep 2006 12:47:25 -0000
@@ -1222,6 +1222,7 @@
 	 * pool.  That is a job for the function that called us.
 	 */
 	if (tbl->at_parent) {
+		KASSERT(tbl->at_wcnt == 0);
 		pmap = tbl->at_parent;
 		free_a_table(tbl, FALSE);
 		pmap->pm_a_tmgr = NULL;
@@ -1245,6 +1246,7 @@
 		panic("get_b_table: out of B tables.");
 	TAILQ_REMOVE(&b_pool, tbl, bt_link);
 	if (tbl->bt_parent) {
+		KASSERT(tbl->bt_wcnt == 0);
 		tbl->bt_parent->at_dtbl[tbl->bt_pidx].attr.raw = MMU_DT_INVALID;
 		tbl->bt_parent->at_ecnt--;
 		free_b_table(tbl, FALSE);
@@ -1267,6 +1269,7 @@
 		panic("get_c_table: out of C tables.");
 	TAILQ_REMOVE(&c_pool, tbl, ct_link);
 	if (tbl->ct_parent) {
+		KASSERT(tbl->ct_wcnt == 0);
 		tbl->ct_parent->bt_dtbl[tbl->ct_pidx].attr.raw = MMU_DT_INVALID;
 		tbl->ct_parent->bt_ecnt--;
 		free_c_table(tbl, FALSE);
@@ -1307,7 +1310,8 @@
 	int i, removed_cnt;
 	mmu_long_dte_t	*dte;
 	mmu_short_dte_t *dtbl;
-	b_tmgr_t	*tmgr;
+	b_tmgr_t	*b_tbl;
+	uint8_t at_wired, bt_wired;
 
 	/*
 	 * Flush the ATC cache of all cached descriptors derived
@@ -1330,6 +1334,7 @@
 	 * stopping short of the kernel's entries.
 	 */
 	removed_cnt = 0;
+	at_wired = a_tbl->at_wcnt;
 	if (a_tbl->at_ecnt) {
 		dte = a_tbl->at_dtbl;
 		for (i=0; i < MMU_TIA(KERNBASE); i++) {
@@ -1357,16 +1362,22 @@
 				 *    details.)
 				 */
 				dtbl = mmu_ptov(dte[i].addr.raw);
-				tmgr = mmuB2tmgr(dtbl);
-				removed_cnt += free_b_table(tmgr, TRUE);
+				b_tbl = mmuB2tmgr(dtbl);
+				bt_wired = b_tbl->bt_wcnt;
+				removed_cnt += free_b_table(b_tbl, TRUE);
+				if (bt_wired)
+					a_tbl->at_wcnt--;
 				dte[i].attr.raw = MMU_DT_INVALID;
 			}
 		}
 		a_tbl->at_ecnt = 0;
 	}
+	KASSERT(a_tbl->at_wcnt == 0);
+
 	if (relink) {
 		a_tbl->at_parent = NULL;
-		TAILQ_REMOVE(&a_pool, a_tbl, at_link);
+		if (!at_wired)
+			TAILQ_REMOVE(&a_pool, a_tbl, at_link);
 		TAILQ_INSERT_HEAD(&a_pool, a_tbl, at_link);
 	}
 	return removed_cnt;
@@ -1384,25 +1395,32 @@
 	int i, removed_cnt;
 	mmu_short_dte_t *dte;
 	mmu_short_pte_t	*dtbl;
-	c_tmgr_t	*tmgr;
+	c_tmgr_t	*c_tbl;
+	uint8_t bt_wired, ct_wired;
 
 	removed_cnt = 0;
+	bt_wired = b_tbl->bt_wcnt;
 	if (b_tbl->bt_ecnt) {
 		dte = b_tbl->bt_dtbl;
 		for (i=0; i < MMU_B_TBL_SIZE; i++) {
 			if (MMU_VALID_DT(dte[i])) {
 				dtbl = mmu_ptov(MMU_DTE_PA(dte[i]));
-				tmgr = mmuC2tmgr(dtbl);
-				removed_cnt += free_c_table(tmgr, TRUE);
+				c_tbl = mmuC2tmgr(dtbl);
+				ct_wired = c_tbl->ct_wcnt;
+				removed_cnt += free_c_table(c_tbl, TRUE);
+				if (ct_wired)
+					b_tbl->bt_wcnt--;
 				dte[i].attr.raw = MMU_DT_INVALID;
 			}
 		}
 		b_tbl->bt_ecnt = 0;
 	}
+	KASSERT(b_tbl->bt_wcnt == 0);
 
 	if (relink) {
 		b_tbl->bt_parent = NULL;
-		TAILQ_REMOVE(&b_pool, b_tbl, bt_link);
+		if (!bt_wired)
+			TAILQ_REMOVE(&b_pool, b_tbl, bt_link);
 		TAILQ_INSERT_HEAD(&b_pool, b_tbl, bt_link);
 	}
 	return removed_cnt;
@@ -1420,22 +1438,31 @@
 int 
 free_c_table(c_tmgr_t *c_tbl, boolean_t relink)
 {
+	mmu_short_pte_t *c_pte;
 	int i, removed_cnt;
+	uint8_t ct_wired;
 
 	removed_cnt = 0;
+	ct_wired = c_tbl->ct_wcnt;
 	if (c_tbl->ct_ecnt) {
 		for (i=0; i < MMU_C_TBL_SIZE; i++) {
-			if (MMU_VALID_DT(c_tbl->ct_dtbl[i])) {
-				pmap_remove_pte(&c_tbl->ct_dtbl[i]);
+			c_pte = &c_tbl->ct_dtbl[i];
+			if (MMU_VALID_DT(*c_pte)) {
+				if (c_pte->attr.raw & MMU_SHORT_PTE_WIRED) {
+					c_tbl->ct_wcnt--;
+				}
+				pmap_remove_pte(c_pte);
 				removed_cnt++;
 			}
 		}
 		c_tbl->ct_ecnt = 0;
 	}
+	KASSERT(c_tbl->ct_wcnt == 0);
 
 	if (relink) {
 		c_tbl->ct_parent = NULL;
-		TAILQ_REMOVE(&c_pool, c_tbl, ct_link);
+		if (!ct_wired)
+			TAILQ_REMOVE(&c_pool, c_tbl, ct_link);
 		TAILQ_INSERT_HEAD(&c_pool, c_tbl, ct_link);
 	}
 	return removed_cnt;
@@ -1821,23 +1848,34 @@
 		 *     change protection of a page
 		 *     change wiring status of a page
 		 *     remove the mapping of a page
-		 *
-		 * XXX - Semi critical: This code should unwire the PTE
-		 * and, possibly, associated parent tables if this is a
-		 * change wiring operation.  Currently it does not.
-		 *
-		 * This may be ok if pmap_unwire() is the only
-		 * interface used to UNWIRE a page.
 		 */
 
 		/* First check if this is a wiring operation. */
-		if (wired && (c_pte->attr.raw & MMU_SHORT_PTE_WIRED)) {
+		if (c_pte->attr.raw & MMU_SHORT_PTE_WIRED) {
 			/*
-			 * The PTE is already wired.  To prevent it from being
-			 * counted as a new wiring operation, reset the 'wired'
-			 * variable.
+			 * The existing mapping is wired, so adjust wired
+			 * entry count here. If new mapping is still wired,
+			 * wired entry count will be incremented again later.
 			 */
-			wired = FALSE;
+			c_tbl->ct_wcnt--;
+			if (!wired) {
+				/*
+				 * The mapping of this PTE is being changed
+				 * from wired to unwired.
+				 * Adjust wired entry counts in each table and
+				 * set llevel flag to put unwired tables back
+				 * into the active pool.
+				 */
+				if (c_tbl->ct_wcnt == 0) {
+					llevel = NEWC;
+					if (--b_tbl->bt_wcnt == 0) {
+						llevel = NEWB;
+						if (--a_tbl->at_wcnt == 0) {
+							llevel = NEWA;
+						}
+					}
+				}
+			}
 		}
 
 		/* Is the new address the same as the old? */
@@ -1944,7 +1982,7 @@
 		pv->pv_idx = nidx;
 	}
 
-	/* Move any allocated tables back into the active pool. */
+	/* Move any allocated or unwired tables back into the active pool. */
 	
 	switch (llevel) {
 		case NEWA:
@@ -2071,7 +2109,7 @@
 	int idx, eidx;
 
 #ifdef	PMAP_DEBUG
-	if ((sva & PGOFSET) || (eva & PGOFSET))
+	if ((va & PGOFSET) || (va & PGOFSET))
 		panic("pmap_kremove: alignment");
 #endif
 
@@ -2902,8 +2940,6 @@
  **
  * Remove the mapping of a range of virtual addresses from the given pmap.
  *
- * If the range contains any wired entries, this function will probably create
- * disaster.
  */
 void 
 pmap_remove(pmap_t pmap, vaddr_t sva, vaddr_t eva)
@@ -2972,6 +3008,7 @@
 	b_tmgr_t *b_tbl;
 	mmu_long_dte_t  *a_dte;
 	mmu_short_dte_t *b_dte;
+	uint8_t at_wired, bt_wired;
 
 	/*
 	 * The following code works with what I call a 'granularity
@@ -3005,6 +3042,8 @@
 	nstart = MMU_ROUND_UP_A(sva);
 	nend = MMU_ROUND_A(eva);
 
+	at_wired = a_tbl->at_wcnt;
+
 	if (sva < nstart) {
 		/*
 		 * This block is executed if the range starts between
@@ -3024,6 +3063,7 @@
 		if (MMU_VALID_DT(*a_dte)) {
 			b_dte = mmu_ptov(a_dte->addr.raw);
 			b_tbl = mmuB2tmgr(b_dte);
+			bt_wired = b_tbl->bt_wcnt;
 
 			/*
 			 * The sub range to be removed starts at the start
@@ -3039,6 +3079,13 @@
 				empty = pmap_remove_b(b_tbl, sva, nstart);
 
 			/*
+			 * If the child table no longer has wired entries,
+			 * decrement wired entry count.
+			 */
+			if (bt_wired && b_tbl->bt_wcnt == 0)
+				a_tbl->at_wcnt--;
+
+			/*
 			 * If the removal resulted in an empty B table,
 			 * invalidate the DTE that points to it and decrement
 			 * the valid entry count of the A table.
@@ -3076,9 +3123,19 @@
 				 */
 				b_dte = mmu_ptov(a_dte->addr.raw);
 				b_tbl = mmuB2tmgr(b_dte);
+				bt_wired = b_tbl->bt_wcnt;
+
 				free_b_table(b_tbl, TRUE);
 
 				/*
+				 * All child entries has been removed.
+				 * If there were any wired entries in it,
+				 * decrement wired entry count.
+				 */
+				if (bt_wired)
+					a_tbl->at_wcnt--;
+
+				/*
 				 * Invalidate the DTE that points to the
 				 * B table and decrement the valid entry
 				 * count of the A table.
@@ -3112,10 +3169,17 @@
 			 */
 			b_dte = mmu_ptov(a_dte->addr.raw);
 			b_tbl = mmuB2tmgr(b_dte);
+			bt_wired = b_tbl->bt_wcnt;
 
 			empty = pmap_remove_b(b_tbl, nend, eva);
 
 			/*
+			 * If the child table no longer has wired entries,
+			 * decrement wired entry count.
+			 */
+			if (bt_wired && b_tbl->bt_wcnt == 0)
+				a_tbl->at_wcnt--;
+			/*
 			 * If the removal resulted in an empty B table,
 			 * invalidate the DTE that points to it and decrement
 			 * the valid entry count of the A table.
@@ -3132,11 +3196,20 @@
 	 * back to the available pool and return TRUE.
 	 */
 	if (a_tbl->at_ecnt == 0) {
+		KASSERT(a_tbl->at_wcnt == 0);
 		a_tbl->at_parent = NULL;
-		TAILQ_REMOVE(&a_pool, a_tbl, at_link);
+		if (!at_wired)
+			TAILQ_REMOVE(&a_pool, a_tbl, at_link);
 		TAILQ_INSERT_HEAD(&a_pool, a_tbl, at_link);
 		empty = TRUE;
 	} else {
+		/*
+		 * If the table doesn't have wired entries any longer
+		 * but still has unwired entries, put it back into
+		 * the available queue.
+		 */
+		if (at_wired && a_tbl->at_wcnt == 0)
+			TAILQ_INSERT_TAIL(&a_pool, a_tbl, at_link);
 		empty = FALSE;
 	}
 
@@ -3159,21 +3232,33 @@
 	c_tmgr_t *c_tbl;
 	mmu_short_dte_t  *b_dte;
 	mmu_short_pte_t  *c_dte;
+	uint8_t bt_wired, ct_wired;
 	
-
 	nstart = MMU_ROUND_UP_B(sva);
 	nend = MMU_ROUND_B(eva);
 
+	bt_wired = b_tbl->bt_wcnt;
+
 	if (sva < nstart) {
 		idx = MMU_TIB(sva);
 		b_dte = &b_tbl->bt_dtbl[idx];
 		if (MMU_VALID_DT(*b_dte)) {
 			c_dte = mmu_ptov(MMU_DTE_PA(*b_dte));
 			c_tbl = mmuC2tmgr(c_dte);
+			ct_wired = c_tbl->ct_wcnt;
+
 			if (eva < nstart)
 				empty = pmap_remove_c(c_tbl, sva, eva);
 			else
 				empty = pmap_remove_c(c_tbl, sva, nstart);
+
+			/*
+			 * If the child table no longer has wired entries,
+			 * decrement wired entry count.
+			 */
+			if (ct_wired && c_tbl->ct_wcnt == 0)
+				b_tbl->bt_wcnt--;
+
 			if (empty) {
 				b_dte->attr.raw = MMU_DT_INVALID;
 				b_tbl->bt_ecnt--;
@@ -3188,7 +3273,18 @@
 			if (MMU_VALID_DT(*b_dte)) {
 				c_dte = mmu_ptov(MMU_DTE_PA(*b_dte));
 				c_tbl = mmuC2tmgr(c_dte);
+				ct_wired = c_tbl->ct_wcnt;
+
 				free_c_table(c_tbl, TRUE);
+
+				/*
+				 * All child entries has been removed.
+				 * If there were any wired entries in it,
+				 * decrement wired entry count.
+				 */
+				if (ct_wired)
+					b_tbl->bt_wcnt--;
+
 				b_dte->attr.raw = MMU_DT_INVALID;
 				b_tbl->bt_ecnt--;
 			}
@@ -3202,7 +3298,16 @@
 		if (MMU_VALID_DT(*b_dte)) {
 			c_dte = mmu_ptov(MMU_DTE_PA(*b_dte));
 			c_tbl = mmuC2tmgr(c_dte);
+			ct_wired = c_tbl->ct_wcnt;
 			empty = pmap_remove_c(c_tbl, nend, eva);
+
+			/*
+			 * If the child table no longer has wired entries,
+			 * decrement wired entry count.
+			 */
+			if (ct_wired && c_tbl->ct_wcnt == 0)
+				b_tbl->bt_wcnt--;
+
 			if (empty) {
 				b_dte->attr.raw = MMU_DT_INVALID;
 				b_tbl->bt_ecnt--;
@@ -3211,11 +3316,21 @@
 	}
 
 	if (b_tbl->bt_ecnt == 0) {
+		KASSERT(b_tbl->bt_wcnt == 0);
 		b_tbl->bt_parent = NULL;
-		TAILQ_REMOVE(&b_pool, b_tbl, bt_link);
+		if (!bt_wired)
+			TAILQ_REMOVE(&b_pool, b_tbl, bt_link);
 		TAILQ_INSERT_HEAD(&b_pool, b_tbl, bt_link);
 		empty = TRUE;
 	} else {
+		/*
+		 * If the table doesn't have wired entries any longer
+		 * but still has unwired entries, put it back into
+		 * the available queue.
+		 */
+		if (bt_wired && b_tbl->bt_wcnt == 0)
+			TAILQ_INSERT_TAIL(&b_pool, b_tbl, bt_link);
+
 		empty = FALSE;
 	}
 
@@ -3232,22 +3347,36 @@
 	boolean_t empty;
 	int idx;
 	mmu_short_pte_t *c_pte;
+	uint8_t ct_wired;
 	
+	ct_wired = c_tbl->ct_wcnt;
+
 	idx = MMU_TIC(sva);
 	c_pte = &c_tbl->ct_dtbl[idx];
 	for (;sva < eva; sva += MMU_PAGE_SIZE, c_pte++) {
 		if (MMU_VALID_DT(*c_pte)) {
+			if (c_pte->attr.raw & MMU_SHORT_PTE_WIRED)
+				c_tbl->ct_wcnt--;
 			pmap_remove_pte(c_pte);
 			c_tbl->ct_ecnt--;
 		}
 	}
 
 	if (c_tbl->ct_ecnt == 0) {
+		KASSERT(c_tbl->ct_wcnt == 0);
 		c_tbl->ct_parent = NULL;
-		TAILQ_REMOVE(&c_pool, c_tbl, ct_link);
+		if (!ct_wired)
+			TAILQ_REMOVE(&c_pool, c_tbl, ct_link);
 		TAILQ_INSERT_HEAD(&c_pool, c_tbl, ct_link);
 		empty = TRUE;
 	} else {
+		/*
+		 * If the table doesn't have wired entries any longer
+		 * but still has unwired entries, put it back into
+		 * the available queue.
+		 */
+		if (ct_wired && c_tbl->ct_wcnt == 0)
+			TAILQ_INSERT_TAIL(&c_pool, c_tbl, ct_link);
 		empty = FALSE;
 	}