The patch titled
lock-validator-introduce-warn_on_oncecond speedup
has been added to the -mm tree. Its filename is
lock-validator-introduce-warn_on_oncecond-speedup.patch
See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
out what to do about this
------------------------------------------------------
Subject: lock-validator-introduce-warn_on_oncecond speedup
From: Steven Rostedt <rostedt@xxxxxxxxxxx>
Here's my code:
----- with the current WARN_ON_ONCE ----
#define unlikely(x) __builtin_expect(!!(x), 0)
#define WARN_ON_ONCE(condition) \
({ \
static int __warn_once = 1; \
int __ret = 0; \
\
if (__warn_once && unlikely((condition))) { \
__warn_once = 0; \
WARN_ON(1); \
__ret = 1; \
} \
__ret; \
})
int warn (int x)
{
WARN_ON_ONCE(x==1);
return x+1;
}
----- with the version I suggest. ----
#define unlikely(x) __builtin_expect(!!(x), 0)
#define WARN_ON_ONCE(condition) \
({ \
static int __warn_once = 1; \
int __ret = 0; \
\
if (unlikely((condition)) && __warn_once) { \
__warn_once = 0; \
WARN_ON(1); \
__ret = 1; \
} \
__ret; \
})
int warn(int x)
{
WARN_ON_ONCE(x==1);
return x+1;
}
-------
Compiling these two I get this:
current warn.o:
00000000 <warn>:
0: 55 push %ebp
1: 89 e5 mov %esp,%ebp
3: 53 push %ebx
4: 83 ec 04 sub $0x4,%esp
7: a1 00 00 00 00 mov 0x0,%eax
c: 8b 5d 08 mov 0x8(%ebp),%ebx
# here we test the __warn_once first and if it is not zero
# it jumps to warn+0x20 to do the condition test
f: 85 c0 test %eax,%eax
11: 75 0d jne 20 <warn+0x20>
13: 5a pop %edx
14: 8d 43 01 lea 0x1(%ebx),%eax
17: 5b pop %ebx
18: 5d pop %ebp
19: c3 ret
1a: 8d b6 00 00 00 00 lea 0x0(%esi),%esi
20: 83 fb 01 cmp $0x1,%ebx
23: 75 ee jne 13 <warn+0x13>
25: 31 c9 xor %ecx,%ecx
27: 89 0d 00 00 00 00 mov %ecx,0x0
2d: c7 04 24 01 00 00 00 movl $0x1,(%esp)
34: e8 fc ff ff ff call 35 <warn+0x35>
39: eb d8 jmp 13 <warn+0x13>
Disassembly of section .data:
My suggested change of doing the condition first:
00000000 <warn>:
0: 55 push %ebp
1: 89 e5 mov %esp,%ebp
3: 53 push %ebx
4: 83 ec 04 sub $0x4,%esp
7: 8b 5d 08 mov 0x8(%ebp),%ebx
# here we test the condition first, and if it the
# unlikely condition is true, then we jump to test
# the __warn_once.
a: 83 fb 01 cmp $0x1,%ebx
d: 74 07 je 16 <warn+0x16>
f: 5a pop %edx
10: 8d 43 01 lea 0x1(%ebx),%eax
13: 5b pop %ebx
14: 5d pop %ebp
15: c3 ret
16: a1 00 00 00 00 mov 0x0,%eax
1b: 85 c0 test %eax,%eax
1d: 74 f0 je f <warn+0xf>
1f: 31 c9 xor %ecx,%ecx
21: 89 0d 00 00 00 00 mov %ecx,0x0
27: c7 04 24 01 00 00 00 movl $0x1,(%esp)
2e: e8 fc ff ff ff call 2f <warn+0x2f>
33: eb da jmp f <warn+0xf>
Disassembly of section .data:
As you can see, because the whole thing is unlikely, the first condition
is expected to fail. With the current WARN_ON logic, that means that
the __warn_once is expected to fail, but that's not the case. So on a
normal system where the WARN_ON_ONCE condition would never happen, you
are always branching. So simply reversing the order to test the
condition before testing the __warn_once variable should improve cache
performance.
Cc: Ingo Molnar <mingo@xxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
---
include/asm-generic/bug.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -puN include/asm-generic/bug.h~lock-validator-introduce-warn_on_oncecond-speedup include/asm-generic/bug.h
--- devel/include/asm-generic/bug.h~lock-validator-introduce-warn_on_oncecond-speedup 2006-06-03 17:12:41.000000000 -0700
+++ devel-akpm/include/asm-generic/bug.h 2006-06-03 17:12:41.000000000 -0700
@@ -43,7 +43,7 @@
static int __warn_once = 1; \
int __ret = 0; \
\
- if (unlikely(__warn_once && (condition))) { \
+ if (unlikely((condition) && __warn_once)) { \
__warn_once = 0; \
WARN_ON(1); \
__ret = 1; \
_
Patches currently in -mm which might be from rostedt@xxxxxxxxxxx are
jbd-avoid-kfree-null.patch
ext3_clear_inode-avoid-kfree-null.patch
lock-validator-introduce-warn_on_oncecond-speedup.patch
sched-comment-bitmap-size-accounting.patch
unnecessary-long-index-i-in-sched.patch
pi-futex-rt-mutex-docs.patch
pi-futex-rt-mutex-docs-update.patch
-
To unsubscribe from this list: send the line "unsubscribe mm-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
