[v5,2/5] powerpc: kretprobes: override default function entry offset

With ABIv2, we offset 8 bytes into a function to get at the local entry
point.

Acked-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/kprobes.c | 9 +++++++++
 1 file changed, 9 insertions(+)

"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:

> With ABIv2, we offset 8 bytes into a function to get at the local entry
> point.
>
> Acked-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/kprobes.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

I'm OK with this change, and I'm happy for it to go with the rest of the
series via acme's tree:

Acked-by: Michael Ellerman <mpe@ellerman.id.au>


But, you've also sent a series to do KPROBES_ON_FTRACE, and that also
touches this function, see the 2nd to last hunk at:

  https://patchwork.ozlabs.org/patch/730675/


If this goes via acme's tree it will be awkward for me to merge the
series above via the powerpc tree.

So we could do topic branches and so on, or we could just drop this
patch from this series, and I'll merge it as part of the other series.
It won't do anything useful until it's merged with a tree that also has
the rest of this series. Or something else I haven't thought of.

cheers

> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index fce05a38851c..331751701fed 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -131,6 +131,15 @@ static void __kprobes set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
>  	kcb->kprobe_saved_msr = regs->msr;
>  }
>  
> +bool arch_function_offset_within_entry(unsigned long offset)
> +{
> +#ifdef PPC64_ELF_ABI_v2
> +	return offset <= 8;
> +#else
> +	return !offset;
> +#endif
> +}
> +
>  void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
>  				      struct pt_regs *regs)
>  {
> -- 
> 2.11.1

Hi Michael,

On 2017/03/08 09:43PM, Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> 
> > With ABIv2, we offset 8 bytes into a function to get at the local entry
> > point.
> >
> > Acked-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> > Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > ---
> >  arch/powerpc/kernel/kprobes.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> 
> I'm OK with this change, and I'm happy for it to go with the rest of the
> series via acme's tree:
> 
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> 
> 
> But, you've also sent a series to do KPROBES_ON_FTRACE, and that also
> touches this function, see the 2nd to last hunk at:
> 
>   https://patchwork.ozlabs.org/patch/730675/
> 
> 
> If this goes via acme's tree it will be awkward for me to merge the
> series above via the powerpc tree.

Ah yes, indeed.

> 
> So we could do topic branches and so on, or we could just drop this
> patch from this series, and I'll merge it as part of the other series.
> It won't do anything useful until it's merged with a tree that also has
> the rest of this series. Or something else I haven't thought of.

The arch-independent change that this depends on has been picked up by 
Arnaldo and pushed to Ingo:
https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg115211.html

I'm guessing this will go into v4.11? In which case, this powerpc patch 
should also go in. Otherwise kretprobes will be broken on powerpc64le.

I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for 
v4.11. If so, it would be good to take this patch through the powerpc 
tree. Otherwise, this can go via Ingo's tree.


Thanks,
Naveen

Em Wed, Mar 08, 2017 at 07:54:12PM +0530, Naveen N. Rao escreveu:
> Hi Michael,
> 
> On 2017/03/08 09:43PM, Michael Ellerman wrote:
> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> > 
> > > With ABIv2, we offset 8 bytes into a function to get at the local entry
> > > point.
> > >
> > > Acked-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> > > Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > > ---
> > >  arch/powerpc/kernel/kprobes.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > 
> > I'm OK with this change, and I'm happy for it to go with the rest of the
> > series via acme's tree:
> > 
> > Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> > 
> > 
> > But, you've also sent a series to do KPROBES_ON_FTRACE, and that also
> > touches this function, see the 2nd to last hunk at:
> > 
> >   https://patchwork.ozlabs.org/patch/730675/
> > 
> > 
> > If this goes via acme's tree it will be awkward for me to merge the
> > series above via the powerpc tree.
> 
> Ah yes, indeed.
> 
> > 
> > So we could do topic branches and so on, or we could just drop this
> > patch from this series, and I'll merge it as part of the other series.
> > It won't do anything useful until it's merged with a tree that also has
> > the rest of this series. Or something else I haven't thought of.
> 
> The arch-independent change that this depends on has been picked up by 
> Arnaldo and pushed to Ingo:
> https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg115211.html
> 
> I'm guessing this will go into v4.11? In which case, this powerpc patch 
> should also go in. Otherwise kretprobes will be broken on powerpc64le.

I don't think so, I've put it in a perf/core branch, meaning its not
strictly fixes, could be processed in the next merge window if Ingo
thinks we've passed the current merge window threshold for such kind of
changes, and he merged it into perf/core, meaning, at this time, that it
is aimed for 4.12.
 
> I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for 
> v4.11. If so, it would be good to take this patch through the powerpc 
> tree. Otherwise, this can go via Ingo's tree.

If you guys convince Ingo that this should go _now_, then just cherry
pick what was merged into tip/perf/core that is needed for the arch
specific stuff and go from there.

- Arnaldo

On 2017/03/08 11:29AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 08, 2017 at 07:54:12PM +0530, Naveen N. Rao escreveu:
> > Hi Michael,
> > 
> > On 2017/03/08 09:43PM, Michael Ellerman wrote:
> > > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> > > 
> > > > With ABIv2, we offset 8 bytes into a function to get at the local entry
> > > > point.
> > > >
> > > > Acked-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> > > > Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> > > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > > > ---
> > > >  arch/powerpc/kernel/kprobes.c | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > 
> > > I'm OK with this change, and I'm happy for it to go with the rest of the
> > > series via acme's tree:
> > > 
> > > Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> > > 
> > > 
> > > But, you've also sent a series to do KPROBES_ON_FTRACE, and that also
> > > touches this function, see the 2nd to last hunk at:
> > > 
> > >   https://patchwork.ozlabs.org/patch/730675/
> > > 
> > > 
> > > If this goes via acme's tree it will be awkward for me to merge the
> > > series above via the powerpc tree.
> > 
> > Ah yes, indeed.
> > 
> > > 
> > > So we could do topic branches and so on, or we could just drop this
> > > patch from this series, and I'll merge it as part of the other series.
> > > It won't do anything useful until it's merged with a tree that also has
> > > the rest of this series. Or something else I haven't thought of.
> > 
> > The arch-independent change that this depends on has been picked up by 
> > Arnaldo and pushed to Ingo:
> > https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg115211.html
> > 
> > I'm guessing this will go into v4.11? In which case, this powerpc patch 
> > should also go in. Otherwise kretprobes will be broken on powerpc64le.
> 
> I don't think so, I've put it in a perf/core branch, meaning its not
> strictly fixes, could be processed in the next merge window if Ingo
> thinks we've passed the current merge window threshold for such kind of
> changes, and he merged it into perf/core, meaning, at this time, that it
> is aimed for 4.12.

Ah, thanks for clarifying.

> 
> > I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for 
> > v4.11. If so, it would be good to take this patch through the powerpc 
> > tree. Otherwise, this can go via Ingo's tree.
> 
> If you guys convince Ingo that this should go _now_, then just cherry
> pick what was merged into tip/perf/core that is needed for the arch
> specific stuff and go from there.

Ok, in hindsight, I think Michael's concern was actually for v4.12 
itself, in which case this particular patch can go via powerpc tree, 
while the rest of the patches in this series can go via your tree.

Michael?


Thanks,
Naveen

"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> On 2017/03/08 11:29AM, Arnaldo Carvalho de Melo wrote:
>> > I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for 
>> > v4.11. If so, it would be good to take this patch through the powerpc 
>> > tree. Otherwise, this can go via Ingo's tree.
>> 
>> If you guys convince Ingo that this should go _now_, then just cherry
>> pick what was merged into tip/perf/core that is needed for the arch
>> specific stuff and go from there.
>
> Ok, in hindsight, I think Michael's concern was actually for v4.12 

Yes I was talking about 4.12, sorry I thought that was implied :)

