I sent this same message to LKML a while ago, but thought I'd get it a more targeted audience:
========================= Noticed today that the combination of 4KSTACKS and DEBUG_STACKOVERFLOW config options is a bit deadly.
DEBUG_STACKOVERFLOW warns in do_IRQ if we're within THREAD_SIZE/8 of the end of useable stack space, or 512 bytes on a 4k stack.
If we are, then it goes down the dump_stack path, which uses most, if not all, of the remaining stack, thereby turning a well-intentioned warning into a full-blown catastrophe.
The callchain from the warning looks something like this, with stack usage shown as found on my x86 box:
4 dump_stack 4 show_trace 8 show_trace_log_lvl 4 dump_trace print_context_stack 12 print_trace_address print_symbol 232 __print_symbol 164 sprint_symbol 20 printk ___ 448
448 bytes to tell us that we're within 512 bytes (or less) of certain doom... and I think there's call overhead on top of that?
The large stack usage in those 2 functions is due to big char arrays, of size KSYM_NAME_LEN (128 bytes) and KSYM_SYMBOL_LEN (223 bytes).
IOW, the stack warning effectively reduces useful stack left in our itty bitty 4k stacks by over 10%.
...
=========================
In light of this, I'd like to propose that we turn off DEBUG_STACKOVERFLOW in Fedora, at least on x86/4KSTACKS. I think it does more harm than good; the warning is going to turn deadly most of the time.
I also had sent a patch to LKML to print whether or not the stack was overflowing, or had ever overflowed, on a kernel panic. It's not yet been merged.
... any comments? I can file a bug but thought some discussion might be in order.
Thanks,
-Eric
On Tue, Sep 11, 2007 at 03:05:52PM -0500, Eric Sandeen wrote:
In light of this, I'd like to propose that we turn off DEBUG_STACKOVERFLOW in Fedora, at least on x86/4KSTACKS. I think it does more harm than good; the warning is going to turn deadly most of the time.
I also had sent a patch to LKML to print whether or not the stack was overflowing, or had ever overflowed, on a kernel panic. It's not yet been merged.
... any comments? I can file a bug but thought some discussion might be in order.
We could turn it off everywhere expect the -debug kernels and just have XFS + -debug be a 'dont do that' thing.
Dave
Dave Jones wrote:
On Tue, Sep 11, 2007 at 03:05:52PM -0500, Eric Sandeen wrote:
In light of this, I'd like to propose that we turn off DEBUG_STACKOVERFLOW in Fedora, at least on x86/4KSTACKS. I think it does more harm than good; the warning is going to turn deadly most of the time.
I also had sent a patch to LKML to print whether or not the stack was overflowing, or had ever overflowed, on a kernel panic. It's not yet been merged.
... any comments? I can file a bug but thought some discussion might be in order.
We could turn it off everywhere expect the -debug kernels and just have XFS + -debug be a 'dont do that' thing.
Well, I didn't actually *say* xfs ... *cough* ;-)
There have been reports of mount.nfs, for example, using within 600 bytes of the end of the stack... if they'd caught an interrupt at the end of whatever callchain that was, they'de be uncomfortably close to the edge too.
And - what is the use of keeping the warning turned on *anywhere* when really, all it does is turn a near-miss into the real thing? If you actually overrun the stack, you're probably going to oops anyway, and get about the same backtrace - except, hopefully less often, w/o the warning going off.
I see this almost entirely as statistically shaving another ~500 bytes off the usable stack, without generating much useful information in return.
Or am I missing some other usefulness of this?
-Eric
On Tue, Sep 11, 2007 at 04:05:09PM -0500, Eric Sandeen wrote:
Dave Jones wrote:
On Tue, Sep 11, 2007 at 03:05:52PM -0500, Eric Sandeen wrote:
In light of this, I'd like to propose that we turn off DEBUG_STACKOVERFLOW in Fedora, at least on x86/4KSTACKS. I think it does more harm than good; the warning is going to turn deadly most of the time.
I also had sent a patch to LKML to print whether or not the stack was overflowing, or had ever overflowed, on a kernel panic. It's not yet been merged.
... any comments? I can file a bug but thought some discussion might be in order.
We could turn it off everywhere expect the -debug kernels and just have XFS + -debug be a 'dont do that' thing.
Well, I didn't actually *say* xfs ... *cough* ;-)
There have been reports of mount.nfs, for example, using within 600 bytes of the end of the stack... if they'd caught an interrupt at the end of whatever callchain that was, they'de be uncomfortably close to the edge too.
And - what is the use of keeping the warning turned on *anywhere* when really, all it does is turn a near-miss into the real thing? If you actually overrun the stack, you're probably going to oops anyway, and get about the same backtrace - except, hopefully less often, w/o the warning going off.
I see this almost entirely as statistically shaving another ~500 bytes off the usable stack, without generating much useful information in return.
Or am I missing some other usefulness of this?
Hmm, good point.
Dave
On Tue, 2007-09-11 at 15:05 -0500, Eric Sandeen wrote:
I sent this same message to LKML a while ago, but thought I'd get it a more targeted audience:
========================= Noticed today that the combination of 4KSTACKS and DEBUG_STACKOVERFLOW config options is a bit deadly.
DEBUG_STACKOVERFLOW warns in do_IRQ if we're within THREAD_SIZE/8 of the end of useable stack space, or 512 bytes on a 4k stack.
If we are, then it goes down the dump_stack path, which uses most, if not all, of the remaining stack, thereby turning a well-intentioned warning into a full-blown catastrophe.
The callchain from the warning looks something like this, with stack usage shown as found on my x86 box:
4 dump_stack 4 show_trace 8 show_trace_log_lvl 4 dump_trace print_context_stack 12 print_trace_address print_symbol 232 __print_symbol 164 sprint_symbol 20 printk ___ 448
448 bytes to tell us that we're within 512 bytes (or less) of certain doom... and I think there's call overhead on top of that?
The large stack usage in those 2 functions is due to big char arrays, of size KSYM_NAME_LEN (128 bytes) and KSYM_SYMBOL_LEN (223 bytes).
IOW, the stack warning effectively reduces useful stack left in our itty bitty 4k stacks by over 10%.
...
=========================
In light of this, I'd like to propose that we turn off DEBUG_STACKOVERFLOW in Fedora, at least on x86/4KSTACKS. I think it does more harm than good; the warning is going to turn deadly most of the time.
I also had sent a patch to LKML to print whether or not the stack was overflowing, or had ever overflowed, on a kernel panic. It's not yet been merged.
... any comments? I can file a bug but thought some discussion might be in order.
Thanks,
Sorry for butting in... but isn't disabling STACKOVERFLOW the wrong answer to this problem? Does anyone see any reason why both sprint_symbol and __print_symbol shouldn't use dynamically allocated buffers instead of wasting stack space? *
- GIlboa * If performance is an issue, memory can be statically allocated per CPU with additional locking in dump_trace.
On Sat, Sep 15, 2007 at 01:34:24AM +0300, Gilboa Davara wrote:
In light of this, I'd like to propose that we turn off DEBUG_STACKOVERFLOW in Fedora, at least on x86/4KSTACKS. I think it does more harm than good; the warning is going to turn deadly most of the time.
Sorry for butting in... but isn't disabling STACKOVERFLOW the wrong answer to this problem? Does anyone see any reason why both sprint_symbol and __print_symbol shouldn't use dynamically allocated buffers instead of wasting stack space? *
I think the reason is that kmalloc can sleep, and those functions may be called from code that can't sleep.
Dave
On Fri, 2007-09-14 at 18:42 -0400, Dave Jones wrote:
On Sat, Sep 15, 2007 at 01:34:24AM +0300, Gilboa Davara wrote:
In light of this, I'd like to propose that we turn off DEBUG_STACKOVERFLOW in Fedora, at least on x86/4KSTACKS. I think it does more harm than good; the warning is going to turn deadly most of the time.
Sorry for butting in... but isn't disabling STACKOVERFLOW the wrong answer to this problem? Does anyone see any reason why both sprint_symbol and __print_symbol shouldn't use dynamically allocated buffers instead of wasting stack space? *
I think the reason is that kmalloc can sleep, and those functions may be called from code that can't sleep.
Dave
kmalloc(GFP_ATOMIC) should not sleep.
... I wonder if there's a way, with no local_irq_disable/enable to use some pre-allocated per_cpu buffer.
.. Either way, if such a patch is required, I should really propose and push it upstream.
- Gilboa
Gilboa Davara wrote:
Sorry for butting in... but isn't disabling STACKOVERFLOW the wrong answer to this problem? Does anyone see any reason why both sprint_symbol and __print_symbol shouldn't use dynamically allocated buffers instead of wasting stack space? *
- GIlboa
- If performance is an issue, memory can be statically allocated per CPU
with additional locking in dump_trace.
Well, I agree that the dump_stack path should be lightened up stack-wise; and I don't think performance should be an issue (dump_stack is used when something has gone wrong, probably not going to be performance critical?) Locked global buffers may be just fine (we did this for xfs error messages, I remember...)
I was looking at this from a slightly different angle, which is that the stack overflow warning is largely pointless - no matter how much you lighten up the dump_stack path, it will add something to the stack depth of the current process, effectively *reducing* the available stack for all processes, and increasing the risk that you'll actually overflow. (if you take an interrupt towards the end of the stack, the warning will go off and use the last bit - so you can't count on that stack space to be available).
And, if you overflow the stack, you'll almost certainly get an oops and a backtrace anyway - usually thread_info gets overwritten and you BUG because it looks like you sleep in an interrupt, or somesuch. So, what's the point of the IRQ stack-depth check, again? Especially with 4k stacks and separate IRQ stacks? And the more deterministic max-stack-depth excursion checker (CONFIG_DEBUG_STACK_USAGE) as well...
Finally, the patch I sent upstream would clearly show on an oops whether or not the stack was currently overflowing, or whether the stack had ever overflowed prior to the oops. Seemed useful to me.
-Eric
On Fri, 2007-09-14 at 22:07 -0500, Eric Sandeen wrote:
Gilboa Davara wrote:
Sorry for butting in... but isn't disabling STACKOVERFLOW the wrong answer to this problem? Does anyone see any reason why both sprint_symbol and __print_symbol shouldn't use dynamically allocated buffers instead of wasting stack space? *
- GIlboa
- If performance is an issue, memory can be statically allocated per CPU
with additional locking in dump_trace.
Well, I agree that the dump_stack path should be lightened up stack-wise; and I don't think performance should be an issue (dump_stack is used when something has gone wrong, probably not going to be performance critical?) Locked global buffers may be just fine (we did this for xfs error messages, I remember...)
I chose the easy wait out and generated a simple, alloc-by-demand patch. http://lkml.org/lkml/2007/9/15/69
I was looking at this from a slightly different angle, which is that the stack overflow warning is largely pointless - no matter how much you lighten up the dump_stack path, it will add something to the stack depth of the current process, effectively *reducing* the available stack for all processes, and increasing the risk that you'll actually overflow. (if you take an interrupt towards the end of the stack, the warning will go off and use the last bit - so you can't count on that stack space to be available).
While it is true, A. If adding ~40 bytes to the kernel's stack usage is critical, we're already passed the all-doom-and-gloom-point. B. We can always calculate the available stack size, and if stack_remain is bigger then say, 80 bytes, call dump_stack.
And, if you overflow the stack, you'll almost certainly get an oops and a backtrace anyway - usually thread_info gets overwritten and you BUG because it looks like you sleep in an interrupt, or somesuch.
Yeah, but at least to me, as a developer, having a warning before all-hell-breaks-lose, is a good thing (tm).
Though, one can always argue that people who play around with kernel development can build their own kernel with STACKOVERFLOW enabled.
So, what's the point of the IRQ stack-depth check, again? Especially with 4k stacks and separate IRQ stacks? And the more deterministic max-stack-depth excursion checker (CONFIG_DEBUG_STACK_USAGE) as well...
Finally, the patch I sent upstream would clearly show on an oops whether or not the stack was currently overflowing, or whether the stack had ever overflowed prior to the oops. Seemed useful to me.
Just to satisfy my curiosity, can you post a link to the patch?
- Gilboa
Gilboa Davara wrote:
So, what's the point of the IRQ stack-depth check, again? Especially with 4k stacks and separate IRQ stacks? And the more deterministic max-stack-depth excursion checker (CONFIG_DEBUG_STACK_USAGE) as well...
Finally, the patch I sent upstream would clearly show on an oops whether or not the stack was currently overflowing, or whether the stack had ever overflowed prior to the oops. Seemed useful to me.
Just to satisfy my curiosity, can you post a link to the patch?
Sure: http://lkml.org/lkml/2007/8/31/28
-Eric
Gilboa Davara wrote:
I was looking at this from a slightly different angle, which is that the stack overflow warning is largely pointless - no matter how much you lighten up the dump_stack path, it will add something to the stack depth of the current process, effectively *reducing* the available stack for all processes, and increasing the risk that you'll actually overflow. (if you take an interrupt towards the end of the stack, the warning will go off and use the last bit - so you can't count on that stack space to be available).
While it is true, A. If adding ~40 bytes to the kernel's stack usage is critical, we're already passed the all-doom-and-gloom-point.
Though, just to play devil's advocate, say your absolute worst case stack depth goes to within 35 bytes of the end. The warning (even if trimmed down to 40 bytes) now renders your system unstable in the long run. Why waste it?
B. We can always calculate the available stack size, and if stack_remain is bigger then say, 80 bytes, call dump_stack.
That seems reasonable. Today, the dump_stack depth ~= the warning threshold, so it's just broken and pointless as it stands.
And, if you overflow the stack, you'll almost certainly get an oops and a backtrace anyway - usually thread_info gets overwritten and you BUG because it looks like you sleep in an interrupt, or somesuch.
Yeah, but at least to me, as a developer, having a warning before all-hell-breaks-lose, is a good thing (tm).
Perhaps. The current warning is fairly random, anyway, since it only fires on an IRQ. If you randomly get the warning at your max stack excursion, but never go so far as to actually blow the stack, then you really didn't need the warning, did you? If you get the warning at maybe 85% of your stack excursion, then your thread continues post-IRQ and blows the stack, you get a nice backtrace anyway and the warning didn't help much. I'm still not convinced that it's that useful.
I like the CONFIG_DEBUG_STACKUSAGE which accurately tells you what your max stack excursions have been. I just wish it could tell you what the callchain *was* (not really possible, as it's written) - and making it resettable would be nice too (easy).
-Eric
On Sat, 2007-09-15 at 17:45 -0500, Eric Sandeen wrote:
Gilboa Davara wrote:
I was looking at this from a slightly different angle, which is that the stack overflow warning is largely pointless - no matter how much you lighten up the dump_stack path, it will add something to the stack depth of the current process, effectively *reducing* the available stack for all processes, and increasing the risk that you'll actually overflow. (if you take an interrupt towards the end of the stack, the warning will go off and use the last bit - so you can't count on that stack space to be available).
While it is true, A. If adding ~40 bytes to the kernel's stack usage is critical, we're already passed the all-doom-and-gloom-point.
Though, just to play devil's advocate, say your absolute worst case stack depth goes to within 35 bytes of the end. The warning (even if trimmed down to 40 bytes) now renders your system unstable in the long run. Why waste it?
True. ... Though being the devil's advocate, if you don't have enough stack space to dump_strace, do_IRQ should -really- call BUG() and halt everything before things turn really ugly.
B. We can always calculate the available stack size, and if stack_remain is bigger then say, 80 bytes, call dump_stack.
That seems reasonable. Today, the dump_stack depth ~= the warning threshold, so it's just broken and pointless as it stands.
Hopefully my patch will get admitted, reducing the stack usage to ~<80 bytes. (I need to recalculate the callstack usage)
Speaking of which, who's the maintainer of kernel/*syms*? The (second) patch [1] seemed to have passed the LKML without any (negative) comments, so I'll try my luck with the actual code maintainer. (I couldn't find anyone in MAINTAINERS)
Yeah, but at least to me, as a developer, having a warning before all-hell-breaks-lose, is a good thing (tm).
Perhaps. The current warning is fairly random, anyway, since it only fires on an IRQ. If you randomly get the warning at your max stack excursion, but never go so far as to actually blow the stack, then you really didn't need the warning, did you? If you get the warning at maybe 85% of your stack excursion, then your thread continues post-IRQ and blows the stack, you get a nice backtrace anyway and the warning didn't help much. I'm still not convinced that it's that useful.
I'm usually afraid of the randomness of things. A minor stack over run can only show it's nasty head -long- -long- after the actual trash took place making it next to impossible to pin-point the original source of the problem.
I like the CONFIG_DEBUG_STACKUSAGE which accurately tells you what your max stack excursions have been. I just wish it could tell you what the callchain *was* (not really possible, as it's written) - and making it resettable would be nice too (easy).
/me adds a note to self to lookup the CONFIG_DEBUG_STACKUSAGE code so I'll know what you're talking about ;)
-Eric
- Gilboa
Gilboa Davara wrote:
I like the CONFIG_DEBUG_STACKUSAGE which accurately tells you what your max stack excursions have been. I just wish it could tell you what the callchain *was* (not really possible, as it's written) - and making it resettable would be nice too (easy).
/me adds a note to self to lookup the CONFIG_DEBUG_STACKUSAGE code so I'll know what you're talking about ;)
Hm it'd be easier to find if I spelled it right: CONFIG_DEBUG_STACK_USAGE :)
It zeros the whole stack when it's allocated. When a thread exits, it finds the first non-zero value on the stack, and from that calculates the max stack excursion. See check_stack_usage() called from do_exit().
-Eric
kernel@lists.fedoraproject.org
-
Dave Jones -
Eric Sandeen -
Gilboa Davara