Re: [PATCH] perf probe: Fix add event failed when 32-bit perf running in 64-bit kernel

From: Yang Jihong
Date: Thu Jul 15 2021 - 02:47:39 EST


Hello Hiramatsu,

On 2021/7/15 12:58, Masami Hiramatsu wrote:
On Thu, 15 Jul 2021 09:20:01 +0800
Yang Jihong <yangjihong1@xxxxxxxxxx> wrote:

Hello Hiramatsu,

On 2021/7/14 16:35, Masami Hiramatsu wrote:
Hi Yang,

On Wed, 14 Jul 2021 14:54:32 +0800
Yang Jihong <yangjihong1@xxxxxxxxxx> wrote:

The "address" member of "struct probe_trace_point" uses long data type.
If kernel is 64-bit and perf program is 32-bit, size of "address" variable is
32 bits. As a result, upper 32 bits of address read from kernel are truncated,
An error occurs during address comparison in kprobe_warn_out_range function.

Good catch!
I didn't imagine that such a use case. But that is important because perf
probe can be used for cross-arch probe definition too.


Before:

# perf probe -a schedule
schedule is out of .text, skip it.
Error: Failed to add events.

Solution:
Change data type of "address" variable to u64 and change corresponding
address printing and value assignment.

OK, as far as I can see, the other parts of the perf also uses u64 for
"address" storing variables. (e.g. symbols, maps etc.)


After:

# perf.new.new probe -a schedule
Added new event:
probe:schedule (on schedule)

You can now use it in all perf tools, such as:

perf record -e probe:schedule -aR sleep 1

# perf probe -l
probe:schedule (on schedule)

I think you missed one thing here.
Usually, this shows the filename and line number of schedule().
Yes, I tried the following diff and now it can show the filename (as is
function entry, relative line number is 0), thank you very much :)

The test result in my environment is as follows:

# perf probe -a schedule
Added new event:
probe:schedule (on schedule)

You can now use it in all perf tools, such as:

perf record -e probe:schedule -aR sleep 1

# perf probe -l schedule -k vmlinux.debug
probe:schedule (on schedule@kernel/sched/core.c)


OK, good!


Can I put the following diff together and submit a v2 patch?

Yes, please include this fix in your patch, because it is what your patch
has to do :)

OK,I have submitted the v2 patch:
https://lore.kernel.org/patchwork/patch/1460154/

Thanks very much for your patient review.:)

Jihong
Thank you!


Could you try below diff?

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 7d2ba8419b0c..609ca1671501 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -113,14 +113,14 @@ static Dwarf_Line *cu_getsrc_die(Dwarf_Die *cu_die, Dwarf_Addr addr)
*
* Find a line number and file name for @addr in @cu_die.
*/
-int cu_find_lineinfo(Dwarf_Die *cu_die, unsigned long addr,
- const char **fname, int *lineno)
+int cu_find_lineinfo(Dwarf_Die *cu_die, Dwarf_Addr addr,
+ const char **fname, int *lineno)
{
Dwarf_Line *line;
Dwarf_Die die_mem;
Dwarf_Addr faddr;
- if (die_find_realfunc(cu_die, (Dwarf_Addr)addr, &die_mem)
+ if (die_find_realfunc(cu_die, addr, &die_mem)
&& die_entrypc(&die_mem, &faddr) == 0 &&
faddr == addr) {
*fname = dwarf_decl_file(&die_mem);
@@ -128,7 +128,7 @@ int cu_find_lineinfo(Dwarf_Die *cu_die, unsigned long addr,
goto out;
}
- line = cu_getsrc_die(cu_die, (Dwarf_Addr)addr);
+ line = cu_getsrc_die(cu_die, addr);
if (line && dwarf_lineno(line, lineno) == 0) {
*fname = dwarf_linesrc(line, NULL, NULL);
if (!*fname)
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index cb99646843a9..7ee0fa19b5c4 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -19,7 +19,7 @@ const char *cu_find_realpath(Dwarf_Die *cu_die, const char *fname);
const char *cu_get_comp_dir(Dwarf_Die *cu_die);
/* Get a line number and file name for given address */
-int cu_find_lineinfo(Dwarf_Die *cudie, unsigned long addr,
+int cu_find_lineinfo(Dwarf_Die *cudie, Dwarf_Addr addr,
const char **fname, int *lineno);
/* Walk on functions at given address */
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 8ba01bbdf05c..50d861a80f57 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1727,7 +1727,7 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr,
}
/* Find a corresponding line (filename and lineno) */
- cu_find_lineinfo(&cudie, addr, &fname, &lineno);
+ cu_find_lineinfo(&cudie, (Dwarf_Addr)addr, &fname, &lineno);
/* Don't care whether it failed or not */
/* Find a corresponding function (name, baseline and baseaddr) */
@@ -1828,8 +1828,7 @@ static int line_range_add_line(const char *src, unsigned int lineno,
}
static int line_range_walk_cb(const char *fname, int lineno,
- Dwarf_Addr addr __maybe_unused,
- void *data)
+ Dwarf_Addr addr, void *data)
{
struct line_finder *lf = data;
const char *__fname;