Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1342069 > unrolled thread

[PATCH 01/12] powerpc/module: Only try to generate the ftrace_caller() stub once

Back to article view | Back to linux.kernel

Contents

  [PATCH 01/12] powerpc/module: Only try to generate the ftrace_caller() stub once Michael Ellerman <mpe@ellerman.id.au> - 2016-02-24 15:30 +0100
    [PATCH 02/12] powerpc/module: Mark module stubs with a magic value Michael Ellerman <mpe@ellerman.id.au> - 2016-02-24 15:40 +0100
      Re: [PATCH 02/12] powerpc/module: Mark module stubs with a magic  value Balbir Singh <bsingharora@gmail.com> - 2016-02-25 01:10 +0100
        Re: [PATCH 02/12] powerpc/module: Mark module stubs with a magic  value Michael Ellerman <mpe@ellerman.id.au> - 2016-02-25 07:50 +0100
      Re: [PATCH 02/12] powerpc/module: Mark module stubs with a magic  value Torsten Duwe <duwe@lst.de> - 2016-02-25 14:20 +0100
        Re: [PATCH 02/12] powerpc/module: Mark module stubs with a magic  value Michael Ellerman <mpe@ellerman.id.au> - 2016-02-26 11:40 +0100
    [PATCH 06/12] powerpc/module: Rework is_early_mcount_callsite() Michael Ellerman <mpe@ellerman.id.au> - 2016-02-24 15:40 +0100
      Re: [PATCH 06/12] powerpc/module: Rework is_early_mcount_callsite() Balbir Singh <bsingharora@gmail.com> - 2016-02-25 00:40 +0100
        Re: [PATCH 06/12] powerpc/module: Rework is_early_mcount_callsite() Michael Ellerman <mpe@ellerman.id.au> - 2016-02-25 11:30 +0100
          Re: [PATCH 06/12] powerpc/module: Rework is_early_mcount_callsite() Torsten Duwe <duwe@lst.de> - 2016-02-25 15:10 +0100
    [PATCH 05/12] powerpc/ftrace: ftrace_graph_caller() needs to save/restore toc Michael Ellerman <mpe@ellerman.id.au> - 2016-02-24 15:40 +0100
      Re: [PATCH 05/12] powerpc/ftrace: ftrace_graph_caller() needs to  save/restore toc Balbir Singh <bsingharora@gmail.com> - 2016-02-25 01:40 +0100
        Re: [PATCH 05/12] powerpc/ftrace: ftrace_graph_caller() needs to  save/restore toc Michael Ellerman <mpe@ellerman.id.au> - 2016-02-25 11:50 +0100
        Re: [PATCH 05/12] powerpc/ftrace: ftrace_graph_caller() needs to  save/restore toc Torsten Duwe <duwe@lst.de> - 2016-02-25 15:10 +0100
    [PATCH 10/12] powerpc/ftrace: FTRACE_WITH_REGS configuration variables Michael Ellerman <mpe@ellerman.id.au> - 2016-02-24 15:40 +0100
      Re: [PATCH 10/12] powerpc/ftrace: FTRACE_WITH_REGS configuration  variables Balbir Singh <bsingharora@gmail.com> - 2016-02-25 02:20 +0100
        Re: [PATCH 10/12] powerpc/ftrace: FTRACE_WITH_REGS configuration  variables Torsten Duwe <duwe@lst.de> - 2016-02-25 15:40 +0100
    [PATCH 07/12] powerpc/ftrace: FTRACE_WITH_REGS implementation for ppc64le Michael Ellerman <mpe@ellerman.id.au> - 2016-02-24 15:40 +0100
      Re: [PATCH 07/12] powerpc/ftrace: FTRACE_WITH_REGS implementation for  ppc64le Balbir Singh <bsingharora@gmail.com> - 2016-02-25 01:50 +0100
        Re: [PATCH 07/12] powerpc/ftrace: FTRACE_WITH_REGS implementation  for ppc64le Torsten Duwe <duwe@lst.de> - 2016-02-25 16:20 +0100
          Re: [PATCH 07/12] powerpc/ftrace: FTRACE_WITH_REGS implementation  for ppc64le Michael Ellerman <mpe@ellerman.id.au> - 2016-02-26 11:20 +0100
    [PATCH 09/12] powerpc/ftrace: Use generic ftrace_modify_all_code() Michael Ellerman <mpe@ellerman.id.au> - 2016-02-24 15:40 +0100
      Re: [PATCH 09/12] powerpc/ftrace: Use generic  ftrace_modify_all_code() Balbir Singh <bsingharora@gmail.com> - 2016-02-25 02:20 +0100
    [PATCH 03/12] powerpc/module: Create a special stub for ftrace_caller() Michael Ellerman <mpe@ellerman.id.au> - 2016-02-24 15:40 +0100
      Re: [PATCH 03/12] powerpc/module: Create a special stub for  ftrace_caller() Balbir Singh <bsingharora@gmail.com> - 2016-02-25 01:10 +0100
        Re: [PATCH 03/12] powerpc/module: Create a special stub for  ftrace_caller() Michael Ellerman <mpe@ellerman.id.au> - 2016-02-25 11:50 +0100
        Re: [PATCH 03/12] powerpc/module: Create a special stub for  ftrace_caller() Torsten Duwe <duwe@lst.de> - 2016-02-25 14:40 +0100
          Re: [PATCH 03/12] powerpc/module: Create a special stub for  ftrace_caller() Michael Ellerman <mpe@ellerman.id.au> - 2016-02-26 11:40 +0100
    Re: [PATCH 01/12] powerpc/module: Only try to generate the  ftrace_caller() stub once Balbir Singh <bsingharora@gmail.com> - 2016-02-25 01:00 +0100
      Re: [PATCH 01/12] powerpc/module: Only try to generate the  ftrace_caller() stub once Balbir Singh <bsingharora@gmail.com> - 2016-02-25 05:40 +0100
    Re: [PATCH 01/12] powerpc/module: Only try to generate the  ftrace_caller() stub once Torsten Duwe <duwe@lst.de> - 2016-02-25 14:10 +0100
    Re: [PATCH 01/12] powerpc/module: Only try to generate the  ftrace_caller() stub once Kamalesh Babulal <kamalesh@linux.vnet.ibm.com> - 2016-02-25 15:40 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1344003 — Re: [PATCH 07/12] powerpc/ftrace: FTRACE_WITH_REGS implementation for ppc64le

