arm pmap locking

[Alan Cox]

On 09/26/2012 01:13, John-Mark Gurney wrote:
> Alan Cox wrote this message on Tue, Sep 25, 2012 at 10:50 -0500:
>> On 09/16/2012 22:33, John-Mark Gurney wrote:
>>> Still getting the LOR..  It always right after boot..  after trying
>>> to mount root and warning about no time-of-day clock, but before setting
>>> hostuuid..
>> Could you please try the attached patch on your armv6 BEAGLEBONE?  My
>> hope is that this addresses the root cause of the LORs involving the
>> pmap lock.
> Good news...  That patch boots and doesn't have the LOR...
>
> FreeBSD 10.0-CURRENT #10 r240947M: Mon Sep 24 17:01:11 PDT 2012
>      jmg at pcbsd-779:/usr/obj/arm.armv6/usr/src.HEAD/sys/helium arm
>
> [...]
>
> Trying to mount root from ufs:mmcsd0s2 []...
> warning: no time-of-day clock registered, system time will not be set accurately
> Setting hostuuid: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx.
>
> Thanks for your work tracking it down!
>

Great.  Now that the LOR is out of the way, let's try further 
simplifications to the locking.  I don't believe that 
pmap_alloc_l2_bucket() needs to unlock and relock around every 
allocation.  Here is another patch to eliminate that.  Also, the patch 
replaces an explicit bzero() call with M_ZERO.

Alan

-------------- next part --------------
Index: arm/arm/pmap-v6.c
===================================================================
--- arm/arm/pmap-v6.c	(revision 240983)
+++ arm/arm/pmap-v6.c	(working copy)
@@ -607,36 +607,20 @@ pmap_alloc_l2_bucket(pmap_t pm, vm_offset_t va)
 	l1idx = L1_IDX(va);
 
 	PMAP_ASSERT_LOCKED(pm);
-	rw_assert(&pvh_global_lock, RA_WLOCKED);
 	if ((l2 = pm->pm_l2[L2_IDX(l1idx)]) == NULL) {
 		/*
 		 * No mapping at this address, as there is
 		 * no entry in the L1 table.
 		 * Need to allocate a new l2_dtable.
 		 */
-		PMAP_UNLOCK(pm);
-		rw_wunlock(&pvh_global_lock);
-		if ((l2 = uma_zalloc(l2table_zone, M_NOWAIT)) == NULL) {
-			rw_wlock(&pvh_global_lock);
-			PMAP_LOCK(pm);
+		l2 = uma_zalloc(l2table_zone, M_NOWAIT | M_ZERO);
+		if (l2 == NULL)
 			return (NULL);
-		}
-		rw_wlock(&pvh_global_lock);
-		PMAP_LOCK(pm);
-		if (pm->pm_l2[L2_IDX(l1idx)] != NULL) {
-			/*
-			 * Someone already allocated the l2_dtable while
-			 * we were doing the same.
-			 */
-			uma_zfree(l2table_zone, l2);
-			l2 = pm->pm_l2[L2_IDX(l1idx)];
-		} else {
-			bzero(l2, sizeof(*l2));
-			/*
-			 * Link it into the parent pmap
-			 */
-			pm->pm_l2[L2_IDX(l1idx)] = l2;
-		}
+
+		/*
+		 * Link it into the parent pmap.
+		 */
+		pm->pm_l2[L2_IDX(l1idx)] = l2;
 	}
 
 	l2b = &l2->l2_bucket[L2_BUCKET(l1idx)];
@@ -651,17 +635,7 @@ pmap_alloc_l2_bucket(pmap_t pm, vm_offset_t va)
 		 * No L2 page table has been allocated. Chances are, this
 		 * is because we just allocated the l2_dtable, above.
 		 */
-		PMAP_UNLOCK(pm);
-		rw_wunlock(&pvh_global_lock);
 		ptep = uma_zalloc(l2zone, M_NOWAIT);
-		rw_wlock(&pvh_global_lock);
-		PMAP_LOCK(pm);
-		if (l2b->l2b_kva != 0) {
-			/* We lost the race. */
-			uma_zfree(l2zone, ptep);
-			return (l2b);
-		}
-		l2b->l2b_phys = vtophys(ptep);
 		if (ptep == NULL) {
 			/*
 			 * Oops, no more L2 page tables available at this
@@ -677,6 +651,7 @@ pmap_alloc_l2_bucket(pmap_t pm, vm_offset_t va)
 
 		l2->l2_occupancy++;
 		l2b->l2b_kva = ptep;
+		l2b->l2b_phys = pmap_kextract((vm_offset_t)ptep);
 		l2b->l2b_l1idx = l1idx;
 	}