All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: David Miller <davem@davemloft.net>
Cc: mingo@elte.hu, sfr@canb.auug.org.au, peterz@infradead.org,
	fweisbec@gmail.com, linux-kernel@vger.kernel.org,
	linux-next@vger.kernel.org, hpa@zytor.com, tglx@linutronix.de,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: linux-next: PowerPC WARN_ON_ONCE() after merge of the final tree (tip related)
Date: Fri, 16 Apr 2010 11:56:56 +1000	[thread overview]
Message-ID: <1271383016.13059.173.camel@pasglop> (raw)
In-Reply-To: <20100414.235557.123118153.davem@davemloft.net>

On Wed, 2010-04-14 at 23:55 -0700, David Miller wrote:
> From: Ingo Molnar <mingo@elte.hu>
> Date: Thu, 15 Apr 2010 08:49:40 +0200
> 
> > Btw., WARN_ON trapping on PowerPC is clearly a PowerPC bug - there's a good 
> > reason we have WARN_ON versus BUG_ON - it should be fixed.
> 
> I disagree, an implementation should be allowed to use the most
> efficient implementation possible for both interfaces.

Right, and I don't think the reason why we have WARN_ON vs. BUG_ON ever
had anything to do with whether it's implemented with a trap or not :-) 

It's purely related to whether it's supposed to be fatal or not. Now,
there is indeed the potential problem you mention of WARN_ON being
called in places where such a trap is unsafe, but so far, this is the
first time I can remember we hit that problem so we've been getting away
with it for quite a while :-)

Now, whether the trap is or is not more efficient than an explicit test
is something that is still being debated on powerpc. It has the
advantage of not un-leafing functions (and thus not creating stack
frames, adding register reloads, etc... when not needed), basically
putting the burden of saving/restoring registers to the (hopefully) rare
path of actually taking the WARN/BUG.

We could probably manufacture something similar with careful use of
inline asm and an out of line asm trampoline.

The benefit of the trap instruction vs. conditional branches per-se is
probably nil. It's really more about the codegen impact, register
clobber due to the added function call, etc.. at least for us.

Cheers,
Ben.

> I would be using traps for both on sparc64 if that were really
> feasible on sparc64 (and actually with gcc-4.5's "asm goto" it might
> actually be now)
> 
> The WARN and BUG macros, when implemented without traps, have serious
> implications for overall code size and register pressure.
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: David Miller <davem@davemloft.net>
Cc: sfr@canb.auug.org.au, peterz@infradead.org, fweisbec@gmail.com,
	linux-kernel@vger.kernel.org, linux-next@vger.kernel.org,
	hpa@zytor.com, mingo@elte.hu, linuxppc-dev@lists.ozlabs.org,
	tglx@linutronix.de
Subject: Re: linux-next: PowerPC WARN_ON_ONCE() after merge of the final tree (tip related)
Date: Fri, 16 Apr 2010 11:56:56 +1000	[thread overview]
Message-ID: <1271383016.13059.173.camel@pasglop> (raw)
In-Reply-To: <20100414.235557.123118153.davem@davemloft.net>

On Wed, 2010-04-14 at 23:55 -0700, David Miller wrote:
> From: Ingo Molnar <mingo@elte.hu>
> Date: Thu, 15 Apr 2010 08:49:40 +0200
> 
> > Btw., WARN_ON trapping on PowerPC is clearly a PowerPC bug - there's a good 
> > reason we have WARN_ON versus BUG_ON - it should be fixed.
> 
> I disagree, an implementation should be allowed to use the most
> efficient implementation possible for both interfaces.

Right, and I don't think the reason why we have WARN_ON vs. BUG_ON ever
had anything to do with whether it's implemented with a trap or not :-) 

It's purely related to whether it's supposed to be fatal or not. Now,
there is indeed the potential problem you mention of WARN_ON being
called in places where such a trap is unsafe, but so far, this is the
first time I can remember we hit that problem so we've been getting away
with it for quite a while :-)

Now, whether the trap is or is not more efficient than an explicit test
is something that is still being debated on powerpc. It has the
advantage of not un-leafing functions (and thus not creating stack
frames, adding register reloads, etc... when not needed), basically
putting the burden of saving/restoring registers to the (hopefully) rare
path of actually taking the WARN/BUG.

We could probably manufacture something similar with careful use of
inline asm and an out of line asm trampoline.

The benefit of the trap instruction vs. conditional branches per-se is
probably nil. It's really more about the codegen impact, register
clobber due to the added function call, etc.. at least for us.

Cheers,
Ben.

> I would be using traps for both on sparc64 if that were really
> feasible on sparc64 (and actually with gcc-4.5's "asm goto" it might
> actually be now)
> 
> The WARN and BUG macros, when implemented without traps, have serious
> implications for overall code size and register pressure.
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

  parent reply	other threads:[~2010-04-16  1:58 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-15  6:12 linux-next: boot failure after merge of the final tree (tip related) Stephen Rothwell
2010-04-15  6:12 ` Stephen Rothwell
2010-04-15  6:48 ` Benjamin Herrenschmidt
2010-04-15  6:48   ` Benjamin Herrenschmidt
2010-04-15  6:49 ` linux-next: PowerPC WARN_ON_ONCE() " Ingo Molnar
2010-04-15  6:49   ` Ingo Molnar
2010-04-15  6:55   ` David Miller
2010-04-15  6:55     ` David Miller
2010-04-15  7:32     ` Ingo Molnar
2010-04-15  7:32       ` Ingo Molnar
2010-04-16  1:51       ` Benjamin Herrenschmidt
2010-04-16  1:51         ` Benjamin Herrenschmidt
2010-04-16  1:56     ` Benjamin Herrenschmidt [this message]
2010-04-16  1:56       ` Benjamin Herrenschmidt
2010-04-15 10:04   ` Stephen Rothwell
2010-04-15 10:04     ` Stephen Rothwell
2010-04-15 13:37     ` [PATCH] lockdep: Fix redundant_hardirqs_on incremented with irqs enabled Frederic Weisbecker
2010-04-27  7:32       ` Stephen Rothwell
2010-04-27 18:07         ` Frederic Weisbecker
2010-04-28  7:12       ` Stephen Rothwell
2010-04-15 13:00   ` linux-next: PowerPC WARN_ON_ONCE() after merge of the final tree (tip related) Frederic Weisbecker
2010-04-15 13:00     ` Frederic Weisbecker
2010-04-15 14:03     ` Ingo Molnar
2010-04-15 14:03       ` Ingo Molnar
2010-04-15 17:15       ` Frederic Weisbecker
2010-04-15 17:15         ` Frederic Weisbecker
2010-04-16 10:38         ` Peter Zijlstra
2010-04-16 10:38           ` Peter Zijlstra
2010-04-16 12:32           ` Frederic Weisbecker
2010-04-16 12:32             ` Frederic Weisbecker
2010-04-15 17:24       ` Frederic Weisbecker
2010-04-15 17:24         ` Frederic Weisbecker
2010-04-15 17:39         ` Ingo Molnar
2010-04-15 17:39           ` Ingo Molnar

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=1271383016.13059.173.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=davem@davemloft.net \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=sfr@canb.auug.org.au \
    --cc=tglx@linutronix.de \
    /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.