Re: [PATCH tip/core/rcu 8/9] nohz_full: Add full-system-idle state machine

Re: [PATCH tip/core/rcu 8/9] nohz_full: Add full-system-idle state machine

[Geert Uytterhoeven]

On Tue, Aug 20, 2013 at 4:47 AM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> --- a/kernel/time/Kconfig
> +++ b/kernel/time/Kconfig
> @@ -157,6 +157,33 @@ config NO_HZ_FULL_SYSIDLE
>
>          Say N if you are unsure.
>
> +config NO_HZ_FULL_SYSIDLE_SMALL
> +       int "Number of CPUs above which large-system approach is used"
> +       depends on NO_HZ_FULL_SYSIDLE
> +       range 1 NR_CPUS

This causes "kernel/time/Kconfig:162:warning: range is invalid" on m68k and
all other architectures that do not support SMP.

How to reproduce:
    make ARCH=m68k defconfig

Furthermore, it seems only hexagon, metag, mips, and x86 set NR_CPUS to 1
if !SMP. On other architectures, NR_CPUS is not defined and presumed to be 0.

Hence in non-interactive configs (e.g. "make defconfig"),
NO_HZ_FULL_SYSIDLE_SMALL will end up as 0.
In interactive configs (e.g. "make oldconfig") Kconfig suggest "0" as
the default,
but refuses to accept it as it doesn't fall within the range 1..0.

How to reproduce:
    Remove the "depends on NO_HZ_FULL_SYSIDLE"
    make ARCH=powerpc mpc83xx_defconfig
    grep NO_HZ_FULL_SYSIDLE_SMALL .config
    -> CONFIG_NO_HZ_FULL_SYSIDLE_SMALL=0
    sed 's/CONFIG_NO_HZ_FULL_SYSIDLE_SMALL=0//g' -i .config
    make ARCH=powerpc oldconfig
    -> no value is accepted

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH tip/core/rcu 8/9] nohz_full: Add full-system-idle state machine

[Paul E. McKenney]

On Fri, Sep 06, 2013 at 10:08:22AM +0200, Geert Uytterhoeven wrote:
> On Tue, Aug 20, 2013 at 4:47 AM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > --- a/kernel/time/Kconfig
> > +++ b/kernel/time/Kconfig
> > @@ -157,6 +157,33 @@ config NO_HZ_FULL_SYSIDLE
> >
> >          Say N if you are unsure.
> >
> > +config NO_HZ_FULL_SYSIDLE_SMALL
> > +       int "Number of CPUs above which large-system approach is used"
> > +       depends on NO_HZ_FULL_SYSIDLE
> > +       range 1 NR_CPUS
> 
> This causes "kernel/time/Kconfig:162:warning: range is invalid" on m68k and
> all other architectures that do not support SMP.
> 
> How to reproduce:
>     make ARCH=m68k defconfig

OK, this does complain, but seems to give a reasonable .config file.
(From what I can tell.)  It would clearly be good to get rid of the
complaint.

> Furthermore, it seems only hexagon, metag, mips, and x86 set NR_CPUS to 1
> if !SMP. On other architectures, NR_CPUS is not defined and presumed to be 0.

Would it make sense to require that NR_CPUS=1 for !SMP?

> Hence in non-interactive configs (e.g. "make defconfig"),
> NO_HZ_FULL_SYSIDLE_SMALL will end up as 0.
> In interactive configs (e.g. "make oldconfig") Kconfig suggest "0" as
> the default,
> but refuses to accept it as it doesn't fall within the range 1..0.
> 
> How to reproduce:
>     Remove the "depends on NO_HZ_FULL_SYSIDLE"
>     make ARCH=powerpc mpc83xx_defconfig
>     grep NO_HZ_FULL_SYSIDLE_SMALL .config
>     -> CONFIG_NO_HZ_FULL_SYSIDLE_SMALL=0
>     sed 's/CONFIG_NO_HZ_FULL_SYSIDLE_SMALL=0//g' -i .config
>     make ARCH=powerpc oldconfig
>     -> no value is accepted

If it turns out that there is some reason by NR_CPUS=1 is impossible,
there are a few things that I could do:

I could just leave the range off, which would allow people to give
nonsense values.  This would be harmless in the code, for example,
a negative value would simply disable small-system handling, while
a too-large value would similarly disable large-system handling.
Might be a bit obnoxious for the guy who typoed and then wasted a
kernel build/boot/test cycle, but it is an option.

