On Fri, Apr 8, 2022 at 10:15 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Ian,
>
> On Thu, Apr 7, 2022 at 4:05 PM Ian Rogers <irogers@google.com> wrote:
> >
> > Fixing up more symbol ends as introduced in:
> > https://lore.kernel.org/lkml/20220317135536.805-1-mpetlan@redhat.com/
> > caused perf annotate to run into memory limits - every symbol holds
> > all the disassembled code in the annotation, and so making symbols
> > ends further away dramatically increased memory usage (40MB to
> > >1GB). Modify the symbol end logic so that special kernel cases aren't
> > applied in the common case.
>
> I'm not sure what was the actual problem the patch tried to solve.
> It seems like a specific problem on powerpc + kprobes and now
> it affects all other architectures.
Thanks Namhyung, this patch set reverts that.
> In the above commit, optinsn_slot and kprobe_optinsn_page will
> have the same address and optinsn_slot cannot be fixed up.
> But I guess the kprobe_optinsn_page still can be fixed up and
> you can use the symbol instead, no?
>
> To me, it'd be better to revert the change and add a special
> handling for the kprobe insn pages as they appear as
> modules.
>
> Also, all the arch symbols fixup routine seem to do the same
> then we might move it to the general logic.
Agreed. It isn't possible for me to test that behavior and so the
change tries to make the behavior clearer and to isolate Michael's
change to PowerPC. I like the idea of removing the weak symbols and
making the behavior common. I also don't know why we're trying to fix
broken elf file symbols and don't just let the assembly/compiler
writers fix that.
Thanks,
Ian
> Thanks,
> Namhyung
>
> >
> > Minor fix to perf annotate to not stall when stderr is full.
> >
> > Ian Rogers (5):
> > perf annotate: Drop objdump stderr
> > perf symbols: Always do architecture specific fixups
> > perf symbols: Add is_kernel argument to fixup end
> > perf symbol: By default only fix zero length symbols
> > perf symbols: More specific architecture end fixing
> >
> > tools/perf/arch/arm64/util/machine.c | 14 +++++++++-----
> > tools/perf/arch/powerpc/util/machine.c | 10 +++++++---
> > tools/perf/arch/s390/util/machine.c | 12 ++++++++----
> > tools/perf/util/annotate.c | 1 +
> > tools/perf/util/symbol-elf.c | 2 +-
> > tools/perf/util/symbol.c | 14 ++++++++------
> > tools/perf/util/symbol.h | 4 ++--
> > 7 files changed, 36 insertions(+), 21 deletions(-)
> >
> > --
> > 2.35.1.1178.g4f1659d476-goog
> >