| Message ID | 5199D565.4070307@asianux.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 20, 2013 at 03:48:53PM +0800, Chen Gang wrote: > > When neither CONFIG_BUG nor HAVE_ARCH_BUG is defined, need let function > return failure value ('NULL') instead of random value. What will such a kernel do? Happily continue running whenever we hit a BUG? that seems like a particularly bad idea. Should we not have a stub BUG() function like: void BUG(void) __attribute__((noreturn)) { local_irq_disable(); while (1) ; } Which would at least halt things?
On 05/22/2013 05:11 PM, Peter Zijlstra wrote: > On Mon, May 20, 2013 at 03:48:53PM +0800, Chen Gang wrote: >> > >> > When neither CONFIG_BUG nor HAVE_ARCH_BUG is defined, need let function >> > return failure value ('NULL') instead of random value. > What will such a kernel do? Happily continue running whenever we hit a > BUG? that seems like a particularly bad idea. Should we not have a stub > BUG() function like: > > void BUG(void) __attribute__((noreturn)) > { > local_irq_disable(); > while (1) ; > } > > Which would at least halt things? > > At least for me, it is a good idea. :-) In menuconfig we can set !CONFIG_BUG and !HAVE_ARCH_BUG manually under any architectures: "> General setup > Configure standard kernel features (expert users) > BUG() Support" So I think, we really need your patch. Thanks.
On Wed, May 22, 2013 at 11:11:56AM +0200, Peter Zijlstra wrote: > On Mon, May 20, 2013 at 03:48:53PM +0800, Chen Gang wrote: > > > > When neither CONFIG_BUG nor HAVE_ARCH_BUG is defined, need let function > > return failure value ('NULL') instead of random value. > > What will such a kernel do? Happily continue running whenever we hit a > BUG? that seems like a particularly bad idea. Should we not have a stub > BUG() function like: > > void BUG(void) __attribute__((noreturn)) > { > local_irq_disable(); > while (1) ; > } Eww. So you've a platform where you have things like panic_on_oops enabled, and you hit this bug... do we really want to just stop? Wouldn't replacing BUG() with panic("BUG"); be better ? But, this begs the question - what is the point of being able to turn off BUG() ? As BUG() on any sensible architecture is implemented by placing the minimum of code at the callsite (eg, one instruction if not using verbose) anything like the above is likely to be bigger. So, I'd actually argue that rather than trying to "fix" this, get rid of CONFIG_BUG and make it always enabled everywhere - just like what has recently been done with hotplug.
On Wed, May 22, 2013 at 02:33:17PM +0100, Russell King - ARM Linux wrote: > On Wed, May 22, 2013 at 11:11:56AM +0200, Peter Zijlstra wrote: > > On Mon, May 20, 2013 at 03:48:53PM +0800, Chen Gang wrote: > > > > > > When neither CONFIG_BUG nor HAVE_ARCH_BUG is defined, need let function > > > return failure value ('NULL') instead of random value. > > > > What will such a kernel do? Happily continue running whenever we hit a > > BUG? that seems like a particularly bad idea. Should we not have a stub > > BUG() function like: > > > > void BUG(void) __attribute__((noreturn)) > > { > > local_irq_disable(); > > while (1) ; > > } > > Eww. So you've a platform where you have things like panic_on_oops > enabled, and you hit this bug... do we really want to just stop? > Wouldn't replacing BUG() with panic("BUG"); be better ? > > But, this begs the question - what is the point of being able to turn > off BUG() ? As BUG() on any sensible architecture is implemented by > placing the minimum of code at the callsite (eg, one instruction if > not using verbose) anything like the above is likely to be bigger. > > So, I'd actually argue that rather than trying to "fix" this, get rid > of CONFIG_BUG and make it always enabled everywhere - just like what > has recently been done with hotplug. Works for me.
On 05/23/2013 12:19 AM, Peter Zijlstra wrote: > On Wed, May 22, 2013 at 02:33:17PM +0100, Russell King - ARM Linux wrote: >> > On Wed, May 22, 2013 at 11:11:56AM +0200, Peter Zijlstra wrote: >>> > > On Mon, May 20, 2013 at 03:48:53PM +0800, Chen Gang wrote: >>>> > > > >>>> > > > When neither CONFIG_BUG nor HAVE_ARCH_BUG is defined, need let function >>>> > > > return failure value ('NULL') instead of random value. >>> > > >>> > > What will such a kernel do? Happily continue running whenever we hit a >>> > > BUG? that seems like a particularly bad idea. Should we not have a stub >>> > > BUG() function like: >>> > > >>> > > void BUG(void) __attribute__((noreturn)) >>> > > { >>> > > local_irq_disable(); >>> > > while (1) ; >>> > > } >> > >> > Eww. So you've a platform where you have things like panic_on_oops >> > enabled, and you hit this bug... do we really want to just stop? >> > Wouldn't replacing BUG() with panic("BUG"); be better ? >> > >> > But, this begs the question - what is the point of being able to turn >> > off BUG() ? As BUG() on any sensible architecture is implemented by >> > placing the minimum of code at the callsite (eg, one instruction if >> > not using verbose) anything like the above is likely to be bigger. >> > >> > So, I'd actually argue that rather than trying to "fix" this, get rid >> > of CONFIG_BUG and make it always enabled everywhere - just like what >> > has recently been done with hotplug. > Works for me. > Thanks all, I should send the related patch for it. Excuse me, I have to do another things, so I will finish it within this week (2013-05-26) Welcome any additional suggestions and completions. Thanks.
diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 9ff7744..94ef8ff 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2350,6 +2350,7 @@ pick_next_task(struct rq *rq) } BUG(); /* the idle class will always have a runnable task */ + return NULL; } /*
When neither CONFIG_BUG nor HAVE_ARCH_BUG is defined, need let function return failure value ('NULL') instead of random value. It is generated by randconfig with MMU for arm s5pv210 with EXTRA_CFALGS=-W. The related warnings: kernel/sched/core.c:2353:1: warning: control reaches end of non-void function [-Wreturn-type] Signed-off-by: Chen Gang <gang.chen@asianux.com> --- kernel/sched/core.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)