I could use a small fixed range (say from 1 to 64), which would
provide at least some checking.  In the unlikely event that someone
really wants more than 64 CPUs handled with small-system handling,
we could revisit at that point.

I tried creating a NR_CPUS_REALLY as follows:

config NR_CPUS_REALLY
	int "Fixed version of NR_CPUS"
	default NR_CPUS if NR_CPUS
	default 1 if !NR_CPUS

But this still gave a warning on the first "default" even though it
was not in effect.  I also tried using Kconfig "if":

if SMP
config NR_CPUS_REALLY
	int "Fixed version of NR_CPUS"
	default NR_CPUS
endif
if !SMP
config NR_CPUS_REALLY
	int "Fixed version of NR_CPUS"
	default 1 if !SMP
endif

However, Kconfig complained about the use of NR_CPUS even though this
was under an "if" whose condition was not set.  Perhaps someone with
better Kconfig-fu than I have can come up with something.

Defining NR_CPUS=1 if !SMP is looking pretty good to me just now.
This would probably have other benefits -- I cannot be the only
person who ever wanted this.  ;-)

							Thanx, Paul

Re: [PATCH tip/core/rcu 8/9] nohz_full: Add full-system-idle state machine

[Geert Uytterhoeven]

On Fri, Sep 6, 2013 at 7:30 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
>> Furthermore, it seems only hexagon, metag, mips, and x86 set NR_CPUS to 1
>> if !SMP. On other architectures, NR_CPUS is not defined and presumed to be 0.
>
> Would it make sense to require that NR_CPUS=1 for !SMP?

Yes, this looks reasonable to me.

> I tried creating a NR_CPUS_REALLY as follows:
>
> config NR_CPUS_REALLY
>         int "Fixed version of NR_CPUS"
>         default NR_CPUS if NR_CPUS
>         default 1 if !NR_CPUS
>
> But this still gave a warning on the first "default" even though it
> was not in effect.  I also tried using Kconfig "if":

IIRC, it tries to use the first default first, so the below may work
(the "if SMP" is probably not needed):

config NR_CPUS_REALLY
        int "Fixed version of NR_CPUS"
        default 1 if !SMP
        default NR_CPUS if SMP

> Defining NR_CPUS=1 if !SMP is looking pretty good to me just now.
> This would probably have other benefits -- I cannot be the only
> person who ever wanted this.  ;-)

Sure. I just didn't want to create patches for all architectures without
having a discussion first.

And it would be nice if it cuould be done in a central place, without
touching all architectures.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH tip/core/rcu 8/9] nohz_full: Add full-system-idle state machine

[Paul E. McKenney]

On Fri, Sep 06, 2013 at 08:50:41PM +0200, Geert Uytterhoeven wrote:
> On Fri, Sep 6, 2013 at 7:30 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> >> Furthermore, it seems only hexagon, metag, mips, and x86 set NR_CPUS to 1
> >> if !SMP. On other architectures, NR_CPUS is not defined and presumed to be 0.
> >
> > Would it make sense to require that NR_CPUS=1 for !SMP?
> 
> Yes, this looks reasonable to me.
> 
> > I tried creating a NR_CPUS_REALLY as follows:
> >
> > config NR_CPUS_REALLY
> >         int "Fixed version of NR_CPUS"
> >         default NR_CPUS if NR_CPUS
> >         default 1 if !NR_CPUS
> >
> > But this still gave a warning on the first "default" even though it
> > was not in effect.  I also tried using Kconfig "if":
> 
> IIRC, it tries to use the first default first, so the below may work
> (the "if SMP" is probably not needed):
> 
> config NR_CPUS_REALLY
>         int "Fixed version of NR_CPUS"
>         default 1 if !SMP
>         default NR_CPUS if SMP

Seemed like a good idea, but I still get:

	make O=/tmp/e ARCH=m68k defconfig
	  GEN     /tmp/e/Makefile
	*** Default configuration is based on 'multi_defconfig'
	kernel/time/Kconfig:140:warning: 'NR_CPUS_REALLY': number is invalid
	#
	# configuration written to .config
	#

Diff below in case I messed something up.

> > Defining NR_CPUS=1 if !SMP is looking pretty good to me just now.
> > This would probably have other benefits -- I cannot be the only
> > person who ever wanted this.  ;-)
> 
> Sure. I just didn't want to create patches for all architectures without
> having a discussion first.
> 
> And it would be nice if it cuould be done in a central place, without
> touching all architectures.

