subject:"\[PATCH\] ARM\: oprofile\: Always allow backtraces"

Re: [PATCH] ARM: oprofile: Always allow backtraces

2011-01-20 Thread Robert Richter

On 20.01.11 04:42:24, Will Deacon wrote:
 Hi Ari,
 
  Always allow backtrace when using oprofile on ARM, even if a PMU
  isn't present.
  
  Restores functionality originally introduced in
  1b7b56982fdcd9d85effd76f3928cf5d6eb26155 by Richard Purdie.
 
 Well spotted, I'd forgotten we still want this for timer mode.
 Comments inline.
  
  diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
  index 8aa9744..a23e5ae 100644
  --- a/arch/arm/oprofile/common.c
  +++ b/arch/arm/oprofile/common.c
  @@ -46,6 +46,7 @@ char *op_name_from_perf_id(void)
  return NULL;
  }
   }
  +#endif
  
   static int report_trace(struct stackframe *frame, void *d)
   {
  @@ -109,6 +110,7 @@ static void arm_backtrace(struct pt_regs * const regs, 
  unsigned int depth)
  tail = user_backtrace(tail);
   }
  
  +#ifdef CONFIG_HW_PERF_EVENTS
 
 Can you reorder the backtrace stuff so that we don't need the extra
 #endif/#ifdef please?

I would like to go even further, see the diff below.

-Robert

---
 arch/arm/oprofile/common.c |   45 ---
 1 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 8aa9744..916aa2e 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -47,18 +47,6 @@ char *op_name_from_perf_id(void)
}
 }
 
