preempt_schedule_irq missing from mfinfo[]?

preempt_schedule_irq missing from mfinfo[]?

[Dave Johnson]

Got this crash with recent 2.6 CVS version.

CONFIG_SMP and CONFIG_PREEMPT are both on.  This in on a sb1250 rev
B2.

The PC it was trying to lookup is in preempt_schedule_irq().  Without
preempt_schedule_irq in mfinfo[] it ended up with the wrong function
and then dereferenced a NULL FP.

Should this be in mfinfo or am I on the wrong track here?

#ifdef CONFIG_PREEMPT
	{ preempt_schedule_irq, 1 },
#endif


Also, __preempt_spin_lock and __preempt_write_lock are nowhere to be
found so I had to remove those from the table.



CPU 0 Unable to handle kernel paging request at virtual address 0000000000000018, epc == ffffffff801055ac, ra == ffffffff80105500
Oops in arch/mips/mm/fault.c::do_page_fault, line 167[#2]:
Cpu 0
$ 0   : 0000000000000000 0000000010001fe1 0000000000000018 0000000000000020
$ 4   : ffffffff80372708 0000000000000003 0000000000000000 0000000000000000
$ 8   : a80000013e69f6a0 0000000000000081 0000000000000001 ffffffffffffffff
$12   : 00000000000055e3 ffffffff80000010 ffffffff8018ffd0 0000000000000000
$16   : ffffffff803726b0 ffffffff8031f940 0000000000000000 ffffffff803726b8
$20   : ffffffff803726c0 0000000000102798 0000000000114d02 0000000000000000
$24   : 0000000000000000 000000002ac1adc8                                  
$28   : a800000000784000 a800000000787ba0 0000000000000000 ffffffff80105500
Hi    : 000000000037f629
Lo    : 000000000012a763
epc   : ffffffff801055ac get_wchan+0x144/0x168     Not tainted
ra    : ffffffff80105500 get_wchan+0x98/0x168
Status: 10001fe3    KX SX UX KERNEL EXL IE 
Cause : 00808008
BadVA : 0000000000000018
PrId  : 01040102
Modules linked in:
Process top (pid: 142, threadinfo=a800000000784000, task=a8000000007799a0)
Stack : 0000000000000001 a80000012a763000 a80000013d59e660 a8000000007786f8
        a8000000cfd15200 ffffffff801d3648 000000007fb8b4f0 a8000000062f3810
        000000007fb8b4f0 a8000000007786f8 ffffffff801cfee0 a800000000787e00
        a80000013d59e680 a8000000007786f8 a80000013d59fed8 a800000000787e00
        a800000000787c70 a8000000004be108 a800000000787c70 a80000013ed0a620
        ffffffff801b0a00 0000000000000000 a800000000787c70 0000000000000000
        a80000013d59e680 ffffffff801a4cb8 a8000000004be108 a80000013d59fed8
        01c5a71f00000004 a80000011e84400a a800000000787e00 a800000000787e00
        0000000000000000 a80000013fe86430 ffffffff801b0a00 00000000000001b6
        0000000000000000 a800000000787e00 0000000000000000 a800000000721b48
        ...
Call Trace:
 [<ffffffff801d3648>] do_task_stat+0x558/0x5f0
 [<ffffffff801cfee0>] task_dumpable+0x48/0x68
 [<ffffffff801b0a00>] dput+0x38/0x2f0
 [<ffffffff801a4cb8>] __link_path_walk+0x1048/0x1480
 [<ffffffff801b0a00>] dput+0x38/0x2f0
 [<ffffffff80182c58>] vma_adjust+0x220/0x410
 [<ffffffff801d3710>] proc_tgid_stat+0x10/0x20
 [<ffffffff801cf1a8>] proc_info_read+0xa0/0xe0
 [<ffffffff8018fd88>] vfs_read+0xf0/0x138
 [<ffffffff80190028>] sys_read+0x58/0xa0
 [<ffffffff80190000>] sys_read+0x30/0xa0
 [<ffffffff8011b3c0>] handle_sys+0x120/0x13c
 [<ffffffff8011b3c0>] handle_sys+0x120/0x13c
 [<ffffffff801c7e48>] compat_sys_fcntl64+0x0/0x1f0


Code: 000318f8  0052102d  0072182d <dc520000> dc710000  00000000  0c04ce78  0220
202d  1440ffdd 






-- 
Dave Johnson
Starent Networks

Re: preempt_schedule_irq missing from mfinfo[]?

[Dave Johnson]

Dave Johnson writes:
> 
> Got this crash with recent 2.6 CVS version.
> 
> CONFIG_SMP and CONFIG_PREEMPT are both on.  This in on a sb1250 rev
> B2.
> 
> The PC it was trying to lookup is in preempt_schedule_irq().  Without
> preempt_schedule_irq in mfinfo[] it ended up with the wrong function
> and then dereferenced a NULL FP.

Looks like more than preempt_schedule_irq are missing.

I've got these in .sched.text:

ffffffff8031ea70 T __sched_text_start
ffffffff8031eb88 T __down_interruptible
ffffffff8031ed18 T schedule
ffffffff8031f7e8 T preempt_schedule
ffffffff8031f8a8 T preempt_schedule_irq
ffffffff8031f9b0 T wait_for_completion
ffffffff8031fae0 T wait_for_completion_timeout
ffffffff8031fc58 T wait_for_completion_interruptible
ffffffff8031fdf0 T wait_for_completion_interruptible_timeout
ffffffff8031ff98 T interruptible_sleep_on
ffffffff80320070 T interruptible_sleep_on_timeout
ffffffff80320160 T sleep_on
ffffffff80320238 T sleep_on_timeout
ffffffff80320328 T cond_resched
ffffffff803203c0 T cond_resched_softirq
ffffffff80320478 T yield
ffffffff803204b8 T io_schedule
ffffffff80320548 T io_schedule_timeout
ffffffff803205d8 T console_conditional_schedule
ffffffff80320610 T schedule_timeout
ffffffff803206f0 t nanosleep_restart
ffffffff803207f0 T __wait_on_bit
ffffffff80320910 T out_of_line_wait_on_bit
ffffffff803209e0 T __wait_on_bit_lock
ffffffff80320b28 T out_of_line_wait_on_bit_lock
ffffffff80320bf8 T __down_read
ffffffff80320d00 T __down_write
ffffffff80320e10 T __lock_text_start
ffffffff80320e10 T __sched_text_end

All of those should be in mfinfo[] with omit_fp 1 for those in
kernel/sched.c and 0 for elsewhere.  Or am I missing something here?

-- 
Dave Johnson
Starent Networks

[PATCH] Re: preempt_schedule_irq missing from mfinfo[]?

[Dave Johnson]

Dave Johnson writes:
> Looks like more than preempt_schedule_irq are missing.
> 
> I've got these in .sched.text:
> 
> ffffffff8031ea70 T __sched_text_start
> ffffffff8031eb88 T __down_interruptible
> ffffffff8031ed18 T schedule
> ffffffff8031f7e8 T preempt_schedule
> ffffffff8031f8a8 T preempt_schedule_irq
> ffffffff8031f9b0 T wait_for_completion
> ffffffff8031fae0 T wait_for_completion_timeout
> ffffffff8031fc58 T wait_for_completion_interruptible
> ffffffff8031fdf0 T wait_for_completion_interruptible_timeout
> ffffffff8031ff98 T interruptible_sleep_on
> ffffffff80320070 T interruptible_sleep_on_timeout
> ffffffff80320160 T sleep_on
> ffffffff80320238 T sleep_on_timeout
> ffffffff80320328 T cond_resched
> ffffffff803203c0 T cond_resched_softirq
> ffffffff80320478 T yield
> ffffffff803204b8 T io_schedule
> ffffffff80320548 T io_schedule_timeout
> ffffffff803205d8 T console_conditional_schedule
> ffffffff80320610 T schedule_timeout
> ffffffff803206f0 t nanosleep_restart
> ffffffff803207f0 T __wait_on_bit
> ffffffff80320910 T out_of_line_wait_on_bit
> ffffffff803209e0 T __wait_on_bit_lock
> ffffffff80320b28 T out_of_line_wait_on_bit_lock
> ffffffff80320bf8 T __down_read
> ffffffff80320d00 T __down_write
> ffffffff80320e10 T __lock_text_start
> ffffffff80320e10 T __sched_text_end
> 
> All of those should be in mfinfo[] with omit_fp 1 for those in
> kernel/sched.c and 0 for elsewhere.  Or am I missing something here?

make that 0 and 1 not 0 and 1.

Patch for these is below.

-- 
Dave Johnson
Starent Networks


diff -Nru a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
--- a/arch/mips/kernel/process.c	Thu Jun 30 16:11:58 2005
+++ b/arch/mips/kernel/process.c	Thu Jun 30 16:11:58 2005
@@ -24,6 +24,7 @@
 #include <linux/a.out.h>
 #include <linux/init.h>
 #include <linux/completion.h>
+#include <linux/console.h>
 
 #include <asm/abi.h>
 #include <asm/bootinfo.h>
@@ -279,28 +280,44 @@
 	/* arch/mips/kernel/semaphore.c */
 	{ __down, 1 },
 	{ __down_interruptible, 1 },
+	/* kernel/printk.c */
+	{ console_conditional_schedule, 1 },
 	/* kernel/sched.c */
 #ifdef CONFIG_PREEMPT
 	{ preempt_schedule, 0 },
+	{ preempt_schedule_irq, 0 },
 #endif
 	{ wait_for_completion, 0 },
+	{ wait_for_completion_timeout, 0 },
+	{ wait_for_completion_interruptible, 0 },
+	{ wait_for_completion_interruptible_timeout, 0 },
 	{ interruptible_sleep_on, 0 },
 	{ interruptible_sleep_on_timeout, 0 },
 	{ sleep_on, 0 },
 	{ sleep_on_timeout, 0 },
+	{ cond_resched, 0 },
+	{ cond_resched_softirq, 0 },
 	{ yield, 0 },
 	{ io_schedule, 0 },
 	{ io_schedule_timeout, 0 },
-#if defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
-	{ __preempt_spin_lock, 0 },
-	{ __preempt_write_lock, 0 },
-#endif
+	/* kernel/wait.c */
+	{ __wait_on_bit, 1 },
+	{ out_of_line_wait_on_bit, 1 },
+	{ __wait_on_bit_lock, 1 },
+	{ out_of_line_wait_on_bit_lock, 1 },
 	/* kernel/timer.c */
 	{ schedule_timeout, 1 },
-/*	{ nanosleep_restart, 1 }, */
+	{ nanosleep_restart, 1 },
+#ifdef CONFIG_RWSEM_XCHGADD_ALGORITHM
+	/* lib/rwsem.c */
+	{ rwsem_down_read_failed, 1 },
+	{ rwsem_down_write_failed, 1 },
+#endif
+#ifdef CONFIG_RWSEM_GENERIC_SPINLOCK
 	/* lib/rwsem-spinlock.c */
 	{ __down_read, 1 },
 	{ __down_write, 1 },
+#endif
 };
 
 static int mips_frame_info_initialized;
