Skip to contentSkip to conversation

Revert broken edge_shirq optimisation

PATCH Applied Jan Kiszka 2 participants 4 messages Jul 31, 2007

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 Revert broken edge_shirq optimisation

Oh my dear. The ground may open and swallow me. This crappy piece of optimisation in xnintr_edge_shirq_handler() I introduced both to 2.4 and (sadly) the 2.3.x stable series cannot work. It must stay as it used to:

nucleus/intr.c +3 -3
@@ -296,15 +296,15 @@ static void xnintr_edge_shirq_handler(un
 		s |= ret;
 
 		if (code == XN_ISR_HANDLED) {
-			if (!(end = (intr->next)))
-				end = shirq->handlers;
+			end = NULL;
 			xnstat_counter_inc(
 				&intr->stat[xnsched_cpu(sched)].hits);
 			xnstat_runtime_lazy_switch(sched,
 				&intr->stat[xnsched_cpu(sched)].account,
 				start);
 			start = xnstat_runtime_now();
-		}
+		} else if (code == XN_ISR_NONE && end == NULL)
+			end = intr;
 
 		if (counter++ > MAX_EDGEIRQ_COUNTER)
 			break;

(that's for 2.3.x)

Once "end" is moved forward to the next handler in the chain, the loop termination condition "intr==end" will trigger on the immediately following round, even if there are still events pending. Utterly broken.

Philippe, please quickly apply -- and don't tell anyone who did this.

Jan

author
#1 lore raw
Quoted 25 lines · Jan Kiszka

Oh my dear. The ground may open and swallow me. This crappy piece of optimisation in xnintr_edge_shirq_handler() I introduced both to 2.4 and (sadly) the 2.3.x stable series cannot work. It must stay as it used to:

nucleus/intr.c +3 -3
@@ -296,15 +296,15 @@ static void xnintr_edge_shirq_handler(un
 		s |= ret;
 
 		if (code == XN_ISR_HANDLED) {
-			if (!(end = (intr->next)))
-				end = shirq->handlers;
+			end = NULL;
 			xnstat_counter_inc(
 				&intr->stat[xnsched_cpu(sched)].hits);
 			xnstat_runtime_lazy_switch(sched,
 				&intr->stat[xnsched_cpu(sched)].account,
 				start);
 			start = xnstat_runtime_now();
-		}
+		} else if (code == XN_ISR_NONE && end == NULL)
+			end = intr;
 
 		if (counter++ > MAX_EDGEIRQ_COUNTER)
 			break;
Quoted 1 line

(that's for 2.3.x)

For 2.4, one may consider to assume now that "code != XN_ISR_HANDLE => code == XN_ISR_NONE":

nucleus/intr.c +3 -3
@@ -251,15 +251,15 @@ static void xnintr_edge_shirq_handler(un
 		s |= ret;
 
 		if (code == XN_ISR_HANDLED) {
-			if (!(end = (intr->next)))
-				end = shirq->handlers;
+			end = NULL;
 			xnstat_counter_inc(
 				&intr->stat[xnsched_cpu(sched)].hits);
 			xnstat_exectime_lazy_switch(sched,
 				&intr->stat[xnsched_cpu(sched)].account,
 				start);
 			start = xnstat_exectime_now();
-		}
+		} else if (end == NULL)
+			end = intr;
 
 		if (counter++ > MAX_EDGEIRQ_COUNTER)
 			break;

Jan

#2 lore raw
Quoted 24 lines · Jan Kiszka
Quoted 23 lines · Jan Kiszka

Oh my dear. The ground may open and swallow me. This crappy piece of optimisation in xnintr_edge_shirq_handler() I introduced both to 2.4 and (sadly) the 2.3.x stable series cannot work. It must stay as it used to:

nucleus/intr.c +3 -3
@@ -296,15 +296,15 @@ static void xnintr_edge_shirq_handler(un
 		s |= ret;
 
 		if (code == XN_ISR_HANDLED) {
-			if (!(end = (intr->next)))
-				end = shirq->handlers;
+			end = NULL;
 			xnstat_counter_inc(
 				&intr->stat[xnsched_cpu(sched)].hits);
 			xnstat_runtime_lazy_switch(sched,
 				&intr->stat[xnsched_cpu(sched)].account,
 				start);
 			start = xnstat_runtime_now();
-		}
+		} else if (code == XN_ISR_NONE && end == NULL)
+			end = intr;
 
Quoted 7 lines
Quoted 3 lines
nucleus/intr.c +0 -0
 		if (counter++ > MAX_EDGEIRQ_COUNTER)
 			break;

(that's for 2.3.x)

For 2.4, one may consider to assume now that "code != XN_ISR_HANDLE => code == XN_ISR_NONE":

Merged, thanks.

Quoted 24 lines
nucleus/intr.c +3 -3
@@ -251,15 +251,15 @@ static void xnintr_edge_shirq_handler(un
 		s |= ret;
 
 		if (code == XN_ISR_HANDLED) {
-			if (!(end = (intr->next)))
-				end = shirq->handlers;
+			end = NULL;
 			xnstat_counter_inc(
 				&intr->stat[xnsched_cpu(sched)].hits);
 			xnstat_exectime_lazy_switch(sched,
 				&intr->stat[xnsched_cpu(sched)].account,
 				start);
 			start = xnstat_exectime_now();
-		}
+		} else if (end == NULL)
+			end = intr;
 
 		if (counter++ > MAX_EDGEIRQ_COUNTER)
 			break;

Jan

Signature · 1 line
Philippe.
#3 lore raw
In reply to Jan Kiszka
Quoted 27 lines · Jan Kiszka

Oh my dear. The ground may open and swallow me. This crappy piece of optimisation in xnintr_edge_shirq_handler() I introduced both to 2.4 and (sadly) the 2.3.x stable series cannot work. It must stay as it used to:

nucleus/intr.c +3 -3
@@ -296,15 +296,15 @@ static void xnintr_edge_shirq_handler(un
 		s |= ret;
 
 		if (code == XN_ISR_HANDLED) {
-			if (!(end = (intr->next)))
-				end = shirq->handlers;
+			end = NULL;
 			xnstat_counter_inc(
 				&intr->stat[xnsched_cpu(sched)].hits);
 			xnstat_runtime_lazy_switch(sched,
 				&intr->stat[xnsched_cpu(sched)].account,
 				start);
 			start = xnstat_runtime_now();
-		}
+		} else if (code == XN_ISR_NONE && end == NULL)
+			end = intr;
 
 		if (counter++ > MAX_EDGEIRQ_COUNTER)
 			break;

(that's for 2.3.x)

Quoted 6 lines

Once "end" is moved forward to the next handler in the chain, the loop termination condition "intr==end" will trigger on the immediately following round, even if there are still events pending. Utterly broken.

Philippe, please quickly apply -- and don't tell anyone who did this.

No problem. Nobody will know, outside of the Internet, that is.

Quoted 2 lines

Jan

Signature · 1 line
Philippe.