headers
Tony Luck | 18 Jul 2012 19:35
Picon
Favicon

[PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC

The stack_not_used() function in <linux/sched.h> assumes that stacks
grow downwards. This is not true on IA64 or PARISC, so this function
would walk off in the wrong direction and into the weeds.

Found on IA64 because of a compilation failure with recursive dependencies
on IA64_TASKSIZE and IA64_THREAD_INFO_SIZE.

Fixing the code is possible, but should be combined with other
infrastructure additions to set up the "canary" at the end of the stack.

Reported-by: Fengguang Wu <fengguang.wu <at> intel.com> (failed allmodconfig build)
Signed-off-by: Tony Luck <tony.luck <at> intel.com>
---
 lib/Kconfig.debug | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ff5bdee..4a18650 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
 <at>  <at>  -714,7 +714,7  <at>  <at>  config STACKTRACE

 config DEBUG_STACK_USAGE
 	bool "Stack utilization instrumentation"
-	depends on DEBUG_KERNEL
+	depends on DEBUG_KERNEL && !IA64 && !PARISC
 	help
 	  Enables the display of the minimum amount of free stack which each
 	  task has ever had available in the sysrq-T and sysrq-P debug output.
--

--
(Continue reading)

Permalink | Reply |
headers
Ingo Molnar | 25 Jul 2012 09:45

Re: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC


* Tony Luck <tony.luck <at> intel.com> wrote:

> The stack_not_used() function in <linux/sched.h> assumes that stacks
> grow downwards. This is not true on IA64 or PARISC, so this function
> would walk off in the wrong direction and into the weeds.
> 
> Found on IA64 because of a compilation failure with recursive dependencies
> on IA64_TASKSIZE and IA64_THREAD_INFO_SIZE.
> 
> Fixing the code is possible, but should be combined with other
> infrastructure additions to set up the "canary" at the end of the stack.
> 
> Reported-by: Fengguang Wu <fengguang.wu <at> intel.com> (failed allmodconfig build)
> Signed-off-by: Tony Luck <tony.luck <at> intel.com>
> ---
>  lib/Kconfig.debug | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index ff5bdee..4a18650 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
>  <at>  <at>  -714,7 +714,7  <at>  <at>  config STACKTRACE
>  
>  config DEBUG_STACK_USAGE
>  	bool "Stack utilization instrumentation"
> -	depends on DEBUG_KERNEL
> +	depends on DEBUG_KERNEL && !IA64 && !PARISC
(Continue reading)

Permalink | Reply |
headers
James Bottomley | 25 Jul 2012 10:02

Re: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC

On Wed, 2012-07-25 at 09:45 +0200, Ingo Molnar wrote:
> * Tony Luck <tony.luck <at> intel.com> wrote:
> 
> > The stack_not_used() function in <linux/sched.h> assumes that stacks
> > grow downwards. This is not true on IA64 or PARISC, so this function
> > would walk off in the wrong direction and into the weeds.
> > 
> > Found on IA64 because of a compilation failure with recursive dependencies
> > on IA64_TASKSIZE and IA64_THREAD_INFO_SIZE.
> > 
> > Fixing the code is possible, but should be combined with other
> > infrastructure additions to set up the "canary" at the end of the stack.
> > 
> > Reported-by: Fengguang Wu <fengguang.wu <at> intel.com> (failed allmodconfig build)
> > Signed-off-by: Tony Luck <tony.luck <at> intel.com>
> > ---
> >  lib/Kconfig.debug | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index ff5bdee..4a18650 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> >  <at>  <at>  -714,7 +714,7  <at>  <at>  config STACKTRACE
> >  
> >  config DEBUG_STACK_USAGE
> >  	bool "Stack utilization instrumentation"
> > -	depends on DEBUG_KERNEL
> > +	depends on DEBUG_KERNEL && !IA64 && !PARISC
>
(Continue reading)

Permalink | Reply |
headers
Luck, Tony | 25 Jul 2012 20:23
Picon
Favicon

RE: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC

> Since the problem is an invalid assumption about how the stack grows,
> why not just condition it on that.  We actually have a config option for
> this: CONFIG_STACK_GROWSUP.  But for some reason ia64 doesn't define
> this, why not, Tony?  It looks deliberate because you have replaced a
> lot of
>
> #ifdef CONFIG_STACK_GROWSUP
>
> with
>
> #if defined(CONFIG_STACK_GROWSUP) || defined(CONFIG_IA64)
>
> but not all of them.

ia64 is special - we have stacks that grow both upwards and downwards!

The typical "C" stack for local function variables that need to be allocated in
memory (arrays, structures, things we take the address of, things that just
don't fit because we run out of registers) grows downwards. But local
variables assigned to registers get quietly saved away to an upwards growing
stack by the register stack engine (working somewhat asynchronously from
the cpu).l

-Tony
NrybXǧv^)޺{.n+{{ayʇڙ,jfhzwj:+vwjmzZ+ݢj"!
Permalink | Reply |
headers
Ingo Molnar | 26 Jul 2012 14:01

Re: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC


* James Bottomley <James.Bottomley <at> HansenPartnership.com> wrote:

> On Wed, 2012-07-25 at 09:45 +0200, Ingo Molnar wrote:
> > * Tony Luck <tony.luck <at> intel.com> wrote:
> > 
> > > The stack_not_used() function in <linux/sched.h> assumes that stacks
> > > grow downwards. This is not true on IA64 or PARISC, so this function
> > > would walk off in the wrong direction and into the weeds.
> > > 
> > > Found on IA64 because of a compilation failure with recursive dependencies
> > > on IA64_TASKSIZE and IA64_THREAD_INFO_SIZE.
> > > 
> > > Fixing the code is possible, but should be combined with other
> > > infrastructure additions to set up the "canary" at the end of the stack.
> > > 
> > > Reported-by: Fengguang Wu <fengguang.wu <at> intel.com> (failed allmodconfig build)
> > > Signed-off-by: Tony Luck <tony.luck <at> intel.com>
> > > ---
> > >  lib/Kconfig.debug | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > > index ff5bdee..4a18650 100644
> > > --- a/lib/Kconfig.debug
> > > +++ b/lib/Kconfig.debug
> > >  <at>  <at>  -714,7 +714,7  <at>  <at>  config STACKTRACE
> > >  
> > >  config DEBUG_STACK_USAGE
> > >  	bool "Stack utilization instrumentation"
(Continue reading)

Permalink | Reply |
headers
Peter Chubb | 27 Jul 2012 00:17
X-Face
Picon
Favicon

Re: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC

>>>>> "Ingo" == Ingo Molnar <mingo <at> kernel.org> writes:

Ingo> * James Bottomley <James.Bottomley <at> HansenPartnership.com> wrote:
>> Since the problem is an invalid assumption about how the stack
>> grows, why not just condition it on that.  We actually have a
>> config option for this: CONFIG_STACK_GROWSUP.  But for some reason
>> ia64 doesn't define this, why not, Tony?  It looks deliberate
>> because you have replaced a lot of
>> 
>> #ifdef CONFIG_STACK_GROWSUP
>> 
>> with
>> 
>> #if defined(CONFIG_STACK_GROWSUP) || defined(CONFIG_IA64)
>> 
>> but not all of them.

Ingo> Yes, that's another possible solution, assuming that it's really
Ingo> only about the up/down difference.

Ingo> Thanks,

IA64 has two stacks -- the standard one, that grows down, and the
register stack engine backing store, that grows up.  The usual
mechanisms for stack growth are used, so only some of the bits
predicated on `STACK_GROWSUP' are useful.

Peter C
--
Dr Peter Chubb				        peter.chubb AT nicta.com.au
(Continue reading)

Permalink | Reply |
headers
James Bottomley | 28 Jul 2012 10:12

Re: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC

On Wed, 2012-07-18 at 10:35 -0700, Tony Luck wrote:
> The stack_not_used() function in <linux/sched.h> assumes that stacks
> grow downwards. This is not true on IA64 or PARISC, so this function
> would walk off in the wrong direction and into the weeds.

OK, so looking at all of this, that statement's not quite true ... at
least for parisc, we begin the stack where end_of_stack() says the end
should be and so the walker will likely find the next word after the
canary skip occupied and terminate there, so we think the stack is
larger than it really is.  It gets the wrong value, but it will never
even walk out of the stack area.

> Found on IA64 because of a compilation failure with recursive dependencies
> on IA64_TASKSIZE and IA64_THREAD_INFO_SIZE.
> 
> Fixing the code is possible, but should be combined with other
> infrastructure additions to set up the "canary" at the end of the stack.

I agree with this.  Most of it looks easily fixable, but how would I
enable the fix for ia64?  For PA it's simple: I'll just use
CONFIG_STACK_GROWSUP, but that won't work for you.

James

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
(Continue reading)

Permalink | Reply |
headers
Luck, Tony | 28 Jul 2012 23:43
Picon
Favicon

RE: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC

> I agree with this.  Most of it looks easily fixable, but how would I
> enable the fix for ia64?  For PA it's simple: I'll just use
> CONFIG_STACK_GROWSUP, but that won't work for you.

ia64 has an ugly chicken vs. egg build dependency. When trying to build our asm-offsets.h
file (to get #define constants for various structure sizes and offsets in a format that is
usable in assembly code) we get:

include/linux/sched.h:2539: error: 'IA64_TASK_SIZE' undeclared (first use in this function)

Which is sad because IA64_TASK_SIZE is one of the #defines that asm-offsets.h is trying
to produce.

Which is why I just threw up my hands in despair and said "!IA64" for this option.

-Tony
NrybXǧv^)޺{.n+{{ayʇڙ,jfhzwj:+vwjmzZ+ݢj"!
Permalink | Reply |

Gmane