Subject: pv_pmap removal
To: None <port-i386@netbsd.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: port-i386
Date: 11/02/2003 17:20:20
--NextPart-20031102171355-0029600
Content-Type: Text/Plain; charset=us-ascii

hi,

attached patch shrinks pv_entry by eliminating pv_pmap member.
while this increases cost to get pmap pointer a little,
i think size of pv_entry is more important.

any comments?

YAMAMOTO Takashi

--NextPart-20031102171355-0029600
Content-Type: Text/Plain; charset=us-ascii
Content-Disposition: attachment; filename="i386.pv_pmap.diff"

Index: include/pmap.h
===================================================================
--- include/pmap.h	(revision 374)
+++ include/pmap.h	(working copy)
@@ -239,6 +239,7 @@ LIST_HEAD(pmap_head, pmap); /* struct pm
  */
 
 struct pmap {
+	/* pm_object should be a first member.  see pve_{get,set}pmap. */
 	struct uvm_object pm_obj;	/* object (lck by object lock) */
 #define	pm_lock	pm_obj.vmobjlock
 	LIST_ENTRY(pmap) pm_list;	/* list (lck by pm_list lock) */
@@ -269,7 +270,6 @@ struct pmap {
 
 struct pv_entry {			/* locked by its list's pvh_lock */
 	SPLAY_ENTRY(pv_entry) pv_node;	/* splay-tree node */
-	struct pmap *pv_pmap;		/* the pmap */
 	vaddr_t pv_va;			/* the virtual address */
 	struct vm_page *pv_ptp;		/* the vm_page of the PTP */
 };
Index: i386/pmap.c
===================================================================
--- i386/pmap.c	(revision 385)
+++ i386/pmap.c	(working copy)
@@ -312,6 +312,9 @@ void	pmap_tlb_shootdown_job_put(struct p
 __cpu_simple_lock_t pmap_tlb_shootdown_job_lock;
 union pmap_tlb_shootdown_job_al *pj_page, *pj_free;
 
+__inline static struct pmap *pve_getpmap(const struct pv_entry *);
+__inline static void pve_setpmap(struct pv_entry *, struct pmap *);
+
 /*
  * global data structures
  */
@@ -388,12 +391,43 @@ static vaddr_t pv_cachedva;		/* cached V
 #define PVE_HIWAT (PVE_LOWAT + (PVE_PER_PVPAGE * 2))
 					/* high water mark */
 
+__inline static struct pmap *
+pve_getpmap(const struct pv_entry *pve)
+{
+	struct pmap *pmap;
+	const struct vm_page *ptp = pve->pv_ptp;
+
+	/* assuming that pm_obj is at top of struct pmap */
+
+	if (ptp) {
+		pmap = (struct pmap *)ptp->uobject;
+	} else {
+		pmap = pmap_kernel();
+	}
+
+	return pmap;
+}
+
+__inline static void
+pve_setpmap(struct pv_entry *pve,
+    struct pmap *pmap)
+{
+
+	/* nothing to do here */
+
+	KDASSERT((pve->pv_ptp == NULL && pmap == pmap_kernel()) ||
+	    (pmap == (struct pmap *)pve->pv_ptp->uobject));
+}
+
 static __inline int
 pv_compare(struct pv_entry *a, struct pv_entry *b)
 {
-	if (a->pv_pmap < b->pv_pmap)
+	const struct pmap *apmap = pve_getpmap(a);
+	const struct pmap *bpmap = pve_getpmap(b);
+
+	if (apmap < bpmap)
 		return (-1);
-	else if (a->pv_pmap > b->pv_pmap)
+	else if (apmap > bpmap)
 		return (1);
 	else if (a->pv_va < b->pv_va)
 		return (-1);
@@ -481,7 +515,7 @@ static void		 pmap_free_pvs(struct pmap 
 static void		 pmap_free_pv_doit(struct pv_entry *);
 static void		 pmap_free_pvpage(void);
 static struct vm_page	*pmap_get_ptp(struct pmap *, int);
-static boolean_t	 pmap_is_curpmap(struct pmap *);
+static boolean_t	 pmap_is_curpmap(const struct pmap *);
 static boolean_t	 pmap_is_active(struct pmap *, int);
 static pt_entry_t	*pmap_map_ptes(struct pmap *);
 static struct pv_entry	*pmap_remove_pv(struct pv_head *, struct pmap *,
@@ -512,7 +546,7 @@ static void		 pmap_unmap_ptes(struct pma
 
 __inline static boolean_t
 pmap_is_curpmap(pmap)
-	struct pmap *pmap;
+	const struct pmap *pmap;
 {
 	return((pmap == pmap_kernel()) ||
 	       (pmap->pm_pdirpa == (paddr_t) rcr3()));
@@ -588,13 +622,15 @@ __inline static pt_entry_t *
 pmap_tmpmap_pvepte(pve)
 	struct pv_entry *pve;
 {
+	const struct pmap *pmap = pve_getpmap(pve);
+
 #ifdef DIAGNOSTIC
-	if (pve->pv_pmap == pmap_kernel())
+	if (pmap == pmap_kernel())
 		panic("pmap_tmpmap_pvepte: attempt to map kernel");
 #endif
 
 	/* is it current pmap?  use direct mapping... */
-	if (pmap_is_curpmap(pve->pv_pmap))
+	if (pmap_is_curpmap(pmap))
 		return(vtopte(pve->pv_va));
 
 	return(((pt_entry_t *)pmap_tmpmap_pa(VM_PAGE_TO_PHYS(pve->pv_ptp)))
@@ -610,7 +646,7 @@ pmap_tmpunmap_pvepte(pve)
 	struct pv_entry *pve;
 {
 	/* was it current pmap?   if so, return */
-	if (pmap_is_curpmap(pve->pv_pmap))
+	if (pmap_is_curpmap(pve_getpmap(pve)))
 		return;
 
 	pmap_tmpunmap_pa();
@@ -1520,9 +1556,13 @@ pmap_enter_pv(pvh, pve, pmap, va, ptp)
 	vaddr_t va;
 	struct vm_page *ptp;	/* PTP in pmap that maps this VA */
 {
-	pve->pv_pmap = pmap;
+
+	KDASSERT((pmap == pmap_kernel() && ptp == NULL) ||
+	    &pmap->pm_obj == ptp->uobject);
+
 	pve->pv_va = va;
 	pve->pv_ptp = ptp;			/* NULL for kernel pmap */
+	pve_setpmap(pve, pmap);
 	simple_lock(&pvh->pvh_lock);		/* lock pv_head */
 	SPLAY_INSERT(pvtree, &pvh->pvh_root, pve); /* add to locked list */
 	simple_unlock(&pvh->pvh_lock);		/* unlock, done! */
@@ -1544,14 +1584,22 @@ pmap_remove_pv(pvh, pmap, va)
 	struct pmap *pmap;
 	vaddr_t va;
 {
-	struct pv_entry tmp, *pve;
+	struct pv_entry tmppve, *pve;
+	struct vm_page tmppage;
 
-	tmp.pv_pmap = pmap;
-	tmp.pv_va = va;
-	pve = SPLAY_FIND(pvtree, &pvh->pvh_root, &tmp);
+	tmppve.pv_va = va;
+	if (pmap == pmap_kernel()) {
+		tmppve.pv_ptp = NULL;
+	} else {
+		tmppage.uobject = &pmap->pm_obj;
+		tmppve.pv_ptp = &tmppage;
+	}
+	pve_setpmap(&tmppve, pmap);
+	pve = SPLAY_FIND(pvtree, &pvh->pvh_root, &tmppve);
 	if (pve == NULL)
 		return (NULL);
 	SPLAY_REMOVE(pvtree, &pvh->pvh_root, pve);
+
 	return(pve);				/* return removed pve */
 }
 
@@ -2594,18 +2642,21 @@ pmap_page_remove(pg)
 	simple_lock(&pvh->pvh_lock);
 
 	for (pve = SPLAY_MIN(pvtree, &pvh->pvh_root); pve != NULL; pve = npve) {
+		struct pmap *pmap;
+
 		npve = SPLAY_NEXT(pvtree, &pvh->pvh_root, pve);
-		ptes = pmap_map_ptes(pve->pv_pmap);		/* locks pmap */
+		pmap = pve_getpmap(pve);
+		ptes = pmap_map_ptes(pmap);		/* locks pmap */
 
 #ifdef DIAGNOSTIC
-		if (pve->pv_ptp && (pve->pv_pmap->pm_pdir[pdei(pve->pv_va)] &
+		if (pve->pv_ptp && (pmap->pm_pdir[pdei(pve->pv_va)] &
 				    PG_FRAME)
 		    != VM_PAGE_TO_PHYS(pve->pv_ptp)) {
 			printf("pmap_page_remove: pg=%p: va=%lx, pv_ptp=%p\n",
 			       pg, pve->pv_va, pve->pv_ptp);
 			printf("pmap_page_remove: PTP's phys addr: "
 			       "actual=%x, recorded=%lx\n",
-			       (pve->pv_pmap->pm_pdir[pdei(pve->pv_va)] &
+			       (pmap->pm_pdir[pdei(pve->pv_va)] &
 				PG_FRAME), VM_PAGE_TO_PHYS(pve->pv_ptp));
 			panic("pmap_page_remove: mapped managed page has "
 			      "invalid pv_ptp field");
@@ -2616,10 +2667,10 @@ pmap_page_remove(pg)
 		opte = x86_atomic_testset_ul(&ptes[x86_btop(pve->pv_va)], 0);
 
 		if (opte & PG_W)
-			pve->pv_pmap->pm_stats.wired_count--;
-		pve->pv_pmap->pm_stats.resident_count--;
+			pmap->pm_stats.wired_count--;
+		pmap->pm_stats.resident_count--;
 
-		pmap_tlb_shootdown(pve->pv_pmap, pve->pv_va, opte, &cpumask);
+		pmap_tlb_shootdown(pmap, pve->pv_va, opte, &cpumask);
 
 		/* sync R/M bits */
 		pg->mdpage.mp_attrs |= (opte & (PG_U|PG_M));
@@ -2651,7 +2702,7 @@ pmap_page_remove(pg)
 #endif /* DEBUG */
 				/* zap! */
 				opte = x86_atomic_testset_ul(
-				    &pve->pv_pmap->pm_pdir[pdei(pve->pv_va)],
+				    &pmap->pm_pdir[pdei(pve->pv_va)],
 				    0);
 				pmap_tlb_shootdown(curpcb->pcb_pmap,
 				    ((vaddr_t)ptes) + pve->pv_ptp->offset,
@@ -2661,21 +2712,21 @@ pmap_page_remove(pg)
 				 * Always shoot down the other pmap's
 				 * self-mapping of the PTP.
 				 */
-				pmap_tlb_shootdown(pve->pv_pmap,
+				pmap_tlb_shootdown(pmap,
 				    ((vaddr_t)PTE_BASE) + pve->pv_ptp->offset,
 				    opte, &cpumask);
 #endif
-				pve->pv_pmap->pm_stats.resident_count--;
+				pmap->pm_stats.resident_count--;
 				/* update hint? */
-				if (pve->pv_pmap->pm_ptphint == pve->pv_ptp)
-					pve->pv_pmap->pm_ptphint =
-					    pve->pv_pmap->pm_obj.memq.tqh_first;
+				if (pmap->pm_ptphint == pve->pv_ptp)
+					pmap->pm_ptphint =
+					    pmap->pm_obj.memq.tqh_first;
 				pve->pv_ptp->wire_count = 0;
 				pve->pv_ptp->flags |= PG_ZERO;
 				uvm_pagefree(pve->pv_ptp);
 			}
 		}
-		pmap_unmap_ptes(pve->pv_pmap);		/* unlocks pmap */
+		pmap_unmap_ptes(pmap);		/* unlocks pmap */
 		SPLAY_REMOVE(pvtree, &pvh->pvh_root, pve); /* remove it */
 		SPLAY_RIGHT(pve, pv_node) = killlist;	/* mark it for death */
 		killlist = pve;
@@ -2742,9 +2793,11 @@ pmap_test_attrs(pg, testbits)
 	for (pve = SPLAY_MIN(pvtree, &pvh->pvh_root);
 	     pve != NULL && (*myattrs & testbits) == 0;
 	     pve = SPLAY_NEXT(pvtree, &pvh->pvh_root, pve)) {
-		ptes = pmap_map_ptes(pve->pv_pmap);
+		struct pmap *pmap = pve_getpmap(pve);
+
+		ptes = pmap_map_ptes(pmap);
 		pte = ptes[x86_btop(pve->pv_va)];
-		pmap_unmap_ptes(pve->pv_pmap);
+		pmap_unmap_ptes(pmap);
 		*myattrs |= pte;
 	}
 
@@ -2797,22 +2850,22 @@ pmap_clear_attrs(pg, clearbits)
 	*myattrs &= ~clearbits;
 
 	SPLAY_FOREACH(pve, pvtree, &pvh->pvh_root) {
+		struct pmap *pmap = pve_getpmap(pve);
 #ifdef DIAGNOSTIC
-		if (!pmap_valid_entry(pve->pv_pmap->pm_pdir[pdei(pve->pv_va)]))
+		if (!pmap_valid_entry(pmap->pm_pdir[pdei(pve->pv_va)]))
 			panic("pmap_change_attrs: mapping without PTP "
 			      "detected");
 #endif
 
-		ptes = pmap_map_ptes(pve->pv_pmap);	/* locks pmap */
+		ptes = pmap_map_ptes(pmap);	/* locks pmap */
 		opte = ptes[x86_btop(pve->pv_va)];
 		if (opte & clearbits) {
 			result |= (opte & clearbits);
 			x86_atomic_clearbits_l(&ptes[x86_btop(pve->pv_va)],
 			    (opte & clearbits));	/* zap! */
-			pmap_tlb_shootdown(pve->pv_pmap, pve->pv_va, opte,
-			    &cpumask);
+			pmap_tlb_shootdown(pmap, pve->pv_va, opte, &cpumask);
 		}
-		pmap_unmap_ptes(pve->pv_pmap);		/* unlocks pmap */
+		pmap_unmap_ptes(pmap);		/* unlocks pmap */
 	}
 
 	simple_unlock(&pvh->pvh_lock);

--NextPart-20031102171355-0029600--