Re: [PATCH] Fix WARN_ON / WARN_ON_ONCE regression

[Tim Chen]

Sorry my mailer screwed up the line-wrap.  Here's a resent of the patch.

Tim

The patch "Let WARN_ON/WARN_ON_ONCE return the condition"
http://kernel.org/git/?
p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=684f978347deb42d180373ac4c427f82ef963171

introduced 40% more 2nd level cache miss to tbench workload
being run in a loop back mode on a Core 2 machine.  I think the
introduction of the local variables to WARN_ON and WARN_ON_ONCE

typeof(x) __ret_warn_on = (x);
typeof(condition) __ret_warn_once = (condition);

results in the extra cache misses.  In our test workload profile, we see
heavily used functions like do_softirq and local_bh_enable 
takes a lot longer to execute.  

The modification below helps fix the problem.  I made a slight
modification to sched.c to get around a gcc bug.

Signed-off-by: Tim Chen <tim.c.chen@xxxxxxxxx>
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index a525089..05ed388 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -17,13 +17,12 @@ #endif
 
 #ifndef HAVE_ARCH_WARN_ON
 #define WARN_ON(condition) ({						\
-	typeof(condition) __ret_warn_on = (condition);			\
-	if (unlikely(__ret_warn_on)) {					\
+	if (unlikely(condition)) {					\
 		printk("BUG: warning at %s:%d/%s()\n", __FILE__,	\
 			__LINE__, __FUNCTION__);			\
 		dump_stack();						\
 	}								\
-	unlikely(__ret_warn_on);					\
+	unlikely(condition);						\
 })
 #endif
 
@@ -43,12 +42,16 @@ #endif
 
 #define WARN_ON_ONCE(condition)	({			\
 	static int __warn_once = 1;			\
-	typeof(condition) __ret_warn_once = (condition);\
 							\
-	if (likely(__warn_once))			\
-		if (WARN_ON(__ret_warn_once)) 		\
-			__warn_once = 0;		\
-	unlikely(__ret_warn_once);			\
+	if (unlikely(condition)){ 			\
+		if (likely(__warn_once)){		\
+			__warn_once=0;			\
+			printk("BUG: warning at %s:%d/%s()\n", __FILE__, \
+				__LINE__, __FUNCTION__);\
+			dump_stack(); 			\
+		}					\
+	}						\
+	unlikely(condition);				\
 })
 
 #ifdef CONFIG_SMP
diff --git a/kernel/sched.c b/kernel/sched.c
index 5c848fd..8ae972c 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -5629,7 +5629,8 @@ static unsigned long domain_distance(int
 	struct sched_domain *sd;
 
 	for_each_domain(cpu1, sd) {
-		WARN_ON(!cpu_isset(cpu1, sd->span));
+		if (unlikely(!cpu_isset(cpu1, sd->span)))
+			WARN_ON(1);
 		if (cpu_isset(cpu2, sd->span))
 			return distance;
 		distance++;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/