-static int report_trace(struct stackframe *frame, void *d)
-{
-   unsigned int *depth = d;
-
-   if (*depth) {
-   oprofile_add_trace(frame-pc);
-   (*depth)--;
-   }
-
-   return *depth == 0;
-}
-
 /*
  * The registers we're interested in are at the end of the variable
  * length saved register structure. The fp points at the end of this
@@ -91,6 +79,30 @@ static struct frame_tail* user_backtrace(struct frame_tail 
*tail)
return buftail[0].fp-1;
 }
 
+#else
+
+static int oprofile_arch_init(struct oprofile_operations *ops)
+{
+   pr_info(oprofile: hardware counters not available\n);
+   return -ENODEV;
+}
+
+static void oprofile_arch_exit(void) { }
+
+#endif /* CONFIG_HW_PERF_EVENTS */
+
+static int report_trace(struct stackframe *frame, void *d)
+{
+   unsigned int *depth = d;
+
+   if (*depth) {
+   oprofile_add_trace(frame-pc);
+   (*depth)--;
+   }
+
+   return *depth == 0;
+}
+
 static void arm_backtrace(struct pt_regs * const regs, unsigned int depth)
 {
struct frame_tail *tail = ((struct frame_tail *) regs-ARM_fp) - 1;
@@ -111,6 +123,7 @@ static void arm_backtrace(struct pt_regs * const regs, 
unsigned int depth)
 
 int __init oprofile_arch_init(struct oprofile_operations *ops)
 {
+   /* provide backtrace support also in timer mode: */
ops-backtrace  = arm_backtrace;
 
return oprofile_perf_init(ops);
@@ -120,11 +133,3 @@ void __exit oprofile_arch_exit(void)
 {
oprofile_perf_exit();
 }
-#else
-int __init oprofile_arch_init(struct oprofile_operations *ops)
-{
-   pr_info(oprofile: hardware counters not available\n);
-   return -ENODEV;
-}
-void __exit oprofile_arch_exit(void) {}
-#endif /* CONFIG_HW_PERF_EVENTS */
-- 
1.7.3.4



-- 
Advanced Micro Devices, Inc.
Operating System Research Center

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: oprofile: Always allow backtraces

2011-01-20 Thread Robert Richter

On 20.01.11 10:52:39, Robert Richter wrote:

 I would like to go even further, see the diff below.
 
 -Robert
 
 ---
  arch/arm/oprofile/common.c |   45 ---
  1 files changed, 25 insertions(+), 20 deletions(-)
 
 diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
 index 8aa9744..916aa2e 100644
 --- a/arch/arm/oprofile/common.c
 +++ b/arch/arm/oprofile/common.c
 @@ -47,18 +47,6 @@ char *op_name_from_perf_id(void)
   }
  }
  
 -static int report_trace(struct stackframe *frame, void *d)
 -{
 - unsigned int *depth = d;
 -
 - if (*depth) {
 - oprofile_add_trace(frame-pc);
 - (*depth)--;
 - }
 -
 - return *depth == 0;
 -}
 -
  /*
   * The registers we're interested in are at the end of the variable
   * length saved register structure. The fp points at the end of this
 @@ -91,6 +79,30 @@ static struct frame_tail* user_backtrace(struct frame_tail 
 *tail)
   return buftail[0].fp-1;
  }
  
 +#else
 +
 +static int oprofile_arch_init(struct oprofile_operations *ops)

Of course it should compile, but in this sense...

-Robert

 +{
 + pr_info(oprofile: hardware counters not available\n);
 + return -ENODEV;
 +}
 +
 +static void oprofile_arch_exit(void) { }
 +
 +#endif /* CONFIG_HW_PERF_EVENTS */
 +
 +static int report_trace(struct stackframe *frame, void *d)
 +{
 + unsigned int *depth = d;
 +
 + if (*depth) {
 + oprofile_add_trace(frame-pc);
 + (*depth)--;
 + }
 +
 + return *depth == 0;
 +}
 +
  static void arm_backtrace(struct pt_regs * const regs, unsigned int depth)
  {
   struct frame_tail *tail = ((struct frame_tail *) regs-ARM_fp) - 1;
 @@ -111,6 +123,7 @@ static void arm_backtrace(struct pt_regs * const regs, 
 unsigned int depth)
  
  int __init oprofile_arch_init(struct oprofile_operations *ops)
  {
 + /* provide backtrace support also in timer mode: */
   ops-backtrace  = arm_backtrace;
  
   return oprofile_perf_init(ops);
 @@ -120,11 +133,3 @@ void __exit oprofile_arch_exit(void)
  {
   oprofile_perf_exit();
  }
 -#else
 -int __init oprofile_arch_init(struct oprofile_operations *ops)
 -{
 - pr_info(oprofile: hardware counters not available\n);
 - return -ENODEV;
 -}
 -void __exit oprofile_arch_exit(void) {}
 -#endif /* CONFIG_HW_PERF_EVENTS */


-- 
Advanced Micro Devices, Inc.
Operating System Research Center

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: oprofile: Always allow backtraces

2011-01-20 Thread Robert Richter

On 20.01.11 05:17:16, Ari Kauppi wrote:

  @@ -91,6 +79,30 @@ static struct frame_tail* user_backtrace(struct 
  frame_tail *tail)
  return buftail[0].fp-1;
  }
 
  +#else
  +
  +static int oprofile_arch_init(struct oprofile_operations *ops)
  +{
  +   pr_info(oprofile: hardware counters not available\n);
  +   return -ENODEV;
  +}
  +
  +static void oprofile_arch_exit(void) { }
 
 I guess you mean oprofile_perf_init and oprofile_perf_exit?

Yes, did not compile test it as my arm cross compile chain was broken.

Maybe we move it to include/linux/oprofile.h.

I also observed some mismatches while using CONFIG_HW_PERF_EVENTS and
CONFIG_PERF_EVENTS, esp. with include/linux/oprofile.h. /arch/sh is
also affected. We need to review this too.

 
 Also the user_backtrace and frame_tail need to be moved out from the 
 #ifdef.

Yes, thanks.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: oprofile: Always allow backtraces

2011-01-20 Thread Ari Kauppi


On Thu, 20 Jan 2011, Robert Richter wrote:

Hi,


On 20.01.11 04:42:24, Will Deacon wrote:

Hi Ari,


Always allow backtrace when using oprofile on ARM, even if a PMU
isn't present.

Restores functionality originally introduced in
1b7b56982fdcd9d85effd76f3928cf5d6eb26155 by Richard Purdie.


Well spotted, I'd forgotten we still want this for timer mode.
Comments inline.


diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 8aa9744..a23e5ae 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -46,6 +46,7 @@ char *op_name_from_perf_id(void)
return NULL;
}
 }
+#endif

 static int report_trace(struct stackframe *frame, void *d)
 {
@@ -109,6 +110,7 @@ static void arm_backtrace(struct pt_regs * const regs, 
unsigned int depth)
tail = user_backtrace(tail);
 }

+#ifdef CONFIG_HW_PERF_EVENTS


Can you reorder the backtrace stuff so that we don't need the extra
#endif/#ifdef please?


I would like to go even further, see the diff below.


Thanks for comments, I can prepare a new patch. I think your patch below 
has two typos but the idea was clear, please see comments below.



-Robert

---
arch/arm/oprofile/common.c |   45 ---
1 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 8aa9744..916aa2e 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -47,18 +47,6 @@ char *op_name_from_perf_id(void)
}
}

