shithub: gefs

Download patch

ref: 6d81e5b3fab334a63318d4ed88eccc89971ddeea
parent: 14d1179c21b1c93e696ac7ff23dc65b109ca9e3f
author: Ori Bernstein <[email protected]>
date: Wed Oct 11 11:20:25 EDT 2023 

blk: avoid removing blocks from cache prematurely

the Bfreed flag could be set on a block before the
block was freed; when putting things back into the
cache and resurrecting them, this could lead to a
stale block being left, because lrubot would lead
cache entry becoming unreachable.


--- a/blk.c
+++ b/blk.c
@@ -939,7 +939,7 @@
 }
 
 void
-freeblk(Tree *t, Bptr bp)
+freeblk(Tree *t, Blk *b, Bptr bp)
 {
 	Bfree *f;
 	ulong ge;
@@ -952,6 +952,10 @@
 	if((f = malloc(sizeof(Bfree))) == nil)
 		return;
 	f->bp = bp;
+	if(b != nil)
+		f->b = holdblk(b);
+	else
+		f->b = nil;
 
 	ge = agetl(&fs->epoch);
 	f->next = fs->limbo[ge];
@@ -1002,6 +1006,10 @@
 		qe.b = nil;
 		qe.qgen = agetv(&fs->qgen);
 		qput(a->sync, qe);
+		if(p->b != nil){
+			setflag(p->b, Bfreed);
+			dropblk(p->b);
+		}
 		free(p);
 		p = n;
 	}
@@ -1134,8 +1142,7 @@
 				rwakeupall(&fs->syncrz);
 			qunlock(&fs->synclk);
 		}else{
-			if(checkflag(qe.b, Bfreed))
-				continue;
+			if(checkflag(qe.b, Bfreed) == 0)
 			if(syncblk(qe.b) == -1){
 				ainc(&fs->broken);
 				fprint(2, "write: %r\n");
--- a/cache.c
+++ b/cache.c
@@ -67,8 +67,6 @@
 		fs->ctail->cnext = b;
 	if(fs->chead == nil)
 		fs->chead = b;
-	b->bp.addr = -1;
-	b->bp.hash = -1;
 	b->cprev = fs->ctail;
 	fs->ctail = b;
 	rwakeup(&fs->lrurz);
@@ -84,9 +82,11 @@
 	assert(b->magic == Magic);
 	h = ihash(b->bp.addr);
 	bkt = &fs->bcache[h % fs->cmax];
+	qlock(&fs->lrulk);
 	lock(bkt);
 	if(checkflag(b, Bcached)){
 		unlock(bkt);
+		qunlock(&fs->lrulk);
 		return;
 	}
 	assert(b->hnext == nil);
@@ -97,10 +97,11 @@
 	b->hnext = bkt->b;
 	bkt->b = b;
 	unlock(bkt);
+	qunlock(&fs->lrulk);
 }
 
 void
-cachedel(vlong addr)
+cachedel_lk(vlong addr)
 {
 	Bucket *bkt;
 	Blk *b, **p;
@@ -125,25 +126,30 @@
 	}
 	unlock(bkt);
 }
+void
+cachedel(vlong addr)
+{
+	qlock(&fs->lrulk);
+	cachedel_lk(addr);
+	qunlock(&fs->lrulk);
+}
 
 Blk*