Agreed, should it prove possible.  ;-)

							Thanx, Paul

------------------------------------------------------------------------

diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index 3381f09..cb7a932 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -134,6 +134,11 @@ config NO_HZ_FULL_ALL
 	 Note the boot CPU will still be kept outside the range to
 	 handle the timekeeping duty.
 
+config NR_CPUS_REALLY
+	int "Fixed version of NR_CPUS"
+	default 1 if !SMP
+	default NR_CPUS if SMP
+
 config NO_HZ_FULL_SYSIDLE
 	bool "Detect full-system idle state for full dynticks system"
 	depends on NO_HZ_FULL
@@ -160,7 +165,7 @@ config NO_HZ_FULL_SYSIDLE
 config NO_HZ_FULL_SYSIDLE_SMALL
 	int "Number of CPUs above which large-system approach is used"
 	depends on NO_HZ_FULL_SYSIDLE
-	range 1 NR_CPUS
+	range 1 NR_CPUS_REALLY
 	default 8
 	help
 	 The full-system idle detection mechanism takes a lazy approach

Re: [PATCH tip/core/rcu 8/9] nohz_full: Add full-system-idle state machine

[Geert Uytterhoeven]

On Fri, Sep 6, 2013 at 8:50 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Fri, Sep 6, 2013 at 7:30 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
>>> Furthermore, it seems only hexagon, metag, mips, and x86 set NR_CPUS to 1
>>> if !SMP. On other architectures, NR_CPUS is not defined and presumed to be 0.
>>
>> Would it make sense to require that NR_CPUS=1 for !SMP?
>
> Yes, this looks reasonable to me.

Perhaps we can invert the logic and define only NR_CPUS in arch-specific
code, and derive SMP from NR_CPUS != 1 in generic code?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH tip/core/rcu 8/9] nohz_full: Add full-system-idle state machine

[Paul E. McKenney]

On Sat, Sep 07, 2013 at 01:22:57PM +0200, Geert Uytterhoeven wrote:
> On Fri, Sep 6, 2013 at 8:50 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, Sep 6, 2013 at 7:30 PM, Paul E. McKenney
> > <paulmck@linux.vnet.ibm.com> wrote:
> >>> Furthermore, it seems only hexagon, metag, mips, and x86 set NR_CPUS to 1
> >>> if !SMP. On other architectures, NR_CPUS is not defined and presumed to be 0.
> >>
> >> Would it make sense to require that NR_CPUS=1 for !SMP?
> >
> > Yes, this looks reasonable to me.
> 
> Perhaps we can invert the logic and define only NR_CPUS in arch-specific
> code, and derive SMP from NR_CPUS != 1 in generic code?

If we always had NR_CPUS defined, that might be a good way to go.
We would of course need acks from the various arch maintainers.  I am
guessing that we are OK for m68k.  ;-)

							Thanx, Paul

Re: [PATCH tip/core/rcu 8/9] nohz_full: Add full-system-idle state machine

[Yann E. MORIN]

Paul, All,

On 2013-09-06 10:30 -0700, Paul E. McKenney spake thusly:
[--SNIP--]
> I also tried using Kconfig "if":
> 
> if SMP
> config NR_CPUS_REALLY
> 	int "Fixed version of NR_CPUS"
> 	default NR_CPUS
> endif
> if !SMP
> config NR_CPUS_REALLY
> 	int "Fixed version of NR_CPUS"
> 	default 1 if !SMP

The 'if !SMP' here is unneeded, you're already in a 'if !SMP' if-block.

> endif
> 
> However, Kconfig complained about the use of NR_CPUS even though this
> was under an "if" whose condition was not set.  Perhaps someone with
> better Kconfig-fu than I have can come up with something.

That's because the 'if' condition is added to the dependency list of the
symbol(s) that is(are) enclosed in the if.

'if' in Kconfig behaves the same way as an 'if' in C. What you expected
(I believe) was the behaviour of '#ifdef', which is not the case. From
Documentation/kbuild/kconfig-language.txt:

    ---8<---
    if:

            "if" <expr>
            <if block>
            "endif"

    This defines an if block. The dependency expression <expr> is appended
    to all enclosed menu entries.
    ---8<---

There's no equivlaent to '#ifdef' in Kconfig.