> itself, in which case this particular patch can go via powerpc tree, 
> while the rest of the patches in this series can go via your tree.
>
> Michael?

Yeah I think that's the easiest option. The function will be temporarily
unused until the two trees are merged, but I think that's fine.

cheers

On 2017/03/09 05:37PM, Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> > On 2017/03/08 11:29AM, Arnaldo Carvalho de Melo wrote:
> >> > I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for 
> >> > v4.11. If so, it would be good to take this patch through the powerpc 
> >> > tree. Otherwise, this can go via Ingo's tree.
> >> 
> >> If you guys convince Ingo that this should go _now_, then just cherry
> >> pick what was merged into tip/perf/core that is needed for the arch
> >> specific stuff and go from there.
> >
> > Ok, in hindsight, I think Michael's concern was actually for v4.12 
> 
> Yes I was talking about 4.12, sorry I thought that was implied :)

I suppose it was evident for everyone except the overzealous me :D
Sorry for all the confusion.

> 
> > itself, in which case this particular patch can go via powerpc tree, 
> > while the rest of the patches in this series can go via your tree.
> >
> > Michael?
> 
> Yeah I think that's the easiest option. The function will be temporarily
> unused until the two trees are merged, but I think that's fine.

Sure, thanks!

- Naveen

Em Thu, Mar 09, 2017 at 05:37:38PM +1100, Michael Ellerman escreveu:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> > On 2017/03/08 11:29AM, Arnaldo Carvalho de Melo wrote:
> >> > I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for 
> >> > v4.11. If so, it would be good to take this patch through the powerpc 
> >> > tree. Otherwise, this can go via Ingo's tree.
> >> 
> >> If you guys convince Ingo that this should go _now_, then just cherry
> >> pick what was merged into tip/perf/core that is needed for the arch
> >> specific stuff and go from there.
> >
> > Ok, in hindsight, I think Michael's concern was actually for v4.12 
> 
> Yes I was talking about 4.12, sorry I thought that was implied :)
> 
> > itself, in which case this particular patch can go via powerpc tree, 
> > while the rest of the patches in this series can go via your tree.
> >
> > Michael?
> 
> Yeah I think that's the easiest option. The function will be temporarily
> unused until the two trees are merged, but I think that's fine.

