[GIT PULL] core kernel update

[GIT PULL] core kernel update

[Ingo Molnar]

Linus,

Please pull the latest core-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core-urgent-for-linus

   # HEAD: 9dd1220114e00d8ec5cdc20085bbe198b21e1985 smp/cpumask: Make CONFIG_CPUMASK_OFFSTACK=y usable without debug dependency

A late change/fix decoupling CPUMASK_OFFSTACK from DEBUG_PER_CPU_MAPS, 
fallout of:

  b53b5eda8194 x86/cpu: Increase max CPU count to 8192
  bb61ccc7dbcb x86/cpu: Allow higher NR_CPUS values

 Thanks,

	Ingo

------------------>
Josh Boyer (1):
      smp/cpumask: Make CONFIG_CPUMASK_OFFSTACK=y usable without debug dependency


 lib/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index b3c8be0..50b47cd 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -342,7 +342,8 @@ config CHECK_SIGNATURE
 	bool
 
 config CPUMASK_OFFSTACK
-	bool "Force CPU masks off stack" if DEBUG_PER_CPU_MAPS
+	bool "Force CPU masks off stack"
+	depends on SMP
 	help
 	  Use dynamic allocation for cpumask_var_t, instead of putting
 	  them on the stack.  This is a bit more expensive, but avoids

Re: [GIT PULL] core kernel update

[Linus Torvalds]

On Tue, Nov 19, 2013 at 7:42 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> Josh Boyer (1):
>       smp/cpumask: Make CONFIG_CPUMASK_OFFSTACK=y usable without debug dependency

This seems to be just pure stupid.

Why the hell should we ever ask the user whethr they want to force
CPUMASK offstack?