On Thu, 2016-02-25 at 16:11 +0100, Torsten Duwe wrote:
> On Thu, Feb 25, 2016 at 11:48:59AM +1100, Balbir Singh wrote:

> > > @@ -608,6 +621,9 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
> > >  					return -ENOENT;
> > >  				if (!restore_r2((u32 *)location + 1, me))
> > >  					return -ENOEXEC;
> > > +				/* Squash the TOC saver for profiler calls */
> > > +				if (!strcmp("_mcount", strtab+sym->st_name))
> > > +					SQUASH_TOC_SAVE_INSN(value);
> > I don't think we need this anymore, do we?
>
> I'm not sure. Once a module is loaded, are all the "bl _mcount"s NOPed out
> before any of its functions are run? If not, the _mcount trampoline will
> be used, and it must not save R2!

With dynamic ftrace, yes they are all nop'ed out before the module runs. See
ftrace_module_init() called from load_module().

But with static ftrace they are just left as-is.

As this series is currently written you can't enable mprofile-kernel with
static ftrace. But that's a bit fragile, someone could easily send a patch to
enable it for static ftrace and we'd probably merge it without thinking about
this code. So I'll leave this as is for now, and we will clean it up once the
series is in.

cheers

[toc] | [prev] | [next] | [standalone]

#1342081 — [PATCH 09/12] powerpc/ftrace: Use generic ftrace_modify_all_code()

From: Torsten Duwe <duwe@lst.de>

Convert powerpc's arch_ftrace_update_code() from its own version to use
the generic default functionality (without stop_machine -- our
instructions are properly aligned and the replacements atomic).

With this we gain error checking and the much-needed function_trace_op
handling.

Signed-off-by: Torsten Duwe <duwe@suse.de>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/ftrace.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index c6408a399ac6..56e5bd53c323 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -527,20 +527,13 @@ void ftrace_replace_code(int enable)
 	}
 }
 