Ok, done that, now compile testing building it in my
multi-distro/x-build containers.

- Arnaldo

On 2017/03/14 10:18AM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Mar 09, 2017 at 05:37:38PM +1100, Michael Ellerman escreveu:
> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> > > On 2017/03/08 11:29AM, Arnaldo Carvalho de Melo wrote:
> > >> > I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for 
> > >> > v4.11. If so, it would be good to take this patch through the powerpc 
> > >> > tree. Otherwise, this can go via Ingo's tree.
> > >> 
> > >> If you guys convince Ingo that this should go _now_, then just cherry
> > >> pick what was merged into tip/perf/core that is needed for the arch
> > >> specific stuff and go from there.
> > >
> > > Ok, in hindsight, I think Michael's concern was actually for v4.12 
> > 
> > Yes I was talking about 4.12, sorry I thought that was implied :)
> > 
> > > itself, in which case this particular patch can go via powerpc tree, 
> > > while the rest of the patches in this series can go via your tree.
> > >
> > > Michael?
> > 
> > Yeah I think that's the easiest option. The function will be temporarily
> > unused until the two trees are merged, but I think that's fine.
> 
> Ok, done that, now compile testing building it in my
> multi-distro/x-build containers.

Thanks, Arnaldo!

I did however notice that you don't seem to have applied Patch 1/5 from 
this series:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1347858.html

That patch is needed to ensure perf continues to work when ftrace README 
advertises support for ref_reloc_sym+offset for kretprobes. Can you 
please apply that as well?

- Naveen

On Wed, 2017-03-08 at 08:26:07 UTC, "Naveen N. Rao" wrote:
> With ABIv2, we offset 8 bytes into a function to get at the local entry
> point.
> 
> Acked-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/a64e3f35a45f4a84148d0ba30a3c75

cheers