All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: David Daney <ddaney@caviumnetworks.com>
Cc: linux-mips@linux-mips.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] MIPS: Make BUG() __noreturn.
Date: Fri, 21 Nov 2008 15:00:23 -0800	[thread overview]
Message-ID: <20081121150023.032f7b5b.akpm@linux-foundation.org> (raw)
In-Reply-To: <49260E4C.8080500@caviumnetworks.com>

On Thu, 20 Nov 2008 17:26:36 -0800
David Daney <ddaney@caviumnetworks.com> wrote:

> MIPS: Make BUG() __noreturn.
> 
> Often we do things like put BUG() in the default clause of a case
> statement.  Since it was not declared __noreturn, this could sometimes
> lead to bogus compiler warnings that variables were used
> uninitialized.
> 
> There is a small problem in that we have to put a magic while(1); loop to
> fool GCC into really thinking it is noreturn.  This makes the new
> BUG() function 3 instructions long instead of just 1, but I think it
> is worth it as it is now unnecessary to do extra work to silence the
> 'used uninitialized' warnings.
> 
> I also re-wrote BUG_ON so that if it is given a constant condition, it
> just does BUG() instead of loading a constant value in to a register
> and testing it.
> 

Yup, this change will fix some compile warnings which will never be
fixed in any other way for mips.

> +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.

I wonder if there is any clever way in which we can do this without
introducing additional runtime cost.

  parent reply	other threads:[~2008-11-21 23:01 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 [this message]
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
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=20081121150023.032f7b5b.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=ddaney@caviumnetworks.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.