lkml.org 
[lkml]   [2019]   [Aug]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 1/3] kprobes/x86: use instruction_pointer and instruction_pointer_set
Hi Jisheng,

On Tue, 20 Aug 2019 09:02:59 +0000
Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:

> Hi Thomas,
>
> On Tue, 20 Aug 2019 10:53:58 +0200 (CEST) Thomas Gleixner wrote:
>
> >
> >
> > On Tue, 20 Aug 2019, Jisheng Zhang wrote:
> >
> > > This is to make the x86 kprobe_ftrace_handler() more common so that
> > > the code could be reused in future.
> >
> > While I agree with the change in general, I can't find anything which
> > reuses that code. So the change log is pretty useless and I have no idea
> > how this is related to the rest of the series.
>
> In v1, this code is moved from x86 to common kprobes.c [1]
> But I agree with Masami, consolidation could be done when arm64 kprobes
> on ftrace is stable.

We'll revisit to consolidate the code after we got 3rd or 4th clones.

>
> In v2, actually, the arm64 version's kprobe_ftrace_handler() is the same
> as x86's, the only difference is comment, e.g
>
> /* Kprobe handler expects regs->ip = ip + 1 as breakpoint hit */
>
> while in arm64
>
> /* Kprobe handler expects regs->pc = ip + 1 as breakpoint hit */

As Peter pointed, on arm64, is that really 1 or 4 bytes?
This part is heavily depends on the processor software-breakpoint
implementation.

>
>
> W/ above, any suggestion about the suitable change log?

I think you just need to keep the first half of the description.
Since this patch itself is not related to the series, could you update
the description and resend it as a single cleanup patch out of the series?

Thank you!

>
> Thanks
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2019-August/674417.html


--
Masami Hiramatsu <mhiramat@kernel.org>

\
 
 \ /
  Last update: 2019-08-21 03:53    [W:0.093 / U:11.320 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site