git: 9front

Download patch

ref: 2733c957ef614631c1cb8b80f55a05cd0520dfc0
parent: 4acf15a832d6516f25e01b7a6301f51754405924
author: cinap_lenrek <cinap_lenrek@gmx.de>
date: Thu Jul 25 21:51:03 EDT 2013

ether82563: avoid deadlock due to icansleep() trying to acquire Rbpool.Lock

icansleep() violates the lock ordering due to the following cases:

rbfree(): ilock(Rbpool.Lock) -> wakeup(): spli(), lock(Rbpool.Rendez)
sleep(): splhi(), lock(Rbpool.Rendez) -> icansleep(): ilock(Rbpool.Lock)

erik fixed this moving the wakeup() out of the ilock() in rbfree(),
but i think it is an error to try acquiering a ilock in sleeps wait
condition function in general.

so this is what we do:

in the icansleep() function, we check for the *real* event we care about;
that is, if theres a buffer available in the Rbpool. this is to handle
the case when rbfree() makes a buffer available *before* it sees us
setting p->starve = 1.

p->starve is now just used to gate rbfree() from calling wakeup() as
an optimization.

this might cause spurious wakeups but they are not a problem. missed
wakeups is the thing we have to prevent.

--- a/sys/src/9/pc/ether82563.c
+++ b/sys/src/9/pc/ether82563.c
@@ -792,14 +792,9 @@
 icansleep(void *v)
 {
 	Rbpool *p;
-	int r;
 
 	p = v;
-	ilock(p);
-	r = p->starve == 0;
-	iunlock(p);
-
-	return r;
+	return p->b != nil;
 }
 
 static Block*
@@ -1058,9 +1053,7 @@
 			print("i82563%d: no rx buffers\n", ctlr->pool);
 			if(maysleep == 0)
 				return -1;
-			ilock(p);
 			p->starve = 1;
-			iunlock(p);
 			sleep(p, icansleep, p);
 			goto redux;
 		}
--