* [mm patch] oprofile: backtrace operation does not initialized
@ 2004-12-08 9:30 Akinobu Mita
2004-12-08 9:34 ` Akinobu Mita
2004-12-08 16:00 ` John Levon
0 siblings, 2 replies; 16+ messages in thread
From: Akinobu Mita @ 2004-12-08 9:30 UTC (permalink / raw)
To: phil.el; +Cc: John Levon, linux-kernel
Hello.
When I forced the oprofile to use timer interrupt with specifying
"timer=1" module parameter. "oprofile_operations->backtrace" did
not initialized on i386.
Please apply this patch, or make oprofile initialize the backtrace
operation in case of using timer interrupt in your preferable way.
---
Signed-off-by: Akinobu Mita <amgta@yacht.ocn.ne.jp>
arch/alpha/oprofile/common.c | 3 +++
arch/arm/oprofile/init.c | 3 +++
arch/i386/oprofile/init.c | 5 ++++-
arch/ia64/oprofile/init.c | 5 ++++-
arch/ppc64/oprofile/common.c | 3 +++
drivers/oprofile/oprof.c | 4 ++--
include/linux/oprofile.h | 2 ++
7 files changed, 21 insertions(+), 4 deletions(-)
diff -Nurp 2.6-mm.orig/arch/alpha/oprofile/common.c 2.6-mm/arch/alpha/oprofile/common.c
--- 2.6-mm.orig/arch/alpha/oprofile/common.c 2004-12-07 00:11:29.000000000 +0900
+++ 2.6-mm/arch/alpha/oprofile/common.c 2004-12-07 23:35:48.000000000 +0900
@@ -143,6 +143,9 @@ oprofile_arch_init(struct oprofile_opera
{
struct op_axp_model *lmodel = NULL;
+ if (ops->force_timer)
+ return;
+
switch (implver()) {
case IMPLVER_EV4:
lmodel = &op_model_ev4;
diff -Nurp 2.6-mm.orig/arch/arm/oprofile/init.c 2.6-mm/arch/arm/oprofile/init.c
--- 2.6-mm.orig/arch/arm/oprofile/init.c 2004-12-07 00:11:32.000000000 +0900
+++ 2.6-mm/arch/arm/oprofile/init.c 2004-12-07 23:37:26.000000000 +0900
@@ -14,6 +14,9 @@
void __init oprofile_arch_init(struct oprofile_operations *ops)
{
+ if (ops->force_timer)
+ return;
+
#ifdef CONFIG_CPU_XSCALE
pmu_init(ops, &op_xscale_spec);
#endif
diff -Nurp 2.6-mm.orig/arch/i386/oprofile/init.c 2.6-mm/arch/i386/oprofile/init.c
--- 2.6-mm.orig/arch/i386/oprofile/init.c 2004-12-07 00:11:15.000000000 +0900
+++ 2.6-mm/arch/i386/oprofile/init.c 2004-12-07 23:29:31.000000000 +0900
@@ -25,6 +25,10 @@ void __init oprofile_arch_init(struct op
{
int ret __attribute_used__ = -ENODEV;
+ ops->backtrace = x86_backtrace;
+ if (ops->force_timer)
+ return;
+
#ifdef CONFIG_X86_LOCAL_APIC
ret = nmi_init(ops);
#endif
@@ -32,7 +36,6 @@ void __init oprofile_arch_init(struct op
if (ret < 0)
ret = nmi_timer_init(ops);
#endif
- ops->backtrace = x86_backtrace;
}
diff -Nurp 2.6-mm.orig/arch/ia64/oprofile/init.c 2.6-mm/arch/ia64/oprofile/init.c
--- 2.6-mm.orig/arch/ia64/oprofile/init.c 2004-12-07 00:11:29.000000000 +0900
+++ 2.6-mm/arch/ia64/oprofile/init.c 2004-12-07 23:35:02.000000000 +0900
@@ -18,11 +18,14 @@ extern void ia64_backtrace(struct pt_reg
void __init oprofile_arch_init(struct oprofile_operations * ops)
{
+ ops->backtrace = ia64_backtrace;
+ if (ops->force_timer)
+ return;
+
#ifdef CONFIG_PERFMON
/* perfmon_init() can fail, but we have no way to report it */
perfmon_init(ops);
#endif
- ops->backtrace = ia64_backtrace;
}
diff -Nurp 2.6-mm.orig/arch/ppc64/oprofile/common.c 2.6-mm/arch/ppc64/oprofile/common.c
--- 2.6-mm.orig/arch/ppc64/oprofile/common.c 2004-12-07 00:11:30.000000000 +0900
+++ 2.6-mm/arch/ppc64/oprofile/common.c 2004-12-07 23:36:59.000000000 +0900
@@ -129,6 +129,9 @@ void __init oprofile_arch_init(struct op
{
unsigned int pvr;
+ if (ops->force_timer)
+ return;
+
pvr = mfspr(SPRN_PVR);
switch (PVR_VER(pvr)) {
diff -Nurp 2.6-mm.orig/drivers/oprofile/oprof.c 2.6-mm/drivers/oprofile/oprof.c
--- 2.6-mm.orig/drivers/oprofile/oprof.c 2004-12-07 00:12:07.000000000 +0900
+++ 2.6-mm/drivers/oprofile/oprof.c 2004-12-07 22:51:21.000000000 +0900
@@ -159,10 +159,10 @@ static int __init oprofile_init(void)
oprofile_timer_init(&oprofile_ops);
if (timer) {
+ oprofile_ops.force_timer = 1;
printk(KERN_INFO "oprofile: using timer interrupt.\n");
- } else {
- oprofile_arch_init(&oprofile_ops);
}
+ oprofile_arch_init(&oprofile_ops);
err = oprofilefs_register();
if (err)
diff -Nurp 2.6-mm.orig/include/linux/oprofile.h 2.6-mm/include/linux/oprofile.h
--- 2.6-mm.orig/include/linux/oprofile.h 2004-12-07 00:12:32.000000000 +0900
+++ 2.6-mm/include/linux/oprofile.h 2004-12-07 22:50:07.000000000 +0900
@@ -39,6 +39,8 @@ struct oprofile_operations {
void (*backtrace)(struct pt_regs * const regs, unsigned int depth);
/* CPU identification string. */
char * cpu_type;
+ /* Force use of timer interrupt */
+ int force_timer;
};
/**
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-08 9:30 [mm patch] oprofile: backtrace operation does not initialized Akinobu Mita
@ 2004-12-08 9:34 ` Akinobu Mita
2004-12-08 11:13 ` Ingo Molnar
2004-12-08 16:00 ` John Levon
1 sibling, 1 reply; 16+ messages in thread
From: Akinobu Mita @ 2004-12-08 9:34 UTC (permalink / raw)
To: phil.el; +Cc: John Levon, linux-kernel
On Wednesday 08 December 2004 18:30, Akinobu Mita wrote:
> Please apply this patch, or make oprofile initialize the backtrace
> operation in case of using timer interrupt in your preferable way.
Acutually, I want this change to work my own tailor-made profiler.
It simply moves the point where timer_hook is called.
e.g. move timer_hook to the entry point of schedule().
--- 2.6-mm/kernel/profile.c.orig 2004-12-07 23:52:56.000000000 +0900
+++ 2.6-mm/kernel/profile.c 2004-12-07 23:53:29.000000000 +0900
@@ -383,8 +383,6 @@ void profile_hit(int type, void *__pc)
void profile_tick(int type, struct pt_regs *regs)
{
- if (type == CPU_PROFILING && timer_hook)
- timer_hook(regs);
if (!user_mode(regs) && cpu_isset(smp_processor_id(), prof_cpu_mask))
profile_hit(type, (void *)profile_pc(regs));
}
--- 2.6-mm/kernel/sched.c.orig 2004-12-08 16:08:13.000000000 +0900
+++ 2.6-mm/kernel/sched.c 2004-12-08 16:10:01.000000000 +0900
@@ -2665,6 +2665,30 @@ EXPORT_SYMBOL(sub_preempt_count);
#endif
+#ifdef __i386__
+#define GET_CURRENT_REGS(regs) \
+do { \
+ __asm__ __volatile__("movl %%ebx,%0" : "=m"(regs.ebx)); \
+ __asm__ __volatile__("movl %%ecx,%0" : "=m"(regs.ecx)); \
+ __asm__ __volatile__("movl %%edx,%0" : "=m"(regs.edx)); \
+ __asm__ __volatile__("movl %%esi,%0" : "=m"(regs.esi)); \
+ __asm__ __volatile__("movl %%edi,%0" : "=m"(regs.edi)); \
+ __asm__ __volatile__("movl %%ebp,%0" : "=m"(regs.ebp)); \
+ __asm__ __volatile__("movl %%eax,%0" : "=m"(regs.eax)); \
+ __asm__ __volatile__("movl %%esp,%0" : "=m"(regs.esp)); \
+ __asm__ __volatile__("movw %%ss, %%ax;" :"=a"(regs.xss)); \
+ __asm__ __volatile__("movw %%cs, %%ax;" :"=a"(regs.xcs)); \
+ __asm__ __volatile__("movw %%ds, %%ax;" :"=a"(regs.xds)); \
+ __asm__ __volatile__("movw %%es, %%ax;" :"=a"(regs.xes)); \
+ __asm__ __volatile__("pushfl; popl %0" :"=m"(regs.eflags)); \
+ \
+ regs.eip = (unsigned long)current_text_addr(); \
+} while(0)
+
+#else
+#error please define get_current_regs()
+#endif
+
/*
* schedule() is the main scheduler function.
*/
@@ -2692,7 +2716,12 @@ asmlinkage void __sched schedule(void)
dump_stack();
}
}
- profile_hit(SCHED_PROFILING, __builtin_return_address(0));
+ if (timer_hook) {
+ struct pt_regs regs;
+
+ GET_CURRENT_REGS(regs);
+ timer_hook(®s);
+ }
need_resched:
preempt_disable();
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-08 9:34 ` Akinobu Mita
@ 2004-12-08 11:13 ` Ingo Molnar
2004-12-08 11:37 ` Akinobu Mita
0 siblings, 1 reply; 16+ messages in thread
From: Ingo Molnar @ 2004-12-08 11:13 UTC (permalink / raw)
To: Akinobu Mita; +Cc: phil.el, John Levon, linux-kernel
* Akinobu Mita <amgta@yacht.ocn.ne.jp> wrote:
> - profile_hit(SCHED_PROFILING, __builtin_return_address(0));
> + if (timer_hook) {
> + struct pt_regs regs;
> +
> + GET_CURRENT_REGS(regs);
> + timer_hook(®s);
> + }
ugh. nack.
Ingo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-08 11:13 ` Ingo Molnar
@ 2004-12-08 11:37 ` Akinobu Mita
0 siblings, 0 replies; 16+ messages in thread
From: Akinobu Mita @ 2004-12-08 11:37 UTC (permalink / raw)
To: Ingo Molnar; +Cc: phil.el, John Levon, linux-kernel
On Wednesday 08 December 2004 20:13, Ingo Molnar wrote:
> * Akinobu Mita <amgta@yacht.ocn.ne.jp> wrote:
> > - profile_hit(SCHED_PROFILING, __builtin_return_address(0));
> > + if (timer_hook) {
> > + struct pt_regs regs;
> > +
> > + GET_CURRENT_REGS(regs);
> > + timer_hook(®s);
> > + }
>
> ugh. nack.
>
This second patch is not intended for inclusion.
It's my own tailor-made profiler. Actually it breaks all architectures
except for i386 with CONFIG_PROFILING.
It just demonstrates why I want to apply the first patch.
It fixes specifying "timer=1" as oprofile module parameter avoids to
set oprofile_operations.backtrace.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-08 9:30 [mm patch] oprofile: backtrace operation does not initialized Akinobu Mita
2004-12-08 9:34 ` Akinobu Mita
@ 2004-12-08 16:00 ` John Levon
2004-12-08 22:31 ` Greg Banks
1 sibling, 1 reply; 16+ messages in thread
From: John Levon @ 2004-12-08 16:00 UTC (permalink / raw)
To: Akinobu Mita; +Cc: phil.el, linux-kernel, gnb
On Wed, Dec 08, 2004 at 06:30:51PM +0900, Akinobu Mita wrote:
> When I forced the oprofile to use timer interrupt with specifying
> "timer=1" module parameter. "oprofile_operations->backtrace" did
> not initialized on i386.
>
> Please apply this patch, or make oprofile initialize the backtrace
> operation in case of using timer interrupt in your preferable way.
I don't like this patch. The arches should just set the backtrace
always, then try to init the hardware. oprofile_init() should then force
the timer ops as needed.
Greg?
john
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-08 16:00 ` John Levon
@ 2004-12-08 22:31 ` Greg Banks
2004-12-08 23:56 ` Philippe Elie
0 siblings, 1 reply; 16+ messages in thread
From: Greg Banks @ 2004-12-08 22:31 UTC (permalink / raw)
To: John Levon; +Cc: Akinobu Mita, phil.el, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 855 bytes --]
On Wed, Dec 08, 2004 at 04:00:55PM +0000, John Levon wrote:
> On Wed, Dec 08, 2004 at 06:30:51PM +0900, Akinobu Mita wrote:
>
> > When I forced the oprofile to use timer interrupt with specifying
> > "timer=1" module parameter. "oprofile_operations->backtrace" did
> > not initialized on i386.
> >
> > Please apply this patch, or make oprofile initialize the backtrace
> > operation in case of using timer interrupt in your preferable way.
>
> I don't like this patch. The arches should just set the backtrace
> always, then try to init the hardware. oprofile_init() should then force
> the timer ops as needed.
>
> Greg?
Agreed, that's a cleaner approach. The attached patch (untested)
implements that. Akinobu-san, can you please test the patch?
Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.
[-- Attachment #2: oprofile-timer-backtrace-fix --]
[-- Type: text/plain, Size: 878 bytes --]
Allow stack tracing to work when sampling on timer is forced
using the timer=1 boot option. Reported by Akinobu Mita.
Signed-off-by: Greg Banks <gnb@melbourne.sgi.com>
---
oprof.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
Index: linux/drivers/oprofile/oprof.c
===================================================================
--- linux.orig/drivers/oprofile/oprof.c 2004-12-04 19:43:37.%N +1100
+++ linux/drivers/oprofile/oprof.c 2004-12-09 09:25:02.%N +1100
@@ -155,13 +155,11 @@ static int __init oprofile_init(void)
{
int err = 0;
- /* this is our fallback case */
- oprofile_timer_init(&oprofile_ops);
+ oprofile_arch_init(&oprofile_ops);
if (timer) {
printk(KERN_INFO "oprofile: using timer interrupt.\n");
- } else {
- oprofile_arch_init(&oprofile_ops);
+ oprofile_timer_init(&oprofile_ops);
}
err = oprofilefs_register();
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-08 22:31 ` Greg Banks
@ 2004-12-08 23:56 ` Philippe Elie
2004-12-09 0:39 ` Greg Banks
0 siblings, 1 reply; 16+ messages in thread
From: Philippe Elie @ 2004-12-08 23:56 UTC (permalink / raw)
To: Greg Banks; +Cc: John Levon, Akinobu Mita, linux-kernel
On Thu, 09 Dec 2004 at 09:31 +0000, Greg Banks wrote:
> On Wed, Dec 08, 2004 at 04:00:55PM +0000, John Levon wrote:
> > On Wed, Dec 08, 2004 at 06:30:51PM +0900, Akinobu Mita wrote:
> >
> > > When I forced the oprofile to use timer interrupt with specifying
> > > "timer=1" module parameter. "oprofile_operations->backtrace" did
> > > not initialized on i386.
> > >
> > > Please apply this patch, or make oprofile initialize the backtrace
> > > operation in case of using timer interrupt in your preferable way.
> >
> > I don't like this patch. The arches should just set the backtrace
> > always, then try to init the hardware. oprofile_init() should then force
> > the timer ops as needed.
> >
> > Greg?
>
> Agreed, that's a cleaner approach. The attached patch (untested)
> implements that. Akinobu-san, can you please test the patch?
> +++ linux/drivers/oprofile/oprof.c 2004-12-09 09:25:02.%N +1100
> @@ -155,13 +155,11 @@ static int __init oprofile_init(void)
> {
> int err = 0;
>
> - /* this is our fallback case */
> - oprofile_timer_init(&oprofile_ops);
> + oprofile_arch_init(&oprofile_ops);
oprofile_arch_init() --> nmi_init() which setup oprofile_ops->setup/shutdown
etc.
> if (timer) {
> printk(KERN_INFO "oprofile: using timer interrupt.\n");
> - } else {
> - oprofile_arch_init(&oprofile_ops);
> + oprofile_timer_init(&oprofile_ops);
> }
oprofile_timer_init doesn't reset op->setup/shutdown, I don't like the idea to
reset them in oprofile_timer_init() it's error prone. John any idea ?
--
Philippe Elie
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-08 23:56 ` Philippe Elie
@ 2004-12-09 0:39 ` Greg Banks
2004-12-09 1:46 ` John Levon
0 siblings, 1 reply; 16+ messages in thread
From: Greg Banks @ 2004-12-09 0:39 UTC (permalink / raw)
To: Philippe Elie; +Cc: Greg Banks, John Levon, Akinobu Mita, linux-kernel
On Thu, Dec 09, 2004 at 12:56:23AM +0100, Philippe Elie wrote:
>
> oprofile_timer_init doesn't reset op->setup/shutdown, I don't like the idea to
> reset them in oprofile_timer_init() it's error prone. John any idea ?
A better long term solution would be not to change the ops structure
but to copy and chain it, as I did in my experimental cswitch patches.
The approach I took allows for a new fake event which can be selected
at runtime with --event instead of with a boot option.
But for now I don't see any drama with leaving in the ->setup() and
->shutdown() methods when rewriting the ops structure. Ditto for
the ->create_files() methods.
Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-09 0:39 ` Greg Banks
@ 2004-12-09 1:46 ` John Levon
2004-12-09 1:50 ` Greg Banks
0 siblings, 1 reply; 16+ messages in thread
From: John Levon @ 2004-12-09 1:46 UTC (permalink / raw)
To: Greg Banks; +Cc: Philippe Elie, Akinobu Mita, linux-kernel
On Thu, Dec 09, 2004 at 11:39:06AM +1100, Greg Banks wrote:
> But for now I don't see any drama with leaving in the ->setup() and
> ->shutdown() methods when rewriting the ops structure. Ditto for
> the ->create_files() methods.
Wouldn't this mean that we try to set up the NMI stuff regardless of
forcing the timer ? I can imagine a flaky system where somebody needs to
avoid going near that stuff.
timer_init() making sure to set all fields seems reasonable to me. Or
oprofile_init() could grab ->backtrace, memset the structure, then
replace ->backtrace...
john
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-09 1:46 ` John Levon
@ 2004-12-09 1:50 ` Greg Banks
2004-12-09 2:23 ` John Levon
2004-12-09 14:22 ` Akinobu Mita
0 siblings, 2 replies; 16+ messages in thread
From: Greg Banks @ 2004-12-09 1:50 UTC (permalink / raw)
To: John Levon; +Cc: Greg Banks, Philippe Elie, Akinobu Mita, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 778 bytes --]
On Thu, Dec 09, 2004 at 01:46:22AM +0000, John Levon wrote:
> On Thu, Dec 09, 2004 at 11:39:06AM +1100, Greg Banks wrote:
>
> > But for now I don't see any drama with leaving in the ->setup() and
> > ->shutdown() methods when rewriting the ops structure. Ditto for
> > the ->create_files() methods.
>
> Wouldn't this mean that we try to set up the NMI stuff regardless of
> forcing the timer ? I can imagine a flaky system where somebody needs to
> avoid going near that stuff.
>
> timer_init() making sure to set all fields seems reasonable to me. Or
> oprofile_init() could grab ->backtrace, memset the structure, then
> replace ->backtrace...
Ok, how about this patch?
Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.
[-- Attachment #2: oprofile-timer-backtrace-fix-2 --]
[-- Type: text/plain, Size: 1429 bytes --]
Allow stack tracing to work when sampling on timer is forced
using the timer=1 boot option. Reported by Akinobu Mita.
Signed-off-by: Greg Banks <gnb@melbourne.sgi.com>
---
oprof.c | 6 ++----
timer_int.c | 3 +++
2 files changed, 5 insertions(+), 4 deletions(-)
Index: linux/drivers/oprofile/oprof.c
===================================================================
--- linux.orig/drivers/oprofile/oprof.c 2004-12-04 19:43:37.%N +1100
+++ linux/drivers/oprofile/oprof.c 2004-12-09 09:25:02.%N +1100
@@ -155,13 +155,11 @@ static int __init oprofile_init(void)
{
int err = 0;
- /* this is our fallback case */
- oprofile_timer_init(&oprofile_ops);
+ oprofile_arch_init(&oprofile_ops);
if (timer) {
printk(KERN_INFO "oprofile: using timer interrupt.\n");
- } else {
- oprofile_arch_init(&oprofile_ops);
+ oprofile_timer_init(&oprofile_ops);
}
err = oprofilefs_register();
Index: linux/drivers/oprofile/timer_int.c
===================================================================
--- linux.orig/drivers/oprofile/timer_int.c 2004-12-04 19:43:37.%N +1100
+++ linux/drivers/oprofile/timer_int.c 2004-12-09 12:48:52.%N +1100
@@ -37,6 +37,9 @@ static void timer_stop(void)
void __init oprofile_timer_init(struct oprofile_operations * ops)
{
+ ops->create_files = NULL;
+ ops->setup = NULL;
+ ops->shutdown = NULL;
ops->start = timer_start;
ops->stop = timer_stop;
ops->cpu_type = "timer";
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-09 1:50 ` Greg Banks
@ 2004-12-09 2:23 ` John Levon
2004-12-09 14:22 ` Akinobu Mita
1 sibling, 0 replies; 16+ messages in thread
From: John Levon @ 2004-12-09 2:23 UTC (permalink / raw)
To: Greg Banks; +Cc: Philippe Elie, Akinobu Mita, linux-kernel
On Thu, Dec 09, 2004 at 12:50:24PM +1100, Greg Banks wrote:
> Ok, how about this patch?
Fine by me
john
> Allow stack tracing to work when sampling on timer is forced
> using the timer=1 boot option. Reported by Akinobu Mita.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-09 1:50 ` Greg Banks
2004-12-09 2:23 ` John Levon
@ 2004-12-09 14:22 ` Akinobu Mita
2004-12-09 14:53 ` Akinobu Mita
2004-12-09 15:23 ` Greg Banks
1 sibling, 2 replies; 16+ messages in thread
From: Akinobu Mita @ 2004-12-09 14:22 UTC (permalink / raw)
To: Greg Banks, John Levon; +Cc: Greg Banks, Philippe Elie, linux-kernel
On Thursday 09 December 2004 10:50, Greg Banks wrote:
> On Thu, Dec 09, 2004 at 01:46:22AM +0000, John Levon wrote:
> > On Thu, Dec 09, 2004 at 11:39:06AM +1100, Greg Banks wrote:
> > > But for now I don't see any drama with leaving in the ->setup() and
> > > ->shutdown() methods when rewriting the ops structure. Ditto for
> > > the ->create_files() methods.
> >
> > Wouldn't this mean that we try to set up the NMI stuff regardless of
> > forcing the timer ? I can imagine a flaky system where somebody needs to
> > avoid going near that stuff.
> >
> > timer_init() making sure to set all fields seems reasonable to me. Or
> > oprofile_init() could grab ->backtrace, memset the structure, then
> > replace ->backtrace...
>
> Ok, how about this patch?
Thanks, but..
This patch is broken on several architectures (sparc64, sh, parisc, s390).
Even though i386 without CONFIG_X86_LOCAL_APIC and CONFIG_X86_IO_APIC.
Since the timer interrupt is the only way of getting sampling for oprofile
on such environments. if no module parameters specified (i.e. timer == 0),
then oprofile_timer_init() is never called. and I have got this error:
Unable to handle kernel NULL pointer dereference at virtual address 00000000
printing eip:
e81a97b0
*pde = 0b690001
Oops: 0000 [#1]
PREEMPT DEBUG_PAGEALLOC
Modules linked in: oprofile 3c59x microcode ntfs video
CPU: 0
EIP: 0060:[<e81a97b0>] Not tainted VLI
EFLAGS: 00010246 (2.6.10-rc2-mm4)
EIP is at oprofilefs_str_to_user+0x10/0x2c [oprofile]
eax: 00000000 ebx: 00000000 ecx: ffffffff edx: 00000000
esi: c7169f54 edi: 00000000 ebp: c7616f6c esp: c7616f68
ds: 007b es: 007b ss: 0068
Process cat (pid: 3366, threadinfo=c7616000 task=c716da60)
Stack: c7616fa8 c7616f90 c0171a80 00000000 0804d888 00001000 c7616fa8 c7169f54
fffffff7 0804d888 c7616fbc c0171cfe c7169f54 0804d888 00001000 c7616fa8
00000000 00000000 00000000 00000003 00001000 c7616000 c0103d41 00000003
Call Trace:
[<c0104397>] show_stack+0x6f/0x88
[<c01044c6>] show_registers+0xfe/0x160
[<c01046f1>] die+0x13d/0x288
[<c0112c74>] do_page_fault+0x324/0x6a1
[<c0103f3b>] error_code+0x2b/0x30
[<c0171a80>] vfs_read+0x88/0x104
[<c0171cfe>] sys_read+0x3a/0x64
[<c0103d41>] sysenter_past_esp+0x52/0x71
Code: 53 58 89 43 54 89 43 4c 89 53 50 89 43 44 89 53 48 89 d8 8b 5d fc c9 c3 8d 76 00 55 89 e5 8b 55 08 57 31 c0 b9 ff ff ff ff 89 d7 <f2> ae f7 d1 49 51 52 ff 75 14 ff 75 10 ff 75 0c e8 97 70 ff d7
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-09 14:22 ` Akinobu Mita
@ 2004-12-09 14:53 ` Akinobu Mita
2004-12-09 14:55 ` John Levon
2004-12-09 15:23 ` Greg Banks
1 sibling, 1 reply; 16+ messages in thread
From: Akinobu Mita @ 2004-12-09 14:53 UTC (permalink / raw)
To: Greg Banks, John Levon; +Cc: Greg Banks, Philippe Elie, linux-kernel
On Thursday 09 December 2004 23:22, Akinobu Mita wrote:
> Since the timer interrupt is the only way of getting sampling for oprofile
> on such environments. if no module parameters specified (i.e. timer == 0),
> then oprofile_timer_init() is never called. and I have got this error:
My first patch is not so bad, I think.
Or, It may be better to revert the return type of oprofile_arch_init()
from "void" to "int" to get the result of initialization.
Though I don't know when/why its interface was changed. and some
architectures (ppc, sh64, m32r) remain to have old interfaces in -mm.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-09 14:53 ` Akinobu Mita
@ 2004-12-09 14:55 ` John Levon
2004-12-09 15:27 ` Greg Banks
0 siblings, 1 reply; 16+ messages in thread
From: John Levon @ 2004-12-09 14:55 UTC (permalink / raw)
To: Akinobu Mita; +Cc: Greg Banks, Philippe Elie, linux-kernel
On Thu, Dec 09, 2004 at 11:53:53PM +0900, Akinobu Mita wrote:
> Or, It may be better to revert the return type of oprofile_arch_init()
> from "void" to "int" to get the result of initialization.
> Though I don't know when/why its interface was changed. and some
> architectures (ppc, sh64, m32r) remain to have old interfaces in -mm.
I've also lost track of why we don't return -ENODEV from the
arch_init().
We should set -ENODEV after setting ->backtrace as appropriate. Then the
code should do what it used to do:
138 int err = oprofile_arch_init(&oprofile_ops);
139
140 if (err == -ENODEV || timer) {
141 timer_init(&oprofile_ops);
142 err = 0;
143 } else if (err) {
144 goto out;
145 }
john
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-09 14:22 ` Akinobu Mita
2004-12-09 14:53 ` Akinobu Mita
@ 2004-12-09 15:23 ` Greg Banks
1 sibling, 0 replies; 16+ messages in thread
From: Greg Banks @ 2004-12-09 15:23 UTC (permalink / raw)
To: Akinobu Mita; +Cc: Greg Banks, John Levon, Philippe Elie, linux-kernel
On Thu, Dec 09, 2004 at 11:22:27PM +0900, Akinobu Mita wrote:
> On Thursday 09 December 2004 10:50, Greg Banks wrote:
> > Ok, how about this patch?
> [...]
> Since the timer interrupt is the only way of getting sampling for oprofile
> on such environments. if no module parameters specified (i.e. timer == 0),
> then oprofile_timer_init() is never called. and I have got this error:
Bother, back to the drawing board.
Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [mm patch] oprofile: backtrace operation does not initialized
2004-12-09 14:55 ` John Levon
@ 2004-12-09 15:27 ` Greg Banks
0 siblings, 0 replies; 16+ messages in thread
From: Greg Banks @ 2004-12-09 15:27 UTC (permalink / raw)
To: John Levon; +Cc: Akinobu Mita, Philippe Elie, linux-kernel
On Thu, Dec 09, 2004 at 02:55:19PM +0000, John Levon wrote:
> On Thu, Dec 09, 2004 at 11:53:53PM +0900, Akinobu Mita wrote:
>
> > Or, It may be better to revert the return type of oprofile_arch_init()
> > from "void" to "int" to get the result of initialization.
>
> I've also lost track of why we don't return -ENODEV from the
> arch_init().
I didn't see a good reason for that change either. Version 0.2
of the callgraph patch had the int return and worked fine. Let's
go back.
Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2004-12-09 15:28 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-12-08 9:30 [mm patch] oprofile: backtrace operation does not initialized Akinobu Mita
2004-12-08 9:34 ` Akinobu Mita
2004-12-08 11:13 ` Ingo Molnar
2004-12-08 11:37 ` Akinobu Mita
2004-12-08 16:00 ` John Levon
2004-12-08 22:31 ` Greg Banks
2004-12-08 23:56 ` Philippe Elie
2004-12-09 0:39 ` Greg Banks
2004-12-09 1:46 ` John Levon
2004-12-09 1:50 ` Greg Banks
2004-12-09 2:23 ` John Levon
2004-12-09 14:22 ` Akinobu Mita
2004-12-09 14:53 ` Akinobu Mita
2004-12-09 14:55 ` John Levon
2004-12-09 15:27 ` Greg Banks
2004-12-09 15:23 ` Greg Banks
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox