changelog shortlog tags branches files raw gz bz2 help

Mercurial > hg > plan9front / changeset: kernel: fix twakeup()/timerdel() race condition

changeset 5848: 381f1cb08002
parent 5847: f0c30306e7d5
child 5849: 4dd0bcaddc91
author: cinap_lenrek@felloff.net
date: Wed, 29 Mar 2017 00:30:53 +0200
files: sys/src/9/port/edf.c sys/src/9/port/portclock.c sys/src/9/port/portdat.h sys/src/9/port/portfns.h sys/src/9/port/proc.c
description: 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.
     1.1--- a/sys/src/9/port/edf.c
     1.2+++ b/sys/src/9/port/edf.c
     1.3@@ -207,7 +207,7 @@ release(Proc *p)
     1.4 }
     1.5 
     1.6 static void
     1.7-releaseintr(Ureg*, Timer *t)
     1.8+releaseintr(Ureg *u, Timer *t)
     1.9 {
    1.10 	Proc *p;
    1.11 	extern int panicking;
    1.12@@ -254,9 +254,7 @@ releaseintr(Ureg*, Timer *t)
    1.13 	case Wakeme:
    1.14 		release(p);
    1.15 		edfunlock();
    1.16-		if(p->trend)
    1.17-			wakeup(p->trend);
    1.18-		p->trend = nil;
    1.19+		twakeup(u, t);
    1.20 		if(up){
    1.21 			up->delaysched++;
    1.22 			delayedscheds++;
    1.23@@ -445,8 +443,7 @@ edfstop(Proc *p)
    1.24 		if(p->trace && (pt = proctrace))
    1.25 			pt(p, SExpel, 0);
    1.26 		e->flags &= ~Admitted;
    1.27-		if(e->tt)
    1.28-			timerdel(e);
    1.29+		timerdel(e);
    1.30 		edfunlock();
    1.31 	}
    1.32 }
    1.33@@ -479,20 +476,23 @@ edfyield(void)
    1.34 	e->r = e->t;
    1.35 	e->flags |= Yield;
    1.36 	e->d = now;
    1.37-	if (up->tt == nil){
    1.38-		n = e->t - now;
    1.39-		if(n < 20)
    1.40-			n = 20;
    1.41-		up->tns = 1000LL * n;
    1.42-		up->tf = releaseintr;
    1.43-		up->tmode = Trelative;
    1.44-		up->ta = up;
    1.45-		up->trend = &up->sleep;
    1.46-		timeradd(up);
    1.47-	}else if(up->tf != releaseintr)
    1.48-		print("edfyield: surprise! %#p\n", up->tf);
    1.49+	n = e->t - now;
    1.50+	if(n < 20)
    1.51+		n = 20;
    1.52+	up->tns = 1000LL * n;
    1.53+	up->tf = releaseintr;
    1.54+	up->tmode = Trelative;
    1.55+	up->ta = up;
    1.56+	up->trend = &up->sleep;
    1.57+	timeradd(up);
    1.58 	edfunlock();
    1.59+	if(waserror()){
    1.60+		up->trend = nil;
    1.61+		timerdel(up);
    1.62+		nexterror();
    1.63+	}
    1.64 	sleep(&up->sleep, yfn, nil);
    1.65+	poperror();
    1.66 }
    1.67 
    1.68 int
     2.1--- a/sys/src/9/port/portclock.c
     2.2+++ b/sys/src/9/port/portclock.c
     2.3@@ -112,9 +112,13 @@ timeradd(Timer *nt)
     2.4 void
     2.5 timerdel(Timer *dt)
     2.6 {
     2.7+	Mach *mp;
     2.8 	Timers *tt;
     2.9 	uvlong when;
    2.10 
    2.11+	/* avoid Tperiodic getting re-added */
    2.12+	dt->tmode = Trelative;
    2.13+
    2.14 	ilock(dt);
    2.15 	if(tt = dt->tt){
    2.16 		ilock(tt);
    2.17@@ -123,7 +127,16 @@ timerdel(Timer *dt)
    2.18 			timerset(tt->head->twhen);
    2.19 		iunlock(tt);
    2.20 	}
    2.21+	if((mp = dt->tactive) == nil || mp->machno == m->machno){
    2.22+		iunlock(dt);
    2.23+		return;
    2.24+	}
    2.25 	iunlock(dt);
    2.26+
    2.27+	/* rare, but tf can still be active on another cpu */
    2.28+	while(dt->tactive == mp && dt->tt == nil)
    2.29+		if(up->nlocks == 0 && islo())
    2.30+			sched();
    2.31 }
    2.32 
    2.33 void
    2.34@@ -190,12 +203,14 @@ timerintr(Ureg *u, Tval)
    2.35 		tt->head = t->tnext;
    2.36 		assert(t->tt == tt);
    2.37 		t->tt = nil;
    2.38+		t->tactive = MACHP(m->machno);
    2.39 		fcallcount[m->machno]++;
    2.40 		iunlock(tt);
    2.41 		if(t->tf)
    2.42 			(*t->tf)(u, t);
    2.43 		else
    2.44 			callhzclock++;
    2.45+		t->tactive = nil;
    2.46 		ilock(tt);
    2.47 		if(t->tmode == Tperiodic)
    2.48 			tadd(tt, t);
     3.1--- a/sys/src/9/port/portdat.h
     3.2+++ b/sys/src/9/port/portdat.h
     3.3@@ -556,6 +556,7 @@ struct Timer
     3.4 	void	*ta;
     3.5 	/* Internal */
     3.6 	Lock;
     3.7+	Mach	*tactive;	/* The cpu that tf is active on */
     3.8 	Timers	*tt;		/* Timers queue this timer runs on */
     3.9 	Tval	tticks;		/* tns converted to ticks */
    3.10 	Tval	twhen;		/* ns represented in fastticks */
     4.1--- a/sys/src/9/port/portfns.h
     4.2+++ b/sys/src/9/port/portfns.h
     4.3@@ -348,6 +348,7 @@ void		todinit(void);
     4.4 void		todset(vlong, vlong, int);
     4.5 Block*		trimblock(Block*, int, int);
     4.6 void		tsleep(Rendez*, int (*)(void*), void*, ulong);
     4.7+void		twakeup(Ureg*, Timer *);
     4.8 int		uartctl(Uart*, char*);
     4.9 int		uartgetc(void);
    4.10 void		uartkick(void*);
     5.1--- a/sys/src/9/port/proc.c
     5.2+++ b/sys/src/9/port/proc.c
     5.3@@ -822,28 +822,17 @@ tfn(void *arg)
     5.4 	return up->trend == nil || up->tfn(arg);
     5.5 }
     5.6 
     5.7-static void
     5.8+void
     5.9 twakeup(Ureg*, Timer *t)
    5.10 {
    5.11 	Proc *p;
    5.12 	Rendez *trend;
    5.13 
    5.14-	ilock(t);
    5.15 	p = t->ta;
    5.16 	trend = p->trend;
    5.17 	if(trend != nil){
    5.18+		p->trend = nil;
    5.19 		wakeup(trend);
    5.20-		p->trend = nil;
    5.21-	}
    5.22-	iunlock(t);
    5.23-}
    5.24-
    5.25-static void
    5.26-stoptimer(void)
    5.27-{
    5.28-	if(up->trend != nil){
    5.29-		up->trend = nil;
    5.30-		timerdel(up);
    5.31 	}
    5.32 }
    5.33 
    5.34@@ -864,11 +853,13 @@ tsleep(Rendez *r, int (*fn)(void*), void
    5.35 	timeradd(up);
    5.36 
    5.37 	if(waserror()){
    5.38-		stoptimer();
    5.39+		up->trend = nil;
    5.40+		timerdel(up);
    5.41 		nexterror();
    5.42 	}
    5.43 	sleep(r, tfn, arg);
    5.44-	stoptimer();
    5.45+	up->trend = nil;
    5.46+	timerdel(up);
    5.47 	poperror();
    5.48 }
    5.49 
    5.50@@ -1102,8 +1093,7 @@ pexit(char *exitstr, int freemem)
    5.51 	void (*pt)(Proc*, int, vlong);
    5.52 
    5.53 	up->alarm = 0;
    5.54-	if(up->tt != nil)
    5.55-		timerdel(up);
    5.56+	timerdel(up);
    5.57 	pt = proctrace;
    5.58 	if(pt != nil)
    5.59 		pt(up, SDead, 0);