shithub: gefs

Download patch

ref: d8f3576e5dc6793b98314f202cfba0c65bfbee12
parent: 93b53d81984032ba86acff73c220c59c40fe9ffe
author: Ori Bernstein <ori@eigenstate.org>
date: Sun Oct 3 23:38:42 EDT 2021 

tree: refcount blocks correctly


--- a/blk.c
+++ b/blk.c
@@ -14,7 +14,7 @@
 };
 
 static vlong	blkalloc_lk(Arena*);
-static int	blkdealloc(vlong);
+static int	blkdealloc_lk(vlong);
 static void	cachedel(vlong);
 static Blk	*cacheblk(Blk*);
 static Blk	*lookupblk(vlong);
@@ -44,6 +44,7 @@
 	b->bp.addr = bp;
 	b->bp.hash = -1;
 	b->bp.gen = -1;
+	b->ref = 0;	/* caller must increment */
 	b->cnext = nil;
 	b->cprev = nil;
 	b->hnext = nil;
@@ -263,6 +264,7 @@
 Nextblk:
 	if((b = readblk(bp, 0)) == nil)
 		return -1;
+	cacheblk(b);
 	p = b->data;
 	bh = GBIT64(p + 0);
 	/* the hash covers the log and offset */
@@ -438,8 +440,12 @@
 					break;
 				}
 			}
-			if(blkdealloc(bp) == -1)
+			lock(a);
+			if(blkdealloc_lk(bp) == -1){
+				unlock(a);
 				return -1;
+			}
+			unlock(a);
 		}
 	}
 	finalize(a->logtl);
@@ -484,7 +490,7 @@
 }
 
 static int
-blkdealloc(vlong b)
+blkdealloc_lk(vlong b)
 {
 	Arena *a;
 	int r;
@@ -491,7 +497,6 @@
 
 	r = -1;
 	a = getarena(b);
-	lock(a);
 	cachedel(b);
 	if(freerange(a->free, b, Blksz) == -1)
 		goto out;
@@ -499,7 +504,6 @@
 		goto out;
 	r = 0;
 out:
-	unlock(a);
 	return r;
 }
 
@@ -559,7 +563,7 @@
 	b->bp.addr = bp;
 	b->bp.hash = -1;
 	b->bp.gen = fs->nextgen;
-	b->ref = 1;
+	b->ref = 0;	/* cacheblk incremnets */
 	b->data = b->buf + Hdrsz;
 	return cacheblk(b);
 }
@@ -578,8 +582,6 @@
 	for(b = bkt->b; b != nil; b = b->hnext)
 		if(b->bp.addr == off)
 			break;
-	if(b != nil)
-		pinblk(b);
 	unlock(bkt);
 	return b;
 }
@@ -592,19 +594,19 @@
 	u32int h;
 
 	/* FIXME: better hash. */
+	refblk(b);
 	assert(b->bp.addr != 0);
+	assert(!(b->flag & Bzombie));
 	h = ihash(b->bp.addr);
-	ainc(&b->ref);
 	bkt = &fs->cache[h % fs->cmax];
 	lock(bkt);
 	for(e = bkt->b; e != nil; e = e->hnext){
 		if(b == e)
-			goto found;
+			goto Found;
 		assert(b->bp.addr != e->bp.addr);
 	}
 	bkt->b = b;
-found:
-	ainc(&b->ref);
+Found:
 	unlock(bkt);
 
 	lock(&fs->lrulk);
@@ -626,10 +628,10 @@
 	b->cnext = fs->chead;
 	b->cprev = nil;
 	fs->chead = b;
-	if((b->flag&Bcache) == 0){
-		b->flag |= Bcache;
+	if((b->flag&Bcached) == 0){
+		b->flag |= Bcached;
 		fs->ccount++;
-		ainc(&b->ref);
+		refblk(b);
 	}
 	c=0;
 	USED(c);
@@ -794,7 +796,7 @@
 }
 
 Blk*
