In article <20010124115307.S2291@dr-evil.shagadelic.org>,
Jason R Thorpe <thorpej@zembu.com> wrote:
>So I was looking through UVM today, and noticed that pmap_is_referenced()
>is no longer used -- at all.  This appears to have happened with UBC,
>to some extent.
>It looks like, in general, the pagedaemon is a lot less efficient that it
>could be (and, in fact, *used* to be).


yes, i think you are right that it could use improvement.
the one issue that may be a bit sticky is the inactive queue.
the old VM was not consistent about pages on the inactive queue.
most parts of the code assumed that pages on the inactive queue
did not have any pmap mappings but a few parts allowed them to
leak in.

with UVM i went with the assumption that inactive pages do not
have mappings and thus you can modify, clean, free, or generally
mess with them without having to do pmap_page_protect() first.


>In my mind, this is how it SHOULD work:
>	for (pg on inactive list) {
>		if (free target is met)
>			break;
>		if (pg is referenced) {
>			/* DO NOT CLEAR REFERENCE! */
>			move page to active list;
>		}

given the assumptions in UVM (that inactive pages have no mappings)
the above if statement should never fire.   that is prob. why chucks
deleted this statement in UBC.


>		pmap_page_protect(pg, VM_PROT_NONE);
>		if (pg is modified)
>			start a clean of the page;
>		else if (page is clean)
>			free page;
>	}

>	for (pg on active list) {
>		if (inactive target is met) {
>			/*
>			 * HAVE TO RETHINK THIS IF UBC CACHE PAGES ARE
>			 * PLACED ONTO THE INACTIVE LIST (causing a glut
>			 * of inactive pages).
>			 */
>			break;
>		}
>		if (pg is referenced)
>			clear reference;

hmmm.   this part was supposed to be in the UVM code, but it doesn't
seem to be there.   that's an error, sorry about that!

>		else {
>			/* NOTE: PAGE IS NOT PROTECTED IN ANY WAY! */
>			move page to inactive list;

i think you should be careful about not protecting it.   by placing
a page that has possible active mappings on the inactive queue you are
violating one of the assumptions that i wrote the rest of the code with
and it could cause trouble later....


>		}
>	}


chuck

-- 
Chuck Cranor                            http://www.research.att.com/info/chuck
Senior Technical Staff Member		chuck@research.att.com
Internet and Networking Systems Research Lab
AT&T Labs-Research, Florham Park, NJ