Date: Tue, 25 Sep 2012 10:50:02 -0500 From: Alan Cox <alc@rice.edu> To: "arm@freebsd.org" <arm@freebsd.org>, John-Mark Gurney <jmg@funkthat.com> Cc: Alan Cox <alc@rice.edu> Subject: Re: arm pmap locking Message-ID: <5061D2AA.3000100@rice.edu> In-Reply-To: <20120917033308.GB58312@funkthat.com> References: <1345315508.27688.260.camel@revolution.hippie.lan> <503D12AE.1050705@rice.edu> <1346350374.1140.525.camel@revolution.hippie.lan> <5045351F.6060201@rice.edu> <1346723041.1140.602.camel@revolution.hippie.lan> <504B85BE.3030101@rice.edu> <1347316458.1137.41.camel@revolution.hippie.lan> <504F8BAC.4040902@rice.edu> <20120915045040.GZ58312@funkthat.com> <5054D69B.40209@rice.edu> <20120917033308.GB58312@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------060804050202020708080306
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
On 09/16/2012 22:33, John-Mark Gurney wrote:
> Alan Cox wrote this message on Sat, Sep 15, 2012 at 14:27 -0500:
>> On 09/14/2012 23:50, John-Mark Gurney wrote:
>>> Alan Cox wrote this message on Tue, Sep 11, 2012 at 14:06 -0500:
>>>> On 09/10/2012 17:34, Ian Lepore wrote:
>>>>> On Sat, 2012-09-08 at 12:51 -0500, Alan Cox wrote:
>>>>>> Here is another patch. This simplifies the kernel pmap locking in
>>>>>> pmap_enter_pv() and corrects some comments.
>>>>>>
>>>>>> Thanks in advance,
>>>>>> Alan
>>>>>>
>>>>> I'm afraid I'm not going to be able to do this any time soon. I bricked
>>>>> my DreamPlug last Friday (the bright side: the nandfs_newfs command in
>>>>> -current apparently works just fine when applied to real hardware rather
>>>>> than the simulator device). I haven't had any success in getting
>>>>> openocd to transfer a new uboot image to it. So I'm probably going to
>>>>> be without arm hardware that can run anything newer than 8.2 for a few
>>>>> weeks (until my new atmel eval board arrives).
>>>>>
>>>> Thanks for letting me know.
>>>>
>>>> Could someone else here please test the attached patch?
>>> I figure since you've been looking at arm's pmap, that I should report
>>> to you a LOR that I observered recently on my armv6 board:
>>> lock order reversal:
>>> 1st 0xc1cf70b0 pmap (pmap) @ /usr/src.HEAD/sys/arm/arm/pmap-v6.c:673
>>> 2nd 0xc091e608 PV ENTRY (UMA zone) @ /usr/src.HEAD/sys/vm/uma_core.c:2084
>>> KDB: stack backtrace:
>>> db_trace_self() at db_trace_self+0xc
>>> scp=0xc05a294c rlv=0xc025b298 (X_db_sym_numargs+0x1bc)
>>> rsp=0xc9d1a8fc rfp=0xc9d1aa18
>>> X_db_sym_numargs() at X_db_sym_numargs+0x194
>>> scp=0xc025b270 rlv=0xc0398340 (kdb_backtrace+0x3c)
>>> rsp=0xc9d1aa1c rfp=0xc9d1aa2c
>>> r4=0xc06bda44
>>> kdb_backtrace() at kdb_backtrace+0xc
>>> scp=0xc0398310 rlv=0xc03ad3f8 (witness_display_spinlock+0x80)
>>> rsp=0xc9d1aa30 rfp=0xc9d1aa44
>>> r4=0x00000001
>>> witness_display_spinlock() at witness_display_spinlock+0x5c
>>> scp=0xc03ad3d4 rlv=0xc03ae6d8 (witness_checkorder+0x884)
>>> rsp=0xc9d1aa48 rfp=0xc9d1aa98
>>> r5=0xc1cf70b0 r4=0xc06263d4
>>> witness_checkorder() at witness_checkorder+0xc
>>> scp=0xc03ade60 rlv=0xc0355b9c (_mtx_lock_flags+0xcc)
>>> rsp=0xc9d1aa9c rfp=0xc9d1aabc
>>> r10=0xc1cf70b0 r9=0x00000000
>>> r8=0xc091d6e0 r7=0xc0620730 r6=0x00000824 r5=0x00000000
>>> r4=0xc091e608
>>> _mtx_lock_flags() at _mtx_lock_flags+0xc
>>> scp=0xc0355adc rlv=0xc057d290 (uma_zalloc_arg+0x1a8)
>>> rsp=0xc9d1aac0 rfp=0xc9d1aafc
>>> r7=0xc091d748 r6=0x00000000
>>> r5=0xc08fc0b8 r4=0xc0620730
>>> uma_zalloc_arg() at uma_zalloc_arg+0xc
>>> scp=0xc057d0f4 rlv=0xc05abb04 (pmap_growkernel+0xf20)
>>> rsp=0xc9d1ab00 rfp=0xc9d1ab70
>>> r10=0xc1cf70b0 r9=0x00000000
>>> r8=0x00000000 r7=0x8122e032 r6=0x00000000 r5=0xc08fc0b8
>>> r4=0x00000001
>>> pmap_growkernel() at pmap_growkernel+0x3fc
>>> scp=0xc05aafe0 rlv=0xc05ac14c (pmap_enter+0x70)
>>> rsp=0xc9d1ab74 rfp=0xc9d1aba0
>>> r10=0x00000007 r9=0x00000000
>>> r8=0xc09885a8 r7=0xbffff000 r6=0xc08278c0 r5=0xc1cf70b0
>>> r4=0xc06261e0
>>> pmap_enter() at pmap_enter+0xc
>>> scp=0xc05ac0e8 rlv=0xc057ff44 (vm_fault_hold+0x1638)
>>> rsp=0xc9d1aba4 rfp=0xc9d1ad14
>>> r10=0xc9d1ade4 r8=0x00000000
>>> r7=0x00000002 r6=0x00000000 r5=0xc1cf7000 r4=0xc09885a8
>>> vm_fault_hold() at vm_fault_hold+0xc
>>> scp=0xc057e918 rlv=0xc058054c (vm_fault+0x8c)
>>> rsp=0xc9d1ad18 rfp=0xc9d1ad3c
>>> r10=0xc9d1ade4 r9=0x00000002
>>> r8=0x00000000 r7=0x00000002 r6=0xbffff000 r5=0xc1cf7000
>>> r4=0xc1cf5000
>>> vm_fault() at vm_fault+0xc
>>> scp=0xc05804cc rlv=0xc05b0ac8 (data_abort_handler+0x35c)
>>> rsp=0xc9d1ad40 rfp=0xc9d1ade0
>>> r8=0xbffff000 r7=0xc1cf5000
>>> r6=0x00000000 r5=0xc0626844 r4=0xc1cf3088
>>> data_abort_handler() at data_abort_handler+0xc
>>> scp=0xc05b0778 rlv=0xc05a4150 (address_exception_entry+0x50)
>>> rsp=0xc9d1ade4 rfp=0xc9d1ae84
>>> r10=0xc0682bde r9=0xc1cf3000
>>> r8=0xc9d1aeac r7=0xc0682bde r6=0xc0682bd4 r5=0xc05f1648
>>> r4=0xbfffffff
>>> exec_shell_imgact() at exec_shell_imgact+0x75c
>>> scp=0xc031d348 rlv=0xc033b68c (fork_exit+0x94)
>>> rsp=0xc9d1ae88 rfp=0xc9d1aea8
>>> r10=0x00000000 r9=0x00000000
>>> r8=0xc9d1aeac r7=0x00000000 r6=0xc031d33c r5=0xc1cf3000
>>> r4=0xc1cf5000
>>> fork_exit() at fork_exit+0xc
>>> scp=0xc033b604 rlv=0xc05afe44 (fork_trampoline+0x14)
>>> rsp=0xc9d1aeac rfp=0x00000000
>>> r8=0x00000000 r7=0x8ffbf8ef
>>> r6=0xf5f7f4a9 r5=0x00000000 r4=0xc031d33c
>>>
>>> FreeBSD beaglebone 10.0-CURRENT FreeBSD 10.0-CURRENT #1 r240480: Thu Nov
>>> 3 14:57:01 PDT 2011
>>> jmg@pcbsd-779:/usr/obj/arm.armv6/usr/src.HEAD/sys/helium arm
>>>
>>> Do you need any more information?
>> I'm not sure what to make of this stack trace. In particular,
>> pmap_enter() does not directly call pmap_growkernel(). In fact, the
>> only caller to pmap_growkernel() in the entire kernel is
>> vm_map_insert(), which doesn't appear in this stack trace. Moreover,
>> this appears to be a legitimate page fault within the kernel address
>> space. (The image activator touches pageable kernel memory.) However,
>> such page faults should never need to grow the kernel page table. The
>> necessary page table pages should have been allocated during
>> initialization when the various kernel map submaps were created. The
>> bottom line is that I don't have an immediate answer. I'm going to need
>> to think about this. In general, I'm a bit uncomfortable with the way
>> that the l2 and l2 table zones are created.
> 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.
Alan
--------------060804050202020708080306
Content-Type: text/plain;
name="arm_pmap20.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="arm_pmap20.patch"
Index: arm/arm/pmap-v6.c
===================================================================
--- arm/arm/pmap-v6.c (revision 240913)
+++ arm/arm/pmap-v6.c (working copy)
@@ -148,9 +148,11 @@ __FBSDID("$FreeBSD$");
#include <sys/systm.h>
#include <sys/kernel.h>
#include <sys/ktr.h>
+#include <sys/lock.h>
#include <sys/proc.h>
#include <sys/malloc.h>
#include <sys/msgbuf.h>
+#include <sys/mutex.h>
#include <sys/vmmeter.h>
#include <sys/mman.h>
#include <sys/rwlock.h>
@@ -167,8 +169,7 @@ __FBSDID("$FreeBSD$");
#include <vm/vm_page.h>
#include <vm/vm_pageout.h>
#include <vm/vm_extern.h>
-#include <sys/lock.h>
-#include <sys/mutex.h>
+
#include <machine/md_var.h>
#include <machine/cpu.h>
#include <machine/cpufunc.h>
@@ -202,6 +203,7 @@ static pv_entry_t pmap_get_pv_entry(void);
static void pmap_enter_locked(pmap_t, vm_offset_t, vm_page_t,
vm_prot_t, boolean_t, int);
+static vm_paddr_t pmap_extract_locked(pmap_t pmap, vm_offset_t va);
static void pmap_alloc_l1(pmap_t);
static void pmap_free_l1(pmap_t);
@@ -1659,7 +1661,7 @@ pmap_bootstrap(vm_offset_t firstaddr, vm_offset_t
/*
* Initialize the global pv list lock.
*/
- rw_init_flags(&pvh_global_lock, "pmap pv global", RW_RECURSE);
+ rw_init(&pvh_global_lock, "pmap pv global");
/*
* Reserve some special page table entries/VA space for temporary
@@ -2100,6 +2102,13 @@ pmap_kenter_user(vm_offset_t va, vm_paddr_t pa)
pmap_fault_fixup(pmap_kernel(), va, VM_PROT_READ|VM_PROT_WRITE, 1);
}
+vm_paddr_t
+pmap_kextract(vm_offset_t va)
+{
+
+ return (pmap_extract_locked(kernel_pmap, va));
+}
+
/*
* remove a page from the kernel pagetables
*/
@@ -2850,22 +2859,34 @@ pmap_copy(pmap_t dst_pmap, pmap_t src_pmap, vm_off
* with the given map/virtual_address pair.
*/
vm_paddr_t
-pmap_extract(pmap_t pm, vm_offset_t va)
+pmap_extract(pmap_t pmap, vm_offset_t va)
{
+ vm_paddr_t pa;
+
+ PMAP_LOCK(pmap);
+ pa = pmap_extract_locked(pmap, va);
+ PMAP_UNLOCK(pmap);
+ return (pa);
+}
+
+static vm_paddr_t
+pmap_extract_locked(pmap_t pmap, vm_offset_t va)
+{
struct l2_dtable *l2;
pd_entry_t l1pd;
pt_entry_t *ptep, pte;
vm_paddr_t pa;
u_int l1idx;
+
+ if (pmap != kernel_pmap)
+ PMAP_ASSERT_LOCKED(pmap);
l1idx = L1_IDX(va);
-
- PMAP_LOCK(pm);
- l1pd = pm->pm_l1->l1_kva[l1idx];
+ l1pd = pmap->pm_l1->l1_kva[l1idx];
if (l1pte_section_p(l1pd)) {
/*
- * These should only happen for pmap_kernel()
+ * These should only happen for the kernel pmap.
*/
- KASSERT(pm == pmap_kernel(), ("huh"));
+ KASSERT(pmap == kernel_pmap, ("huh"));
/* XXX: what to do about the bits > 32 ? */
if (l1pd & L1_S_SUPERSEC)
pa = (l1pd & L1_SUP_FRAME) | (va & L1_SUP_OFFSET);
@@ -2877,34 +2898,22 @@ vm_paddr_t
* descriptor as an indication that a mapping exists.
* We have to look it up in the L2 dtable.
*/
- l2 = pm->pm_l2[L2_IDX(l1idx)];
-
+ l2 = pmap->pm_l2[L2_IDX(l1idx)];
if (l2 == NULL ||
- (ptep = l2->l2_bucket[L2_BUCKET(l1idx)].l2b_kva) == NULL) {
- PMAP_UNLOCK(pm);
+ (ptep = l2->l2_bucket[L2_BUCKET(l1idx)].l2b_kva) == NULL)
return (0);
- }
-
- ptep = &ptep[l2pte_index(va)];
- pte = *ptep;
-
- if (pte == 0) {
- PMAP_UNLOCK(pm);
+ pte = ptep[l2pte_index(va)];
+ if (pte == 0)
return (0);
- }
-
switch (pte & L2_TYPE_MASK) {
case L2_TYPE_L:
pa = (pte & L2_L_FRAME) | (va & L2_L_OFFSET);
break;
-
default:
pa = (pte & L2_S_FRAME) | (va & L2_S_OFFSET);
break;
}
}
-
- PMAP_UNLOCK(pm);
return (pa);
}
Index: arm/arm/pmap.c
===================================================================
--- arm/arm/pmap.c (revision 240913)
+++ arm/arm/pmap.c (working copy)
@@ -145,9 +145,11 @@ __FBSDID("$FreeBSD$");
#include <sys/systm.h>
#include <sys/kernel.h>
#include <sys/ktr.h>
+#include <sys/lock.h>
#include <sys/proc.h>
#include <sys/malloc.h>
#include <sys/msgbuf.h>
+#include <sys/mutex.h>
#include <sys/vmmeter.h>
#include <sys/mman.h>
#include <sys/rwlock.h>
@@ -164,8 +166,7 @@ __FBSDID("$FreeBSD$");
#include <vm/vm_page.h>
#include <vm/vm_pageout.h>
#include <vm/vm_extern.h>
-#include <sys/lock.h>
-#include <sys/mutex.h>
+
#include <machine/md_var.h>
#include <machine/cpu.h>
#include <machine/cpufunc.h>
@@ -197,6 +198,7 @@ static pv_entry_t pmap_get_pv_entry(void);
static void pmap_enter_locked(pmap_t, vm_offset_t, vm_page_t,
vm_prot_t, boolean_t, int);
+static vm_paddr_t pmap_extract_locked(pmap_t pmap, vm_offset_t va);
static void pmap_fix_cache(struct vm_page *, pmap_t, vm_offset_t);
static void pmap_alloc_l1(pmap_t);
static void pmap_free_l1(pmap_t);
@@ -2840,6 +2842,13 @@ pmap_kenter_user(vm_offset_t va, vm_paddr_t pa)
pmap_fault_fixup(pmap_kernel(), va, VM_PROT_READ|VM_PROT_WRITE, 1);
}
+vm_paddr_t
+pmap_kextract(vm_offset_t va)
+{
+
+ return (pmap_extract_locked(kernel_pmap, va));
+}
+
/*
* remove a page from the kernel pagetables
*/
@@ -3644,22 +3653,34 @@ pmap_copy(pmap_t dst_pmap, pmap_t src_pmap, vm_off
* with the given map/virtual_address pair.
*/
vm_paddr_t
-pmap_extract(pmap_t pm, vm_offset_t va)
+pmap_extract(pmap_t pmap, vm_offset_t va)
{
+ vm_paddr_t pa;
+
+ PMAP_LOCK(pmap);
+ pa = pmap_extract_locked(pmap, va);
+ PMAP_UNLOCK(pmap);
+ return (pa);
+}
+
+static vm_paddr_t
+pmap_extract_locked(pmap_t pmap, vm_offset_t va)
+{
struct l2_dtable *l2;
pd_entry_t l1pd;
pt_entry_t *ptep, pte;
vm_paddr_t pa;
u_int l1idx;
+
+ if (pmap != kernel_pmap)
+ PMAP_ASSERT_LOCKED(pmap);
l1idx = L1_IDX(va);
-
- PMAP_LOCK(pm);
- l1pd = pm->pm_l1->l1_kva[l1idx];
+ l1pd = pmap->pm_l1->l1_kva[l1idx];
if (l1pte_section_p(l1pd)) {
/*
- * These should only happen for pmap_kernel()
+ * These should only happen for the kernel pmap.
*/
- KASSERT(pm == pmap_kernel(), ("huh"));
+ KASSERT(pmap == kernel_pmap, ("huh"));
/* XXX: what to do about the bits > 32 ? */
if (l1pd & L1_S_SUPERSEC)
pa = (l1pd & L1_SUP_FRAME) | (va & L1_SUP_OFFSET);
@@ -3671,34 +3692,22 @@ vm_paddr_t
* descriptor as an indication that a mapping exists.
* We have to look it up in the L2 dtable.
*/
- l2 = pm->pm_l2[L2_IDX(l1idx)];
-
+ l2 = pmap->pm_l2[L2_IDX(l1idx)];
if (l2 == NULL ||
- (ptep = l2->l2_bucket[L2_BUCKET(l1idx)].l2b_kva) == NULL) {
- PMAP_UNLOCK(pm);
+ (ptep = l2->l2_bucket[L2_BUCKET(l1idx)].l2b_kva) == NULL)
return (0);
- }
-
- ptep = &ptep[l2pte_index(va)];
- pte = *ptep;
-
- if (pte == 0) {
- PMAP_UNLOCK(pm);
+ pte = ptep[l2pte_index(va)];
+ if (pte == 0)
return (0);
- }
-
switch (pte & L2_TYPE_MASK) {
case L2_TYPE_L:
pa = (pte & L2_L_FRAME) | (va & L2_L_OFFSET);
break;
-
default:
pa = (pte & L2_S_FRAME) | (va & L2_S_OFFSET);
break;
}
}
-
- PMAP_UNLOCK(pm);
return (pa);
}
Index: arm/include/pmap.h
===================================================================
--- arm/include/pmap.h (revision 240803)
+++ arm/include/pmap.h (working copy)
@@ -92,8 +92,7 @@ enum mem_type {
#ifdef _KERNEL
-#define vtophys(va) pmap_extract(pmap_kernel(), (vm_offset_t)(va))
-#define pmap_kextract(va) pmap_extract(pmap_kernel(), (vm_offset_t)(va))
+#define vtophys(va) pmap_kextract((vm_offset_t)(va))
#endif
@@ -228,6 +227,7 @@ void pmap_kenter(vm_offset_t va, vm_paddr_t pa);
void pmap_kenter_nocache(vm_offset_t va, vm_paddr_t pa);
void *pmap_kenter_temp(vm_paddr_t pa, int i);
void pmap_kenter_user(vm_offset_t va, vm_paddr_t pa);
+vm_paddr_t pmap_kextract(vm_offset_t va);
void pmap_kremove(vm_offset_t);
void *pmap_mapdev(vm_offset_t, vm_size_t);
void pmap_unmapdev(vm_offset_t, vm_size_t);
--------------060804050202020708080306--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5061D2AA.3000100>