[PATCH] kernel/sched/core.c: need return NULL when BUG() is defined as empty.

[PATCH] kernel/sched/core.c: need return NULL when BUG() is defined as empty.

[Chen Gang]

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(-)

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;
 }
 
 /*
-- 
1.7.7.6

Re: [PATCH] kernel/sched/core.c: need return NULL when BUG() is defined as empty.

[Peter Zijlstra]

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?

Re: [PATCH] kernel/sched/core.c: need return NULL when BUG() is defined as empty.

[Chen Gang]

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.
-- 
Chen Gang

Asianux Corporation

Re: [PATCH] kernel/sched/core.c: need return NULL when BUG() is defined as empty.

[Russell King - ARM Linux]

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.

Re: [PATCH] kernel/sched/core.c: need return NULL when BUG() is defined as empty.

[Peter Zijlstra]

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. 

Re: [PATCH] kernel/sched/core.c: need return NULL when BUG() is defined as empty.

[Chen Gang]

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.
-- 
Chen Gang

Asianux Corporation