public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Arjan van de Ven <arjan@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org, torvalds@linux-foundation.org,
	bp@alien8.de, jpoimboe@redhat.com, richard.weinberger@gmail.com
Subject: Re: [PATCH] x86: Implement __WARN using UD0
Date: Thu, 23 Feb 2017 15:57:51 +0100	[thread overview]
Message-ID: <20170223145751.GE6515@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <c82a7090-7894-2569-fec5-c628e04ab120@linux.intel.com>

On Thu, Feb 23, 2017 at 06:14:45AM -0800, Arjan van de Ven wrote:
> On 2/23/2017 5:28 AM, Peter Zijlstra wrote:
> >
> >By using "UD0" for WARNs we remove the function call and its possible
> >__FILE__ and __LINE__ immediate arguments from the instruction stream.
> >
> >Total image size will not change much, what we win in the instruction
> >stream we'll loose because of the __bug_table entries. Still, saves on
> >I$ footprint and the total image size does go down a bit.
> 
> well I am a little sceptical; WARNs are rare so the code (other than the test)
> should be waaay out of line already (unlikely() and co).

There's only so much you can do in small functions. Sure it tries to
move the crud to the end, but at the end of the day, its still in the
same function.

> And I assume you're not removing the __FILE__ and __LINE__ info, since that info
> is actually high value for us developers... so what are you actually saving?

OK, so going by my own numbers the total image size does not in fact go
down (it did earlier when I initially wrote this patch) :/

I think back when I wrote this I had refcount_t generate inline WARNs
and that generates a huge amount of junk all over the place. This very
much did clear much of that up.

And as said; there's only so much you can do in small functions.

Look at the below for example (frobbed the code to do WARN_ON instead of
WARN), depending on alignment the WARN code will be in the same
cacheline as 'normal' code.


0000000000000016 <refcount_add_not_zero>:
  16:   55                      push   %rbp
  17:   8b 16                   mov    (%rsi),%edx
  19:   41 83 c8 ff             or     $0xffffffff,%r8d
  1d:   48 89 e5                mov    %rsp,%rbp
  20:   85 d2                   test   %edx,%edx
  22:   74 25                   je     49 <refcount_add_not_zero+0x33>
  24:   83 fa ff                cmp    $0xffffffff,%edx
  27:   74 1c                   je     45 <refcount_add_not_zero+0x2f>
  29:   89 d1                   mov    %edx,%ecx
  2b:   89 d0                   mov    %edx,%eax
  2d:   01 f9                   add    %edi,%ecx
  2f:   41 0f 42 c8             cmovb  %r8d,%ecx
  33:   f0 0f b1 0e             lock cmpxchg %ecx,(%rsi)
  37:   39 c2                   cmp    %eax,%edx
  39:   74 04                   je     3f <refcount_add_not_zero+0x29>
  3b:   89 c2                   mov    %eax,%edx
  3d:   eb e1                   jmp    20 <refcount_add_not_zero+0xa>
  3f:   ff c1                   inc    %ecx
  41:   75 02                   jne    45 <refcount_add_not_zero+0x2f>
  43:   0f ff                   (bad)  
  45:   b0 01                   mov    $0x1,%al
  47:   eb 02                   jmp    4b <refcount_add_not_zero+0x35>
  49:   31 c0                   xor    %eax,%eax
  4b:   5d                      pop    %rbp
  4c:   c3                      retq  



0000000000000016 <refcount_add_not_zero>:
  16:   8b 16                   mov    (%rsi),%edx
  18:   41 83 c8 ff             or     $0xffffffff,%r8d
  1c:   85 d2                   test   %edx,%edx
  1e:   74 3b                   je     5b <refcount_add_not_zero+0x45>
  20:   83 fa ff                cmp    $0xffffffff,%edx
  23:   75 03                   jne    28 <refcount_add_not_zero+0x12>
  25:   b0 01                   mov    $0x1,%al
  27:   c3                      retq   
  28:   89 d1                   mov    %edx,%ecx
  2a:   89 d0                   mov    %edx,%eax
  2c:   01 f9                   add    %edi,%ecx
  2e:   41 0f 42 c8             cmovb  %r8d,%ecx
  32:   f0 0f b1 0e             lock cmpxchg %ecx,(%rsi)
  36:   39 c2                   cmp    %eax,%edx
  38:   74 04                   je     3e <refcount_add_not_zero+0x28>
  3a:   89 c2                   mov    %eax,%edx
  3c:   eb de                   jmp    1c <refcount_add_not_zero+0x6>
  3e:   ff c1                   inc    %ecx
  40:   75 e3                   jne    25 <refcount_add_not_zero+0xf>
  42:   55                      push   %rbp
  43:   be 3f 00 00 00          mov    $0x3f,%esi
  48:   48 c7 c7 00 00 00 00    mov    $0x0,%rdi
                        4b: R_X86_64_32S        .rodata.str1.1
  4f:   48 89 e5                mov    %rsp,%rbp
  52:   e8 00 00 00 00          callq  57 <refcount_add_not_zero+0x41>
                        53: R_X86_64_PC32       warn_slowpath_null-0x4
  57:   b0 01                   mov    $0x1,%al
  59:   5d                      pop    %rbp
  5a:   c3                      retq   
  5b:   31 c0                   xor    %eax,%eax
  5d:   c3                      retq   

  reply	other threads:[~2017-02-23 14:58 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-23 13:28 [PATCH] x86: Implement __WARN using UD0 Peter Zijlstra
2017-02-23 14:09 ` Peter Zijlstra
2017-02-23 14:59   ` Peter Zijlstra
2017-02-23 15:09   ` hpa
2017-02-23 15:23     ` Peter Zijlstra
2017-02-23 15:32       ` hpa
2017-02-23 16:03         ` Borislav Petkov
2017-02-23 14:12 ` Josh Poimboeuf
2017-02-23 14:17   ` Borislav Petkov
2017-02-23 14:36   ` Peter Zijlstra
2017-02-23 14:14 ` Arjan van de Ven
2017-02-23 14:57   ` Peter Zijlstra [this message]
2017-02-24  7:43     ` Ingo Molnar
2017-02-24  8:31       ` Peter Zijlstra
2017-02-24  9:06         ` hpa
2017-02-24  9:11         ` hpa
2017-02-24  9:15           ` Ingo Molnar
2017-02-24  9:46             ` Borislav Petkov
2017-02-24  9:52               ` H. Peter Anvin
     [not found]             ` <21ac6e53-2fcd-52e9-e72d-9faf7da14d1e@zytor.com>
2017-02-24 10:41               ` Peter Zijlstra
2017-02-25 10:38                 ` Borislav Petkov
2017-02-25 17:55                   ` hpa
2017-02-25 19:38                     ` Borislav Petkov
2017-02-25 20:04                       ` hpa
2017-02-25 20:29                         ` Borislav Petkov
2017-02-24 11:16 ` [PATCH -v2] " Peter Zijlstra
2017-02-25  8:19   ` [RFC][PATCH] bug: Add _ONCE logic to report_bug() Peter Zijlstra
2017-02-25  9:18     ` Ingo Molnar
2017-02-25  9:44       ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170223145751.GE6515@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=arjan@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=richard.weinberger@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox