shithub: gefs

Download patch

ref: f8236f161ce449a53c4f4a8dff312b756cbebde8
parent: ee29d2cef8988a46ce7819172cc382f603832232
author: Ori Bernstein <ori@eigenstate.org>
date: Thu Nov 30 18:23:47 EST 2023 

fs: be safer about snap deletion

we can get the same block affected by messages
in the tree multiple times; use a proper iterator
to clean up our data blocks, instead of walking
them naively.


--- a/fs.c
+++ b/fs.c
@@ -32,7 +32,7 @@
 }
 
 static void
-snapfs(Amsg *a, Bptr *bp, vlong *pred)
+snapfs(Amsg *a, Tree **tp)
 {
 	Tree *t, *s;
 	Mount *mnt;
@@ -43,8 +43,7 @@
 		nexterror();
 	}
 	t = nil;
-	*pred = -1;
-	*bp = (Bptr){-1, -1, -1};
+	*tp = nil;
 	for(mnt = fs->mounts; mnt != nil; mnt = mnt->next){
 		if(strcmp(a->old, mnt->name) == 0){
 			updatesnap(&mnt->root, mnt->root, mnt->name);
@@ -69,10 +68,8 @@
 			return;
 		}
 		if(t->nlbl == 1 && t->nref <= 1 && t->succ == -1){
-			lock(&t->lk);
-			*pred = t->pred;
-			*bp = t->bp;
-			unlock(&t->lk);
+			aincl(&t->memref, 1);
+			*tp = t;
 		}
 		delsnap(t, t->succ, a->old);
 	}else{
@@ -2098,71 +2095,65 @@
 	}
 }
 
-/*
- * Here, we clean epochs frequently, but we run outside of
- * an epoch; this is because the caller of this function
- * has already waited for an epoch to tick over, there's
- * nobody that can be accessing the tree other than us,
- * and we just need to keep the limbo list short.
- */
 void
-sweeptree(Bptr pb, vlong pred)
+freetree(Bptr rb, vlong pred)
 {
 	Bptr bp;
 	Blk *b;
 	Kvp kv;
-	Msg m;
 	int i;
 
-	if((b = getblk(pb, 0)) == nil){
-		fprint(2, "sweep %B: %r", bp);
-		return;
-	}
-	switch(b->type){
-	case Tleaf:
+	b = getblk(rb, 0);
+	if(b->type == Tpivot){
 		for(i = 0; i < b->nval; i++){
 			getval(b, i, &kv);
-			if(kv.k[0] == Kdat){
-				bp = unpackbp(kv.v, kv.nv);
-				if(bp.gen > pred)
-					freeblk(nil, nil, bp);
-			}
-			epochclean();
-		}
-		break;
-	case Tpivot:
-		for(i = 0; i < b->nbuf; i++){
-			getmsg(b, i, &m);
-			if(m.op == Oinsert && m.k[0] == Kdat){
-				bp = unpackbp(m.v, m.nv);
-				if(bp.gen > pred)
-					freeblk(nil, nil, bp);
-				epochclean();
-			}
-		}
-		for(i = 0; i < b->nval; i++){
-			getval(b, i, &kv);
 			bp = unpackbp(kv.v, kv.nv);
-			sweeptree(bp, pred);
+			freetree(bp, pred);
 			epochclean();
 		}
-		break;
-	default:
-		fprint(2, "broken tree %B\n", pb);
-		abort();
 	}
-	if(pb.gen > pred)
-		freeblk(nil, nil, pb);
+	if(rb.gen > pred)
+		freeblk(nil, nil, rb);
 	dropblk(b);
 }
 
+/*
+ * Here, we clean epochs frequently, but we run outside of
+ * an epoch; this is because the caller of this function
+ * has already waited for an epoch to tick over, there's
+ * nobody that can be accessing the tree other than us,
+ * and we just need to keep the limbo list short.
+ */
 void
+sweeptree(Tree *t)
+{
+	char pfx[1];
+	Scan s;
+	Bptr bp;
+
+	pfx[0] = Kdat;
+	btnewscan(&s, pfx, 1);
+	btenter(t, &s);
+	while(1){
+		if(!btnext(&s, &s.kv))
+			break;
+		bp = unpackbp(s.kv.v, s.kv.nv);
+		if(bp.gen > t->pred)
+			freeblk(nil, nil, bp);
+		epochclean();
+	}
+	btexit(&s);
+	freetree(t->bp, t->pred);
+}
+
+void
 runsweep(int id, void*)
 {
 	char buf[Offksz];
 	Bptr bp, nb, *oldhd;
-	vlong off, pred;
+	vlong off;
 	Mount *mnt;
+	Tree *t;
 	Arena *a;
 	Amsg *am;
 	Blk *b;
@@ -2234,15 +2225,16 @@
 				nexterror();
 			}
 			epochstart(id);
-			snapfs(am, &bp, &pred);
+			snapfs(am, &t);
 			sync();
 			epochend(id);
 			poperror();
 			qunlock(&fs->mutlk);
 
-			if(pred != -1){
+			if(t != nil){
 				epochwait();
-				sweeptree(bp, pred);
+				sweeptree(t);
+				closesnap(t);
 			}
 			poperror();
 			break;