All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Arnd Bergmann <arnd@arndb.de>,
	linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Chen Gang <gang.chen@asianux.com>,
	Ingo Molnar <mingo@kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>
Subject: Re: [PATCH, re-send] Always trap on BUG()
Date: Mon, 15 Jul 2013 23:47:59 +0100	[thread overview]
Message-ID: <20130715224759.GZ24642@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <51E47924.9030005@zytor.com>

On Mon, Jul 15, 2013 at 03:35:16PM -0700, H. Peter Anvin wrote:
> On 07/15/2013 03:27 PM, Russell King - ARM Linux wrote:
> > On Mon, Jul 15, 2013 at 03:16:12PM -0700, Andrew Morton wrote:
> >> I've been thinking for a while that CONFIG_BUG=n is a pretty dumb thing
> >> to do, and that maintaining it (and trying to fix the warnings it
> >> produces) aren't worth the effort and that we should remove the whole
> >> thing.  Perhaps your patch changes that calculus, dunno.  Please discuss.
> > 
> > This isn't about introducing "CONFIG_BUG=n" - this is about making a
> > kernel with CONFIG_BUG=n build without producing tonnes and tonnes of
> > warnings, as it does today.  It makes building randconfig pretty
> > useless to find what could be more important warnings.
> > 
> 
> Well, there are three alternatives here, right:

There's many more than three alternatives - some of them may not pass
your taste filter.

> 1. We can use unreachable(), which means that the compiler can assume it
> never happens.

Arnd tried that... see the commit message for the build results from
that.  Arnd included a whole boat-load of other solutions to this
problem and documented the build results there too.

What this actually means in reality is that you're giving permission for
the CPU to run off the end of a function and start executing whatever it
finds there - which could be literal tables.  That might cause some
memory to be corrupted but otherwise go by unnoticed until very much
later.

Infinite while loops on the other hand... probably a single instruction,
but if we're using a single instruction why not make it trap with an
exception anyway so that the system can panic() and maybe reboot without
needing to wait for the watchdog?

> 2. We can trap without metadata.
> 
> 3. We can trap with metadata (current CONFIG_BUG=y).
> 
> I am *guessing* this does 2, but it isn't clear.

That is what this paragraph in Arnd's mail conveys to me:

  This basically loses any of the BUG() reporting, but leaves the
  logic to trap and kill the task in place when CONFIG_BUG is disabled.

WARNING: multiple messages have this Message-ID (diff)
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH, re-send] Always trap on BUG()
Date: Mon, 15 Jul 2013 23:47:59 +0100	[thread overview]
Message-ID: <20130715224759.GZ24642@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <51E47924.9030005@zytor.com>

On Mon, Jul 15, 2013 at 03:35:16PM -0700, H. Peter Anvin wrote:
> On 07/15/2013 03:27 PM, Russell King - ARM Linux wrote:
> > On Mon, Jul 15, 2013 at 03:16:12PM -0700, Andrew Morton wrote:
> >> I've been thinking for a while that CONFIG_BUG=n is a pretty dumb thing
> >> to do, and that maintaining it (and trying to fix the warnings it
> >> produces) aren't worth the effort and that we should remove the whole
> >> thing.  Perhaps your patch changes that calculus, dunno.  Please discuss.
> > 
> > This isn't about introducing "CONFIG_BUG=n" - this is about making a
> > kernel with CONFIG_BUG=n build without producing tonnes and tonnes of
> > warnings, as it does today.  It makes building randconfig pretty
> > useless to find what could be more important warnings.
> > 
> 
> Well, there are three alternatives here, right:

There's many more than three alternatives - some of them may not pass
your taste filter.

> 1. We can use unreachable(), which means that the compiler can assume it
> never happens.

Arnd tried that... see the commit message for the build results from
that.  Arnd included a whole boat-load of other solutions to this
problem and documented the build results there too.

What this actually means in reality is that you're giving permission for
the CPU to run off the end of a function and start executing whatever it
finds there - which could be literal tables.  That might cause some
memory to be corrupted but otherwise go by unnoticed until very much
later.

Infinite while loops on the other hand... probably a single instruction,
but if we're using a single instruction why not make it trap with an
exception anyway so that the system can panic() and maybe reboot without
needing to wait for the watchdog?

> 2. We can trap without metadata.
> 
> 3. We can trap with metadata (current CONFIG_BUG=y).
> 
> I am *guessing* this does 2, but it isn't clear.

That is what this paragraph in Arnd's mail conveys to me:

  This basically loses any of the BUG() reporting, but leaves the
  logic to trap and kill the task in place when CONFIG_BUG is disabled.

  reply	other threads:[~2013-07-15 22:48 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-05 15:38 [PATCH, re-send] Always trap on BUG() Arnd Bergmann
2013-07-05 15:38 ` Arnd Bergmann
2013-07-12 10:15 ` Ingo Molnar
2013-07-12 10:15   ` Ingo Molnar
2013-07-15 22:16 ` Andrew Morton
2013-07-15 22:16   ` Andrew Morton
2013-07-15 22:27   ` Russell King - ARM Linux
2013-07-15 22:27     ` Russell King - ARM Linux
2013-07-15 22:35     ` H. Peter Anvin
2013-07-15 22:35       ` H. Peter Anvin
2013-07-15 22:35       ` H. Peter Anvin
2013-07-15 22:47       ` Russell King - ARM Linux [this message]
2013-07-15 22:47         ` Russell King - ARM Linux
2013-07-23  9:36       ` Ingo Molnar
2013-07-23  9:36         ` Ingo Molnar
2013-07-23  9:54         ` H. Peter Anvin
2013-07-23  9:54           ` H. Peter Anvin
2013-07-23  9:54           ` H. Peter Anvin
2013-07-24  0:00         ` Chen Gang
2013-07-24  0:00           ` Chen Gang
2013-07-25 22:05           ` Ingo Molnar
2013-07-25 22:05             ` Ingo Molnar
2013-07-26  1:05             ` Chen Gang
2013-07-26  1:05               ` Chen Gang

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=20130715224759.GZ24642@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=ebiederm@xmission.com \
    --cc=gang.chen@asianux.com \
    --cc=geert@linux-m68k.org \
    --cc=hpa@zytor.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.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.