shithub: riscv

Info  •  Files  •  Log  •  Branches

kernel: getting rid of duppage() (thanks charles)

simplifying paging code by getting rid of duppage(). instead,
fixfault() now always makes a copy of the shared/cached page
and leaves the cache alone. newpage() uncaches pages as
neccesary.

thanks charles forsyth for the suggestion.

from http://9fans.net/archive/2014/03/26:

> It isn't needed at all. When a cached page is written, it's trying hard to
> replace the page in the cache by a new copy,
> to return the previously cached page. Instead, I copy the cached page and
> return the copy, which is what it already
> does in another instance. ...

--- a/sys/src/9/port/fault.c

+++ b/sys/src/9/port/fault.c

@@ -80,7 +80,6 @@

 fixfault(Segment *s, uintptr addr, int read, int doputmmu)

 {

 	int type;

-	int ref;

 	Pte **p, *etp;

 	uintptr soff, mmuphys=0;

 	Page **pg, *lkp, *new;

@@ -121,7 +120,6 @@

 			new = newpage(1, &s, addr);

 			if(s == 0)

 				return -1;

-

 			*pg = new;

 		}

 		goto common;

@@ -143,24 +141,15 @@

 		lkp = *pg;

 		lock(lkp);

-		ref = lkp->ref;

-		if(ref == 0)

+		if(lkp->ref == 0)

 			panic("fault %#p ref == 0", lkp);

-		if(lkp->image == &swapimage)

-			ref += swapcount(lkp->daddr);

-		if(ref == 1 && lkp->image) {

-			/*

-			 * save a copy of the original for the image cache

-			 * and uncache the page. page might temporarily be

-			 * unlocked while trying to acquire palloc lock so

-			 * recheck ref in case it got grabbed.

-			 */

-			duppage(lkp);

-

-			ref = lkp->ref;

-		}

-		unlock(lkp);

-		if(ref > 1){

+		if(lkp->ref == 1 && lkp->image == nil) {

+			unlock(lkp);

+		} else if(lkp->image == &swapimage && (lkp->ref + swapcount(lkp->daddr)) == 1) {

+			uncachepage(lkp);

+			unlock(lkp);

+		} else {

+			unlock(lkp);

 			new = newpage(0, &s, addr);

 			if(s == 0)

 				return -1;

--- a/sys/src/9/port/page.c

+++ b/sys/src/9/port/page.c

@@ -265,83 +265,6 @@

 }

 void

-duppage(Page *p)				/* Always call with p locked */

-{

-	Page *np;

-	int color;

-

-	/*

-	 *  normal lock ordering is to call

-	 *  lock(&palloc) before lock(p).

-	 *  To avoid deadlock, we have to drop

-	 *  our locks and try again. as the page

-	 *  is from the image cache, this might

-	 *  let someone else come in and grab it

-	 *  so we check page ref below.

-	 */

-	if(!canlock(&palloc)){

-		unlock(p);

-		lock(&palloc);

-		lock(p);

-	}

-

-	/* don't dup pages that are shared or have no image */

-	if(p->ref != 1 || p->image == nil || p->image->notext){

-		unlock(&palloc);

-		return;

-	}

-

-	/* No freelist cache when memory is very low */

-	if(palloc.freecount < swapalloc.highwater) {

-		unlock(&palloc);

-		uncachepage(p);

-		return;

-	}

-

-	color = getpgcolor(p->va);

-	for(np = palloc.head; np; np = np->next)

-		if(np->color == color)

-			break;

-

-	/* No page of the correct color */

-	if(np == 0) {

-		unlock(&palloc);

-		uncachepage(p);

-		return;

-	}

-

-	pageunchain(np);

-	pagechaintail(np);

-/*

-* XXX - here's a bug? - np is on the freelist but it's not really free.

-* when we unlock palloc someone else can come in, decide to

-* use np, and then try to lock it.  they succeed after we've 

-* run copypage and cachepage and unlock(np).  then what?

-* they call pageunchain before locking(np), so it's removed

-* from the freelist, but still in the cache because of

-* cachepage below.  if someone else looks in the cache

-* before they remove it, the page will have a nonzero ref

-* once they finally lock(np). This does not happen because

-* newpage, auxpage, duppage and lookpage all lock(&palloc)

-* so while they hold it nobody is going to grab anything

-* from the cache.

-*/

-	lock(np);

-	if(np->ref != 0)	/* should never happen */

-		panic("duppage: np->ref %d != 0", np->ref);

-	unlock(&palloc);

-

-	/* Cache the new version */

-	uncachepage(np);

-	np->va = p->va;

-	np->daddr = p->daddr;

-	copypage(p, np);

-	cachepage(np, p->image);

-	unlock(np);

-	uncachepage(p);

-}

-

-void

 copypage(Page *f, Page *t)

 {

 	KMap *ks, *kd;

--- a/sys/src/9/port/portfns.h

+++ b/sys/src/9/port/portfns.h

@@ -82,7 +82,6 @@

 void		dumpregs(Ureg*);

 void		dumpstack(void);

 Fgrp*		dupfgrp(Fgrp*);

-void		duppage(Page*);

 void		dupswap(Page*);

 void		edfinit(Proc*);

 char*		edfadmit(Proc*);

--- a/sys/src/9/port/swap.c

+++ b/sys/src/9/port/swap.c

@@ -49,7 +49,7 @@

 	swapimage.notext = 1;

 }

-ulong

+uintptr

 newswap(void)

 {

 	uchar *look;