Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 30 Sep 2012 12:35:55 -0500
From:      Alan Cox <alc@rice.edu>
To:        Aleksandr Rybalko <ray@freebsd.org>
Cc:        "arm@freebsd.org" <arm@freebsd.org>
Subject:   Re: armv6 pmap patch
Message-ID:  <506882FB.9060507@rice.edu>
In-Reply-To: <20120930014939.9d277f0d.ray@freebsd.org>
References:  <504BDC56.3060607@rice.edu> <20120910211817.2d8a340d@fubar.geek.nz> <504E1A1B.90101@rice.edu> <20120928160227.99d2b126.ray@dlink.ua> <5066AB0D.6000901@rice.edu> <20120930014939.9d277f0d.ray@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------000404080808070608070201
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

On 09/29/2012 17:49, Aleksandr Rybalko wrote:
> On Sat, 29 Sep 2012 03:02:21 -0500
> Alan Cox<alc@rice.edu>  wrote:
>
>> On 09/28/2012 08:02, Aleksandr Rybalko wrote:
>>> On Mon, 10 Sep 2012 11:49:31 -0500
>>> Alan Cox<alc@rice.edu>   wrote:
>>>
>>>>> On 09/10/2012 04:18, Andrew Turner wrote:
>>>>>> On Sat, 08 Sep 2012 19:01:26 -0500
>>>>>> Alan Cox<alc@rice.edu>    wrote:
>>>>>>
>>>>>>> Can someone here please test this patch to the new armv6 pmap?
>>>>>>> It eliminates the use of the page queues lock and updates some
>>>>>>> comments. Similar changes were recently made to the original arm
>>>>>>> pmap.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Alan
>>>>>>>
>>>>>> I have booted FreeBSD with the patch on a Pandaboard and it
>>>>>> appears to work. Are there any tests you would like me to run?
>>>>>>
>>>>> Nothing in particular, since almost anything that you do on the
>>>>> machine will exercise the changed code.
>>>>>
>>>>> There appears to be a lot of unnecessary dropping and reacquiring
>>>>> of locks around UMA calls in both pmap.c and pmap-v6.c.  I will
>>>>> try to generate a patch to eliminate this later in the week.
>>>>>
>>>>> Alan
>>>>>
>>> Hi Alan and ARM hackers,
>>>
>>> Don't know exact, but think it is related to current pmap work.
>>> So here is two panics observed on R-Pi recently (HEAD @r240985),
>>> both on attempt to untar ports.tar.gz :)
>>>
>> *snip*
>>
>> The attached patch should eliminate the panic.  Please let me know
>> when you've had a chance to test it.
> I don't know when it done (extracting ports.tar.gz :) ) but it still
> works w/o panic :)
>
> Thanks a lot Alan!
>
>

Great.  I've committed the change.

Can someone here please test the attached arm v6 pmap patch?  I'd like 
to verify that my earlier efforts to eliminate the lock order reversals 
allows for eliminating all of the unlocking and relocking in 
pmap_alloc_l2_bucket().  If there's a problem, it should be apparent 
immediately.

Alan


--------------000404080808070608070201
Content-Type: text/plain;
 name="armv6-pmap8.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="armv6-pmap8.patch"

Index: arm/arm/pmap-v6.c
===================================================================
--- arm/arm/pmap-v6.c	(revision 241063)
+++ arm/arm/pmap-v6.c	(working copy)
@@ -598,36 +598,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)];
@@ -642,17 +626,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
@@ -668,17 +642,13 @@ 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;
 	}
 
 	return (l2b);
 }
 
-static PMAP_INLINE void
-pmap_free_l2_ptp(pt_entry_t *l2)
-{
-	uma_zfree(l2zone, l2);
-}
 /*
  * One or more mappings in the specified L2 descriptor table have just been
  * invalidated.
@@ -744,7 +714,7 @@ pmap_free_l2_bucket(pmap_t pm, struct l2_bucket *l
 	/*
 	 * Release the L2 descriptor table back to the pool cache.
 	 */
-	pmap_free_l2_ptp(ptep);
+	uma_zfree(l2zone, ptep);
 
 	/*
 	 * Update the reference count in the associated l2_dtable

--------------000404080808070608070201--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?506882FB.9060507>