On Thu, 17 Aug 2006 15:57:28 +0200, Franck Bui-Huu <vagabon.xyz@xxxxxxxxx> wrote:
> This array was used to 'cache' some frame info about scheduler
> functions to speed up get_wchan(). This array was 1Ko size and
> was only used when CONFIG_KALLSYMS was set but declared for all
> configs.
>
> Rather than make the array statement conditional, this patches
> removes this array and its uses. Indeed the common case doesn't
> seem to use this array and get_wchan() is not a critical path
> anyways.
It looks good basically, but a few fixes are required.
> static int __init frame_info_init(void)
> {
> - int i;
> + unsigned long size = 0;
You must pass some non-zero size even if CONFIG_KALLSYMS was not set.
Otherwise schedule_mfi will not be initialized as expected. Actually,
this is not a problem of this patch, but we missed this point on
previous cleanups for the get_frame_info()...
> +unsigned long get_wchan(struct task_struct *task)
> +{
> + unsigned long stack_page = (unsigned long)task_stack_page(task);
This should be done after "if (!task ..." check.
> + unsigned long pc = 0;
> +#ifdef CONFIG_KALLSYMS
> + unsigned long sp = task->thread.reg29;
Same. And you missed one stack level.
sp = task->thread.reg29 + schedule_mfi.frame_size;
> +#endif
> +
> + if (!task || task == current || task->state == TASK_RUNNING)
> + goto out;
> + if (!stack_page)
> + goto out;
> +
> + pc = thread_saved_pc(task);
> +
> +#ifdef CONFIG_KALLSYMS
> + while (in_sched_functions(pc))
> + pc = unwind_stack(task, &sp, pc, 0);
> +#endif
> +
> +out:
> + return pc;
> +}
Thanks for your work.
---
Atsushi Nemoto
