2007-08-29 22:34:32

by Eric Sandeen

[permalink] [raw]
Subject: 4KSTACKS + DEBUG_STACKOVERFLOW harmful

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%.

Any suggestions for ways around this? The warning is somewhat helpful,
and I guess the obvious option is to lighten up the dump_stack path, but
it's still effectively reducing precious available stack space by some
amount.

With CONFIG_DEBUG_STACK_USAGE, we could print at oops time: "oh, and by
the way, you blew your stack" if there is no zeroed stack space left, as
a post-mortem. Even without that option, I think we could still check
whether the *current* %esp at oops time has gone too far? But if we
blew the stack, returned, and *then* oops, I think it'd be hard to know
without the DEBUG_STACK_USAGE option that we ran out of room.

-Eric


2007-08-29 22:54:11

by Jesper Juhl

[permalink] [raw]
Subject: Re: 4KSTACKS + DEBUG_STACKOVERFLOW harmful

On 30/08/2007, Eric Sandeen <[email protected]> wrote:
> 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.
>
...
>
> 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%.
>
> Any suggestions for ways around this? The warning is somewhat helpful,
> and I guess the obvious option is to lighten up the dump_stack path, but
> it's still effectively reducing precious available stack space by some
> amount.
>
A first step could be to allocate those two char arrays with kmalloc()
instead of on the stack, but then I guess that dump_stack() gets
called from places where we may not really want to be calling
kmalloc(). I guess we could allocate the buffers earlier (like at boot
time) and store pointers somewhere where dump stack can get to them
later when it needs them.


> With CONFIG_DEBUG_STACK_USAGE, we could print at oops time: "oh, and by
> the way, you blew your stack" if there is no zeroed stack space left, as
> a post-mortem. Even without that option, I think we could still check
> whether the *current* %esp at oops time has gone too far? But if we
> blew the stack, returned, and *then* oops, I think it'd be hard to know
> without the DEBUG_STACK_USAGE option that we ran out of room.
>

We could also simply have it warn at a higher limit, like 1024 bytes
instead of 512. But I guess then we would get too many false positives
and make it less useful.

--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

2007-08-29 23:02:17

by Eric Sandeen

[permalink] [raw]
Subject: Re: 4KSTACKS + DEBUG_STACKOVERFLOW harmful

Jesper Juhl wrote:

>> Any suggestions for ways around this? The warning is somewhat helpful,
>> and I guess the obvious option is to lighten up the dump_stack path, but
>> it's still effectively reducing precious available stack space by some
>> amount.
>>
> A first step could be to allocate those two char arrays with kmalloc()
> instead of on the stack, but then I guess that dump_stack() gets
> called from places where we may not really want to be calling
> kmalloc(). I guess we could allocate the buffers earlier (like at boot
> time) and store pointers somewhere where dump stack can get to them
> later when it needs them.

Yep, I thought about something like that... and I assume you'd need a
bit of locking around them too.

>> With CONFIG_DEBUG_STACK_USAGE, we could print at oops time: "oh, and by
>> the way, you blew your stack" if there is no zeroed stack space left, as
>> a post-mortem. Even without that option, I think we could still check
>> whether the *current* %esp at oops time has gone too far? But if we
>> blew the stack, returned, and *then* oops, I think it'd be hard to know
>> without the DEBUG_STACK_USAGE option that we ran out of room.
>>
>
> We could also simply have it warn at a higher limit, like 1024 bytes
> instead of 512. But I guess then we would get too many false positives
> and make it less useful.

Yes, but if you happen to warn deeper anyway, just because you got
"lucky" with IRQ timing, you'll still explode. Regardless of where the
threshold is, there's still a risk of starting the warning deeper than
that. Whatever stack the warning takes effectively reduces the useable
stack size.

-Eric

2007-08-29 23:55:22

by Kyle Moffett

[permalink] [raw]
Subject: Re: 4KSTACKS + DEBUG_STACKOVERFLOW harmful

On Aug 29, 2007, at 19:01:57, Eric Sandeen wrote:
> Jesper Juhl wrote:
>> A first step could be to allocate those two char arrays with
>> kmalloc() instead of on the stack, but then I guess that dump_stack
>> () gets called from places where we may not really want to be
>> calling kmalloc(). I guess we could allocate the buffers earlier
>> (like at boot time) and store pointers somewhere where dump stack
>> can get to them later when it needs them.
>
> Yep, I thought about something like that... and I assume you'd need
> a bit of locking around them too.

How about turning off preemption and using a per-CPU buffer?
Alternatively you could turn off IRQs, poke a per-CPU value to clue
in any incoming NMIs, and switch to a separate stack. I suppose if
you wanted it to work with all of 16 bytes of stack left on both
thread and IRQ stacks, you could have separate per-CPU NMI stacks;
the stack-dump would be poking a special per-CPU value and sending
ourselves an NMI.

There are probably a half dozen other variants on ways to run
screaming to the CPU saying "It hurts mommy!" and get a new stack in
which we can play for a while.

Cheers,
Kyle Moffett

2007-08-31 11:12:06

by Denys Vlasenko

[permalink] [raw]
Subject: Re: 4KSTACKS + DEBUG_STACKOVERFLOW harmful

On Wednesday 29 August 2007 23:34, Eric Sandeen wrote:
> 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.
...
> 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%.

KSYM_NAME_LEN = 128 sounds stupid. The name which is wider than 80 chars??
Kernel shouldn't have names that long.
Say, 50 chars ought to be enough.
--
vda

2007-08-31 14:40:07

by Jörn Engel

[permalink] [raw]
Subject: Re: 4KSTACKS + DEBUG_STACKOVERFLOW harmful

On Fri, 31 August 2007 12:11:25 +0100, Denys Vlasenko wrote:
>
> KSYM_NAME_LEN = 128 sounds stupid. The name which is wider than 80 chars??
> Kernel shouldn't have names that long.
> Say, 50 chars ought to be enough.

Might be an enforcement problem, unless someone also writes a
check_name_len.pl or so.

Jörn

--
The rabbit runs faster than the fox, because the rabbit is rinning for
his life while the fox is only running for his dinner.
-- Aesop

2007-08-31 17:16:27

by Denys Vlasenko

[permalink] [raw]
Subject: Re: 4KSTACKS + DEBUG_STACKOVERFLOW harmful

On Friday 31 August 2007 15:35, Jörn Engel wrote:
> On Fri, 31 August 2007 12:11:25 +0100, Denys Vlasenko wrote:
> >
> > KSYM_NAME_LEN = 128 sounds stupid. The name which is wider than 80 chars??
> > Kernel shouldn't have names that long.
> > Say, 50 chars ought to be enough.
>
> Might be an enforcement problem, unless someone also writes a
> check_name_len.pl or so.

It's trivial to do it in scripts/mksysmap (script which generates System.map)

Currently it simply does

$NM -n $1 | grep -v '\( [aUw] \)\|\(__crc_\)\|\( \$[adt]\)' > $2

--
vda