+ lock-validator-introduce-warn_on_oncecond-speedup.patch added to -mm tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Kernel Newbies FAQ]     [Kernel Archive]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Photo]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]

  Powered by Linux