Re: [PATCH 0/5] Tidy up symbol end fixup

From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>,
	Michael Petlan <mpetlan@redhat.com>,
	 Masami Hiramatsu <mhiramat@kernel.org>
Cc: "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>,
	"Martin Liška" <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>
Subject: Re: [PATCH 0/5] Tidy up symbol end fixup
Date: Fri, 8 Apr 2022 10:15:22 -0700	[thread overview]
Message-ID: <CAM9d7cjM5jpPfdWUvoezYYJoz_Sh1rHpGHVGAvxOqxzb=AKBVQ@mail.gmail.com> (raw)
In-Reply-To: <20220407230503.1265036-1-irogers@google.com>

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.

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.

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
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel