The following diff has two pieces, one that switches all VP allocations
for userland to be NOWAIT, the other is to add all the the necessary mutexes
to protect the pmap data structures.

Having the pool_get calls for userland vp and pted_desc entries is necessary
because allowing the pool code to wait can end up with a recursive mutex
deadlock.

If the userland entry is marked with CANFAIL the uvm code will retry the
request.

The case of pm == pmap_kernel() is invalid here, the kernel map should
NEVER allocate pages from the pool, all kernel VP mappings will be created
either on boot or perhaps in the future in pmap_growkernel()

I now notice that some of these mutex occur at the same location as splXX()
splx() calls. It may make sense to combine these in the future, but has
not been done at this time.

diff --git a/sys/arch/arm64/arm64/pmap.c b/sys/arch/arm64/arm64/pmap.c
index f28439c0943..fab895043f3 100644
--- a/sys/arch/arm64/arm64/pmap.c
+++ b/sys/arch/arm64/arm64/pmap.c
@@ -357,12 +357,7 @@ pmap_vp_enter(pmap_t pm, vaddr_t va, struct pte_desc 
*pted, int flags)
        struct pmapvp2 *vp2;
        struct pmapvp3 *vp3;
 
-       int vp_pool_flags;
-       if (pm == pmap_kernel()) {
-               vp_pool_flags  = PR_NOWAIT;
-       } else {
-               vp_pool_flags  = PR_WAITOK |PR_ZERO;
-       }
+       int vp_pool_flags = PR_NOWAIT |PR_ZERO;
 
        if (pm->have_4_level_pt) {
                vp1 = pm->pm_vp.l0->vp[VP_IDX0(va)];
@@ -465,14 +460,21 @@ pmap_enter_pv(struct pte_desc *pted, struct vm_page *pg)
                return;
        }
 
+       mtx_enter(&pg->mdpage.pv_mtx);
        LIST_INSERT_HEAD(&(pg->mdpage.pv_list), pted, pted_pv_list);
        pted->pted_va |= PTED_VA_MANAGED_M;
+       mtx_leave(&pg->mdpage.pv_mtx);
 }
 
 void
 pmap_remove_pv(struct pte_desc *pted)
 {
+       struct vm_page *pg;
+       pg = PHYS_TO_VM_PAGE(pted->pted_pte & PTE_RPGN);
+
+       mtx_enter(&pg->mdpage.pv_mtx);
        LIST_REMOVE(pted, pted_pv_list);
+       mtx_leave(&pg->mdpage.pv_mtx);
 }
 
 volatile int supportuserland;
@@ -490,6 +492,8 @@ pmap_enter(pmap_t pm, vaddr_t va, paddr_t pa, vm_prot_t 
prot, int flags)
        /* MP - Acquire lock for this pmap */
 
        s = splvm();
+       mtx_enter(&pm->pm_mtx);
+
        pted = pmap_vp_lookup(pm, va, NULL);
        if (pted && PTED_VALID(pted)) {
                pmap_remove_pted(pm, pted);
@@ -571,6 +575,7 @@ pmap_enter(pmap_t pm, vaddr_t va, paddr_t pa, vm_prot_t 
prot, int flags)
 
        error = 0;
 out:
+       mtx_leave(&pm->pm_mtx);
        splx(s);
        /* MP - free pmap lock */
        return error;
@@ -626,11 +631,12 @@ pmap_remove_pted(pmap_t pm, struct pte_desc *pted)
                pted->pted_va &= ~PTED_VA_EXEC_M;
        }
 
-       pted->pted_pte = 0;
-
        if (PTED_MANAGED(pted))
                pmap_remove_pv(pted);
 
+       pted->pted_pte = 0;
+       pted->pted_va = 0;
+
        if (pm != pmap_kernel())
                pool_put(&pmap_pted_pool, pted);
        splx(s);
@@ -724,14 +730,22 @@ pmap_kremove_pg(vaddr_t va)
        //if (!cold) printf("%s: %x\n", __func__, va);
 
        pm = pmap_kernel();
+       s = splvm();
+       mtx_enter(&pm->pm_mtx);
+
        pted = pmap_vp_lookup(pm, va, NULL);
-       if (pted == NULL)
+       if (pted == NULL) {
+               mtx_leave(&pm->pm_mtx);
+               splx(s);
                return;
+       }
 
-       if (!PTED_VALID(pted))
-               return; /* not mapped */
-
-       s = splvm();
+       if (!PTED_VALID(pted)) {
+               /* not mapped */
+               mtx_leave(&pm->pm_mtx);
+               splx(s);
+               return;
+       }
 
        pm->pm_stats.resident_count--;
 
@@ -757,6 +771,7 @@ pmap_kremove_pg(vaddr_t va)
        pted->pted_pte = 0;
        pted->pted_va = 0;
 
+       mtx_leave(&pm->pm_mtx);
        splx(s);
 }
 
@@ -895,6 +910,7 @@ pmap_pinit(pmap_t pm)
 
        //pmap_allocate_asid(pm); // by default global (allocate asid later!?!)
        pm->pm_asid = -1;
