xfnw/plan9fox
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

kernel: cachedel() lock order, lookpage, cleanup

Browse source 
the lock order of page.Lock -> palloc.hashlock was
violated in cachedel() which is called from the
pager. change the code to do it in the right oder
to prevent deadlock.

change lookpage to retry on false hit. i assume that
a false hit means:

a) we'r low on memory -> cached page got uncached/reused

b) duppage() got called on the page, meaning theres another
cached copy in the image now.

paging in is expensive compared to the hashtable lookup, so
i think retrying is better.

cleanup fixfault, adding comments.
This commit is contained in:
cinap_lenrek 2012-10-16 14:12:21 +02:00
parent 1335be8276
commit 45b99937be
2 changed files with 29 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
sys/src/9/port/fault.c
View file

@ -140,15 +140,20 @@ fixfault(Segment *s, ulong addr, int read, int doputmmu)
lkp = *pg;
lock(lkp);
if(lkp->ref < 1)
panic("fault: lkp->ref %d < 1", lkp->ref);
ref = lkp->ref;
if(ref == 0)
panic("fault %#p ref == 0", lkp);
if(lkp->image == &swapimage)
ref = lkp->ref + swapcount(lkp->daddr);
else
ref = lkp->ref;
if(ref == 1 && lkp->image){
/* save a copy of the original for the image cache */
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);

29
sys/src/9/port/page.c
View file

@ -335,7 +335,10 @@ retry:
* 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).
* 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 */
@ -412,23 +415,24 @@ cachepage(Page *p, Image *i)
void
cachedel(Image *i, ulong daddr)
{
Page *f, **l;
Page *f;
retry:
lock(&palloc.hashlock);
l = &pghash(daddr);
for(f = *l; f; f = f->hash) {
for(f = pghash(daddr); f; f = f->hash) {
if(f->image == i && f->daddr == daddr) {
unlock(&palloc.hashlock);
lock(f);
if(f->image == i && f->daddr == daddr){
*l = f->hash;
putimage(f->image);
f->image = 0;
f->daddr = 0;
if(f->image != i || f->daddr != daddr) {
unlock(f);
goto retry;
}
uncachepage(f);
unlock(f);
break;
return;
}
l = &f->hash;
}
unlock(&palloc.hashlock);
}
@ -438,6 +442,7 @@ lookpage(Image *i, ulong daddr)
{
Page *f;
retry:
lock(&palloc.hashlock);
for(f = pghash(daddr); f; f = f->hash) {
if(f->image == i && f->daddr == daddr) {
@ -448,7 +453,7 @@ lookpage(Image *i, ulong daddr)
if(f->image != i || f->daddr != daddr) {
unlock(f);
unlock(&palloc);
return 0;
goto retry;
}
if(++f->ref == 1)
pageunchain(f);