Re: [PATCH 23/31] perf tools: Introduce regs_query_register_offset() for x86
From: Arnaldo Carvalho de Melo
Date: Tue Sep 01 2015 - 10:50:31 EST
Em Tue, Sep 01, 2015 at 09:52:30PM +0800, Wangnan (F) escreveu:
> On 2015/9/1 19:47, åæéå / HIRAMATUïMASAMI wrote:
> >>From: Wang Nan [mailto:wangnan0@xxxxxxxxxx]
> >>regs_query_register_offset() is a helper function which converts
> >>register name like "%rax" to offset of a register in 'struct pt_regs',
> >>which is required by BPF prologue generator. Since the function is
> >>identical, try to reuse the code in arch/x86/kernel/ptrace.c.
> >>Comment inside dwarf-regs.c list the differences between this
> >>implementation and kernel code.
> >Hmm, this also introduce a duplication of the code...
> >It might be a good time to move them into arch/x86/lib/ and
> >reuse it directly from perf code.
> So you want to move it from ./arch/x86/kernel/ptrace.c to arch/x86/lib and
> let perf link against arch/x86/lib/lib.a to use it?
> I think it worth a specific work to do it. Currently we lake
> scaffold to compile and link against the kernel side library. Moreover,
> we should also consider other archs. Seems not very easy.
> This is not the only one file which can benifite from kernel's arch/x86/lib
> Newly introduced tools/perf/util/intel-pt-decoder/insn.c, and I believe
> more. Therefore I think it should be a separated work from perf BPF patches.
> So how about keep this patch at this time? Or do you have some idea?
I would go with what Wang did at this time, its a step in the right
direction in the sense that we're trying to use the same function names
and semantics, and, as much as possible, possibly in verbatim form,
using the same implementation.
Doing the work to fully share it is something being discussed, but that
should not get in the way of eBPF work, IMHO.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/