git: 9front

Download patch

ref: 46e7aaa19a6cdf258ff2f60292c5f074ac9d73d9
parent: 2fce59a13a759fd099d46f735f2a10face403130
author: Ori Bernstein <ori@eigenstate.org>
date: Fri May 30 09:50:53 EDT 2025 

gefs: use rwlock instead of atomics for mount list

Using an rwlock here simplifies reasoning about the code.
The individial elements of the mount list still participate
in EBR, becuase accessing them via the FID needs to be safe,
but we don't need the performance of EBR when walking the
list.


--- a/sys/src/cmd/gefs/cons.c
+++ b/sys/src/cmd/gefs/cons.c
@@ -173,7 +173,9 @@
 	for(i = 0; i < fs->nusers; i++){
 		u = &fs->users[i];
 		fprint(fd, "%d:%s:", u->id, u->name);
-		if((v = uid2user(u->lead)) == nil)
+		if(u->lead == noneid)
+			fprint(fd, ":");
+		else if((v = uid2user(u->lead)) == nil)
 			fprint(fd, "???:");
 		else
 			fprint(fd, "%s:", v->name);
--- a/sys/src/cmd/gefs/dat.h
+++ b/sys/src/cmd/gefs/dat.h
@@ -533,7 +533,7 @@
 	QLock	synclk;
 	Rendez	syncrz;
 
-	QLock	mountlk;
+	RWLock	mountlk;
 	Mount	*mounts;
 	Mount	*snapmnt;
 	Lock	connlk;
--- a/sys/src/cmd/gefs/fs.c
+++ b/sys/src/cmd/gefs/fs.c
@@ -114,8 +114,16 @@
 		nexterror();
 	}
 	tracem("packb");
-	for(mnt = agetp(&fs->mounts); mnt != nil; mnt = mnt->next)
+
+	rlock(&fs->mountlk);
+	if(waserror()){
+		runlock(&fs->mountlk);
+		nexterror();
+	}
+	for(mnt = fs->mounts; mnt != nil; mnt = mnt->next)
 		updatesnap(&mnt->root, mnt->root, mnt->name, mnt->flag);
