On Wed, Feb 16, 2005 at 08:22:30AM +0900, YAMAMOTO Takashi wrote:
> hi,
> 
> > +	/*
> > +	 * None of the obvious reasons why we might not be able to do the loan
> > +	 * are true.  If we need to COW the amap, try to do it now.
> > +	 */
> 
> don't you need to check (entry->protection & VM_PROT_WRITE) ?

indeed.  I was sure that there needed to be more checks at the top of that,
there didn't seem to be quite enough yet.  :-)


> > +	aref = entry->aref;
> 
> why copy aref?

I don't remember, that's one of the few bits I didn't rewrite recently.
but yea, it would be better to use the entry's directly.


> > +		simple_lock(&uobj->vmobjlock);
> > +		uvm_lock_pageq();
> > +		for (i = 0; i < npages; i++) {
> > +			uvm_pageactivate(pgs[i]);
> > +		}
> > +		uvm_page_unbusy(pgs, npages);
> > +		uvm_unlock_pageq();
> > +		simple_unlock(&uobj->vmobjlock);
> 
> (although somewhat offtopic,) how do you think about PR/28372 ?

yea, that's a whole 'nuther discussion.
I'll send a separate note about that.


> > +		pg->loan_count++;
> > +		uvm_pageactivate(pg);
> > +		anon = amap_lookup(&aref, aoff + (i << PAGE_SHIFT));
> > +		if (anon) {
> > +			oanons[oanon++] = anon;
> > +			amap_unadd(&aref, aoff + (i << PAGE_SHIFT));
> > +		}
> > +		if (pg->uanon) {
> > +			anon = pg->uanon;
> > +			simple_lock(&anon->an_lock);
> > +			anon->an_ref++;
> 
> i think you shouldn't increment loan_count if the page is
> already ->A loaned.

right again, I'll fix that too.

thanks,
-Chuck