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