[PATCH 0/2] ftrace: updates for powerpc

[PATCH 0/2] ftrace: updates for powerpc

[Steven Rostedt]

Paul,

The first patch is a ftrace clean up to use the proper pr_debug macro.

The second patch is to fix a compile error when dynamic ftrace
is compiled in, but modules are not.

The following patches are in:

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git

    branch: ppc/ftrace


Steven Rostedt (2):
      ftrace, powerpc: replace debug macro with proper pr_deug
      powerpc, ftrace: fix compile error when modules not configured

----
 arch/powerpc/kernel/ftrace.c |   42 +++++++++++++++++++++++++-----------------
 1 files changed, 25 insertions(+), 17 deletions(-)
-- 

[PATCH 1/2] ftrace, powerpc: replace debug macro with proper pr_deug

[Steven Rostedt]

[-- Attachment #1: 0001-ftrace-powerpc-replace-debug-macro-with-proper-pr_.patch --]
[-- Type: text/plain, Size: 3336 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Impact: cleanup

The PowerPC ftrace code uses a hacked up DEBUGP macro for prints.
This patch converts it to the standard pr_debug.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 arch/powerpc/kernel/ftrace.c |   26 ++++++++++----------------
 1 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index 5355244..a913f91 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -20,12 +20,6 @@
 #include <asm/code-patching.h>
 #include <asm/ftrace.h>
 
-#if 0
-#define DEBUGP printk
-#else
-#define DEBUGP(fmt , ...)	do { } while (0)
-#endif
-
 static unsigned int ftrace_nop = PPC_NOP_INSTR;
 
 #ifdef CONFIG_PPC32
@@ -175,7 +169,7 @@ __ftrace_make_nop(struct module *mod,
 	 * 0xe8, 0x4c, 0x00, 0x28,    ld      r2,40(r12)
 	 */
 
-	DEBUGP("ip:%lx jumps to %lx r2: %lx", ip, tramp, mod->arch.toc);
+	pr_debug("ip:%lx jumps to %lx r2: %lx", ip, tramp, mod->arch.toc);
 
 	/* Find where the trampoline jumps to */
 	if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
@@ -183,7 +177,7 @@ __ftrace_make_nop(struct module *mod,
 		return -EFAULT;
 	}
 
-	DEBUGP(" %08x %08x", jmp[0], jmp[1]);
+	pr_debug(" %08x %08x", jmp[0], jmp[1]);
 
 	/* verify that this is what we expect it to be */
 	if (((jmp[0] & 0xffff0000) != 0x3d820000) ||
@@ -198,18 +192,18 @@ __ftrace_make_nop(struct module *mod,
 	offset = (unsigned)((unsigned short)jmp[0]) << 16 |
 		(unsigned)((unsigned short)jmp[1]);
 
-	DEBUGP(" %x ", offset);
+	pr_debug(" %x ", offset);
 
 	/* get the address this jumps too */
 	tramp = mod->arch.toc + offset + 32;
-	DEBUGP("toc: %lx", tramp);
+	pr_debug("toc: %lx", tramp);
 
 	if (probe_kernel_read(jmp, (void *)tramp, 8)) {
 		printk(KERN_ERR "Failed to read %lx\n", tramp);
 		return -EFAULT;
 	}
 
-	DEBUGP(" %08x %08x\n", jmp[0], jmp[1]);
+	pr_debug(" %08x %08x\n", jmp[0], jmp[1]);
 
 	ptr = ((unsigned long)jmp[0] << 32) + jmp[1];
 
@@ -286,7 +280,7 @@ __ftrace_make_nop(struct module *mod,
 	 *  0x4e, 0x80, 0x04, 0x20  bctr
 	 */
 
-	DEBUGP("ip:%lx jumps to %lx", ip, tramp);
+	pr_debug("ip:%lx jumps to %lx", ip, tramp);
 
 	/* Find where the trampoline jumps to */
 	if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
@@ -294,7 +288,7 @@ __ftrace_make_nop(struct module *mod,
 		return -EFAULT;
 	}
 
-	DEBUGP(" %08x %08x ", jmp[0], jmp[1]);
+	pr_debug(" %08x %08x ", jmp[0], jmp[1]);
 
 	/* verify that this is what we expect it to be */
 	if (((jmp[0] & 0xffff0000) != 0x3d600000) ||
@@ -310,7 +304,7 @@ __ftrace_make_nop(struct module *mod,
 	if (tramp & 0x8000)
 		tramp -= 0x10000;
 
-	DEBUGP(" %x ", tramp);
+	pr_debug(" %x ", tramp);
 
 	if (tramp != addr) {
 		printk(KERN_ERR
@@ -413,7 +407,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	/* ld r2,40(r1) */
 	op[1] = 0xe8410028;
 
-	DEBUGP("write to %lx\n", rec->ip);
+	pr_debug("write to %lx\n", rec->ip);
 
 	if (probe_kernel_write((void *)ip, op, MCOUNT_INSN_SIZE * 2))
 		return -EPERM;
@@ -453,7 +447,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	DEBUGP("write to %lx\n", rec->ip);
+	pr_debug("write to %lx\n", rec->ip);
 
 	if (probe_kernel_write((void *)ip, &op, MCOUNT_INSN_SIZE))
 		return -EPERM;
-- 
1.5.6.5

-- 

Re: [PATCH 1/2] ftrace, powerpc: replace debug macro with proper pr_deug

[Steven Rostedt]

[ forgot to add linuxppc ]

On Fri, 6 Feb 2009, Steven Rostedt wrote:

> From: Steven Rostedt <srostedt@redhat.com>
> 
> Impact: cleanup
> 
> The PowerPC ftrace code uses a hacked up DEBUGP macro for prints.
> This patch converts it to the standard pr_debug.
> 
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> ---
>  arch/powerpc/kernel/ftrace.c |   26 ++++++++++----------------
>  1 files changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
> index 5355244..a913f91 100644
> --- a/arch/powerpc/kernel/ftrace.c
> +++ b/arch/powerpc/kernel/ftrace.c
> @@ -20,12 +20,6 @@
>  #include <asm/code-patching.h>
>  #include <asm/ftrace.h>
>  
> -#if 0
> -#define DEBUGP printk
> -#else
> -#define DEBUGP(fmt , ...)	do { } while (0)
> -#endif
> -
>  static unsigned int ftrace_nop = PPC_NOP_INSTR;
>  
>  #ifdef CONFIG_PPC32
> @@ -175,7 +169,7 @@ __ftrace_make_nop(struct module *mod,
>  	 * 0xe8, 0x4c, 0x00, 0x28,    ld      r2,40(r12)
>  	 */
>  
> -	DEBUGP("ip:%lx jumps to %lx r2: %lx", ip, tramp, mod->arch.toc);
> +	pr_debug("ip:%lx jumps to %lx r2: %lx", ip, tramp, mod->arch.toc);
>  
>  	/* Find where the trampoline jumps to */
>  	if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
> @@ -183,7 +177,7 @@ __ftrace_make_nop(struct module *mod,
>  		return -EFAULT;
>  	}
>  
> -	DEBUGP(" %08x %08x", jmp[0], jmp[1]);
> +	pr_debug(" %08x %08x", jmp[0], jmp[1]);
>  
>  	/* verify that this is what we expect it to be */
>  	if (((jmp[0] & 0xffff0000) != 0x3d820000) ||
> @@ -198,18 +192,18 @@ __ftrace_make_nop(struct module *mod,
>  	offset = (unsigned)((unsigned short)jmp[0]) << 16 |
>  		(unsigned)((unsigned short)jmp[1]);
>  
> -	DEBUGP(" %x ", offset);
> +	pr_debug(" %x ", offset);
>  
>  	/* get the address this jumps too */
>  	tramp = mod->arch.toc + offset + 32;
> -	DEBUGP("toc: %lx", tramp);
> +	pr_debug("toc: %lx", tramp);
>  
>  	if (probe_kernel_read(jmp, (void *)tramp, 8)) {
>  		printk(KERN_ERR "Failed to read %lx\n", tramp);
>  		return -EFAULT;
>  	}
>  
> -	DEBUGP(" %08x %08x\n", jmp[0], jmp[1]);
> +	pr_debug(" %08x %08x\n", jmp[0], jmp[1]);
>  
>  	ptr = ((unsigned long)jmp[0] << 32) + jmp[1];
>  
> @@ -286,7 +280,7 @@ __ftrace_make_nop(struct module *mod,
>  	 *  0x4e, 0x80, 0x04, 0x20  bctr
>  	 */
>  
> -	DEBUGP("ip:%lx jumps to %lx", ip, tramp);
> +	pr_debug("ip:%lx jumps to %lx", ip, tramp);
>  
>  	/* Find where the trampoline jumps to */
>  	if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
> @@ -294,7 +288,7 @@ __ftrace_make_nop(struct module *mod,
>  		return -EFAULT;
>  	}
>  
> -	DEBUGP(" %08x %08x ", jmp[0], jmp[1]);
> +	pr_debug(" %08x %08x ", jmp[0], jmp[1]);
>  
>  	/* verify that this is what we expect it to be */
>  	if (((jmp[0] & 0xffff0000) != 0x3d600000) ||
> @@ -310,7 +304,7 @@ __ftrace_make_nop(struct module *mod,
>  	if (tramp & 0x8000)
>  		tramp -= 0x10000;
>  
> -	DEBUGP(" %x ", tramp);
> +	pr_debug(" %x ", tramp);
>  
>  	if (tramp != addr) {
>  		printk(KERN_ERR
> @@ -413,7 +407,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
>  	/* ld r2,40(r1) */
>  	op[1] = 0xe8410028;
>  
> -	DEBUGP("write to %lx\n", rec->ip);
> +	pr_debug("write to %lx\n", rec->ip);
>  
>  	if (probe_kernel_write((void *)ip, op, MCOUNT_INSN_SIZE * 2))
>  		return -EPERM;
> @@ -453,7 +447,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
>  		return -EINVAL;
>  	}
>  
> -	DEBUGP("write to %lx\n", rec->ip);
> +	pr_debug("write to %lx\n", rec->ip);
>  
>  	if (probe_kernel_write((void *)ip, &op, MCOUNT_INSN_SIZE))
>  		return -EPERM;
> -- 
> 1.5.6.5
> 
> -- 
> 
> 

Re: [PATCH 1/2] ftrace, powerpc: replace debug macro with proper pr_deug

[Benjamin Herrenschmidt]

On Fri, 2009-02-06 at 01:03 -0500, Steven Rostedt wrote:
> plain text document attachment
> (0001-ftrace-powerpc-replace-debug-macro-with-proper-pr_.patch)
> From: Steven Rostedt <srostedt@redhat.com>
> 
> Impact: cleanup
> 
> The PowerPC ftrace code uses a hacked up DEBUGP macro for prints.
> This patch converts it to the standard pr_debug.
>
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Want me to go through powerpc tree or you'll merge via Ingo ?

Re: [PATCH 1/2] ftrace, powerpc: replace debug macro with proper pr_deug

[Steven Rostedt]

On Fri, 6 Feb 2009, Benjamin Herrenschmidt wrote:

> On Fri, 2009-02-06 at 01:03 -0500, Steven Rostedt wrote:
> > plain text document attachment
> > (0001-ftrace-powerpc-replace-debug-macro-with-proper-pr_.patch)
> > From: Steven Rostedt <srostedt@redhat.com>
> > 
> > Impact: cleanup
> > 
> > The PowerPC ftrace code uses a hacked up DEBUGP macro for prints.
> > This patch converts it to the standard pr_debug.
> >
> > Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> 
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> Want me to go through powerpc tree or you'll merge via Ingo ?

These two changes are powerpc specific. I think it may get more testing 
going through the powerpc tree than through tip.

-- Steve

[PATCH 2/2] powerpc, ftrace: fix compile error when modules not configured

[Steven Rostedt]

[-- Attachment #1: 0002-powerpc-ftrace-fix-compile-error-when-modules-not.patch --]
[-- Type: text/plain, Size: 2473 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Michael Neuling reported a compile bug when dynamic ftrace was
configured in and modules were not. This was due to the ftrace
code referencing module specific structures.

Reported-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 arch/powerpc/kernel/ftrace.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index a913f91..88c641d 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -113,6 +113,8 @@ static int test_24bit_addr(unsigned long ip, unsigned long addr)
 	return create_branch((unsigned int *)ip, addr, 0);
 }
 
+#ifdef CONFIG_MODULES
+
 static int is_bl_op(unsigned int op)
 {
 	return (op & 0xfc000003) == 0x48000001;
@@ -323,6 +325,7 @@ __ftrace_make_nop(struct module *mod,
 	return 0;
 }
 #endif /* PPC64 */
+#endif /* CONFIG_MODULES */
 
 int ftrace_make_nop(struct module *mod,
 		    struct dyn_ftrace *rec, unsigned long addr)
@@ -342,6 +345,7 @@ int ftrace_make_nop(struct module *mod,
 		return ftrace_modify_code(ip, old, new);
 	}
 
+#ifdef CONFIG_MODULES
 	/*
 	 * Out of range jumps are called from modules.
 	 * We should either already have a pointer to the module
@@ -366,9 +370,13 @@ int ftrace_make_nop(struct module *mod,
 		mod = rec->arch.mod;
 
 	return __ftrace_make_nop(mod, rec, addr);
-
+#else
+	/* We should not get here without modules */
+	return -EINVAL;
+#endif /* CONFIG_MODULES */
 }
 
+#ifdef CONFIG_MODULES
 #ifdef CONFIG_PPC64
 static int
 __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
@@ -457,6 +465,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	return 0;
 }
 #endif /* CONFIG_PPC64 */
+#endif /* CONFIG_MODULES */
 
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
@@ -475,6 +484,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 		return ftrace_modify_code(ip, old, new);
 	}
 
+#ifdef CONFIG_MODULES
 	/*
 	 * Out of range jumps are called from modules.
 	 * Being that we are converting from nop, it had better
@@ -486,6 +496,10 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	}
 
 	return __ftrace_make_call(rec, addr);
+#else
+	/* We should not get here without modules */
+	return -EINVAL;
+#endif /* CONFIG_MODULES */
 }
 
 int ftrace_update_ftrace_func(ftrace_func_t func)
-- 
1.5.6.5

-- 

Re: [PATCH 2/2] powerpc, ftrace: fix compile error when modules not configured

[Steven Rostedt]

[ forgot to add linuxppc ]


On Fri, 6 Feb 2009, Steven Rostedt wrote:

> From: Steven Rostedt <srostedt@redhat.com>
> 
> Michael Neuling reported a compile bug when dynamic ftrace was
> configured in and modules were not. This was due to the ftrace
> code referencing module specific structures.
> 
> Reported-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> ---
>  arch/powerpc/kernel/ftrace.c |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
> index a913f91..88c641d 100644
> --- a/arch/powerpc/kernel/ftrace.c
> +++ b/arch/powerpc/kernel/ftrace.c
> @@ -113,6 +113,8 @@ static int test_24bit_addr(unsigned long ip, unsigned long addr)
>  	return create_branch((unsigned int *)ip, addr, 0);
>  }
>  
> +#ifdef CONFIG_MODULES
> +
>  static int is_bl_op(unsigned int op)
>  {
>  	return (op & 0xfc000003) == 0x48000001;
> @@ -323,6 +325,7 @@ __ftrace_make_nop(struct module *mod,
>  	return 0;
>  }
>  #endif /* PPC64 */
> +#endif /* CONFIG_MODULES */
>  
>  int ftrace_make_nop(struct module *mod,
>  		    struct dyn_ftrace *rec, unsigned long addr)
> @@ -342,6 +345,7 @@ int ftrace_make_nop(struct module *mod,
>  		return ftrace_modify_code(ip, old, new);
>  	}
>  
> +#ifdef CONFIG_MODULES
>  	/*
>  	 * Out of range jumps are called from modules.
>  	 * We should either already have a pointer to the module
> @@ -366,9 +370,13 @@ int ftrace_make_nop(struct module *mod,
>  		mod = rec->arch.mod;
>  
>  	return __ftrace_make_nop(mod, rec, addr);
> -
> +#else
> +	/* We should not get here without modules */
> +	return -EINVAL;
> +#endif /* CONFIG_MODULES */
>  }
>  
> +#ifdef CONFIG_MODULES
>  #ifdef CONFIG_PPC64
>  static int
>  __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> @@ -457,6 +465,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
>  	return 0;
>  }
>  #endif /* CONFIG_PPC64 */
> +#endif /* CONFIG_MODULES */
>  
>  int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
>  {
> @@ -475,6 +484,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
>  		return ftrace_modify_code(ip, old, new);
>  	}
>  
> +#ifdef CONFIG_MODULES
>  	/*
>  	 * Out of range jumps are called from modules.
>  	 * Being that we are converting from nop, it had better
> @@ -486,6 +496,10 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
>  	}
>  
>  	return __ftrace_make_call(rec, addr);
> +#else
> +	/* We should not get here without modules */
> +	return -EINVAL;
> +#endif /* CONFIG_MODULES */
>  }
>  
>  int ftrace_update_ftrace_func(ftrace_func_t func)
> -- 
> 1.5.6.5
> 
> -- 
> 
> 

Re: [PATCH 2/2] powerpc, ftrace: fix compile error when modules not configured

[Michael Neuling]

In message <20090206060527.369616736@goodmis.org> you wrote:
> From: Steven Rostedt <srostedt@redhat.com>
> 
> Michael Neuling reported a compile bug when dynamic ftrace was
> configured in and modules were not. This was due to the ftrace
> code referencing module specific structures.
> 
> Reported-by: Michael Neuling <mikey@neuling.org>
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>

Steve,

Thanks, fixes the error I was seeing.  

As an aside, is there anyway we can merge some of the code in
arch/powerpc/kernel/ftrace.c between 32 and 64bit?  There seems to be a
lot of repeated code in there with only minor changes.

Mikey

> ---
>  arch/powerpc/kernel/ftrace.c |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
> index a913f91..88c641d 100644
> --- a/arch/powerpc/kernel/ftrace.c
> +++ b/arch/powerpc/kernel/ftrace.c
> @@ -113,6 +113,8 @@ static int test_24bit_addr(unsigned long ip, unsigned lon
g addr)
>  	return create_branch((unsigned int *)ip, addr, 0);
>  }
>  
> +#ifdef CONFIG_MODULES
> +
>  static int is_bl_op(unsigned int op)
>  {
>  	return (op & 0xfc000003) == 0x48000001;
> @@ -323,6 +325,7 @@ __ftrace_make_nop(struct module *mod,
>  	return 0;
>  }
>  #endif /* PPC64 */
> +#endif /* CONFIG_MODULES */
>  
>  int ftrace_make_nop(struct module *mod,
>  		    struct dyn_ftrace *rec, unsigned long addr)
> @@ -342,6 +345,7 @@ int ftrace_make_nop(struct module *mod,
>  		return ftrace_modify_code(ip, old, new);
>  	}
>  
> +#ifdef CONFIG_MODULES
>  	/*
>  	 * Out of range jumps are called from modules.
>  	 * We should either already have a pointer to the module
> @@ -366,9 +370,13 @@ int ftrace_make_nop(struct module *mod,
>  		mod = rec->arch.mod;
>  
>  	return __ftrace_make_nop(mod, rec, addr);
> -
> +#else
> +	/* We should not get here without modules */
> +	return -EINVAL;
> +#endif /* CONFIG_MODULES */
>  }
>  
> +#ifdef CONFIG_MODULES
>  #ifdef CONFIG_PPC64
>  static int
>  __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> @@ -457,6 +465,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long 
addr)
>  	return 0;
>  }
>  #endif /* CONFIG_PPC64 */
> +#endif /* CONFIG_MODULES */
>  
>  int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
>  {
> @@ -475,6 +484,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned lon
g addr)
>  		return ftrace_modify_code(ip, old, new);
>  	}
>  
> +#ifdef CONFIG_MODULES
>  	/*
>  	 * Out of range jumps are called from modules.
>  	 * Being that we are converting from nop, it had better
> @@ -486,6 +496,10 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned lo
ng addr)
>  	}
>  
>  	return __ftrace_make_call(rec, addr);
> +#else
> +	/* We should not get here without modules */
> +	return -EINVAL;
> +#endif /* CONFIG_MODULES */
>  }
>  
>  int ftrace_update_ftrace_func(ftrace_func_t func)
> -- 
> 1.5.6.5
> 
> -- 
> 

Re: [PATCH 0/2] ftrace: updates for powerpc

[Benjamin Herrenschmidt]

On Fri, 2009-02-06 at 01:03 -0500, Steven Rostedt wrote:
> Paul,
> 
> The first patch is a ftrace clean up to use the proper pr_debug macro.
> 
> The second patch is to fix a compile error when dynamic ftrace
> is compiled in, but modules are not.

Did you find out the problem with the large USB module ?

Cheers,
Ben.

> The following patches are in:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> 
>     branch: ppc/ftrace
> 
> 
> Steven Rostedt (2):
>       ftrace, powerpc: replace debug macro with proper pr_deug
>       powerpc, ftrace: fix compile error when modules not configured
> 
> ----
>  arch/powerpc/kernel/ftrace.c |   42 +++++++++++++++++++++++++-----------------
>  1 files changed, 25 insertions(+), 17 deletions(-)

Re: [PATCH 0/2] ftrace: updates for powerpc

[Steven Rostedt]

On Fri, 6 Feb 2009, Benjamin Herrenschmidt wrote:

> On Fri, 2009-02-06 at 01:03 -0500, Steven Rostedt wrote:
> > Paul,
> > 
> > The first patch is a ftrace clean up to use the proper pr_debug macro.
> > 
> > The second patch is to fix a compile error when dynamic ftrace
> > is compiled in, but modules are not.
> 
> Did you find out the problem with the large USB module ?

Not yet, I'm still trying to figure out why it does things differently on 
large modules.

-- Steve

hrtimer: about hres_active

[Iram Shahzad]

Hi

I am trying to understand the purpose of "hres_active" of hrtimer
and have the following question in this regard.

It seems "hres_active" indicates whether high resolution mode is
active or not. But I am not clear about the idea behind it.

I see that hres_active is initialized to 0 here:
    hrtimer_init_hres

and set to 1 here:
    hrtimer_run_pending
       -> hrtimer_switch_to_hres

That means hrtimer becomes "active" at the 1st timer softirq
and remains so forever. Is this understanding correct?

My original concern is as follows:

hrtimer_get_next_event returns KTIME_MAX when hrtimer is "active".
So if the above understanding is correct, then after the 1st timer
softirq it will always return KTIME_MAX. This means cpu_idle will never
take the hrtimer event into account and will always base its decision
on the next event of the timer wheel. Is this intended behaviour?

I would highly appreciate any information about this.
Please CC me because I am not a member of this ML.

Best regards
Iram

Re: hrtimer: about hres_active

[Thomas Gleixner]

Iram,

On Mon, 10 May 2010, Iram Shahzad wrote:

> I am trying to understand the purpose of "hres_active" of hrtimer
> and have the following question in this regard.
> 
> It seems "hres_active" indicates whether high resolution mode is
> active or not. But I am not clear about the idea behind it.
> 
> I see that hres_active is initialized to 0 here:
>    hrtimer_init_hres
> 
> and set to 1 here:
>    hrtimer_run_pending
>       -> hrtimer_switch_to_hres
> 
> That means hrtimer becomes "active" at the 1st timer softirq
> and remains so forever. Is this understanding correct?

No. The system switches to high resolution mode late in the boot
process and it does so only when there is high res capable hardware
available.
 
> My original concern is as follows:
> 
> hrtimer_get_next_event returns KTIME_MAX when hrtimer is "active".
> So if the above understanding is correct, then after the 1st timer
> softirq it will always return KTIME_MAX. This means cpu_idle will never
> take the hrtimer event into account and will always base its decision
> on the next event of the timer wheel. Is this intended behaviour?

Yes it is. In the case of high res active the hrtimer which is the
next to expire is already armed on that CPU in the clock event
device. Therefor we know already when the next hrtimer will fire. No
need to lookup further. But we have to check the timer wheel as it
might have a timer which is due earlier than the first to expire
hrtimer.

Thanks,

	tglx

Re: hrtimer: about hres_active

[Iram Shahzad]

Thomas,

Many thanks for the reply. I understood why hrtimer list
is not checked in hrtimer_get_next_event in the case of
high res active.

Please let me ask more about the following.

> No. The system switches to high resolution mode late in the boot
> process and it does so only when there is high res capable hardware
> available.

So far as I checked, hres_active is set to 1 in hrtimer_switch_to_hres.
hrtimer_switch_to_hres is only called from hrtimer_run_pending.
And hrtimer_run_pending is called from timer wheel softirq.
That is why I concluded that hres_active is set at 1st timer softirq.
Do you just mean that it is not the 1st softirq, or do you
mean that in some other place in the late boot process
there is another code which sets hres_active?

And is it correct that hres_active remains 1 forever once set to it?

Best regards
Iram

----- Original Message ----- 
From: "Thomas Gleixner" <tglx@linutronix.de>
To: "Iram Shahzad" <iram.shahzad@jp.fujitsu.com>
Cc: <linux-kernel@vger.kernel.org>
Sent: Tuesday, May 11, 2010 12:12 AM
Subject: Re: hrtimer: about hres_active


> Iram,
> 
> On Mon, 10 May 2010, Iram Shahzad wrote:
> 
>> I am trying to understand the purpose of "hres_active" of hrtimer
>> and have the following question in this regard.
>> 
>> It seems "hres_active" indicates whether high resolution mode is
>> active or not. But I am not clear about the idea behind it.
>> 
>> I see that hres_active is initialized to 0 here:
>>    hrtimer_init_hres
>> 
>> and set to 1 here:
>>    hrtimer_run_pending
>>       -> hrtimer_switch_to_hres
>> 
>> That means hrtimer becomes "active" at the 1st timer softirq
>> and remains so forever. Is this understanding correct?
> 
> No. The system switches to high resolution mode late in the boot
> process and it does so only when there is high res capable hardware
> available.
> 
>> My original concern is as follows:
>> 
>> hrtimer_get_next_event returns KTIME_MAX when hrtimer is "active".
>> So if the above understanding is correct, then after the 1st timer
>> softirq it will always return KTIME_MAX. This means cpu_idle will never
>> take the hrtimer event into account and will always base its decision
>> on the next event of the timer wheel. Is this intended behaviour?
> 
> Yes it is. In the case of high res active the hrtimer which is the
> next to expire is already armed on that CPU in the clock event
> device. Therefor we know already when the next hrtimer will fire. No
> need to lookup further. But we have to check the timer wheel as it
> might have a timer which is due earlier than the first to expire
> hrtimer.
> 
> Thanks,
> 
> tglx
>

Re: hrtimer: about hres_active

[Thomas Gleixner]

Iram,

please do _NOT_ top post.

On Tue, 11 May 2010, Iram Shahzad wrote:
> > No. The system switches to high resolution mode late in the boot
> > process and it does so only when there is high res capable hardware
> > available.
> 
> So far as I checked, hres_active is set to 1 in hrtimer_switch_to_hres.
> hrtimer_switch_to_hres is only called from hrtimer_run_pending.
> And hrtimer_run_pending is called from timer wheel softirq.
> That is why I concluded that hres_active is set at 1st timer softirq.
> Do you just mean that it is not the 1st softirq, or do you
> mean that in some other place in the late boot process
> there is another code which sets hres_active?

It happens from softirq context, but not from the 1st softirq.
 
> And is it correct that hres_active remains 1 forever once set to it?

Yes. There is no way back.
 
Thanks,

	tglx