-cacheget(vlong off)
+cacheget(vlong addr)
 {
 	Bucket *bkt;
 	u32int h;
 	Blk *b;
 
-	h = ihash(off);
-
+	h = ihash(addr);
 	bkt = &fs->bcache[h % fs->cmax];
-
 	qlock(&fs->lrulk);
 	lock(bkt);
 	for(b = bkt->b; b != nil; b = b->hnext){
-		if(b->bp.addr == off){
- 			holdblk(b);
+		if(b->bp.addr == addr){
+			holdblk(b);
 			lrudel(b);
-			b->lasthold = getcallerpc(&off);
+			b->lasthold = getcallerpc(&addr);
 			break;
 		}
 	}
@@ -168,9 +174,10 @@
 	b = fs->ctail;
 	assert(b->magic == Magic);
 	assert(b->ref == 0);
-	cachedel(b->bp.addr);
+	if(checkflag(b, Bcached))
+		cachedel_lk(b->bp.addr);
+	if(checkflag(b, Bcached)) print("%B cached %#p freed %#p\n", b->bp, b->cached, b->freed);
 	lrudel(b);
-	if(checkflag(b, Bcached)) print("cached %B freed %#p\n", b->bp, b->freed);
 	assert(!checkflag(b, Bcached));
 	b->flag = 0;
 	b->lasthold = 0;
--- a/fns.h
+++ b/fns.h
@@ -53,7 +53,7 @@
 void	epochend(int);
 void	epochclean(void);
 void	freesync(void);
-void	freeblk(Tree*, Bptr);
+void	freeblk(Tree*, Blk*, Bptr);
 int	dlappend(Dlist *dl, Bptr);
 int	killblk(Tree*, Bptr);
 int	blkdealloc(vlong);
--- a/fs.c
+++ b/fs.c
@@ -376,7 +376,7 @@
 			memcpy(b->buf, t->buf, Blksz);
 			dropblk(t);
 		}
-		freeblk(f->mnt->root, bp);
+		freeblk(f->mnt->root, nil, bp);
 	}else if(e != Esrch){
 		werrstr("%s", e);
 		return -1;
@@ -1994,7 +1994,7 @@
 		n = writeb(f, &kv[i], &bp[i], p, o, c, f->dent->length);
 		if(n == -1){
 			for(j = 0; j < i; j++)
-				freeblk(f->mnt->root, bp[i]);
+				freeblk(f->mnt->root, nil, bp[i]);
 			wunlock(f->dent);
 			fprint(2, "%r");
 			putfid(f);
--- a/snap.c
+++ b/snap.c
@@ -115,7 +115,7 @@
 	}
 Found:
 	h = ihash(gen) ^ ihash(bgen);
-	p = &fs->dlcache[h % nelem(fs->dlcache)];
+	p = &fs->dlcache[h % fs->dlcmax];
 	dl->chain = *p;
 	*p = dl;
 	return dl;
@@ -161,8 +161,7 @@
 			}
 		}
 		bp = b->deadp;
-		freeblk(&fs->snap, b->bp);
-		setflag(b, Bfreed);
+		freeblk(&fs->snap, b, b->bp);
 		dropblk(b);
 	}
 	return nil;
--- a/tree.c
+++ b/tree.c
@@ -497,7 +497,7 @@
 			while(j < up->hi){
 				if(m.op == Oclearb){
 					bp = unpackbp(v.v, v.nv);
-					freeblk(t, bp);
+					freeblk(t, nil, bp);
 				}
 				ok = apply(&v, &m, buf, sizeof(buf));
 		Copy:
@@ -636,8 +636,8 @@
 	l = newblk(t, b->type);
 	r = newblk(t, b->type);
 	if(l == nil || r == nil){
-		freeblk(t, l->bp);
-		freeblk(t, r->bp);
+		freeblk(t, l, l->bp);
+		freeblk(t, r, r->bp);
 		return -1;
 	}
 
@@ -735,8 +735,8 @@
 	l = newblk(t, b->type);
 	r = newblk(t, b->type);
 	if(l == nil || r == nil){
-		freeblk(t, l->bp);
-		freeblk(t, r->bp);
+		freeblk(t, l, l->bp);
+		freeblk(t, r, r->bp);
 		return -1;
 	}
 	d = l;
@@ -870,8 +870,8 @@
 	l = newblk(t, a->type);
 	r = newblk(t, a->type);
 	if(l == nil || r == nil){
-		freeblk(t, l->bp);
-		freeblk(t, r->bp);
+		freeblk(t, l, l->bp);
+		freeblk(t, r, r->bp);
 		return -1;
 	}
 	d = l;
@@ -1085,14 +1085,10 @@
 	Path *p;
 
 	for(p = path; p != path + npath; p++){
-		if(p->b != nil){
-			freeblk(t, p->b->bp);
-			setflag(p->b, Bfreed);
-		}
-		if(p->m != nil){
-			freeblk(t, p->m->bp);
-			setflag(p->m, Bfreed);
-		}
+		if(p->b != nil)
+			freeblk(t, p->b, p->b->bp);
+		if(p->m != nil)
+			freeblk(t, p->b, p->m->bp);
 		dropblk(p->b);
 		dropblk(p->nl);
 		dropblk(p->nr);
@@ -1196,8 +1192,7 @@
 	t->dirty = 1;
 	unlock(&t->lk);
 
-	freeblk(t, b->bp);
-	setflag(b, Bfreed);
+	freeblk(t, b, b->bp);
 	dropblk(b);
 	dropblk(r);
 	return nil;