* [PATCH v2] perf machine: Fix load kernel symbol with '-k' option
@ 2018-03-09 6:05 Leo Yan
2018-03-09 17:31 ` Mathieu Poirier
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Leo Yan @ 2018-03-09 6:05 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-kernel,
Mathieu Poirier
Cc: Leo Yan
On Hikey arm64 octa A53 platform, when use command './perf report -v
-k vmlinux --stdio' it outputs below error info, and it skips to load
kernel symbol and doesn't print symbol for event:
Failed to open [kernel.kallsyms]_text, continuing without symbols.
The regression is introduced by commit ("8c7f1bb37b29 perf machine: Move
kernel mmap name into struct machine"), which changes the logic for
machine mmap_name by removing function machine__mmap_name() and always
use 'machine->mmap_name'. Comparing difference between
machine__mmap_name() and 'machine->mmap_name', the later one includes
the string for specified kernel vmlinux string with option '-k' in
command, but the old function machine__mmap_name() ignores vmlinux
path string. As result, event's mmap file name doesn't match with
machine mmap file name anymore and it skips to load kernel symbol from
vmlinux file.
To resolve this issue, this patch adds extra checking for
'symbol_conf.vmlinux_name', when it has been set string so we can know
it includes vmlinux path string specified for option '-k'. For this
case it sets 'is_kernel_mmap' to true and run into flow to load kernel
symbol from vmlinux.
This patch has been verified with two commands: './perf report -v
-k vmlinux --stdio' and './perf script -v -F cpu,event,ip,sym,symoff
-k vmlinux'.
Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
tools/perf/util/machine.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 12b7427..3125871 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1299,9 +1299,18 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
else
kernel_type = DSO_TYPE_GUEST_KERNEL;
- is_kernel_mmap = memcmp(event->mmap.filename,
- machine->mmap_name,
- strlen(machine->mmap_name) - 1) == 0;
+ /*
+ * When symbol_conf.vmlinux_name is not NULL, it includes the specified
+ * kernel vmlinux path with option '-k'. So set 'is_kernel_mmap' to
+ * true for creating machine symbol map.
+ */
+ if (symbol_conf.vmlinux_name)
+ is_kernel_mmap = true;
+ else
+ is_kernel_mmap = memcmp(event->mmap.filename,
+ machine->mmap_name,
+ strlen(machine->mmap_name) - 1) == 0;
+
if (event->mmap.filename[0] == '/' ||
(!is_kernel_mmap && event->mmap.filename[0] == '[')) {
map = machine__findnew_module_map(machine, event->mmap.start,
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf machine: Fix load kernel symbol with '-k' option
2018-03-09 6:05 [PATCH v2] perf machine: Fix load kernel symbol with '-k' option Leo Yan
@ 2018-03-09 17:31 ` Mathieu Poirier
2018-03-09 17:42 ` Arnaldo Carvalho de Melo
2018-03-09 20:56 ` Jiri Olsa
2 siblings, 0 replies; 6+ messages in thread
From: Mathieu Poirier @ 2018-03-09 17:31 UTC (permalink / raw)
To: Leo Yan
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, linux-kernel
On 8 March 2018 at 23:05, Leo Yan <leo.yan@linaro.org> wrote:
> On Hikey arm64 octa A53 platform, when use command './perf report -v
> -k vmlinux --stdio' it outputs below error info, and it skips to load
> kernel symbol and doesn't print symbol for event:
> Failed to open [kernel.kallsyms]_text, continuing without symbols.
>
> The regression is introduced by commit ("8c7f1bb37b29 perf machine: Move
> kernel mmap name into struct machine"), which changes the logic for
> machine mmap_name by removing function machine__mmap_name() and always
> use 'machine->mmap_name'. Comparing difference between
> machine__mmap_name() and 'machine->mmap_name', the later one includes
> the string for specified kernel vmlinux string with option '-k' in
> command, but the old function machine__mmap_name() ignores vmlinux
> path string. As result, event's mmap file name doesn't match with
> machine mmap file name anymore and it skips to load kernel symbol from
> vmlinux file.
>
> To resolve this issue, this patch adds extra checking for
> 'symbol_conf.vmlinux_name', when it has been set string so we can know
> it includes vmlinux path string specified for option '-k'. For this
> case it sets 'is_kernel_mmap' to true and run into flow to load kernel
> symbol from vmlinux.
>
> This patch has been verified with two commands: './perf report -v
> -k vmlinux --stdio' and './perf script -v -F cpu,event,ip,sym,symoff
> -k vmlinux'.
>
> Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/util/machine.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 12b7427..3125871 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1299,9 +1299,18 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> else
> kernel_type = DSO_TYPE_GUEST_KERNEL;
>
> - is_kernel_mmap = memcmp(event->mmap.filename,
> - machine->mmap_name,
> - strlen(machine->mmap_name) - 1) == 0;
> + /*
> + * When symbol_conf.vmlinux_name is not NULL, it includes the specified
> + * kernel vmlinux path with option '-k'. So set 'is_kernel_mmap' to
> + * true for creating machine symbol map.
> + */
> + if (symbol_conf.vmlinux_name)
> + is_kernel_mmap = true;
> + else
> + is_kernel_mmap = memcmp(event->mmap.filename,
> + machine->mmap_name,
> + strlen(machine->mmap_name) - 1) == 0;
> +
I have tested this on my side and things work properly.
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> if (event->mmap.filename[0] == '/' ||
> (!is_kernel_mmap && event->mmap.filename[0] == '[')) {
> map = machine__findnew_module_map(machine, event->mmap.start,
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf machine: Fix load kernel symbol with '-k' option
2018-03-09 6:05 [PATCH v2] perf machine: Fix load kernel symbol with '-k' option Leo Yan
2018-03-09 17:31 ` Mathieu Poirier
@ 2018-03-09 17:42 ` Arnaldo Carvalho de Melo
2018-03-09 20:56 ` Jiri Olsa
2 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-09 17:42 UTC (permalink / raw)
To: Jiri Olsa
Cc: Leo Yan, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, linux-kernel, Mathieu Poirier
Em Fri, Mar 09, 2018 at 02:05:23PM +0800, Leo Yan escreveu:
> On Hikey arm64 octa A53 platform, when use command './perf report -v
> -k vmlinux --stdio' it outputs below error info, and it skips to load
> kernel symbol and doesn't print symbol for event:
> Failed to open [kernel.kallsyms]_text, continuing without symbols.
>
> The regression is introduced by commit ("8c7f1bb37b29 perf machine: Move
> kernel mmap name into struct machine"), which changes the logic for
commit 8c7f1bb37b29f140e08175132f3abb4d5ad229fc
Author: Jiri Olsa <jolsa@kernel.org>
Date: Thu Feb 15 13:26:30 2018 +0100
perf machine: Move kernel mmap name into struct machine
Jiri, can you please check this and Ack?
- Arnaldo
> machine mmap_name by removing function machine__mmap_name() and always
> use 'machine->mmap_name'. Comparing difference between
> machine__mmap_name() and 'machine->mmap_name', the later one includes
> the string for specified kernel vmlinux string with option '-k' in
> command, but the old function machine__mmap_name() ignores vmlinux
> path string. As result, event's mmap file name doesn't match with
> machine mmap file name anymore and it skips to load kernel symbol from
> vmlinux file.
>
> To resolve this issue, this patch adds extra checking for
> 'symbol_conf.vmlinux_name', when it has been set string so we can know
> it includes vmlinux path string specified for option '-k'. For this
> case it sets 'is_kernel_mmap' to true and run into flow to load kernel
> symbol from vmlinux.
>
> This patch has been verified with two commands: './perf report -v
> -k vmlinux --stdio' and './perf script -v -F cpu,event,ip,sym,symoff
> -k vmlinux'.
>
> Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/util/machine.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 12b7427..3125871 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1299,9 +1299,18 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> else
> kernel_type = DSO_TYPE_GUEST_KERNEL;
>
> - is_kernel_mmap = memcmp(event->mmap.filename,
> - machine->mmap_name,
> - strlen(machine->mmap_name) - 1) == 0;
> + /*
> + * When symbol_conf.vmlinux_name is not NULL, it includes the specified
> + * kernel vmlinux path with option '-k'. So set 'is_kernel_mmap' to
> + * true for creating machine symbol map.
> + */
> + if (symbol_conf.vmlinux_name)
> + is_kernel_mmap = true;
> + else
> + is_kernel_mmap = memcmp(event->mmap.filename,
> + machine->mmap_name,
> + strlen(machine->mmap_name) - 1) == 0;
> +
> if (event->mmap.filename[0] == '/' ||
> (!is_kernel_mmap && event->mmap.filename[0] == '[')) {
> map = machine__findnew_module_map(machine, event->mmap.start,
> --
> 2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf machine: Fix load kernel symbol with '-k' option
2018-03-09 6:05 [PATCH v2] perf machine: Fix load kernel symbol with '-k' option Leo Yan
2018-03-09 17:31 ` Mathieu Poirier
2018-03-09 17:42 ` Arnaldo Carvalho de Melo
@ 2018-03-09 20:56 ` Jiri Olsa
2018-03-10 9:50 ` Leo Yan
2 siblings, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2018-03-09 20:56 UTC (permalink / raw)
To: Leo Yan
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Namhyung Kim, linux-kernel, Mathieu Poirier
On Fri, Mar 09, 2018 at 02:05:23PM +0800, Leo Yan wrote:
> On Hikey arm64 octa A53 platform, when use command './perf report -v
> -k vmlinux --stdio' it outputs below error info, and it skips to load
> kernel symbol and doesn't print symbol for event:
> Failed to open [kernel.kallsyms]_text, continuing without symbols.
>
> The regression is introduced by commit ("8c7f1bb37b29 perf machine: Move
> kernel mmap name into struct machine"), which changes the logic for
> machine mmap_name by removing function machine__mmap_name() and always
> use 'machine->mmap_name'. Comparing difference between
> machine__mmap_name() and 'machine->mmap_name', the later one includes
> the string for specified kernel vmlinux string with option '-k' in
> command, but the old function machine__mmap_name() ignores vmlinux
> path string. As result, event's mmap file name doesn't match with
> machine mmap file name anymore and it skips to load kernel symbol from
> vmlinux file.
>
> To resolve this issue, this patch adds extra checking for
> 'symbol_conf.vmlinux_name', when it has been set string so we can know
> it includes vmlinux path string specified for option '-k'. For this
> case it sets 'is_kernel_mmap' to true and run into flow to load kernel
> symbol from vmlinux.
>
> This patch has been verified with two commands: './perf report -v
> -k vmlinux --stdio' and './perf script -v -F cpu,event,ip,sym,symoff
> -k vmlinux'.
>
> Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/util/machine.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 12b7427..3125871 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1299,9 +1299,18 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> else
> kernel_type = DSO_TYPE_GUEST_KERNEL;
>
> - is_kernel_mmap = memcmp(event->mmap.filename,
> - machine->mmap_name,
> - strlen(machine->mmap_name) - 1) == 0;
> + /*
> + * When symbol_conf.vmlinux_name is not NULL, it includes the specified
> + * kernel vmlinux path with option '-k'. So set 'is_kernel_mmap' to
> + * true for creating machine symbol map.
> + */
> + if (symbol_conf.vmlinux_name)
> + is_kernel_mmap = true;
> + else
> + is_kernel_mmap = memcmp(event->mmap.filename,
> + machine->mmap_name,
> + strlen(machine->mmap_name) - 1) == 0;
> +
> if (event->mmap.filename[0] == '/' ||
> (!is_kernel_mmap && event->mmap.filename[0] == '[')) {
> map = machine__findnew_module_map(machine, event->mmap.start,
right, the mmap gets confused with the vmlinux path, but I wonder
the fix should be not to include symbol_conf.vmlinux_name in the
mmap_name like below.. untested
thanks,
jirka
---
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 43fbbee409ec..f0cb72022177 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -51,15 +51,9 @@ static void machine__threads_init(struct machine *machine)
static int machine__set_mmap_name(struct machine *machine)
{
if (machine__is_host(machine)) {
- if (symbol_conf.vmlinux_name)
- machine->mmap_name = strdup(symbol_conf.vmlinux_name);
- else
- machine->mmap_name = strdup("[kernel.kallsyms]");
+ machine->mmap_name = strdup("[kernel.kallsyms]");
} else if (machine__is_default_guest(machine)) {
- if (symbol_conf.default_guest_vmlinux_name)
- machine->mmap_name = strdup(symbol_conf.default_guest_vmlinux_name);
- else
- machine->mmap_name = strdup("[guest.kernel.kallsyms]");
+ machine->mmap_name = strdup("[guest.kernel.kallsyms]");
} else {
if (asprintf(&machine->mmap_name, "[guest.kernel.kallsyms.%d]",
machine->pid) < 0)
@@ -794,9 +788,15 @@ static struct dso *machine__get_kernel(struct machine *machine)
struct dso *kernel;
if (machine__is_host(machine)) {
+ if (symbol_conf.vmlinux_name)
+ vmlinux_name = symbol_conf.vmlinux_name;
+
kernel = machine__findnew_kernel(machine, vmlinux_name,
"[kernel]", DSO_TYPE_KERNEL);
} else {
+ if (symbol_conf.default_guest_vmlinux_name)
+ vmlinux_name = symbol_conf.default_guest_vmlinux_name;
+
kernel = machine__findnew_kernel(machine, vmlinux_name,
"[guest.kernel]",
DSO_TYPE_GUEST_KERNEL);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf machine: Fix load kernel symbol with '-k' option
2018-03-09 20:56 ` Jiri Olsa
@ 2018-03-10 9:50 ` Leo Yan
2018-03-11 19:49 ` Jiri Olsa
0 siblings, 1 reply; 6+ messages in thread
From: Leo Yan @ 2018-03-10 9:50 UTC (permalink / raw)
To: Jiri Olsa
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Namhyung Kim, linux-kernel, Mathieu Poirier
Hi Jiri,
On Fri, Mar 09, 2018 at 09:56:00PM +0100, Jiri Olsa wrote:
> On Fri, Mar 09, 2018 at 02:05:23PM +0800, Leo Yan wrote:
> > On Hikey arm64 octa A53 platform, when use command './perf report -v
> > -k vmlinux --stdio' it outputs below error info, and it skips to load
> > kernel symbol and doesn't print symbol for event:
> > Failed to open [kernel.kallsyms]_text, continuing without symbols.
> >
> > The regression is introduced by commit ("8c7f1bb37b29 perf machine: Move
> > kernel mmap name into struct machine"), which changes the logic for
> > machine mmap_name by removing function machine__mmap_name() and always
> > use 'machine->mmap_name'. Comparing difference between
> > machine__mmap_name() and 'machine->mmap_name', the later one includes
> > the string for specified kernel vmlinux string with option '-k' in
> > command, but the old function machine__mmap_name() ignores vmlinux
> > path string. As result, event's mmap file name doesn't match with
> > machine mmap file name anymore and it skips to load kernel symbol from
> > vmlinux file.
> >
> > To resolve this issue, this patch adds extra checking for
> > 'symbol_conf.vmlinux_name', when it has been set string so we can know
> > it includes vmlinux path string specified for option '-k'. For this
> > case it sets 'is_kernel_mmap' to true and run into flow to load kernel
> > symbol from vmlinux.
> >
> > This patch has been verified with two commands: './perf report -v
> > -k vmlinux --stdio' and './perf script -v -F cpu,event,ip,sym,symoff
> > -k vmlinux'.
> >
> > Suggested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> > tools/perf/util/machine.c | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index 12b7427..3125871 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -1299,9 +1299,18 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> > else
> > kernel_type = DSO_TYPE_GUEST_KERNEL;
> >
> > - is_kernel_mmap = memcmp(event->mmap.filename,
> > - machine->mmap_name,
> > - strlen(machine->mmap_name) - 1) == 0;
> > + /*
> > + * When symbol_conf.vmlinux_name is not NULL, it includes the specified
> > + * kernel vmlinux path with option '-k'. So set 'is_kernel_mmap' to
> > + * true for creating machine symbol map.
> > + */
> > + if (symbol_conf.vmlinux_name)
> > + is_kernel_mmap = true;
> > + else
> > + is_kernel_mmap = memcmp(event->mmap.filename,
> > + machine->mmap_name,
> > + strlen(machine->mmap_name) - 1) == 0;
> > +
> > if (event->mmap.filename[0] == '/' ||
> > (!is_kernel_mmap && event->mmap.filename[0] == '[')) {
> > map = machine__findnew_module_map(machine, event->mmap.start,
>
> right, the mmap gets confused with the vmlinux path, but I wonder
> the fix should be not to include symbol_conf.vmlinux_name in the
> mmap_name like below.. untested
I tested below fixing at my side, and confirm your fixing also can
resolve this issue. After reviewing the change, it's more neat than my
own change . So you could add my test tag :)
Tested-by: Leo Yan <leo.yan@linaro.org>
BTW, I have a minor comment for code refactoring, it's up to you if
need or not and if need use a new patch for refactoring, please see below
inline comment.
Thanks,
Leo Yan
> ---
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 43fbbee409ec..f0cb72022177 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -51,15 +51,9 @@ static void machine__threads_init(struct machine *machine)
> static int machine__set_mmap_name(struct machine *machine)
> {
> if (machine__is_host(machine)) {
> - if (symbol_conf.vmlinux_name)
> - machine->mmap_name = strdup(symbol_conf.vmlinux_name);
> - else
> - machine->mmap_name = strdup("[kernel.kallsyms]");
> + machine->mmap_name = strdup("[kernel.kallsyms]");
> } else if (machine__is_default_guest(machine)) {
> - if (symbol_conf.default_guest_vmlinux_name)
> - machine->mmap_name = strdup(symbol_conf.default_guest_vmlinux_name);
> - else
> - machine->mmap_name = strdup("[guest.kernel.kallsyms]");
> + machine->mmap_name = strdup("[guest.kernel.kallsyms]");
> } else {
> if (asprintf(&machine->mmap_name, "[guest.kernel.kallsyms.%d]",
> machine->pid) < 0)
Do you think below format is better or not?
static int machine__set_mmap_name(struct machine *machine)
{
- if (machine__is_host(machine)) {
- if (symbol_conf.vmlinux_name)
- machine->mmap_name = strdup(symbol_conf.vmlinux_name);
- else
- machine->mmap_name = strdup("[kernel.kallsyms]");
- } else if (machine__is_default_guest(machine)) {
- if (symbol_conf.default_guest_vmlinux_name)
- machine->mmap_name = strdup(symbol_conf.default_guest_vmlinux_name);
- else
- machine->mmap_name = strdup("[guest.kernel.kallsyms]");
- } else {
- if (asprintf(&machine->mmap_name, "[guest.kernel.kallsyms.%d]",
- machine->pid) < 0)
- machine->mmap_name = NULL;
- }
+ if (machine__is_host(machine))
+ machine->mmap_name = strdup("[kernel.kallsyms]");
+ else if (machine__is_default_guest(machine))
+ machine->mmap_name = strdup("[guest.kernel.kallsyms]");
+ else if (asprintf(&machine->mmap_name, "[guest.kernel.kallsyms.%d]",
+ machine->pid) < 0)
+ machine->mmap_name = NULL;
return machine->mmap_name ? 0 : -ENOMEM;
}
> @@ -794,9 +788,15 @@ static struct dso *machine__get_kernel(struct machine *machine)
> struct dso *kernel;
>
> if (machine__is_host(machine)) {
> + if (symbol_conf.vmlinux_name)
> + vmlinux_name = symbol_conf.vmlinux_name;
> +
> kernel = machine__findnew_kernel(machine, vmlinux_name,
> "[kernel]", DSO_TYPE_KERNEL);
> } else {
> + if (symbol_conf.default_guest_vmlinux_name)
> + vmlinux_name = symbol_conf.default_guest_vmlinux_name;
> +
> kernel = machine__findnew_kernel(machine, vmlinux_name,
> "[guest.kernel]",
> DSO_TYPE_GUEST_KERNEL);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] perf machine: Fix load kernel symbol with '-k' option
2018-03-10 9:50 ` Leo Yan
@ 2018-03-11 19:49 ` Jiri Olsa
0 siblings, 0 replies; 6+ messages in thread
From: Jiri Olsa @ 2018-03-11 19:49 UTC (permalink / raw)
To: Leo Yan
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Namhyung Kim, linux-kernel, Mathieu Poirier
On Sat, Mar 10, 2018 at 05:50:56PM +0800, Leo Yan wrote:
SNIP
> > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > > index 12b7427..3125871 100644
> > > --- a/tools/perf/util/machine.c
> > > +++ b/tools/perf/util/machine.c
> > > @@ -1299,9 +1299,18 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> > > else
> > > kernel_type = DSO_TYPE_GUEST_KERNEL;
> > >
> > > - is_kernel_mmap = memcmp(event->mmap.filename,
> > > - machine->mmap_name,
> > > - strlen(machine->mmap_name) - 1) == 0;
> > > + /*
> > > + * When symbol_conf.vmlinux_name is not NULL, it includes the specified
> > > + * kernel vmlinux path with option '-k'. So set 'is_kernel_mmap' to
> > > + * true for creating machine symbol map.
> > > + */
> > > + if (symbol_conf.vmlinux_name)
> > > + is_kernel_mmap = true;
> > > + else
> > > + is_kernel_mmap = memcmp(event->mmap.filename,
> > > + machine->mmap_name,
> > > + strlen(machine->mmap_name) - 1) == 0;
> > > +
> > > if (event->mmap.filename[0] == '/' ||
> > > (!is_kernel_mmap && event->mmap.filename[0] == '[')) {
> > > map = machine__findnew_module_map(machine, event->mmap.start,
> >
> > right, the mmap gets confused with the vmlinux path, but I wonder
> > the fix should be not to include symbol_conf.vmlinux_name in the
> > mmap_name like below.. untested
>
> I tested below fixing at my side, and confirm your fixing also can
> resolve this issue. After reviewing the change, it's more neat than my
> own change . So you could add my test tag :)
>
> Tested-by: Leo Yan <leo.yan@linaro.org>
thanks
SNIP
> static int machine__set_mmap_name(struct machine *machine)
> {
> - if (machine__is_host(machine)) {
> - if (symbol_conf.vmlinux_name)
> - machine->mmap_name = strdup(symbol_conf.vmlinux_name);
> - else
> - machine->mmap_name = strdup("[kernel.kallsyms]");
> - } else if (machine__is_default_guest(machine)) {
> - if (symbol_conf.default_guest_vmlinux_name)
> - machine->mmap_name = strdup(symbol_conf.default_guest_vmlinux_name);
> - else
> - machine->mmap_name = strdup("[guest.kernel.kallsyms]");
> - } else {
> - if (asprintf(&machine->mmap_name, "[guest.kernel.kallsyms.%d]",
> - machine->pid) < 0)
> - machine->mmap_name = NULL;
> - }
> + if (machine__is_host(machine))
> + machine->mmap_name = strdup("[kernel.kallsyms]");
> + else if (machine__is_default_guest(machine))
> + machine->mmap_name = strdup("[guest.kernel.kallsyms]");
> + else if (asprintf(&machine->mmap_name, "[guest.kernel.kallsyms.%d]",
> + machine->pid) < 0)
> + machine->mmap_name = NULL;
yea, that looks better.. will change
jirka
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-11 19:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-09 6:05 [PATCH v2] perf machine: Fix load kernel symbol with '-k' option Leo Yan
2018-03-09 17:31 ` Mathieu Poirier
2018-03-09 17:42 ` Arnaldo Carvalho de Melo
2018-03-09 20:56 ` Jiri Olsa
2018-03-10 9:50 ` Leo Yan
2018-03-11 19:49 ` Jiri Olsa
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).