linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf tools: Fixup end address of modules
@ 2024-12-18 22:04 Namhyung Kim
  2024-12-18 22:15 ` Ian Rogers
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2024-12-18 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
  Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
	linux-perf-users, Blake Jones, Luis Chamberlain, Petr Pavlu,
	Sami Tolvanen, Daniel Gomez, linux-modules

In machine__create_module(), it reads /proc/modules to get a list of
modules in the system.  The file shows the start address (of text) and
the size of the module so it uses the info to reconstruct system memory
maps for symbol resolution.

But module memory consists of multiple segments and they can be
scaterred.  Currently perf tools assume they are contiguous and see some
overlaps.  This can confuse the tool when it finds a map containing a
given address.

As we mostly care about the function symbols in the text segment, it can
fixup the size or end address of modules when there's an overlap.  We
can use maps__fixup_end() which updates the end address using the start
address of the next map.

Ideally it should be able to track other segments (like data/rodata),
but that would require some changes in /proc/modules IMHO.

Reported-by: Blake Jones <blakejones@google.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Petr Pavlu <petr.pavlu@suse.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Daniel Gomez <da.gomez@samsung.com>
Cc: linux-modules@vger.kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/machine.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 27d5345d2b307a97..8bb34689e3ceeec4 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1468,6 +1468,8 @@ static int machine__create_modules(struct machine *machine)
 	if (modules__parse(modules, machine, machine__create_module))
 		return -1;
 
+	maps__fixup_end(machine__kernel_maps(machine));
+
 	if (!machine__set_modules_path(machine))
 		return 0;
 
-- 
2.47.1.613.gc27f4b7a9f-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf tools: Fixup end address of modules
  2024-12-18 22:04 [PATCH] perf tools: Fixup end address of modules Namhyung Kim
@ 2024-12-18 22:15 ` Ian Rogers
  2025-01-09 21:19   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2024-12-18 22:15 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Kan Liang, Jiri Olsa, Adrian Hunter,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Blake Jones,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	linux-modules

On Wed, Dec 18, 2024 at 2:04 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> In machine__create_module(), it reads /proc/modules to get a list of
> modules in the system.  The file shows the start address (of text) and
> the size of the module so it uses the info to reconstruct system memory
> maps for symbol resolution.
>
> But module memory consists of multiple segments and they can be
> scaterred.  Currently perf tools assume they are contiguous and see some

nit: s/scaterred/scattered/

> overlaps.  This can confuse the tool when it finds a map containing a
> given address.
>
> As we mostly care about the function symbols in the text segment, it can
> fixup the size or end address of modules when there's an overlap.  We
> can use maps__fixup_end() which updates the end address using the start
> address of the next map.
>
> Ideally it should be able to track other segments (like data/rodata),
> but that would require some changes in /proc/modules IMHO.
>
> Reported-by: Blake Jones <blakejones@google.com>
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Petr Pavlu <petr.pavlu@suse.com>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: Daniel Gomez <da.gomez@samsung.com>
> Cc: linux-modules@vger.kernel.org
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/machine.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 27d5345d2b307a97..8bb34689e3ceeec4 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1468,6 +1468,8 @@ static int machine__create_modules(struct machine *machine)
>         if (modules__parse(modules, machine, machine__create_module))
>                 return -1;
>
> +       maps__fixup_end(machine__kernel_maps(machine));
> +
>         if (!machine__set_modules_path(machine))
>                 return 0;
>
> --
> 2.47.1.613.gc27f4b7a9f-goog
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf tools: Fixup end address of modules
  2024-12-18 22:15 ` Ian Rogers
@ 2025-01-09 21:19   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-01-09 21:19 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Namhyung Kim, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users, Blake Jones,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	linux-modules

On Wed, Dec 18, 2024 at 02:15:35PM -0800, Ian Rogers wrote:
> On Wed, Dec 18, 2024 at 2:04 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > In machine__create_module(), it reads /proc/modules to get a list of
> > modules in the system.  The file shows the start address (of text) and
> > the size of the module so it uses the info to reconstruct system memory
> > maps for symbol resolution.
> >
> > But module memory consists of multiple segments and they can be
> > scaterred.  Currently perf tools assume they are contiguous and see some
> 
> nit: s/scaterred/scattered/
> 
> > overlaps.  This can confuse the tool when it finds a map containing a
> > given address.
> >
> > As we mostly care about the function symbols in the text segment, it can
> > fixup the size or end address of modules when there's an overlap.  We
> > can use maps__fixup_end() which updates the end address using the start
> > address of the next map.
> >
> > Ideally it should be able to track other segments (like data/rodata),
> > but that would require some changes in /proc/modules IMHO.
> >
> > Reported-by: Blake Jones <blakejones@google.com>
> > Cc: Luis Chamberlain <mcgrof@kernel.org>
> > Cc: Petr Pavlu <petr.pavlu@suse.com>
> > Cc: Sami Tolvanen <samitolvanen@google.com>
> > Cc: Daniel Gomez <da.gomez@samsung.com>
> > Cc: linux-modules@vger.kernel.org
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Acked-by: Ian Rogers <irogers@google.com>

Thanks, applied to perf-tools-next,

- Arnaldo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-01-09 21:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-18 22:04 [PATCH] perf tools: Fixup end address of modules Namhyung Kim
2024-12-18 22:15 ` Ian Rogers
2025-01-09 21:19   ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).