kernel: stop_machine: report (un)dead code (and feedback request)

kernel: stop_machine: report (un)dead code (and feedback request)

[Valentin Rothberg]

Hi Chris,

your commit 4c477de14237 ("kernel: remove stop_machine() Kconfig
dependency") has shown up in today's linux-next tree (20151126).
The commit changes the #ifdef condition in kernel/stop_machine.c
from/to:

    -#if defined(CONFIG_STOP_MACHINE) && defined(CONFIG_SMP)
    +#if defined(CONFIG_SMP) || defined(CONFIG_HOTPLUG_CPU)

Although this change fixes certain configs on X86, the condition now is
a tautology since CONFIG_SMP is already required to compile the file:

    kernel/Makefile:65:obj-$(CONFIG_SMP) += stop_machine.o

AFAIU, we can safely remove this #ifdef?

I detected this issue with undertaker-checkpatch [1].  We're a research
group at the University of Erlangen-Nuremberg, running daily checks on
linux-next to report Kconfig related issues, such as dead and undead
code, or references on undefined symbols.

One of the students, Patrick, who writes his Bachelor's thesis in our
group implemented a web-framework to visualize kernel code w.r.t. CPP
#ifdef statements and the bugs our tools [1] detect.  You can find the
visualization of the mentioned issue at [2].  The description of the
tautology in the #ifdef condition can be found by clicking on the
'details' menu of the block.

We'll be happy to receive any kind of feedback of the recipients of this
mail.  Do you like/dislike the visualization?  Does it help you to
understand the described issue? etc.  The main purpose of the framework
is to aid when sending reports.  It's much easier and nicer to point to
a website visualizing a problem than just describing it textually.

Kind regards,
 Valentin

[1] https://undertaker.cs.fau.de
[2] http://cados.cs.fau.de:4567/4c477de14237193c99329ca82965c1211b4ee81e/kernel/stop_machine.c

Re: kernel: stop_machine: report (un)dead code (and feedback request)

[Chris Wilson]

On Thu, Nov 26, 2015 at 03:45:59PM +0100, Valentin Rothberg wrote:
> Hi Chris,
> 
> your commit 4c477de14237 ("kernel: remove stop_machine() Kconfig
> dependency") has shown up in today's linux-next tree (20151126).
> The commit changes the #ifdef condition in kernel/stop_machine.c
> from/to:
> 
>     -#if defined(CONFIG_STOP_MACHINE) && defined(CONFIG_SMP)
>     +#if defined(CONFIG_SMP) || defined(CONFIG_HOTPLUG_CPU)
> 
> Although this change fixes certain configs on X86, the condition now is
> a tautology since CONFIG_SMP is already required to compile the file:
> 
>     kernel/Makefile:65:obj-$(CONFIG_SMP) += stop_machine.o
> 
> AFAIU, we can safely remove this #ifdef?

That seems logical. The argument in favour of it would be to keep the
ifdeffery around the function defintion the same as the function
declaration in stop_machine.h.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: kernel: stop_machine: report (un)dead code (and feedback request)

[Ingo Molnar]

* Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Thu, Nov 26, 2015 at 03:45:59PM +0100, Valentin Rothberg wrote:
> > Hi Chris,
> > 
> > your commit 4c477de14237 ("kernel: remove stop_machine() Kconfig
> > dependency") has shown up in today's linux-next tree (20151126).
> > The commit changes the #ifdef condition in kernel/stop_machine.c
> > from/to:
> > 
> >     -#if defined(CONFIG_STOP_MACHINE) && defined(CONFIG_SMP)
> >     +#if defined(CONFIG_SMP) || defined(CONFIG_HOTPLUG_CPU)
> > 
> > Although this change fixes certain configs on X86, the condition now is
> > a tautology since CONFIG_SMP is already required to compile the file:
> > 
> >     kernel/Makefile:65:obj-$(CONFIG_SMP) += stop_machine.o
> > 
> > AFAIU, we can safely remove this #ifdef?
> 
> That seems logical. The argument in favour of it would be to keep the
> ifdeffery around the function defintion the same as the function
> declaration in stop_machine.h.

But this would introduce a bit of fragility: we could re-introduce the same 
regression that the commit fixes, if we ever changed the SMP dependency for 
stop_machine.c.

Thanks,

	Ingo