Seriously, it only leaves room for mistakes and stupidities. There is
no reason for any normal use-case to ask the user about this. Even the
help message is pure and utter garbage ("This is a bit more expensive,
but avoids stack overflow").

The fact is, an on-stack CPUMASK is *smaller* than a pointer to an
offstack one when NR_CPU is small. The question makes no sense then.
And when NR_CPU is huge, the question makes no sense _either_, since
we can't have the cpumasks on stack.

Asking the user questions that make no f*cking sense to ask is stupid.
And I'm not knowingly pulling stupid crap.

                   Linus

Re: [GIT PULL] core kernel update

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Nov 19, 2013 at 7:42 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > Josh Boyer (1):
> >       smp/cpumask: Make CONFIG_CPUMASK_OFFSTACK=y usable without debug dependency
> 
> This seems to be just pure stupid.
> 
> Why the hell should we ever ask the user whethr they want to force 
> CPUMASK offstack?
> 
> Seriously, it only leaves room for mistakes and stupidities. There 
> is no reason for any normal use-case to ask the user about this. 
> Even the help message is pure and utter garbage ("This is a bit more 
> expensive, but avoids stack overflow").
> 
> The fact is, an on-stack CPUMASK is *smaller* than a pointer to an 
> offstack one when NR_CPU is small. The question makes no sense then. 
> And when NR_CPU is huge, the question makes no sense _either_, since 
> we can't have the cpumasks on stack.

You are completely right. I missed that, will fix it up.

So we have these CPU count ranges right now:

  config NR_CPUS
        int "Maximum number of CPUs" if SMP && !MAXSMP
        range 2 8 if SMP && X86_32 && !X86_BIGSMP
        range 2 512 if SMP && !MAXSMP && !CPUMASK_OFFSTACK
        range 2 8192 if SMP && !MAXSMP && CPUMASK_OFFSTACK && X86_64

As you pointed out offstack is definitely nonsensical for
NR_CPUS <= 64. (on 64-bit CPUs) On-stack is known to crash with 
NR_CPUS >= 1024.

The 128..512 CPUs range is somewhat of an unknown.

So the fixed code would always use on-stack cpumasks up to a limit 
(which limit falls into the 128...512 range) and it would always use 
off-stack cpumasks above that limit. No user configurability in the 
regular case. [*]

The actual value of the limit - here's the on-stack cpumask sizes of 
the candidate range:

	128 CPUs: 16 byte cpumasks
	256 CPUs: 32 byte cpumasks
	512 CPUs: 64 byte cpumasks

512 definitely 'feels' dangerous, so I'd say the limit should be 
either 128 or 256. Both are ridiculously high counts for regular 
systems, so it doesn't really matter which one we pick.

I'd guess we should pick the higher limit for the time being (256 CPUs 
- maybe even 512 CPUs), and lower it on contrary evidence (evidence of 
stack overflows)?

Thoughts?

Thanks,

	Ingo

[*] Plus perhaps allow offstack to be configurable arbitrarily if 
    debugging is enabled [DEBUG_PER_CPU_MAP=y], to allow easy
    experiments/measurements?

Re: [GIT PULL] core kernel update

[Peter Zijlstra]

On Tue, Nov 19, 2013 at 08:09:04PM +0100, Ingo Molnar wrote:
> The actual value of the limit - here's the on-stack cpumask sizes of 
> the candidate range:
> 
> 	128 CPUs: 16 byte cpumasks
> 	256 CPUs: 32 byte cpumasks
> 	512 CPUs: 64 byte cpumasks

So 512 / 64bytes is a single cacheline and feels like a nice cut-off
before requiring an extra indirection and more cachelines.

64 bytes also doesn't sound _that_ big to have on-stack.

So I'd go for having the cut-off on >512, unless of course theres
evidence 64bytes is already too much.

Re: [GIT PULL] core kernel update

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Nov 19, 2013 at 08:09:04PM +0100, Ingo Molnar wrote:
> > The actual value of the limit - here's the on-stack cpumask sizes of 
> > the candidate range:
> > 
> > 	128 CPUs: 16 byte cpumasks
> > 	256 CPUs: 32 byte cpumasks
> > 	512 CPUs: 64 byte cpumasks
> 
> So 512 / 64bytes is a single cacheline and feels like a nice cut-off
> before requiring an extra indirection and more cachelines.
> 
> 64 bytes also doesn't sound _that_ big to have on-stack.

The cacheline size itself isn't necessarily super meaningful for 
on-stack variables: they are rarely cacheline aligned so they will 
take part in two cachelines.

> So I'd go for having the cut-off on >512, unless of course theres 
> evidence 64bytes is already too much.

I'm fine with that in any case, for the other reason I outlined: it's 
the highest one and we can iterate down if it proves to be bad. If we 
start out too low we'll probably never know it was too low.

Thanks,

	Ingo

Re: [GIT PULL] core kernel update

[Linus Torvalds]

On Tue, Nov 19, 2013 at 11:09 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> As you pointed out offstack is definitely nonsensical for
> NR_CPUS <= 64. (on 64-bit CPUs) On-stack is known to crash with
> NR_CPUS >= 1024.
>
> The 128..512 CPUs range is somewhat of an unknown.

Agreed. And we could even decide to ask somewhere in that range.

I think 128 bits is still safely "don't bother with an external
pointer" (it's just two words, it's like a "struct list_head" -
there's no way that should be unsafe on the stack). Once we get to 256
bits I start going "Hmm, that's 32 bytes, maybe an external allocation
makes sense..."

I'd personally put the cut-off point at just keeping it on-stack if
it's smaller than or equal to 256. But I agree that at that point it's
really just a judgement call.

             Linus

Re: [GIT PULL] core kernel update

[Rusty Russell]

Ingo Molnar <mingo@kernel.org> writes:
> [*] Plus perhaps allow offstack to be configurable arbitrarily if 
>     debugging is enabled [DEBUG_PER_CPU_MAP=y], to allow easy
>     experiments/measurements?

Yes, it was good for i386 testing.

But now it seems that DEBUG_KERNEL is on for every distribution
(AFAICT), it's no longer an obscure option.  I'm happy to kill it.

Cheers,
Rusty.

Re: [GIT PULL] core kernel update

[Ingo Molnar]

* Rusty Russell <rusty@rustcorp.com.au> wrote:

> Ingo Molnar <mingo@kernel.org> writes:
> > [*] Plus perhaps allow offstack to be configurable arbitrarily if 
> >     debugging is enabled [DEBUG_PER_CPU_MAP=y], to allow easy
> >     experiments/measurements?
> 
> Yes, it was good for i386 testing.
> 
> But now it seems that DEBUG_KERNEL is on for every distribution 
> (AFAICT), it's no longer an obscure option.  I'm happy to kill it.

It would depend on DEBUG_PER_CPU_MAP=y, not DEBUG_KERNEL=y.

Thanks,

	Ingo