diff -Nru a/include/linux/preempt.h b/include/linux/preempt.h
--- a/include/linux/preempt.h	Thu Jun 30 16:11:58 2005
+++ b/include/linux/preempt.h	Thu Jun 30 16:11:58 2005
@@ -25,6 +25,7 @@
 #ifdef CONFIG_PREEMPT
 
 asmlinkage void preempt_schedule(void);
+asmlinkage void preempt_schedule_irq(void);
 
 #define preempt_disable() \
 do { \
diff -Nru a/include/linux/timer.h b/include/linux/timer.h
--- a/include/linux/timer.h	Thu Jun 30 16:11:58 2005
+++ b/include/linux/timer.h	Thu Jun 30 16:11:58 2005
@@ -99,4 +99,6 @@
 extern void run_local_timers(void);
 extern void it_real_fn(unsigned long);
 
+extern long nanosleep_restart(struct restart_block *restart);
+
 #endif
diff -Nru a/kernel/timer.c b/kernel/timer.c
--- a/kernel/timer.c	Thu Jun 30 16:11:58 2005
+++ b/kernel/timer.c	Thu Jun 30 16:11:58 2005
@@ -1135,7 +1135,7 @@
 	return current->pid;
 }
 
-static long __sched nanosleep_restart(struct restart_block *restart)
+long __sched nanosleep_restart(struct restart_block *restart)
 {
 	unsigned long expire = restart->arg0, now = jiffies;
 	struct timespec __user *rmtp = (struct timespec __user *) restart->arg1;

Re: preempt_schedule_irq missing from mfinfo[]?

[Atsushi Nemoto]

>>>>> On Thu, 30 Jun 2005 11:50:57 -0400, Dave Johnson <djohnson+linuxmips@sw.starentnetworks.com> said:
dave> The PC it was trying to lookup is in preempt_schedule_irq().
dave> Without preempt_schedule_irq in mfinfo[] it ended up with the
dave> wrong function and then dereferenced a NULL FP.

dave> Should this be in mfinfo or am I on the wrong track here?
...
dave> Also, __preempt_spin_lock and __preempt_write_lock are nowhere
dave> to be found so I had to remove those from the table.

Well, The current get_wchan implementation is still problematic
because:

* Some functions in __sched (and __lock) section are static.
* Functions in __lock are not handled.
* Maintenance of the struct mips_frame_info mfinfo[] is a pain.
* sleep_on*() is deprecated.  kernel-janitors people want to remove
  references for them.

I suppose searching a caller of scheduling functions in get_wchan is
not necessary.  Calling thread_saved_pc() will be enough for most
usage.

So I posted a patch to simplify things a while ago.

http://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=20050126.120916.88699500.nemoto%40toshiba-tops.co.jp

And here is a revised patch.

diff -u linux-mips/arch/mips/kernel/process.c linux/arch/mips/kernel/process.c
--- linux-mips/arch/mips/kernel/process.c	2005-06-21 17:34:10.000000000 +0900
+++ linux/arch/mips/kernel/process.c	2005-07-01 11:15:26.000000000 +0900
@@ -270,47 +270,15 @@
 }
 
 static struct mips_frame_info {
-	void *func;
-	int omit_fp;	/* compiled without fno-omit-frame-pointer */
 	int frame_offset;
 	int pc_offset;
-} schedule_frame, mfinfo[] = {
-	{ schedule, 0 },	/* must be first */
-	/* arch/mips/kernel/semaphore.c */
-	{ __down, 1 },
-	{ __down_interruptible, 1 },
-	/* kernel/sched.c */
-#ifdef CONFIG_PREEMPT
-	{ preempt_schedule, 0 },
-#endif
-	{ wait_for_completion, 0 },
-	{ interruptible_sleep_on, 0 },
-	{ interruptible_sleep_on_timeout, 0 },
-	{ sleep_on, 0 },
-	{ sleep_on_timeout, 0 },
-	{ yield, 0 },
-	{ io_schedule, 0 },
-	{ io_schedule_timeout, 0 },
-#if defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
-	{ __preempt_spin_lock, 0 },
-	{ __preempt_write_lock, 0 },
-#endif
-	/* kernel/timer.c */
-	{ schedule_timeout, 1 },
-/*	{ nanosleep_restart, 1 }, */
-	/* lib/rwsem-spinlock.c */
-	{ __down_read, 1 },
-	{ __down_write, 1 },
-};
-
-static int mips_frame_info_initialized;
-static int __init get_frame_info(struct mips_frame_info *info)
+} schedule_frame;
+static int __init get_frame_info(struct mips_frame_info *info, void *func)
 {
 	int i;
-	void *func = info->func;
 	union mips_instruction *ip = (union mips_instruction *)func;
 	info->pc_offset = -1;
-	info->frame_offset = info->omit_fp ? 0 : -1;
+	info->frame_offset = -1;
 	for (i = 0; i < 128; i++, ip++) {
 		/* if jal, jalr, jr, stop. */
 		if (ip->j_format.opcode == jal_op ||
@@ -358,26 +326,7 @@
 
 static int __init frame_info_init(void)
 {
-	int i, found;
-	for (i = 0; i < ARRAY_SIZE(mfinfo); i++)
-		if (get_frame_info(&mfinfo[i]))
-			return -1;
-	schedule_frame = mfinfo[0];
-	/* bubble sort */
-	do {
-		struct mips_frame_info tmp;
-		found = 0;
-		for (i = 1; i < ARRAY_SIZE(mfinfo); i++) {
-			if (mfinfo[i-1].func > mfinfo[i].func) {
-				tmp = mfinfo[i];
-				mfinfo[i] = mfinfo[i-1];
-				mfinfo[i-1] = tmp;
-				found = 1;
-			}
-		}
-	} while (found);
-	mips_frame_info_initialized = 1;
-	return 0;
+	return get_frame_info(&schedule_frame, schedule);
 }
 
 arch_initcall(frame_info_init);
@@ -398,39 +347,12 @@
 	return ((unsigned long *)t->reg29)[schedule_frame.pc_offset];
 }
 
-/* get_wchan - a maintenance nightmare^W^Wpain in the ass ...  */
 unsigned long get_wchan(struct task_struct *p)
 {
-	unsigned long frame, pc;
-
 	if (!p || p == current || p->state == TASK_RUNNING)
 		return 0;
 
-	if (!mips_frame_info_initialized)
-		return 0;
-	pc = thread_saved_pc(p);
-
-	if (!in_sched_functions(pc))
-		goto out;
-
-	frame = ((unsigned long *)p->thread.reg30)[schedule_frame.frame_offset];
-	do {
-		int i;
-		for (i = ARRAY_SIZE(mfinfo) - 1; i >= 0; i--) {
-			if (pc >= (unsigned long) mfinfo[i].func)
-				break;
-		}
-		if (i < 0)
-			break;
-
-		if (mfinfo[i].omit_fp)
-			break;
-		pc = ((unsigned long *)frame)[mfinfo[i].pc_offset];
-		frame = ((unsigned long *)frame)[mfinfo[i].frame_offset];
-	} while (in_sched_functions(pc));
-
-out:
-	return pc;
+	return thread_saved_pc(p);
 }
 
 EXPORT_SYMBOL(get_wchan);


---
Atsushi Nemoto

Re: preempt_schedule_irq missing from mfinfo[]?

[Dave Johnson]

That'll do it.  My patch wasn't enough.

I added some sanity checks to get_wchan and it hit one while running
overnight.

The task being examined transitioned from !TASK_RUNNING to
TASK_RUNNING while it was being examined. Doh!

Definately not SMP/preempt safe as written today.


-- 
Dave Johnson
Starent Networks


Atsushi Nemoto writes:
> >>>>> On Thu, 30 Jun 2005 11:50:57 -0400, Dave Johnson <djohnson+linuxmips@sw.starentnetworks.com> said:
> dave> The PC it was trying to lookup is in preempt_schedule_irq().
> dave> Without preempt_schedule_irq in mfinfo[] it ended up with the
> dave> wrong function and then dereferenced a NULL FP.
> 
> dave> Should this be in mfinfo or am I on the wrong track here?
> ...
> dave> Also, __preempt_spin_lock and __preempt_write_lock are nowhere
> dave> to be found so I had to remove those from the table.
> 
> Well, The current get_wchan implementation is still problematic
> because:
> 
> * Some functions in __sched (and __lock) section are static.
> * Functions in __lock are not handled.
> * Maintenance of the struct mips_frame_info mfinfo[] is a pain.
> * sleep_on*() is deprecated.  kernel-janitors people want to remove
>   references for them.
> 
> I suppose searching a caller of scheduling functions in get_wchan is
> not necessary.  Calling thread_saved_pc() will be enough for most
> usage.
> 
> So I posted a patch to simplify things a while ago.
> 
> http://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=20050126.120916.88699500.nemoto%40toshiba-tops.co.jp
> 
> And here is a revised patch.
> 
> diff -u linux-mips/arch/mips/kernel/process.c linux/arch/mips/kernel/process.c
> --- linux-mips/arch/mips/kernel/process.c	2005-06-21 17:34:10.000000000 +0900
> +++ linux/arch/mips/kernel/process.c	2005-07-01 11:15:26.000000000 +0900
> @@ -270,47 +270,15 @@
>  }
>  
>  static struct mips_frame_info {
> -	void *func;
> -	int omit_fp;	/* compiled without fno-omit-frame-pointer */
>  	int frame_offset;
>  	int pc_offset;
> -} schedule_frame, mfinfo[] = {
> -	{ schedule, 0 },	/* must be first */
> -	/* arch/mips/kernel/semaphore.c */
> -	{ __down, 1 },
> -	{ __down_interruptible, 1 },
> -	/* kernel/sched.c */
> -#ifdef CONFIG_PREEMPT
> -	{ preempt_schedule, 0 },
> -#endif
> -	{ wait_for_completion, 0 },
> -	{ interruptible_sleep_on, 0 },
> -	{ interruptible_sleep_on_timeout, 0 },
> -	{ sleep_on, 0 },
> -	{ sleep_on_timeout, 0 },
> -	{ yield, 0 },
> -	{ io_schedule, 0 },
> -	{ io_schedule_timeout, 0 },
> -#if defined(CONFIG_SMP) && defined(CONFIG_PREEMPT)
> -	{ __preempt_spin_lock, 0 },
> -	{ __preempt_write_lock, 0 },
> -#endif
> -	/* kernel/timer.c */
> -	{ schedule_timeout, 1 },
> -/*	{ nanosleep_restart, 1 }, */
> -	/* lib/rwsem-spinlock.c */
> -	{ __down_read, 1 },
> -	{ __down_write, 1 },
> -};
> -
> -static int mips_frame_info_initialized;
> -static int __init get_frame_info(struct mips_frame_info *info)
> +} schedule_frame;
> +static int __init get_frame_info(struct mips_frame_info *info, void *func)
>  {
>  	int i;
> -	void *func = info->func;
>  	union mips_instruction *ip = (union mips_instruction *)func;
>  	info->pc_offset = -1;
> -	info->frame_offset = info->omit_fp ? 0 : -1;
> +	info->frame_offset = -1;
>  	for (i = 0; i < 128; i++, ip++) {
>  		/* if jal, jalr, jr, stop. */
>  		if (ip->j_format.opcode == jal_op ||
> @@ -358,26 +326,7 @@
>  
>  static int __init frame_info_init(void)
>  {
> -	int i, found;
> -	for (i = 0; i < ARRAY_SIZE(mfinfo); i++)
> -		if (get_frame_info(&mfinfo[i]))
> -			return -1;
> -	schedule_frame = mfinfo[0];
> -	/* bubble sort */
> -	do {
> -		struct mips_frame_info tmp;
> -		found = 0;
> -		for (i = 1; i < ARRAY_SIZE(mfinfo); i++) {
> -			if (mfinfo[i-1].func > mfinfo[i].func) {
> -				tmp = mfinfo[i];
> -				mfinfo[i] = mfinfo[i-1];
> -				mfinfo[i-1] = tmp;
> -				found = 1;
> -			}
> -		}
> -	} while (found);
> -	mips_frame_info_initialized = 1;
> -	return 0;
> +	return get_frame_info(&schedule_frame, schedule);
>  }
>  
>  arch_initcall(frame_info_init);
> @@ -398,39 +347,12 @@
>  	return ((unsigned long *)t->reg29)[schedule_frame.pc_offset];
>  }
>  
> -/* get_wchan - a maintenance nightmare^W^Wpain in the ass ...  */
>  unsigned long get_wchan(struct task_struct *p)
>  {
> -	unsigned long frame, pc;
> -
>  	if (!p || p == current || p->state == TASK_RUNNING)
>  		return 0;
>  
> -	if (!mips_frame_info_initialized)
> -		return 0;
> -	pc = thread_saved_pc(p);
> -
> -	if (!in_sched_functions(pc))
> -		goto out;
> -
> -	frame = ((unsigned long *)p->thread.reg30)[schedule_frame.frame_offset];
> -	do {
> -		int i;
> -		for (i = ARRAY_SIZE(mfinfo) - 1; i >= 0; i--) {
> -			if (pc >= (unsigned long) mfinfo[i].func)
> -				break;
> -		}
> -		if (i < 0)
> -			break;
> -
> -		if (mfinfo[i].omit_fp)
> -			break;
> -		pc = ((unsigned long *)frame)[mfinfo[i].pc_offset];
> -		frame = ((unsigned long *)frame)[mfinfo[i].frame_offset];
> -	} while (in_sched_functions(pc));
> -
> -out:
> -	return pc;
> +	return thread_saved_pc(p);
>  }
>  
>  EXPORT_SYMBOL(get_wchan);
> 
> 
> ---
> Atsushi Nemoto

Re: preempt_schedule_irq missing from mfinfo[]?

[Atsushi Nemoto]

>>>>> On Fri, 1 Jul 2005 09:54:49 -0400, Dave Johnson <djohnson+linuxmips@sw.starentnetworks.com> said:

dave> That'll do it.  My patch wasn't enough.  I added some sanity
dave> checks to get_wchan and it hit one while running overnight.

dave> The task being examined transitioned from !TASK_RUNNING to
dave> TASK_RUNNING while it was being examined. Doh!

dave> Definately not SMP/preempt safe as written today.

Perhaps you can make it SMP/preempt safe by doing stack_page test in
the unwinding loop as done on i386, etc.

But anyway I think just calling thread_saved_pc is enough.

Ralf, how do you think about this?

---
Atsushi Nemoto

Re: preempt_schedule_irq missing from mfinfo[]?

[Ralf Baechle DL5RB]

On Sun, Jul 03, 2005 at 12:59:21AM +0900, Atsushi Nemoto wrote:

> Ralf, how do you think about this?

If the WCHAN column of ps axl is supposed to be any useful we need to
unwind the stack until we find the caller of the sleeping or scheduling
function.  Very useful for debugging.

  Ralf

Re: preempt_schedule_irq missing from mfinfo[]?

[Atsushi Nemoto]

>>>>> On Tue, 5 Jul 2005 21:03:09 +0100, Ralf Baechle DL5RB <ralf@linux-mips.org> said:
ralf> If the WCHAN column of ps axl is supposed to be any useful we
ralf> need to unwind the stack until we find the caller of the
ralf> sleeping or scheduling function.  Very useful for debugging.

Yes, but many sleeping/scheduling (such as schedule_timeout(),
__down(), etc.)  are compiled without -fno-omit-frame-pointer, so
you can not find the caller of such functions anyway.

And some sleeping/scheduling functions which are compiled with
-fno-omit-frame-pointer are static or deprecated (sleep_on(), etc.)

You can find the caller of "schedule()" even with simple
thread_saved_pc().  I think it is enough so I do not think it is worth
to fix (and maintain) current minfo[].

---
Atsushi Nemoto

Re: preempt_schedule_irq missing from mfinfo[]?

[Maciej W. Rozycki]

On Wed, 6 Jul 2005, Atsushi Nemoto wrote:

> Yes, but many sleeping/scheduling (such as schedule_timeout(),
> __down(), etc.)  are compiled without -fno-omit-frame-pointer, so
> you can not find the caller of such functions anyway.

 Of course you can -- __builtin_return_address().  It should be enough for 
`ps' to fetch useful data from "System.map", shouldn't it?

  Maciej

Re: preempt_schedule_irq missing from mfinfo[]?

[Ralf Baechle]

On Wed, Jul 06, 2005 at 12:29:12PM +0900, Atsushi Nemoto wrote:

> >>>>> On Tue, 5 Jul 2005 21:03:09 +0100, Ralf Baechle <ralf@linux-mips.org> said:
> ralf> If the WCHAN column of ps axl is supposed to be any useful we
> ralf> need to unwind the stack until we find the caller of the
> ralf> sleeping or scheduling function.  Very useful for debugging.
> 
> Yes, but many sleeping/scheduling (such as schedule_timeout(),
> __down(), etc.)  are compiled without -fno-omit-frame-pointer, so
> you can not find the caller of such functions anyway.

Without additional information a framepointer isn't terribly useful on
MIPS.  Unfortunately it's stored at a non-constant offset in a function's
stackframe.

> And some sleeping/scheduling functions which are compiled with
> -fno-omit-frame-pointer are static or deprecated (sleep_on(), etc.)
> 
> You can find the caller of "schedule()" even with simple
> thread_saved_pc().  I think it is enough so I do not think it is worth
> to fix (and maintain) current minfo[].

The alternative would be to finally bite the bullet and add a wchan field
to thread_struct and initialize it in all the sleeping functions.

The IA-64 people have something like a DWARF-based frame unwinder but
that just seems to heavy.

  Ralf

Re: preempt_schedule_irq missing from mfinfo[]?

[Ralf Baechle]

On Wed, Jul 06, 2005 at 09:58:50AM +0100, Maciej W. Rozycki wrote:

> > Yes, but many sleeping/scheduling (such as schedule_timeout(),
> > __down(), etc.)  are compiled without -fno-omit-frame-pointer, so
> > you can not find the caller of such functions anyway.
> 
>  Of course you can -- __builtin_return_address().  It should be enough for 
> `ps' to fetch useful data from "System.map", shouldn't it?

__builtin_return_address() is what those function could use themselves.
In this case it's about another piece of code unwinding the stackframe
until we hit a caller address that is not a scheduling function.

  Ralf

Re: preempt_schedule_irq missing from mfinfo[]?

[Atsushi Nemoto]

>>>>> On Wed, 6 Jul 2005 10:01:38 +0100, Ralf Baechle <ralf@linux-mips.org> said:
>> You can find the caller of "schedule()" even with simple
>> thread_saved_pc().  I think it is enough so I do not think it is
>> worth to fix (and maintain) current minfo[].

ralf> The alternative would be to finally bite the bullet and add a
ralf> wchan field to thread_struct and initialize it in all the
ralf> sleeping functions.

ralf> The IA-64 people have something like a DWARF-based frame
ralf> unwinder but that just seems to heavy.

Another alternative would be:

1.  Using KALLSYMS feature in kernel to obtain an address in
__sched/__lock function.  This might solve static function (and
maintainance) issue.

2.  Unwinding stack based on "addiu sp,sp,-NN" instruction in prologue
of the function.  This might solve omit-frame-pointer issue.

Anybody try? :-)

---
Atsushi Nemoto