-pinblk(Blk *b)
+refblk(Blk *b)
 {
 	ainc(&b->ref);
 	return b;
@@ -820,9 +822,13 @@
 {
 	if(b == nil)
 		return;
+	assert(b->ref > 0);
+	if(b->flag & Bzombie)
+		fprint(2, "reaping zombie: %B @ %ld\n", b->bp, b->ref);
 	if(adec(&b->ref) == 0){
 		assert((b->flag & Bqueued) || !(b->flag & Bdirty));
 		cachedel(b->bp.addr);
+		assert(lookupblk(b->bp.addr) == nil);
 		free(b);
 	}
 }
@@ -832,13 +838,22 @@
 {
 	Arena *a;
 
-	assert(b->ref == 1 && b->flag & (Bdirty|Bqueued) == Bdirty);
+	/* we can have patterns like:
+	 *   b = getblk();
+         *   use(b);
+         *   freeblk(b);
+         *   unref(b);
+         */
+	if(b->ref != 1 && ((b->flag & Bcached) && b->ref != 2)){
+		fprint(2, "warning: dangling refs: %B @ %ld\n", b->bp, b->ref);
+		b->flag |= Bzombie;
+	}
+	assert((b->flag & Bqueued) == 0);
 	a = getarena(b->bp.addr);
 	lock(a);
 	logop(a, b->bp.addr, LogFree);
-	blkdealloc(b->bp.addr);
+	blkdealloc_lk(b->bp.addr);
 	unlock(a);
-	free(b);
 }
 
 int
--- a/check.c
+++ b/check.c
@@ -72,6 +72,7 @@
 			}
 			if(badblk(c, h - 1, &x, &y))
 				fail++;
+			putblk(c);
 		}
 		r = keycmp(&x, &y);
 		switch(r){
@@ -188,11 +189,14 @@
 void
 fshowfs(int fd, char *m)
 {
+	Blk *b;
 	int h;
 
 	fprint(fd, "=== %s\n", m);
 	fprint(fd, "\tht: %d\n", fs->root.ht);
-	rshowblk(fd, getroot(&fs->root, &h), 0, 1);
+	b = getroot(&fs->root, &h);
+	rshowblk(fd, b, 0, 1);
+	putblk(b);
 }
 
 void
--- a/dat.h
+++ b/dat.h
@@ -64,7 +64,6 @@
 	 * ptr:  off[8] hash[8] -- a key for an Dir block.
 	 * dir:  fixed statbuf header, user ids
 	 */
-	Kref,	/* off[8] => ptr[16]:		pointer to refcount page */
 	Kdat,	/* qid[8] off[8] => ptr[16]:	pointer to data page */
 	Kent,	/* pqid[8] name[n] => dir[n]:	serialized Dir */
 	Ksnap,	/* name[n] => dent[16] ptr[16]:	snapshot root */
@@ -75,7 +74,8 @@
 	Bdirty	= 1 << 0,
 	Bqueued	= 1 << 1,
 	Bfinal	= 1 << 2,
-	Bcache	= 1 << 3,
+	Bcached	= 1 << 3,
+	Bzombie	= 1 << 4,
 };
 
 #define Efs	"i will not buy this fs, it is scratched"
--- a/fns.h
+++ b/fns.h
@@ -13,7 +13,7 @@
 Blk*	shadow(Blk*, Path*, Path*);
 Blk*	getroot(Tree*, int*);
 Blk*	getblk(Bptr, int);
-Blk*	pinblk(Blk*);
+Blk*	refblk(Blk*);
 Blk*	readblk(vlong, int);
 Arena*	getarena(vlong);
 void	putblk(Blk*);
--- a/fs.c
+++ b/fs.c
@@ -49,11 +49,13 @@
 		b = getblk(f->root.bp, 0);
 	if(b == nil)
 		return Efs;
+
 	if(lk)
 		rlock(f->dent);
 	e = btlookupat(b, k, kv, bp);
 	if(lk)
 		runlock(f->dent);
+	putblk(b);
 	return e;
 }
 
@@ -552,6 +554,14 @@
 		rerror(m, "no such fid");
 		return;
 	}
+
+	lock(f);
+	if(f->scan != nil){
+		btdone(f->scan);
+		f->scan = nil;
+	}
+	unlock(f);
+
 	clunkfid(f);
 	r.type = Rclunk;
 	respond(m, &r);
--- a/ream.c
+++ b/ream.c
@@ -128,6 +128,7 @@
 	s->type = Tsuper;
 	s->bp.addr = sz;
 	s->data = s->buf + Hdrsz;
+	s->ref = 1;
 	fillsuper(s);
 	finalize(s);
 	syncblk(s);
--- a/tree.c
+++ b/tree.c
@@ -461,6 +461,7 @@
 		}
 		b->nbuf = j;
 	}
