From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from duke.felloff.net ([216.126.196.34]) by ewsd; Sun Nov 17 15:10:36 EST 2019
Message-ID: <6630B066158B3C1C022B5438CA4F5BAC@felloff.net>
Date: Sun, 17 Nov 2019 21:10:25 +0100
From: cinap_lenrek@felloff.net
To: 9front@9front.org
Subject: Re: [9front] ahci led: reset spins.
In-Reply-To: E196E7B9A63F3382559A4EBA852969BC@eigenstate.org
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
List-ID: <9front.9front.org>
List-Help: <http://lists.9front.org>
X-Glyph: ➈
X-Bullshit: virtualized optimized core hardware 

the whole code in ledkproc() looks rather suspect to me.

for example, the map[] is NCtlr sized, so i would
assume that the index is the controller number,
but later, we index into it by the driver number.

theres also a microdelay() loop in blink() that
could be spinning forever. theres no reason to use
delay() or microdelay() as well as we run on behalf
of a kproc so we can properly sleep and not burn
cpu cycles.

diff -r 874554781a81 sys/src/9/pc/sdiahci.c
--- a/sys/src/9/pc/sdiahci.c	Sun Nov 17 19:04:38 2019 +0100
+++ b/sys/src/9/pc/sdiahci.c	Sun Nov 17 21:04:51 2019 +0100
@@ -1215,14 +1215,19 @@
 ahciencreset(Ctlr *c)
 {
 	Ahba *h;
+	int i;
 
 	if(c->enctype == Eesb)
 		return 0;
 	h = c->hba;
 	h->emctl |= Emrst;
-	while(h->emctl & Emrst)
-		delay(1);
-	return 0;
+	for(i = 0; i < 1000; i++){
+		if((h->emctl & Emrst) == 0)
+			return 0;
+		esleep(1);
+	}
+	print("%s: ahciencreset: hung ctlr\n", Tname(c));
+	return -1;
 }
 
 /*
@@ -1301,9 +1306,11 @@
 		return 0;
 	c = d->ctlr;
 	h = c->hba;
+
 	/* ensure last message has been transmitted */
-	while(h->emctl & Tmsg)
-		microdelay(1);
+	if(h->emctl & Tmsg)
+		return -1;
+
 	switch(c->enctype){
 	default:
 		panic("%s: bad led type %d", dnam(d), c->enctype);
@@ -1403,30 +1410,34 @@
 	memset(map, 0, sizeof map);
 	for(i = 0; i < niactlr; i++)
 		if(iactlr[i].enctype != 0){
-			ahciencreset(iactlr + i);
+			if(ahciencreset(iactlr + i) == -1)
+				continue;
 			map[i] = 1;
 			j++;
 		}
 	if(j == 0)
 		pexit("no work", 1);
 	for(i = 0; i < niadrive; i++){
-		iadrive[i]->nled = 3;		/* hardcoded */
-		if(iadrive[i]->ctlr->enctype == Eesb)
-			iadrive[i]->nled = 3;
-		iadrive[i]->ledbits = -1;
+		d = iadrive[i];
+		d->nled = 3;		/* hardcoded */
+		if(d->ctlr->enctype == Eesb)
+			d->nled = 3;
+		d->ledbits = -1;
 	}
 	for(i = 0; ; i++){
 		t0 = Ticks;
 		for(j = 0; j < niadrive; ){
-			c = iadrive[j]->ctlr;
-			if(map[j] == 0)
-				j += c->enctype;
-			else if(c->enctype == Eesb){
+			d = iadrive[j];
+			c = d->ctlr;
+			if(map[c - iactlr] == 0)
+				j++;
+			else
+			if(c->enctype == Eesb){
 				blinkesb(c, i);
 				j += c->ndrive;
 			}else{
-				d = iadrive[j++];
 				blink(d, i);
+				j++;
 			}
 		}
 		t1 = Ticks;

--
cinap