I'll see if I can come up with a meaningfull construct that fixes your
use-case. Don't hold your breath, though! ;-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH tip/core/rcu 8/9] nohz_full: Add full-system-idle state machine

[Paul E. McKenney]

On Sat, Sep 07, 2013 at 11:13:48AM +0200, Yann E. MORIN wrote:
> Paul, All,
> 
> On 2013-09-06 10:30 -0700, Paul E. McKenney spake thusly:
> [--SNIP--]
> > I also tried using Kconfig "if":
> > 
> > if SMP
> > config NR_CPUS_REALLY
> > 	int "Fixed version of NR_CPUS"
> > 	default NR_CPUS
> > endif
> > if !SMP
> > config NR_CPUS_REALLY
> > 	int "Fixed version of NR_CPUS"
> > 	default 1 if !SMP
> 
> The 'if !SMP' here is unneeded, you're already in a 'if !SMP' if-block.

Agreed, though I get the same result even without the !SMP.

> > endif
> > 
> > However, Kconfig complained about the use of NR_CPUS even though this
> > was under an "if" whose condition was not set.  Perhaps someone with
> > better Kconfig-fu than I have can come up with something.
> 
> That's because the 'if' condition is added to the dependency list of the
> symbol(s) that is(are) enclosed in the if.
> 
> 'if' in Kconfig behaves the same way as an 'if' in C. What you expected
> (I believe) was the behaviour of '#ifdef', which is not the case. From
> Documentation/kbuild/kconfig-language.txt:
> 
>     ---8<---
>     if:
> 
>             "if" <expr>
>             <if block>
>             "endif"
> 
>     This defines an if block. The dependency expression <expr> is appended
>     to all enclosed menu entries.
>     ---8<---

OK, I did read this, but misunderstood it.

> There's no equivlaent to '#ifdef' in Kconfig.
> 
> I'll see if I can come up with a meaningfull construct that fixes your
> use-case. Don't hold your breath, though! ;-)

If not, we need to add NR_CPUS to the architectures lacking them...

								Thanx, Paul

Re: [PATCH tip/core/rcu 8/9] nohz_full: Add full-system-idle state machine

[Yann E. MORIN]

Paul, All,

On 2013-09-07 11:57 -0700, Paul E. McKenney spake thusly:
> On Sat, Sep 07, 2013 at 11:13:48AM +0200, Yann E. MORIN wrote:
[--SNIP--]
> > I'll see if I can come up with a meaningfull construct that fixes your
> > use-case. Don't hold your breath, though! ;-)
> 
> If not, we need to add NR_CPUS to the architectures lacking them...

Unfortunately, I was not able to come up with anything suitable.

I think your proposal to always define NR_CPUS=1 for architectures
without SMP support is a good solution.

After all, if !SMP because the architecture does not support it, I
believe it makes sense that NR_CPUS be defined to 1.

Unless NR_CPUS carries with it a hidden meaning about SMP being
possible, that is, which would probably be wrong anyway, since we have
SMP for this.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH tip/core/rcu 8/9] nohz_full: Add full-system-idle state machine

[Frederic Weisbecker]

On Sun, Sep 08, 2013 at 12:32:50PM +0200, Yann E. MORIN wrote:
> Paul, All,
> 
> On 2013-09-07 11:57 -0700, Paul E. McKenney spake thusly:
> > On Sat, Sep 07, 2013 at 11:13:48AM +0200, Yann E. MORIN wrote:
> [--SNIP--]
> > > I'll see if I can come up with a meaningfull construct that fixes your
> > > use-case. Don't hold your breath, though! ;-)
> > 
> > If not, we need to add NR_CPUS to the architectures lacking them...
> 
> Unfortunately, I was not able to come up with anything suitable.
> 
> I think your proposal to always define NR_CPUS=1 for architectures
> without SMP support is a good solution.
> 
> After all, if !SMP because the architecture does not support it, I
> believe it makes sense that NR_CPUS be defined to 1.
> 
> Unless NR_CPUS carries with it a hidden meaning about SMP being
> possible, that is, which would probably be wrong anyway, since we have
> SMP for this.

Right, if possible, the best would be to define CONFIG_NR_CPUS to a single
central place, like arch/Kconfig:

	config NR_CPUS
	default 1 if !SMP

It's a single int after all. Specific arch constraints can probably be overriden
from the arch.

> 
> Regards,
> Yann E. MORIN.
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'