From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: Andrew Morton <akpm@linux-foundation.org>,
David Daney <ddaney@caviumnetworks.com>,
linux-mips@linux-mips.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] MIPS: Make BUG() __noreturn.
Date: Mon, 24 Nov 2008 16:16:36 -0800 [thread overview]
Message-ID: <492B43E4.2020607@goop.org> (raw)
In-Reply-To: <20081123095818.GU30453@elte.hu>
Ingo Molnar wrote:
> * Andrew Morton <akpm@linux-foundation.org> wrote:
>
>
>>> +static inline void __noreturn BUG(void)
>>> +{
>>> + __asm__ __volatile__("break %0" : : "i" (BRK_BUG));
>>> + /* Fool GCC into thinking the function doesn't return. */
>>> + while (1)
>>> + ;
>>> +}
>>>
>> This kind of sucks, doesn't it? It adds instructions into the
>> kernel text, very frequently on fast paths. Those instructions are
>> never executed, and we're blowing away i-cache just to quash
>> compiler warnings.
>>
>> For example, this:
>>
>> --- a/arch/x86/include/asm/bug.h~a
>> +++ a/arch/x86/include/asm/bug.h
>> @@ -22,14 +22,12 @@ do { \
>> ".popsection" \
>> : : "i" (__FILE__), "i" (__LINE__), \
>> "i" (sizeof(struct bug_entry))); \
>> - for (;;) ; \
>> } while (0)
>>
>> #else
>> #define BUG() \
>> do { \
>> asm volatile("ud2"); \
>> - for (;;) ; \
>> } while (0)
>> #endif
>>
>> _
>>
>> reduces the size of i386 mm/vmalloc.o text by 56 bytes.
>>
>
> yes - the total image effect is significantly - recently looked at how
> much larger !CONFIG_BUG builds would get if we inserted an infinite
> loop into them - it was in the 50K text range (!).
>
> but in the x86 ud2 case we could guarantee that we wont ever return
> from that exception. Mind sending a patch with a signoff, a
> description and an infinite loop in the u2d handler?
>
There are two arguments against making BUG() a noreturn:
* if you compile without BUG enabled, then it won't be noreturn anyway
* making it noreturn kills the lifetime of any variables that would
otherwise be considered alive, making the DWARF debug info at that
point less reliable (which is a pain even for post-mortem debugging)
The counter-argument is that not making it noreturn will keep variables
alive that wouldn't otherwise be, causing greater register pressure,
spillage, etc.
If adding an infinite loop really adds 50k to the image, the extra size
must come from the changes to variable lifetime rather than the loop
instructions themselves (which are only 2 bytes per instance, and we
don't have 25,000 BUGs in the kernel, do we?).
J
next prev parent reply other threads:[~2008-11-25 0:16 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-11-21 1:26 [PATCH] MIPS: Make BUG() __noreturn David Daney
2008-11-21 10:00 ` Alan Cox
2008-11-21 10:27 ` Geert Uytterhoeven
2008-11-21 11:14 ` Maciej W. Rozycki
2008-11-21 12:58 ` Andreas Schwab
2008-11-21 16:40 ` David Daney
2008-11-21 18:46 ` Geert Uytterhoeven
2008-11-21 22:16 ` Ralf Baechle
2008-11-24 19:04 ` Maciej W. Rozycki
2008-11-21 23:00 ` Andrew Morton
2008-11-21 23:48 ` David Daney
2008-11-23 9:58 ` Ingo Molnar
2008-11-24 9:20 ` Ralf Baechle
2008-11-25 0:16 ` Jeremy Fitzhardinge [this message]
2008-11-22 9:39 ` Ralf Baechle
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=492B43E4.2020607@goop.org \
--to=jeremy@goop.org \
--cc=akpm@linux-foundation.org \
--cc=ddaney@caviumnetworks.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@linux-mips.org \
--cc=mingo@elte.hu \
/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