+	runlock(&fs->mountlk);
+	poperror();
 	/*
 	 * Now that we've updated the snaps, we can sync the
 	 * dlist; the snap tree will not change from here.
@@ -204,16 +212,18 @@
 static void
 snapfs(Amsg *a, Tree **tp)
 {
-	Tree *t, *s;
+	Tree *t, *s, *r;
 	Mount *mnt;
 
+	t = nil;
+	r = nil;
+	*tp = nil;
+	rlock(&fs->mountlk);
 	if(waserror()){
-		*tp = nil;
+		runlock(&fs->mountlk);
 		nexterror();
 	}
-	t = nil;
-	*tp = nil;
-	for(mnt = agetp(&fs->mounts); mnt != nil; mnt = mnt->next){
+	for(mnt = fs->mounts; mnt != nil; mnt = mnt->next){
 		if(strcmp(a->old, mnt->name) == 0){
 			updatesnap(&mnt->root, mnt->root, mnt->name, mnt->flag);
 			t = agetp(&mnt->root);
@@ -224,6 +234,7 @@
 	if(t == nil && (t = opensnap(a->old, nil)) == nil){
 		if(a->fd != -1)
 			fprint(a->fd, "snap: open '%s': does not exist\n", a->old);
+		runlock(&fs->mountlk);
 		poperror();
 		return;
 	}
@@ -231,12 +242,13 @@
 		if(mnt != nil) {
 			if(a->fd != -1)
 				fprint(a->fd, "snap: snap is mounted: '%s'\n", a->old);
+			runlock(&fs->mountlk);
 			poperror();
 			return;
 		}
 		if(t->nlbl == 1 && t->nref <= 1 && t->succ == -1){
 			ainc(&t->memref);
-			*tp = t;
+			r = t;
 		}
 		delsnap(t, t->succ, a->old);
 	}else{
@@ -244,6 +256,7 @@
 			if(a->fd != -1)
 				fprint(a->fd, "snap: already exists '%s'\n", a->new);
 			closesnap(s);
+			runlock(&fs->mountlk);
 			poperror();
 			return;
 		}
@@ -250,7 +263,9 @@
 		tagsnap(t, a->new, a->flag);
 	}
 	closesnap(t);
+	runlock(&fs->mountlk);
 	poperror();
+	*tp = r;
 	if(a->fd != -1){
 		if(a->delete)
 			fprint(a->fd, "deleted: %s\n", a->old);
@@ -415,8 +430,16 @@
 {
 	if(!(mnt->flag & Lmut))
 		error(Erdonly);
-	if(mnt->root->nlbl != 1 || mnt->root->nref != 0)
+	if(mnt->root->nlbl != 1 || mnt->root->nref != 0){
+		rlock(&fs->mountlk);
+		if(waserror()){
+			runlock(&fs->mountlk);
+			nexterror();
+		}
 		updatesnap(&mnt->root, mnt->root, mnt->name, mnt->flag);
+		poperror();
+		runlock(&fs->mountlk);
+	}
 	btupsert(mnt->root, m, nm);
 }
 
@@ -679,22 +702,19 @@
 		return fs->snapmnt;
 	}
 
-	qlock(&fs->mountlk);
-	for(mnt = agetp(&fs->mounts); mnt != nil; mnt = mnt->next){
+	wlock(&fs->mountlk);
+	for(mnt = fs->mounts; mnt != nil; mnt = mnt->next){
 		if(strcmp(name, mnt->name) == 0){
 			ainc(&mnt->ref);
 			goto Out;
 		}
 	}
-	if((mnt = mallocz(sizeof(*mnt), 1)) == nil){
-		qunlock(&fs->mountlk);
-		error(Enomem);
-	}
 	if(waserror()){
-		qunlock(&fs->mountlk);
+		wunlock(&fs->mountlk);
 		free(mnt);
 		nexterror();
 	}
+	mnt = emalloc(sizeof(*mnt), 1);
 	mnt->ref = 1;
 	snprint(mnt->name, sizeof(mnt->name), "%s", name);
 	if((t = opensnap(name, &flg)) == nil)
@@ -703,11 +723,11 @@
 	mnt->root = t;
 	mnt->next = fs->mounts;
 	loadautos(mnt);
-	asetp(&fs->mounts, mnt);
+	fs->mounts = mnt;
 	poperror();
 
 Out:
-	qunlock(&fs->mountlk);
+	wunlock(&fs->mountlk);
 	return mnt;
 }
 
@@ -718,8 +738,8 @@
 
 	if(mnt == nil)
 		return;
+	wlock(&fs->mountlk);
 	if(adec(&mnt->ref) == 0){
-		qlock(&fs->mountlk);
 		for(p = &fs->mounts; (me = *p) != nil; p = &me->next){
 			if(me == mnt)
 				break;
@@ -727,8 +747,8 @@
 		assert(me != nil);
 		*p = me->next;
 		limbo(DFmnt, me);
-		qunlock(&fs->mountlk);
 	}
+	wunlock(&fs->mountlk);
 }
 
 static void
@@ -3032,12 +3052,14 @@
 
 		tmnow(&tm, nil);
 		now = tmnorm(&tm);
-		for(mnt = agetp(&fs->mounts); mnt != nil; mnt = mnt->next){
+		rlock(&fs->mountlk);
+		for(mnt = fs->mounts; mnt != nil; mnt = mnt->next){
 			if(!(mnt->flag & Lmut))
 				continue;
 			for(i = 0; i < nelem(mnt->cron); i++)
 				cronsync(mnt->name, &mnt->cron[i], &tm, now);
 		}
+		runlock(&fs->mountlk);
 		poperror();
 	}
 }
--- a/sys/src/cmd/gefs/snap.c
+++ b/sys/src/cmd/gefs/snap.c
@@ -310,6 +310,8 @@
  *
  * If it has one successor and no label, then
  * it will be merged with that successor.
+ *
+ * Must be called with mntlock held for r
  */
 void
 delsnap(Tree *t, vlong succ, char *name)
@@ -360,7 +362,8 @@
 	btupsert(&fs->snap, m, nm);
 	if(deltree){
 		reclaimblocks(t->gen, succ, t->pred);
-		for(mnt = agetp(&fs->mounts); mnt != nil; mnt = mnt->next){
+		assert(!canwlock(&fs->mountlk));
+		for(mnt = fs->mounts; mnt != nil; mnt = mnt->next){
 			if(mnt->root->gen == t->succ)
 				mnt->root->pred = t->pred;
 			if(mnt->root->gen == t->pred)
--- a/sys/src/cmd/gefs/user.c
+++ b/sys/src/cmd/gefs/user.c
@@ -133,6 +133,7 @@
 			goto Error;
 		}
 		snprint(u->name, sizeof(u->name), "%s", f);
+		u->lead = noneid;
 		u->memb = nil;
 		u->nmemb = 0;
 		i++;
@@ -162,6 +163,11 @@
 			if(u == nil){
 				fprint(fd, "/adm/users:%d: leader %s does not exist\n", lnum, f);
 				err = Enouser;
+				goto Error;
+			}
+			if(u->id == noneid){
+				fprint(fd, "/adm/users:%d: group leader may not be none\n", lnum);
+				err = Esyntax;
 				goto Error;
 			}
 			users[i].lead = u->id;
-- 
⑨