+/*
+ * Use the default ftrace_modify_all_code, but without
+ * stop_machine().
+ */
 void arch_ftrace_update_code(int command)
 {
-	if (command & FTRACE_UPDATE_CALLS)
-		ftrace_replace_code(1);
-	else if (command & FTRACE_DISABLE_CALLS)
-		ftrace_replace_code(0);
-
-	if (command & FTRACE_UPDATE_TRACE_FUNC)
-		ftrace_update_ftrace_func(ftrace_trace_function);
-
-	if (command & FTRACE_START_FUNC_RET)
-		ftrace_enable_ftrace_graph_caller();
-	else if (command & FTRACE_STOP_FUNC_RET)
-		ftrace_disable_ftrace_graph_caller();
+	ftrace_modify_all_code(command);
 }
 
 int __init ftrace_dyn_arch_init(void)
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1342564 — Re: [PATCH 09/12] powerpc/ftrace: Use generic ftrace_modify_all_code()

On 25/02/16 01:28, Michael Ellerman wrote:
> From: Torsten Duwe <duwe@lst.de>
>
> Convert powerpc's arch_ftrace_update_code() from its own version to use
> the generic default functionality (without stop_machine -- our
> instructions are properly aligned and the replacements atomic).
>
> With this we gain error checking and the much-needed function_trace_op
> handling.
>
> Signed-off-by: Torsten Duwe <duwe@suse.de>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/kernel/ftrace.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
> index c6408a399ac6..56e5bd53c323 100644
> --- a/arch/powerpc/kernel/ftrace.c
> +++ b/arch/powerpc/kernel/ftrace.c
> @@ -527,20 +527,13 @@ void ftrace_replace_code(int enable)
>  	}
>  }
>  
> +/*
> + * Use the default ftrace_modify_all_code, but without
> + * stop_machine().
> + */
>  void arch_ftrace_update_code(int command)
>  {
> -	if (command & FTRACE_UPDATE_CALLS)
> -		ftrace_replace_code(1);
> -	else if (command & FTRACE_DISABLE_CALLS)
> -		ftrace_replace_code(0);
> -
> -	if (command & FTRACE_UPDATE_TRACE_FUNC)
> -		ftrace_update_ftrace_func(ftrace_trace_function);
> -
> -	if (command & FTRACE_START_FUNC_RET)
> -		ftrace_enable_ftrace_graph_caller();
> -	else if (command & FTRACE_STOP_FUNC_RET)
> -		ftrace_disable_ftrace_graph_caller();
> +	ftrace_modify_all_code(command);
>  }
>  
>  int __init ftrace_dyn_arch_init(void)
Reviewed-by: Balbir Singh <bsingharora@gmail.com>

Balbir

[toc] | [prev] | [next] | [standalone]

#1342083 — [PATCH 03/12] powerpc/module: Create a special stub for ftrace_caller()

In order to support the new -mprofile-kernel ABI, we need to be able to
call from the module back to ftrace_caller() (in the kernel) without
using the module's r2. That is because the function in this module which
is calling ftrace_caller() may not have setup r2, if it doesn't
otherwise need it (ie. it accesses no globals).

To make that work we add a new stub which is used for calling
ftrace_caller(), which uses the kernel toc instead of the module toc.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/module_64.c | 48 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 9629966e614b..e711d40a3b8f 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -671,10 +671,56 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 }
 
 #ifdef CONFIG_DYNAMIC_FTRACE
