linux-mips
[Top] [All Lists]

Re: [PATCH] Remove mfinfo[64] used by get_wchan()

To: vagabon.xyz@gmail.com
Subject: Re: [PATCH] Remove mfinfo[64] used by get_wchan()
From: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Date: Fri, 18 Aug 2006 11:52:13 +0900 (JST)
Cc: ralf@linux-mips.org, linux-mips@linux-mips.org
In-reply-to: <44E475C8.5000105@innova-card.com>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <44E475C8.5000105@innova-card.com>
Sender: linux-mips-bounce@linux-mips.org
On Thu, 17 Aug 2006 15:57:28 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> 
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

<Prev in Thread] Current Thread [Next in Thread>