Re: [patch] sched: fix single-depth wchan output

From: Ingo Molnar
Date: Thu Nov 06 2008 - 02:44:47 EST



* Ken Chen <kenchen@xxxxxxxxxx> wrote:

> On Wed, Nov 5, 2008 at 10:59 PM, Ingo Molnar <mingo@xxxxxxx> wrote:
> > If it does stack walking manually then please update it to use
> > save_stack_trace() instead - that is the standard API that will
> > utilize the best possible stack walking machinery on the architecture
> > level.
>
> OK, I pulled the patch out of our code that export stack trace via
> /proc/pid/trace. I didn't write this patch, but I think a better
> choice would be to override struct stacktrace_ops print_trace_ops
> with a memory buffer pointer to dump the stack into. If you have
> any comments, please let me know. I will polish this patch up and
> rebase to git-head.

hm, instead of modifying the lowlevel arch dump code, why not just use
the existing save_stack_trace(), and render the output yourself via a
trivial sprintf, just like kernel/lockdep.c does?

See kernel/stacktrace.c's print_stack_trace() - that could be extended
with a sprintf_stack_trace() method. Allocate a large enough buffer
dynamically, with a max of 128 stacktrace entries or so. (the output
buffer is limited to 4K-ish anyway, right?)

As a bonus this will work on every architecture, not just x86.

a few other details:

> - char namebuf[KSYM_NAME_LEN];
> + char namebuf[128];

... time machine back to old crappy code ;-)

> symname = kallsyms_lookup(address, &symsize, &offset,
> - &modname, namebuf);
> + &modname, namebuf);

ditto. But none of this has to be modified so you can just drop these
bits.

> + read_lock(&tasklist_lock);
> + buf_show_task(posp, end, task);
> + read_unlock(&tasklist_lock);

to get a stable trace we'll need more locking than that (tasklist_lock
does not exclude scheduling, etc.) - but it takes care of the most
important detail: tasks exiting from under us. So this should be OK.

> #endif
> + INF("trace", S_IFREG|S_IRUGO, pid_trace),

that needs to be r-------- instead of r--r--r--, for security reasons.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/