+	freeblk(b);
 	p->n = n;
 	return 0;
 }
@@ -552,6 +553,7 @@
 			setmsg(r, j, &m);
 		}
 	}
+	freeblk(b);
 	p->split = 1;
 	p->l = l;
 	p->r = r;
@@ -640,6 +642,8 @@
 			setmsg(d, o++, &m);
 		}
 	}
+	freeblk(a);
+	freeblk(b);
 //showfs("postmerge");
 	enqueue(d);
 	p->merge = 1;
@@ -756,6 +760,8 @@
 			setmsg(d, o++, &m);
 		}
 	}
+	freeblk(a);
+	freeblk(b);
 	enqueue(l);
 	enqueue(r);
 	p->merge = 1;
@@ -806,7 +812,7 @@
 	if(p->idx == -1)
 		return 0;
 	if(pp != nil){
-		if((m = pp->n) == nil)
+		if((m = refblk(pp->n)) == nil)
 			return 0;
 	}else{
 		if((m = getblk(km.bp, 0)) == nil)
@@ -838,6 +844,7 @@
 done:
 	ret = 0;
 out:
+	putblk(m);
 	putblk(l);
 	putblk(r);
 	if(pp == nil)
@@ -982,18 +989,12 @@
 	Path *p;
 
 	for(p = path; p != path + npath; p++){
-		if(p->b != nil)
-			putblk(p->b);
-		if(p->n != nil)
-			putblk(p->n);
-		if(p->l != nil)
-			putblk(p->l);
-		if(p->r != nil)
-			putblk(p->r);
-		if(p->nl != nil)
-			putblk(p->nl);
-		if(p->nr != nil)
-			putblk(p->nr);
+		putblk(p->b);
+		putblk(p->n);
+		putblk(p->l);
+		putblk(p->r);
+		putblk(p->nl);
+		putblk(p->nr);
 	}
 }
 
@@ -1143,7 +1144,7 @@
 		*h = t->ht;
 	unlock(&t->lk);
 
-	return getblk(t->bp, 0);
+	return getblk(bp, 0);
 }
 
 static char*
@@ -1182,12 +1183,13 @@
 char*
 btlookupat(Blk *b0, Key *k, Kvp *r, Blk **bp)
 {
-	int idx, same, done;
+	int idx, same, done, r0;
 	char *ret;
-	Blk *b;
+	Blk *b, *c;
 
 	*bp = nil;
-	b = pinblk(b0);
+	r0 = b0->ref;
+	b = refblk(b0);
 	assert(k != r);
 	while(b->type == Tpivot){
 		ret = collect(b, k, r, &done);
@@ -1194,18 +1196,24 @@
 		if(done)
 			return ret;
 		idx = blksearch(b, k, r, nil);
-		if(idx == -1)
+		if(idx == -1){
+			assert(b0->ref == r0 + (b == b0) ? 1 : 0);
+			putblk(b);
 			return Eexist;
-		putblk(b);
-		if((b = getblk(r->bp, 0)) == nil)
+		}
+		if((c = getblk(r->bp, 0)) == nil)
 			return Efs;
+		putblk(b);
+		b = c;
 	}
 	assert(b->type == Tleaf);
 	blksearch(b, k, r, &same);
 	if(!same){
+		assert(b0->ref == r0 + (b == b0) ? 1 : 0);
 		putblk(b);
 		return Eexist;
 	}
+	assert(b0->ref == r0 + (b == b0) ? 1 : 0);
 	*bp = b;
 	return nil;
 }
@@ -1287,6 +1295,7 @@
 	case Oinsert:
 		s->present = 1;
 		kv2dir(m, d);
+		fprint(2, "name: %s\n", d->name);
 		break;
 	case Odelete:
 		s->present = 0;
@@ -1349,8 +1358,10 @@
 		}
 		start = i;
 		p[i-1].vi++;
+		putblk(p[i].b);
 		p[i].vi = 0;
 		p[i].bi = 0;
+		p[i].b = nil;
 	}
 	for(i = start; i < h; i++){
 		getval(p[i-1].b, p[i-1].vi, &kv);
@@ -1402,8 +1413,7 @@
 	int i;
 
 	for(i = 0; i < s->root.ht; i++)
-		if(s->path[i].b != nil)
-			putblk(s->path[i].b);
+		putblk(s->path[i].b);
 	free(s->path);
 }