shithub: riscv

Info  •  Files  •  Log  •  Branches

kernel: fix twakeup()/timerdel() race condition

timerdel() did not make sure that the timer function
is not active (on another cpu). just acquiering the
Timer lock in the timer function only blocks the caller
of timerdel()/timeradd() but not the other way arround
(on a multiprocessor).

this changes the timer code to track activity of
the timer function, having timerdel() wait until
the timer has finished executing.

--- a/sys/src/9/port/edf.c

+++ b/sys/src/9/port/edf.c

@@ -207,7 +207,7 @@

 }

 static void

-releaseintr(Ureg*, Timer *t)

+releaseintr(Ureg *u, Timer *t)

 {

 	Proc *p;

 	extern int panicking;

@@ -254,9 +254,7 @@

 	case Wakeme:

 		release(p);

 		edfunlock();

-		if(p->trend)

-			wakeup(p->trend);

-		p->trend = nil;

+		twakeup(u, t);

 		if(up){

 			up->delaysched++;

 			delayedscheds++;

@@ -445,8 +443,7 @@

 		if(p->trace && (pt = proctrace))

 			pt(p, SExpel, 0);

 		e->flags &= ~Admitted;

-		if(e->tt)

-			timerdel(e);

+		timerdel(e);

 		edfunlock();

 	}

 }

@@ -479,20 +476,23 @@

 	e->r = e->t;

 	e->flags |= Yield;

 	e->d = now;

-	if (up->tt == nil){

-		n = e->t - now;

-		if(n < 20)

-			n = 20;

-		up->tns = 1000LL * n;

-		up->tf = releaseintr;

-		up->tmode = Trelative;

-		up->ta = up;

-		up->trend = &up->sleep;

-		timeradd(up);

-	}else if(up->tf != releaseintr)

-		print("edfyield: surprise! %#p\n", up->tf);

+	n = e->t - now;

+	if(n < 20)

+		n = 20;

+	up->tns = 1000LL * n;

+	up->tf = releaseintr;

+	up->tmode = Trelative;

+	up->ta = up;

+	up->trend = &up->sleep;

+	timeradd(up);

 	edfunlock();

+	if(waserror()){

+		up->trend = nil;

+		timerdel(up);

+		nexterror();

+	}

 	sleep(&up->sleep, yfn, nil);

+	poperror();

 }

 int

--- a/sys/src/9/port/portclock.c

+++ b/sys/src/9/port/portclock.c

@@ -112,9 +112,13 @@

 void

 timerdel(Timer *dt)

 {

+	Mach *mp;

 	Timers *tt;

 	uvlong when;

+	/* avoid Tperiodic getting re-added */

+	dt->tmode = Trelative;

+

 	ilock(dt);

 	if(tt = dt->tt){

 		ilock(tt);

@@ -123,7 +127,16 @@

 			timerset(tt->head->twhen);

 		iunlock(tt);

 	}

+	if((mp = dt->tactive) == nil || mp->machno == m->machno){

+		iunlock(dt);

+		return;

+	}

 	iunlock(dt);

+

+	/* rare, but tf can still be active on another cpu */

+	while(dt->tactive == mp && dt->tt == nil)

+		if(up->nlocks == 0 && islo())

+			sched();

 }

 void

@@ -190,6 +203,7 @@

 		tt->head = t->tnext;

 		assert(t->tt == tt);

 		t->tt = nil;

+		t->tactive = MACHP(m->machno);

 		fcallcount[m->machno]++;

 		iunlock(tt);

 		if(t->tf)

@@ -196,6 +210,7 @@

 			(*t->tf)(u, t);

 		else

 			callhzclock++;

+		t->tactive = nil;

 		ilock(tt);

 		if(t->tmode == Tperiodic)

 			tadd(tt, t);

--- a/sys/src/9/port/portdat.h

+++ b/sys/src/9/port/portdat.h

@@ -556,6 +556,7 @@

 	void	*ta;

 	/* Internal */

 	Lock;

+	Mach	*tactive;	/* The cpu that tf is active on */

 	Timers	*tt;		/* Timers queue this timer runs on */

 	Tval	tticks;		/* tns converted to ticks */

 	Tval	twhen;		/* ns represented in fastticks */

--- a/sys/src/9/port/portfns.h

+++ b/sys/src/9/port/portfns.h

@@ -348,6 +348,7 @@

 void		todset(vlong, vlong, int);

 Block*		trimblock(Block*, int, int);

 void		tsleep(Rendez*, int (*)(void*), void*, ulong);

+void		twakeup(Ureg*, Timer *);

 int		uartctl(Uart*, char*);

 int		uartgetc(void);

 void		uartkick(void*);

--- a/sys/src/9/port/proc.c

+++ b/sys/src/9/port/proc.c

@@ -822,31 +822,20 @@

 	return up->trend == nil || up->tfn(arg);

 }

-static void

+void

 twakeup(Ureg*, Timer *t)

 {

 	Proc *p;

 	Rendez *trend;

-	ilock(t);

 	p = t->ta;

 	trend = p->trend;

 	if(trend != nil){

-		wakeup(trend);

 		p->trend = nil;

+		wakeup(trend);

 	}

-	iunlock(t);

 }

-static void

-stoptimer(void)

-{

-	if(up->trend != nil){

-		up->trend = nil;

-		timerdel(up);

-	}

-}

-

 void

 tsleep(Rendez *r, int (*fn)(void*), void *arg, ulong ms)

 {

@@ -864,11 +853,13 @@

 	timeradd(up);

 	if(waserror()){

-		stoptimer();

+		up->trend = nil;

+		timerdel(up);

 		nexterror();

 	}

 	sleep(r, tfn, arg);

-	stoptimer();

+	up->trend = nil;

+	timerdel(up);

 	poperror();

 }

@@ -1102,8 +1093,7 @@

 	void (*pt)(Proc*, int, vlong);

 	up->alarm = 0;

-	if(up->tt != nil)

-		timerdel(up);

+	timerdel(up);

 	pt = proctrace;

 	if(pt != nil)

 		pt(up, SDead, 0);