Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220407230503.1265036-1-irogers@google.com> <CAM9d7cjM5jpPfdWUvoezYYJoz_Sh1rHpGHVGAvxOqxzb=AKBVQ@mail.gmail.com>
In-Reply-To: <CAM9d7cjM5jpPfdWUvoezYYJoz_Sh1rHpGHVGAvxOqxzb=AKBVQ@mail.gmail.com>
From:   Ian Rogers <irogers@google.com>
Date:   Fri, 8 Apr 2022 16:52:00 -0700
Message-ID: <CAP-5=fULT859R4-x4RRsvKEUSrGYySVVSxtWSWYUoKC0M0vCYg@mail.gmail.com>
Subject: Re: [PATCH 0/5] Tidy up symbol end fixup
To:     Namhyung Kim <namhyung@kernel.org>
Cc:     Michael Petlan <mpetlan@redhat.com>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        John Garry <john.garry@huawei.com>,
        Will Deacon <will@kernel.org>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        Leo Yan <leo.yan@linaro.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        James Clark <james.clark@arm.com>,
        Alexandre Truong <alexandre.truong@arm.com>,
        German Gomez <german.gomez@arm.com>,
        Dave Marchevsky <davemarchevsky@fb.com>,
        Song Liu <songliubraving@fb.com>,
        Ravi Bangoria <ravi.bangoria@amd.com>,
        Li Huafei <lihuafei1@huawei.com>,
        =?UTF-8?Q?Martin_Li=C5=A1ka?= <mliska@suse.cz>,
        William Cohen <wcohen@redhat.com>,
        Riccardo Mancini <rickyman7@gmail.com>,
        Thomas Richter <tmricht@linux.ibm.com>,
        Lexi Shao <shaolexi@huawei.com>,
        Remi Bernon <rbernon@codeweavers.com>,
        Denis Nikitin <denik@chromium.org>,
        linux-arm-kernel@lists.infradead.org,
        linux-perf-users <linux-perf-users@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Stephane Eranian <eranian@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
> >