-static int report_trace(struct stackframe *frame, void *d)
-{
-   unsigned int *depth = d;
-
-   if (*depth) {
-   oprofile_add_trace(frame-pc);
-   (*depth)--;
-   }
-
-   return *depth == 0;
-}
-
/*
 * The registers we're interested in are at the end of the variable
 * length saved register structure. The fp points at the end of this
@@ -91,6 +79,30 @@ static struct frame_tail* user_backtrace(struct frame_tail 
*tail)
return buftail[0].fp-1;
}

+#else
+
+static int oprofile_arch_init(struct oprofile_operations *ops)
+{
+   pr_info(oprofile: hardware counters not available\n);
+   return -ENODEV;
+}
+
+static void oprofile_arch_exit(void) { }


I guess you mean oprofile_perf_init and oprofile_perf_exit?

Also the user_backtrace and frame_tail need to be moved out from the 
#ifdef.



+
+#endif /* CONFIG_HW_PERF_EVENTS */
+
+static int report_trace(struct stackframe *frame, void *d)
+{
+   unsigned int *depth = d;
+
+   if (*depth) {
+   oprofile_add_trace(frame-pc);
+   (*depth)--;
+   }
+
+   return *depth == 0;
+}
+
static void arm_backtrace(struct pt_regs * const regs, unsigned int depth)
{
struct frame_tail *tail = ((struct frame_tail *) regs-ARM_fp) - 1;
@@ -111,6 +123,7 @@ static void arm_backtrace(struct pt_regs * const regs, 
unsigned int depth)

int __init oprofile_arch_init(struct oprofile_operations *ops)
{
+   /* provide backtrace support also in timer mode: */
ops-backtrace   = arm_backtrace;

return oprofile_perf_init(ops);
@@ -120,11 +133,3 @@ void __exit oprofile_arch_exit(void)
{
oprofile_perf_exit();
}
-#else
-int __init oprofile_arch_init(struct oprofile_operations *ops)
-{
-   pr_info(oprofile: hardware counters not available\n);
-   return -ENODEV;
-}
-void __exit oprofile_arch_exit(void) {}
-#endif /* CONFIG_HW_PERF_EVENTS */



--
Ari

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: oprofile: Always allow backtraces

2011-01-20 Thread Sergei Shtylyov


Hello.

On 19-01-2011 23:54, Ari Kauppi wrote:


Always allow backtrace when using oprofile on ARM, even if a PMU
isn't present.



Restores functionality originally introduced in
1b7b56982fdcd9d85effd76f3928cf5d6eb26155 by Richard Purdie.


   You're lacking the commit summary specified in parens which Linus asked to 
add.



Signed-off-by: Ari Kauppikau...@papupata.org
Cc: Richard Purdierpur...@rpsys.net
Cc: Will Deaconwill.dea...@arm.com
Cc: Matt Flemingm...@console-pimps.org


WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ARM: oprofile: Always allow backtraces

2011-01-19 Thread Ari Kauppi

Always allow backtrace when using oprofile on ARM, even if a PMU
isn't present.

Restores functionality originally introduced in
1b7b56982fdcd9d85effd76f3928cf5d6eb26155 by Richard Purdie.

Signed-off-by: Ari Kauppi kau...@papupata.org
Cc: Richard Purdie rpur...@rpsys.net
Cc: Will Deacon will.dea...@arm.com
Cc: Matt Fleming m...@console-pimps.org
---
 arch/arm/oprofile/common.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 8aa9744..a23e5ae 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -46,6 +46,7 @@ char *op_name_from_perf_id(void)
return NULL;
}
 }
+#endif
 
 static int report_trace(struct stackframe *frame, void *d)
 {
@@ -109,6 +110,7 @@ static void arm_backtrace(struct pt_regs * const regs, 
unsigned int depth)
tail = user_backtrace(tail);
 }
 
+#ifdef CONFIG_HW_PERF_EVENTS
 int __init oprofile_arch_init(struct oprofile_operations *ops)
 {
ops-backtrace  = arm_backtrace;
@@ -124,6 +126,8 @@ void __exit oprofile_arch_exit(void)
 int __init oprofile_arch_init(struct oprofile_operations *ops)
 {
pr_info(oprofile: hardware counters not available\n);
+   ops-backtrace  = arm_backtrace;
+
return -ENODEV;
 }
 void __exit oprofile_arch_exit(void) {}
-- 
1.7.4.rc2.3.g60a2e

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: oprofile: Always allow backtraces

Re: [PATCH] ARM: oprofile: Always allow backtraces

Re: [PATCH] ARM: oprofile: Always allow backtraces

Re: [PATCH] ARM: oprofile: Always allow backtraces

Re: [PATCH] ARM: oprofile: Always allow backtraces

[PATCH] ARM: oprofile: Always allow backtraces

6 matches

Site Navigation

Mail list logo

Footer information