+       mtx_init(&pm->pm_mtx, IPL_VM);
 
        pmap_extract(pmap_kernel(), l0va, (paddr_t *)&pm->pm_pt0pa);
 
@@ -1511,21 +1527,37 @@ pmap_page_protect(struct vm_page *pg, vm_prot_t prot)
        /* need to lock for this pv */
        s = splvm();
 
-       if (prot == PROT_NONE) {
-               while (!LIST_EMPTY(&(pg->mdpage.pv_list))) {
-                       pted = LIST_FIRST(&(pg->mdpage.pv_list));
-                       pmap_remove_pted(pted->pted_pmap, pted);
+       if (prot != PROT_NONE) {
+               LIST_FOREACH(pted, &(pg->mdpage.pv_list), pted_pv_list) {
+                       pmap_page_ro(pted->pted_pmap, pted->pted_va, prot);
                }
-               /* page is being reclaimed, sync icache next use */
-               atomic_clearbits_int(&pg->pg_flags, PG_PMAP_EXE);
                splx(s);
                return;
        }
+       mtx_enter(&pg->mdpage.pv_mtx);
+       while (!LIST_EMPTY(&(pg->mdpage.pv_list))) {
+               pted = LIST_FIRST(&(pg->mdpage.pv_list));
+               pmap_t pm = pted->pted_pmap;
+               mtx_leave(&pg->mdpage.pv_mtx);
+               mtx_enter(&pm->pm_mtx);
+               if (pted != LIST_FIRST(&(pg->mdpage.pv_list))) {
+                       // race lost, retry
+                       mtx_leave(&pm->pm_mtx);
+                       mtx_enter(&pg->mdpage.pv_mtx);
+                       continue;
+               }
 
-       LIST_FOREACH(pted, &(pg->mdpage.pv_list), pted_pv_list) {
-               pmap_page_ro(pted->pted_pmap, pted->pted_va, prot);
+               pmap_remove_pted(pted->pted_pmap, pted);
+               mtx_leave(&pm->pm_mtx);
+               mtx_enter(&pg->mdpage.pv_mtx);
        }
+
+       /* page is being reclaimed, sync icache next use - XXX cull? */
+       atomic_clearbits_int(&pg->pg_flags, PG_PMAP_EXE);
+
+       mtx_leave(&pg->mdpage.pv_mtx);
        splx(s);
+       return;
 }
 
 
@@ -1910,6 +1942,7 @@ int pmap_clear_modify(struct vm_page *pg)
        int s;
 
        s = splvm();
+       mtx_enter(&pg->mdpage.pv_mtx);
 
        pg->pg_flags &= ~PG_PMAP_MOD;
 
@@ -1921,6 +1954,7 @@ int pmap_clear_modify(struct vm_page *pg)
 
                ttlb_flush(pted->pted_pmap, pted->pted_va & ~PAGE_MASK);
        }
+       mtx_leave(&pg->mdpage.pv_mtx);
        splx(s);
 
        return 0;
@@ -1940,6 +1974,7 @@ int pmap_clear_reference(struct vm_page *pg)
        int s;
 
        s = splvm();
+       mtx_enter(&pg->mdpage.pv_mtx);
 
        pg->pg_flags &= ~PG_PMAP_REF;
 
@@ -1948,6 +1983,7 @@ int pmap_clear_reference(struct vm_page *pg)
                pmap_pte_insert(pted);
                ttlb_flush(pted->pted_pmap, pted->pted_va & ~PAGE_MASK);
        }
+       mtx_leave(&pg->mdpage.pv_mtx);
        splx(s);
 
        return 0;
diff --git a/sys/arch/arm64/include/pmap.h b/sys/arch/arm64/include/pmap.h
index 4f0044cd65a..1825316a217 100644
--- a/sys/arch/arm64/include/pmap.h
+++ b/sys/arch/arm64/include/pmap.h
@@ -73,6 +73,7 @@ struct pmap {
        int pm_asid;
        int pm_active;
        int pm_refs;                            /* ref count */
+       struct mutex pm_mtx;
        struct pmap_statistics  pm_stats;       /* pmap statistics */
 };
 
@@ -125,12 +126,14 @@ extern vaddr_t    pmap_curmaxkvaddr;
 #define __HAVE_VM_PAGE_MD
 struct vm_page_md {
        LIST_HEAD(,pte_desc) pv_list;
+       struct mutex pv_mtx;
        int pvh_attrs;                          /* page attributes */
 };
 
-#define VM_MDPAGE_INIT(pg) do {                        \
-        LIST_INIT(&((pg)->mdpage.pv_list));     \
-       (pg)->mdpage.pvh_attrs = 0;             \
+#define VM_MDPAGE_INIT(pg) do {                                \
+        LIST_INIT(&((pg)->mdpage.pv_list));            \
+       mtx_init(&(pg)->mdpage.pv_mtx, IPL_VM);         \
+       (pg)->mdpage.pvh_attrs = 0;                     \
 } while (0)
 
 #endif /* _LOCORE */

Dale Rahn                               dr...@dalerahn.com

Reply via email to