From mboxrd@z Thu Jan  1 00:00:00 1970
Message-ID: <122a1b862278abf61a07a806168993ac@felloff.net>
Date: Sun,  2 Mar 2014 04:55:22 +0100
From: cinap_lenrek@felloff.net
To: 9fans@9fans.net
In-Reply-To: <4578ec4fd74cba90ed1d96bfb4fa6e25@mikro.quanstro.net>
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
Subject: Re: [9fans] imagereclaim()
Topicbox-Message-UUID: c0ab0d14-ead8-11e9-9d60-3106f5b1d025

checked nix/port/page.c. your duppage() is wrong.

	/* don't dup pages with no image */
	if(p->ref == 0 || p->image == nil || p->image->notext)
		return 0;

	/*
	 *  normal lock ordering is to call
	 *  lock(&pga) before lock(p).
	 *  To avoid deadlock, we have to drop
	 *  our locks and try again.
	 */
	if(!canlock(&pga)){
		unlock(p);
		if(up)
			sched();
		lock(p);
		goto retry;
	}

you need to check p->ref != 1 instead of p->ref == 0. the page
passed to duppage() is still cached. after you unlock(p), someone
can come in and take a reference to the page from the image
cache (lookpage()), making p->ref > 1 once you get the lock back.

put an assert or print in there after the if(!canlock(&pga){} block
to check p->ref.

when this happens, the caller to duppage() (fixfault) must not
modify the page or use it in his procs segment (on copy on write)
but make a copy for itself because the other processes that
grabbed the reference is (commited to) reading it (outside of the
page lock of course, so you already lost when p->ref != 1).

--
cinap