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
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
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 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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC
> "Ingo" == Ingo Molnar writes: Ingo> * James Bottomley 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 http://www.ssrg.nicta.com.au Software Systems Research Group/NICTA -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC
* James Bottomley wrote: > On Wed, 2012-07-25 at 09:45 +0200, Ingo Molnar wrote: > > * Tony Luck wrote: > > > > > The stack_not_used() function in 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 (failed allmodconfig > > > build) > > > Signed-off-by: Tony Luck > > > --- > > > 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 > > > @@ -714,7 +714,7 @@ config STACKTRACE > > > > > > config DEBUG_STACK_USAGE > > > bool "Stack utilization instrumentation" > > > - depends on DEBUG_KERNEL > > > + depends on DEBUG_KERNEL && !IA64 && !PARISC > > > > The modern way of doing this is by adding an ARCH_SUPPORTS_ > > flag. > > That's a bit daft, isn't it? [...] It's generally more maintainable than a random list of architecture exclusions because every (old or new) architecture can just grep for ARCH_SUPPORTS_ pattern and see whether they support everything that others support. The above exclusion list of architectures is much harder to find in a structured way. > [...] We'd have to add ARCH_SUPPORTS_ flags to about 25 > separate architectures just to get it not supported on these > two. That is one off overhead and it makes things easier to maintain going forward. Anyway, that's the current upstream technique and it's been in place for years. > 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. Yes, that's another possible solution, assuming that it's really only about the up/down difference. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC
* James Bottomley james.bottom...@hansenpartnership.com wrote: On Wed, 2012-07-25 at 09:45 +0200, Ingo Molnar wrote: * Tony Luck tony.l...@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...@intel.com (failed allmodconfig build) Signed-off-by: Tony Luck tony.l...@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 @@ -714,7 +714,7 @@ config STACKTRACE config DEBUG_STACK_USAGE bool Stack utilization instrumentation - depends on DEBUG_KERNEL + depends on DEBUG_KERNEL !IA64 !PARISC The modern way of doing this is by adding an ARCH_SUPPORTS_ flag. That's a bit daft, isn't it? [...] It's generally more maintainable than a random list of architecture exclusions because every (old or new) architecture can just grep for ARCH_SUPPORTS_ pattern and see whether they support everything that others support. The above exclusion list of architectures is much harder to find in a structured way. [...] We'd have to add ARCH_SUPPORTS_ flags to about 25 separate architectures just to get it not supported on these two. That is one off overhead and it makes things easier to maintain going forward. Anyway, that's the current upstream technique and it's been in place for years. 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. Yes, that's another possible solution, assuming that it's really only about the up/down difference. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC
Ingo == Ingo Molnar mi...@kernel.org writes: Ingo * James Bottomley james.bottom...@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 http://www.ssrg.nicta.com.au Software Systems Research Group/NICTA -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
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 wrote: > > > The stack_not_used() function in 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 (failed allmodconfig > > build) > > Signed-off-by: Tony Luck > > --- > > 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 > > @@ -714,7 +714,7 @@ config STACKTRACE > > > > config DEBUG_STACK_USAGE > > bool "Stack utilization instrumentation" > > - depends on DEBUG_KERNEL > > + depends on DEBUG_KERNEL && !IA64 && !PARISC > > The modern way of doing this is by adding an ARCH_SUPPORTS_ > flag. That's a bit daft, isn't it? We'd have to add ARCH_SUPPORTS_ flags to about 25 separate architectures just to get it not supported on these two. 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. James -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC
* Tony Luck wrote: > The stack_not_used() function in 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 (failed allmodconfig build) > Signed-off-by: Tony Luck > --- > 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 > @@ -714,7 +714,7 @@ config STACKTRACE > > config DEBUG_STACK_USAGE > bool "Stack utilization instrumentation" > - depends on DEBUG_KERNEL > + depends on DEBUG_KERNEL && !IA64 && !PARISC The modern way of doing this is by adding an ARCH_SUPPORTS_ flag. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [PATCH] debug: Do not permit CONFIG_DEBUG_STACK_USAGE=y on IA64 or PARISC
* Tony Luck tony.l...@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...@intel.com (failed allmodconfig build) Signed-off-by: Tony Luck tony.l...@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 @@ -714,7 +714,7 @@ config STACKTRACE config DEBUG_STACK_USAGE bool Stack utilization instrumentation - depends on DEBUG_KERNEL + depends on DEBUG_KERNEL !IA64 !PARISC The modern way of doing this is by adding an ARCH_SUPPORTS_ flag. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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.l...@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...@intel.com (failed allmodconfig build) Signed-off-by: Tony Luck tony.l...@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 @@ -714,7 +714,7 @@ config STACKTRACE config DEBUG_STACK_USAGE bool Stack utilization instrumentation - depends on DEBUG_KERNEL + depends on DEBUG_KERNEL !IA64 !PARISC The modern way of doing this is by adding an ARCH_SUPPORTS_ flag. That's a bit daft, isn't it? We'd have to add ARCH_SUPPORTS_ flags to about 25 separate architectures just to get it not supported on these two. 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. James -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