+
+#define PACATOC offsetof(struct paca_struct, kernel_toc)
+
+static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs, struct module *me)
+{
+	struct ppc64_stub_entry *entry;
+	unsigned int i, num_stubs;
+	static u32 stub_insns[] = {
+		0xe98d0000 | PACATOC, 	/* ld      r12,PACATOC(r13)	*/
+		0x3d8c0000,		/* addis   r12,r12,<high>	*/
+		0x398c0000, 		/* addi    r12,r12,<low>	*/
+		0x7d8903a6, 		/* mtctr   r12			*/
+		0x4e800420, 		/* bctr				*/
+	};
+	long reladdr;
+
+	num_stubs = sechdrs[me->arch.stubs_section].sh_size / sizeof(*entry);
+
+	/* Find the next available stub entry */
+	entry = (void *)sechdrs[me->arch.stubs_section].sh_addr;
+	for (i = 0; i < num_stubs && stub_func_addr(entry->funcdata); i++, entry++);
+
+	if (i >= num_stubs) {
+		pr_err("%s: Unable to find a free slot for ftrace stub.\n", me->name);
+		return 0;
+	}
+
+	memcpy(entry->jump, stub_insns, sizeof(stub_insns));
+
+	/* Stub uses address relative to kernel_toc */
+	reladdr = (unsigned long)ftrace_caller - get_paca()->kernel_toc;
+	if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) {
+		pr_err("%s: Address of ftrace_caller out of range of kernel_toc.\n", me->name);
+		return 0;
+	}
+
+	entry->jump[1] |= PPC_HA(reladdr);
+	entry->jump[2] |= PPC_LO(reladdr);
+
+	/* Eventhough we don't use funcdata in the stub, it's needed elsewhere. */
+	entry->funcdata = func_desc((unsigned long)ftrace_caller);
+	entry->magic = STUB_MAGIC;
+
+	return (unsigned long)entry;
+}
+
 int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs)
 {
 	mod->arch.toc = my_r2(sechdrs, mod);
-	mod->arch.tramp = stub_for_addr(sechdrs, (unsigned long)ftrace_caller, mod);
+	mod->arch.tramp = create_ftrace_stub(sechdrs, mod);
 
 	if (!mod->arch.tramp)
 		return -ENOENT;
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1342529 — Re: [PATCH 03/12] powerpc/module: Create a special stub for ftrace_caller()

On 25/02/16 01:28, Michael Ellerman wrote:
> In order to support the new -mprofile-kernel ABI, we need to be able to
> call from the module back to ftrace_caller() (in the kernel) without
> using the module's r2. That is because the function in this module which
> is calling ftrace_caller() may not have setup r2, if it doesn't
> otherwise need it (ie. it accesses no globals).
>
> To make that work we add a new stub which is used for calling
> ftrace_caller(), which uses the kernel toc instead of the module toc.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/kernel/module_64.c | 48 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index 9629966e614b..e711d40a3b8f 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -671,10 +671,56 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
>  }
>  
>  #ifdef CONFIG_DYNAMIC_FTRACE
> +
> +#define PACATOC offsetof(struct paca_struct, kernel_toc)
> +
> +static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs, struct module *me)
> +{
> +	struct ppc64_stub_entry *entry;
> +	unsigned int i, num_stubs;
How about some comments on r2
r2 is still pointing to the module's toc, will be saved by ftrace_caller and restored by the instruction following bl ftrace_caller (after patching _mcount/nop)

> +	static u32 stub_insns[] = {
> +		0xe98d0000 | PACATOC, 	/* ld      r12,PACATOC(r13)	*/
> +		0x3d8c0000,		/* addis   r12,r12,<high>	*/
> +		0x398c0000, 		/* addi    r12,r12,<low>	*/
> +		0x7d8903a6, 		/* mtctr   r12			*/
> +		0x4e800420, 		/* bctr				*/
> +	};
> +	long reladdr;
> +
> +	num_stubs = sechdrs[me->arch.stubs_section].sh_size / sizeof(*entry);
> +
> +	/* Find the next available stub entry */
> +	entry = (void *)sechdrs[me->arch.stubs_section].sh_addr;
> +	for (i = 0; i < num_stubs && stub_func_addr(entry->funcdata); i++, entry++);
> +
> +	if (i >= num_stubs) {
> +		pr_err("%s: Unable to find a free slot for ftrace stub.\n", me->name);
> +		return 0;
> +	}
> +
> +	memcpy(entry->jump, stub_insns, sizeof(stub_insns));
> +
> +	/* Stub uses address relative to kernel_toc */
> +	reladdr = (unsigned long)ftrace_caller - get_paca()->kernel_toc;
> +	if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) {
> +		pr_err("%s: Address of ftrace_caller out of range of kernel_toc.\n", me->name);
> +		return 0;
> +	}
> +
> +	entry->jump[1] |= PPC_HA(reladdr);
> +	entry->jump[2] |= PPC_LO(reladdr);
> +
> +	/* Eventhough we don't use funcdata in the stub, it's needed elsewhere. */
> +	entry->funcdata = func_desc((unsigned long)ftrace_caller);
> +	entry->magic = STUB_MAGIC;
> +
> +	return (unsigned long)entry;
> +}
> +
>  int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs)
>  {
>  	mod->arch.toc = my_r2(sechdrs, mod);
> -	mod->arch.tramp = stub_for_addr(sechdrs, (unsigned long)ftrace_caller, mod);
> +	mod->arch.tramp = create_ftrace_stub(sechdrs, mod);
>  
>  	if (!mod->arch.tramp)
>  		return -ENOENT;
Reviewed-by: Balbir Singh <bsingharora@gmail.com>

[toc] | [prev] | [next] | [standalone]

#1343104 — Re: [PATCH 03/12] powerpc/module: Create a special stub for ftrace_caller()

On Thu, 2016-02-25 at 11:08 +1100, Balbir Singh wrote:
> 
> On 25/02/16 01:28, Michael Ellerman wrote:
> > In order to support the new -mprofile-kernel ABI, we need to be able to
> > call from the module back to ftrace_caller() (in the kernel) without
> > using the module's r2. That is because the function in this module which
> > is calling ftrace_caller() may not have setup r2, if it doesn't
> > otherwise need it (ie. it accesses no globals).
> > 
> > To make that work we add a new stub which is used for calling
> > ftrace_caller(), which uses the kernel toc instead of the module toc.
> > 
> > diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> > index 9629966e614b..e711d40a3b8f 100644
> > --- a/arch/powerpc/kernel/module_64.c
> > +++ b/arch/powerpc/kernel/module_64.c
> > @@ -671,10 +671,56 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
> >  }
> >  
> >  #ifdef CONFIG_DYNAMIC_FTRACE
> > +
> > +#define PACATOC offsetof(struct paca_struct, kernel_toc)
> > +
> > +static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs, struct module *me)
> > +{
> > +	struct ppc64_stub_entry *entry;
> > +	unsigned int i, num_stubs;

> How about some comments on r2
> r2 is still pointing to the module's toc, will be saved by ftrace_caller and
> restored by the instruction following bl ftrace_caller (after patching
> _mcount/nop)

Yeah I'll add some commentary.

I think the change log describes it fairly well but a comment is also good.

cheers

[toc] | [prev] | [next] | [standalone]

#1343172 — Re: [PATCH 03/12] powerpc/module: Create a special stub for ftrace_caller()

On Thu, Feb 25, 2016 at 11:08:54AM +1100, Balbir Singh wrote:
> How about some comments on r2
> r2 is still pointing to the module's toc, will be saved by ftrace_caller and restored by the instruction following bl ftrace_caller (after patching _mcount/nop)

To be precise: ftrace_caller needs to save _and_ restore r2 in case of -mprofile-kernel.

> > +	/* Stub uses address relative to kernel_toc */
> > +	reladdr = (unsigned long)ftrace_caller - get_paca()->kernel_toc;

kernel_toc is a compile time constant; do you really want to look it up in
memory at runtime each time? It's a bit tricky to get the +- 0x8000 right
OTOH...

I wrote:
extern unsigned long __toc_start;
    reladdr = addr - ((unsigned long)(&__toc_start) + 0x8000UL);

looks a bit odd, but evaluates to a constant for ftrace_caller.

Either way is fine with me:

Signed-off-by: Torsten Duwe <duwe@suse.de>
Reviewed-by: Torsten Duwe <duwe@suse.de>

> Reviewed-by: Balbir Singh <bsingharora@gmail.com>

	Torsten

[toc] | [prev] | [next] | [standalone]

#1344041 — Re: [PATCH 03/12] powerpc/module: Create a special stub for ftrace_caller()

On Thu, 2016-02-25 at 14:31 +0100, Torsten Duwe wrote:

> On Thu, Feb 25, 2016 at 11:08:54AM +1100, Balbir Singh wrote:

> > How about some comments on r2
> > r2 is still pointing to the module's toc, will be saved by ftrace_caller and restored by the instruction following bl ftrace_caller (after patching _mcount/nop)
> 
> To be precise: ftrace_caller needs to save _and_ restore r2 in case of -mprofile-kernel.

> > > +	/* Stub uses address relative to kernel_toc */
> > > +	reladdr = (unsigned long)ftrace_caller - get_paca()->kernel_toc;

Yeah true. I originally wrote it with the address of ftrace_caller passed as an
argument, so it had to be computed at runtime, and getting it from the paca is
~= to getting it out of the GOT.

> kernel_toc is a compile time constant; do you really want to look it up in
> memory at runtime each time? It's a bit tricky to get the +- 0x8000 right
> OTOH...
> 
> I wrote:
> extern unsigned long __toc_start;
>     reladdr = addr - ((unsigned long)(&__toc_start) + 0x8000UL);
> 
> looks a bit odd, but evaluates to a constant for ftrace_caller.

Yeah that makes sense. I'll add a helper to do the + 32k.

cheers

[toc] | [prev] | [next] | [standalone]

#1342526 — Re: [PATCH 01/12] powerpc/module: Only try to generate the ftrace_caller() stub once

On 25/02/16 01:28, Michael Ellerman wrote:
> Currently we generate the module stub for ftrace_caller() at the bottom
> of apply_relocate_add(). However apply_relocate_add() is potentially
> called more than once per module, which means we will try to generate
> the ftrace_caller() stub multiple times.
This makes sense
Reviewed-by: Balbir Singh <bsingharora@gmail.com>

[toc] | [prev] | [next] | [standalone]

#1342791 — Re: [PATCH 01/12] powerpc/module: Only try to generate the ftrace_caller() stub once

On 25/02/16 10:55, Balbir Singh wrote:
>
> On 25/02/16 01:28, Michael Ellerman wrote:
>> Currently we generate the module stub for ftrace_caller() at the bottom
>> of apply_relocate_add(). However apply_relocate_add() is potentially
>> called more than once per module, which means we will try to generate
>> the ftrace_caller() stub multiple times.
> This makes sense
> Reviewed-by: Balbir Singh <bsingharora@gmail.com>
For the entire series also

Tested-by: Balbir Singh <bsingharora@gmail.com>

[toc] | [prev] | [next] | [standalone]

#1343151 — Re: [PATCH 01/12] powerpc/module: Only try to generate the ftrace_caller() stub once

On Thu, Feb 25, 2016 at 01:28:24AM +1100, Michael Ellerman wrote:
> Currently we generate the module stub for ftrace_caller() at the bottom
> of apply_relocate_add(). However apply_relocate_add() is potentially
> called more than once per module, which means we will try to generate
> the ftrace_caller() stub multiple times.
> 
> Although the current code deals with that correctly, ie. it only
> generates a stub the first time, it would be clearer to only try to
> generate the stub once.
> 
> Note also on first reading it may appear that we generate a different
> stub for each section that requires relocation, but that is not the
> case. The code in stub_for_addr() that searches for an existing stub
> uses sechdrs[me->arch.stubs_section], ie. the single stub section for
> this module.
> 
> A cleaner approach is to only generate the ftrace_caller() stub once,
> from module_finalize(). An additional benefit is we can clean the ifdefs
> up a little.
> 
> Finally we must propagate the const'ness of some of the pointers passed
> to module_finalize(), but that is also an improvement.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Reviewed-by: Torsten Duwe <duwe@suse.de>

	Torsten

[toc] | [prev] | [next] | [standalone]

#1343204 — Re: [PATCH 01/12] powerpc/module: Only try to generate the ftrace_caller() stub once

* Michael Ellerman <mpe@ellerman.id.au> [2016-02-25 01:28:24]:

> Currently we generate the module stub for ftrace_caller() at the bottom
> of apply_relocate_add(). However apply_relocate_add() is potentially
> called more than once per module, which means we will try to generate
> the ftrace_caller() stub multiple times.
> 
> Although the current code deals with that correctly, ie. it only
> generates a stub the first time, it would be clearer to only try to
> generate the stub once.
> 
> Note also on first reading it may appear that we generate a different
> stub for each section that requires relocation, but that is not the
> case. The code in stub_for_addr() that searches for an existing stub
> uses sechdrs[me->arch.stubs_section], ie. the single stub section for
> this module.
> 
> A cleaner approach is to only generate the ftrace_caller() stub once,
> from module_finalize(). An additional benefit is we can clean the ifdefs
> up a little.
> 
> Finally we must propagate the const'ness of some of the pointers passed
> to module_finalize(), but that is also an improvement.
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

For all of the patches in the series.

Tested-by: Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>


Regards,
Kamalesh.

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web