* [PATCH 1/4] perf tools: Mark modules dsos with kernel type
2020-02-10 14:32 [PATCH 0/4] perf tools: Fix kmap handling Jiri Olsa
@ 2020-02-10 14:32 ` Jiri Olsa
2020-02-15 8:41 ` [tip: perf/urgent] perf maps: Mark module DSOs " tip-bot2 for Jiri Olsa
2020-02-10 14:32 ` [PATCH 2/4] perf tools: Mark ksymbol dsos " Jiri Olsa
` (3 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2020-02-10 14:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ravi Bangoria, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan
We add kernel module map into machine->kmaps, so it needs
to be created as 'struct kmap', which is dependent on its
dso having kernel type.
Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/machine.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index c8c5410315e8..e3e5490f6de5 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -686,6 +686,7 @@ static struct dso *machine__findnew_module_dso(struct machine *machine,
dso__set_module_info(dso, m, machine);
dso__set_long_name(dso, strdup(filename), true);
+ dso->kernel = DSO_TYPE_KERNEL;
}
dso__get(dso);
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [tip: perf/urgent] perf maps: Mark module DSOs with kernel type
2020-02-10 14:32 ` [PATCH 1/4] perf tools: Mark modules dsos with kernel type Jiri Olsa
@ 2020-02-15 8:41 ` tip-bot2 for Jiri Olsa
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot2 for Jiri Olsa @ 2020-02-15 8:41 UTC (permalink / raw)
To: linux-tip-commits
Cc: Ravi Bangoria, Jiri Olsa, Kim Phillips, Alexander Shishkin,
Michael Petlan, Namhyung Kim, Peter Zijlstra,
Arnaldo Carvalho de Melo, x86, LKML
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: 02213cec64bbef66d7ad9ddc3b7c47236da64343
Gitweb: https://git.kernel.org/tip/02213cec64bbef66d7ad9ddc3b7c47236da64343
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 10 Feb 2020 15:32:15 +01:00
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 11 Feb 2020 16:41:49 -03:00
perf maps: Mark module DSOs with kernel type
We add kernel module map into machine->kmaps, so it needs to be created
as 'struct kmap', which is dependent on its dso having kernel type.
Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Kim Phillips <kim.phillips@amd.com>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20200210143218.24948-2-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/machine.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index c8c5410..e3e5490 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -686,6 +686,7 @@ static struct dso *machine__findnew_module_dso(struct machine *machine,
dso__set_module_info(dso, m, machine);
dso__set_long_name(dso, strdup(filename), true);
+ dso->kernel = DSO_TYPE_KERNEL;
}
dso__get(dso);
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/4] perf tools: Mark ksymbol dsos with kernel type
2020-02-10 14:32 [PATCH 0/4] perf tools: Fix kmap handling Jiri Olsa
2020-02-10 14:32 ` [PATCH 1/4] perf tools: Mark modules dsos with kernel type Jiri Olsa
@ 2020-02-10 14:32 ` Jiri Olsa
2020-02-10 15:17 ` Arnaldo Carvalho de Melo
2020-02-10 14:32 ` [PATCH 3/4] perf tools: Fix map__clone for struct kmap Jiri Olsa
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2020-02-10 14:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ravi Bangoria, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan
We add ksymbol map into machine->kmaps, so it needs to be
created as 'struct kmap', which is dependent on its dso
having kernel type.
Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/machine.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index e3e5490f6de5..0a43dc83d7b2 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -727,8 +727,14 @@ static int machine__process_ksymbol_register(struct machine *machine,
struct map *map = maps__find(&machine->kmaps, event->ksymbol.addr);
if (!map) {
- map = dso__new_map(event->ksymbol.name);
- if (!map)
+ struct dso *dso = dso__new(event->ksymbol.name);
+
+ if (dso) {
+ dso->kernel = DSO_TYPE_KERNEL;
+ map = map__new2(0, dso);
+ }
+
+ if (!dso || !map)
return -ENOMEM;
map->start = event->ksymbol.addr;
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/4] perf tools: Mark ksymbol dsos with kernel type
2020-02-10 14:32 ` [PATCH 2/4] perf tools: Mark ksymbol dsos " Jiri Olsa
@ 2020-02-10 15:17 ` Arnaldo Carvalho de Melo
2020-02-10 15:25 ` Jiri Olsa
0 siblings, 1 reply; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-10 15:17 UTC (permalink / raw)
To: Jiri Olsa
Cc: Ravi Bangoria, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan
Em Mon, Feb 10, 2020 at 03:32:16PM +0100, Jiri Olsa escreveu:
> We add ksymbol map into machine->kmaps, so it needs to be
> created as 'struct kmap', which is dependent on its dso
> having kernel type.
>
> Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/util/machine.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index e3e5490f6de5..0a43dc83d7b2 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -727,8 +727,14 @@ static int machine__process_ksymbol_register(struct machine *machine,
> struct map *map = maps__find(&machine->kmaps, event->ksymbol.addr);
>
> if (!map) {
> - map = dso__new_map(event->ksymbol.name);
> - if (!map)
> + struct dso *dso = dso__new(event->ksymbol.name);
> +
> + if (dso) {
> + dso->kernel = DSO_TYPE_KERNEL;
> + map = map__new2(0, dso);
> + }
> +
> + if (!dso || !map)
We leak dso if map creation fails?
- Arnaldo
> return -ENOMEM;
>
> map->start = event->ksymbol.addr;
> --
> 2.24.1
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/4] perf tools: Mark ksymbol dsos with kernel type
2020-02-10 15:17 ` Arnaldo Carvalho de Melo
@ 2020-02-10 15:25 ` Jiri Olsa
2020-02-10 19:58 ` Arnaldo Carvalho de Melo
2020-02-10 20:08 ` [PATCHv2 " Jiri Olsa
0 siblings, 2 replies; 17+ messages in thread
From: Jiri Olsa @ 2020-02-10 15:25 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Ravi Bangoria, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan
On Mon, Feb 10, 2020 at 12:17:59PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 10, 2020 at 03:32:16PM +0100, Jiri Olsa escreveu:
> > We add ksymbol map into machine->kmaps, so it needs to be
> > created as 'struct kmap', which is dependent on its dso
> > having kernel type.
> >
> > Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/perf/util/machine.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index e3e5490f6de5..0a43dc83d7b2 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -727,8 +727,14 @@ static int machine__process_ksymbol_register(struct machine *machine,
> > struct map *map = maps__find(&machine->kmaps, event->ksymbol.addr);
> >
> > if (!map) {
> > - map = dso__new_map(event->ksymbol.name);
> > - if (!map)
> > + struct dso *dso = dso__new(event->ksymbol.name);
> > +
> > + if (dso) {
> > + dso->kernel = DSO_TYPE_KERNEL;
> > + map = map__new2(0, dso);
> > + }
> > +
> > + if (!dso || !map)
>
> We leak dso if map creation fails?
yep :-\ will post v2
thanks,
jirka
>
>
> - Arnaldo
>
> > return -ENOMEM;
> >
> > map->start = event->ksymbol.addr;
> > --
> > 2.24.1
> >
>
> --
>
> - Arnaldo
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/4] perf tools: Mark ksymbol dsos with kernel type
2020-02-10 15:25 ` Jiri Olsa
@ 2020-02-10 19:58 ` Arnaldo Carvalho de Melo
2020-02-10 20:34 ` Jiri Olsa
2020-02-10 20:08 ` [PATCHv2 " Jiri Olsa
1 sibling, 1 reply; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-10 19:58 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jiri Olsa, Ravi Bangoria, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan
Em Mon, Feb 10, 2020 at 04:25:37PM +0100, Jiri Olsa escreveu:
> On Mon, Feb 10, 2020 at 12:17:59PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Feb 10, 2020 at 03:32:16PM +0100, Jiri Olsa escreveu:
> > > We add ksymbol map into machine->kmaps, so it needs to be
> > > created as 'struct kmap', which is dependent on its dso
> > > having kernel type.
> > >
> > > Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > > Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > > tools/perf/util/machine.c | 10 ++++++++--
> > > 1 file changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > > index e3e5490f6de5..0a43dc83d7b2 100644
> > > --- a/tools/perf/util/machine.c
> > > +++ b/tools/perf/util/machine.c
> > > @@ -727,8 +727,14 @@ static int machine__process_ksymbol_register(struct machine *machine,
> > > struct map *map = maps__find(&machine->kmaps, event->ksymbol.addr);
> > >
> > > if (!map) {
> > > - map = dso__new_map(event->ksymbol.name);
> > > - if (!map)
> > > + struct dso *dso = dso__new(event->ksymbol.name);
> > > +
> > > + if (dso) {
> > > + dso->kernel = DSO_TYPE_KERNEL;
> > > + map = map__new2(0, dso);
> > > + }
> > > +
> > > + if (!dso || !map)
> >
> > We leak dso if map creation fails?
>
> yep :-\ will post v2
Please collect Kim's Tested-by then,
- Arnaldo
> thanks,
> jirka
>
> >
> >
> > - Arnaldo
> >
> > > return -ENOMEM;
> > >
> > > map->start = event->ksymbol.addr;
> > > --
> > > 2.24.1
> > >
> >
> > --
> >
> > - Arnaldo
> >
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/4] perf tools: Mark ksymbol dsos with kernel type
2020-02-10 19:58 ` Arnaldo Carvalho de Melo
@ 2020-02-10 20:34 ` Jiri Olsa
0 siblings, 0 replies; 17+ messages in thread
From: Jiri Olsa @ 2020-02-10 20:34 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Ravi Bangoria, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan
On Mon, Feb 10, 2020 at 04:58:01PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 10, 2020 at 04:25:37PM +0100, Jiri Olsa escreveu:
> > On Mon, Feb 10, 2020 at 12:17:59PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Mon, Feb 10, 2020 at 03:32:16PM +0100, Jiri Olsa escreveu:
> > > > We add ksymbol map into machine->kmaps, so it needs to be
> > > > created as 'struct kmap', which is dependent on its dso
> > > > having kernel type.
> > > >
> > > > Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > > > Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > > ---
> > > > tools/perf/util/machine.c | 10 ++++++++--
> > > > 1 file changed, 8 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > > > index e3e5490f6de5..0a43dc83d7b2 100644
> > > > --- a/tools/perf/util/machine.c
> > > > +++ b/tools/perf/util/machine.c
> > > > @@ -727,8 +727,14 @@ static int machine__process_ksymbol_register(struct machine *machine,
> > > > struct map *map = maps__find(&machine->kmaps, event->ksymbol.addr);
> > > >
> > > > if (!map) {
> > > > - map = dso__new_map(event->ksymbol.name);
> > > > - if (!map)
> > > > + struct dso *dso = dso__new(event->ksymbol.name);
> > > > +
> > > > + if (dso) {
> > > > + dso->kernel = DSO_TYPE_KERNEL;
> > > > + map = map__new2(0, dso);
> > > > + }
> > > > +
> > > > + if (!dso || !map)
> > >
> > > We leak dso if map creation fails?
> >
> > yep :-\ will post v2
>
> Please collect Kim's Tested-by then,
oops, did not notice, I updated commits with them in the perf/top branch
jirka
>
> - Arnaldo
>
> > thanks,
> > jirka
> >
> > >
> > >
> > > - Arnaldo
> > >
> > > > return -ENOMEM;
> > > >
> > > > map->start = event->ksymbol.addr;
> > > > --
> > > > 2.24.1
> > > >
> > >
> > > --
> > >
> > > - Arnaldo
> > >
> >
>
> --
>
> - Arnaldo
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCHv2 2/4] perf tools: Mark ksymbol dsos with kernel type
2020-02-10 15:25 ` Jiri Olsa
2020-02-10 19:58 ` Arnaldo Carvalho de Melo
@ 2020-02-10 20:08 ` Jiri Olsa
2020-02-11 13:47 ` Arnaldo Carvalho de Melo
2020-02-15 8:41 ` [tip: perf/urgent] perf maps: Mark ksymbol DSOs " tip-bot2 for Jiri Olsa
1 sibling, 2 replies; 17+ messages in thread
From: Jiri Olsa @ 2020-02-10 20:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Ravi Bangoria, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan
On Mon, Feb 10, 2020 at 04:25:37PM +0100, Jiri Olsa wrote:
> On Mon, Feb 10, 2020 at 12:17:59PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Feb 10, 2020 at 03:32:16PM +0100, Jiri Olsa escreveu:
> > > We add ksymbol map into machine->kmaps, so it needs to be
> > > created as 'struct kmap', which is dependent on its dso
> > > having kernel type.
> > >
> > > Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > > Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > > tools/perf/util/machine.c | 10 ++++++++--
> > > 1 file changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > > index e3e5490f6de5..0a43dc83d7b2 100644
> > > --- a/tools/perf/util/machine.c
> > > +++ b/tools/perf/util/machine.c
> > > @@ -727,8 +727,14 @@ static int machine__process_ksymbol_register(struct machine *machine,
> > > struct map *map = maps__find(&machine->kmaps, event->ksymbol.addr);
> > >
> > > if (!map) {
> > > - map = dso__new_map(event->ksymbol.name);
> > > - if (!map)
> > > + struct dso *dso = dso__new(event->ksymbol.name);
> > > +
> > > + if (dso) {
> > > + dso->kernel = DSO_TYPE_KERNEL;
> > > + map = map__new2(0, dso);
> > > + }
> > > +
> > > + if (!dso || !map)
> >
> > We leak dso if map creation fails?
>
> yep :-\ will post v2
v2 attached, it's also all in my perf/top branch
thanks,
jirka
---
We add ksymbol map into machine->kmaps, so it needs to be
created as 'struct kmap', which is dependent on its dso
having kernel type.
Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/machine.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index e3e5490f6de5..0ad026561c7f 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -727,9 +727,17 @@ static int machine__process_ksymbol_register(struct machine *machine,
struct map *map = maps__find(&machine->kmaps, event->ksymbol.addr);
if (!map) {
- map = dso__new_map(event->ksymbol.name);
- if (!map)
+ struct dso *dso = dso__new(event->ksymbol.name);
+
+ if (dso) {
+ dso->kernel = DSO_TYPE_KERNEL;
+ map = map__new2(0, dso);
+ }
+
+ if (!dso || !map) {
+ dso__put(dso);
return -ENOMEM;
+ }
map->start = event->ksymbol.addr;
map->end = map->start + event->ksymbol.len;
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCHv2 2/4] perf tools: Mark ksymbol dsos with kernel type
2020-02-10 20:08 ` [PATCHv2 " Jiri Olsa
@ 2020-02-11 13:47 ` Arnaldo Carvalho de Melo
2020-02-15 8:41 ` [tip: perf/urgent] perf maps: Mark ksymbol DSOs " tip-bot2 for Jiri Olsa
1 sibling, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-02-11 13:47 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jiri Olsa, Ravi Bangoria, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan
Em Mon, Feb 10, 2020 at 09:08:47PM +0100, Jiri Olsa escreveu:
> On Mon, Feb 10, 2020 at 04:25:37PM +0100, Jiri Olsa wrote:
> > On Mon, Feb 10, 2020 at 12:17:59PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Mon, Feb 10, 2020 at 03:32:16PM +0100, Jiri Olsa escreveu:
> > > > We add ksymbol map into machine->kmaps, so it needs to be
> > > > created as 'struct kmap', which is dependent on its dso
> > > > having kernel type.
> > > >
> > > > Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > > > Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > > ---
> > > > tools/perf/util/machine.c | 10 ++++++++--
> > > > 1 file changed, 8 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > > > index e3e5490f6de5..0a43dc83d7b2 100644
> > > > --- a/tools/perf/util/machine.c
> > > > +++ b/tools/perf/util/machine.c
> > > > @@ -727,8 +727,14 @@ static int machine__process_ksymbol_register(struct machine *machine,
> > > > struct map *map = maps__find(&machine->kmaps, event->ksymbol.addr);
> > > >
> > > > if (!map) {
> > > > - map = dso__new_map(event->ksymbol.name);
> > > > - if (!map)
> > > > + struct dso *dso = dso__new(event->ksymbol.name);
> > > > +
> > > > + if (dso) {
> > > > + dso->kernel = DSO_TYPE_KERNEL;
> > > > + map = map__new2(0, dso);
> > > > + }
> > > > +
> > > > + if (!dso || !map)
> > >
> > > We leak dso if map creation fails?
> >
> > yep :-\ will post v2
>
> v2 attached, it's also all in my perf/top branch
Thanks for fixing it, will process it into my perf/urgent branch.
- Arnaldo
> thanks,
> jirka
>
>
> ---
> We add ksymbol map into machine->kmaps, so it needs to be
> created as 'struct kmap', which is dependent on its dso
> having kernel type.
>
> Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/util/machine.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index e3e5490f6de5..0ad026561c7f 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -727,9 +727,17 @@ static int machine__process_ksymbol_register(struct machine *machine,
> struct map *map = maps__find(&machine->kmaps, event->ksymbol.addr);
>
> if (!map) {
> - map = dso__new_map(event->ksymbol.name);
> - if (!map)
> + struct dso *dso = dso__new(event->ksymbol.name);
> +
> + if (dso) {
> + dso->kernel = DSO_TYPE_KERNEL;
> + map = map__new2(0, dso);
> + }
> +
> + if (!dso || !map) {
> + dso__put(dso);
> return -ENOMEM;
> + }
>
> map->start = event->ksymbol.addr;
> map->end = map->start + event->ksymbol.len;
> --
> 2.24.1
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 17+ messages in thread* [tip: perf/urgent] perf maps: Mark ksymbol DSOs with kernel type
2020-02-10 20:08 ` [PATCHv2 " Jiri Olsa
2020-02-11 13:47 ` Arnaldo Carvalho de Melo
@ 2020-02-15 8:41 ` tip-bot2 for Jiri Olsa
1 sibling, 0 replies; 17+ messages in thread
From: tip-bot2 for Jiri Olsa @ 2020-02-15 8:41 UTC (permalink / raw)
To: linux-tip-commits
Cc: Ravi Bangoria, Jiri Olsa, Kim Phillips, Alexander Shishkin,
Michael Petlan, Namhyung Kim, Peter Zijlstra,
Arnaldo Carvalho de Melo, x86, LKML
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: 4a4eb6154d67f7766cc7eb74e9f1db424073e832
Gitweb: https://git.kernel.org/tip/4a4eb6154d67f7766cc7eb74e9f1db424073e832
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Mon, 10 Feb 2020 21:08:47 +01:00
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 11 Feb 2020 16:41:49 -03:00
perf maps: Mark ksymbol DSOs with kernel type
We add ksymbol map into machine->kmaps, so it needs to be created as
'struct kmap', which is dependent on its dso having kernel type.
Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Tested-by: Kim Phillips <kim.phillips@amd.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20200210200847.GA36715@krava
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/machine.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index e3e5490..0ad0265 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -727,9 +727,17 @@ static int machine__process_ksymbol_register(struct machine *machine,
struct map *map = maps__find(&machine->kmaps, event->ksymbol.addr);
if (!map) {
- map = dso__new_map(event->ksymbol.name);
- if (!map)
+ struct dso *dso = dso__new(event->ksymbol.name);
+
+ if (dso) {
+ dso->kernel = DSO_TYPE_KERNEL;
+ map = map__new2(0, dso);
+ }
+
+ if (!dso || !map) {
+ dso__put(dso);
return -ENOMEM;
+ }
map->start = event->ksymbol.addr;
map->end = map->start + event->ksymbol.len;
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/4] perf tools: Fix map__clone for struct kmap
2020-02-10 14:32 [PATCH 0/4] perf tools: Fix kmap handling Jiri Olsa
2020-02-10 14:32 ` [PATCH 1/4] perf tools: Mark modules dsos with kernel type Jiri Olsa
2020-02-10 14:32 ` [PATCH 2/4] perf tools: Mark ksymbol dsos " Jiri Olsa
@ 2020-02-10 14:32 ` Jiri Olsa
2020-02-15 8:41 ` [tip: perf/urgent] perf maps: Fix map__clone() " tip-bot2 for Jiri Olsa
2020-02-10 14:32 ` [PATCH 4/4] perf tools: Move kmap::kmaps setup to maps__insert Jiri Olsa
2020-02-10 16:47 ` [PATCH 0/4] perf tools: Fix kmap handling Kim Phillips
4 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2020-02-10 14:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ravi Bangoria, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan
The map__clone function can be called on kernel maps
as well, so it needs to duplicate the whole kmap data.
Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/map.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index f67960bedebb..cea05fc9595c 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -375,8 +375,13 @@ struct symbol *map__find_symbol_by_name(struct map *map, const char *name)
struct map *map__clone(struct map *from)
{
- struct map *map = memdup(from, sizeof(*map));
+ size_t size = sizeof(struct map);
+ struct map *map;
+
+ if (from->dso && from->dso->kernel)
+ size += sizeof(struct kmap);
+ map = memdup(from, size);
if (map != NULL) {
refcount_set(&map->refcnt, 1);
RB_CLEAR_NODE(&map->rb_node);
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [tip: perf/urgent] perf maps: Fix map__clone() for struct kmap
2020-02-10 14:32 ` [PATCH 3/4] perf tools: Fix map__clone for struct kmap Jiri Olsa
@ 2020-02-15 8:41 ` tip-bot2 for Jiri Olsa
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot2 for Jiri Olsa @ 2020-02-15 8:41 UTC (permalink / raw)
To: linux-tip-commits
Cc: Ravi Bangoria, Jiri Olsa, Kim Phillips, Alexander Shishkin,
Michael Petlan, Namhyung Kim, Peter Zijlstra,
Arnaldo Carvalho de Melo, x86, LKML
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: 7ce66139a99ce57caaf47b64afed5cb6ed02c5ed
Gitweb: https://git.kernel.org/tip/7ce66139a99ce57caaf47b64afed5cb6ed02c5ed
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 10 Feb 2020 15:32:17 +01:00
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 11 Feb 2020 16:41:49 -03:00
perf maps: Fix map__clone() for struct kmap
The map__clone() function can be called on kernel maps as well, so it
needs to duplicate the whole kmap data.
Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Tested-by: Kim Phillips <kim.phillips@amd.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20200210143218.24948-4-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/map.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index f67960b..cea05fc 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -375,8 +375,13 @@ struct symbol *map__find_symbol_by_name(struct map *map, const char *name)
struct map *map__clone(struct map *from)
{
- struct map *map = memdup(from, sizeof(*map));
+ size_t size = sizeof(struct map);
+ struct map *map;
+
+ if (from->dso && from->dso->kernel)
+ size += sizeof(struct kmap);
+ map = memdup(from, size);
if (map != NULL) {
refcount_set(&map->refcnt, 1);
RB_CLEAR_NODE(&map->rb_node);
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4] perf tools: Move kmap::kmaps setup to maps__insert
2020-02-10 14:32 [PATCH 0/4] perf tools: Fix kmap handling Jiri Olsa
` (2 preceding siblings ...)
2020-02-10 14:32 ` [PATCH 3/4] perf tools: Fix map__clone for struct kmap Jiri Olsa
@ 2020-02-10 14:32 ` Jiri Olsa
2020-02-15 8:41 ` [tip: perf/urgent] perf maps: Move kmap::kmaps setup to maps__insert() tip-bot2 for Jiri Olsa
2020-02-10 16:47 ` [PATCH 0/4] perf tools: Fix kmap handling Kim Phillips
4 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2020-02-10 14:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ravi Bangoria, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan
So the kmaps pointer setup is centralized and we do not need
to update it in all those places (2 current places and few
more missing) after calling maps__insert.
Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/machine.c | 13 +------------
tools/perf/util/map.c | 10 ++++++++++
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 0a43dc83d7b2..460315476314 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -979,7 +979,6 @@ int machine__create_extra_kernel_map(struct machine *machine,
kmap = map__kmap(map);
- kmap->kmaps = &machine->kmaps;
strlcpy(kmap->name, xm->name, KMAP_NAME_LEN);
maps__insert(&machine->kmaps, map);
@@ -1089,9 +1088,6 @@ int __weak machine__create_extra_kernel_maps(struct machine *machine __maybe_unu
static int
__machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
{
- struct kmap *kmap;
- struct map *map;
-
/* In case of renewal the kernel map, destroy previous one */
machine__destroy_kernel_maps(machine);
@@ -1100,14 +1096,7 @@ __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
return -1;
machine->vmlinux_map->map_ip = machine->vmlinux_map->unmap_ip = identity__map_ip;
- map = machine__kernel_map(machine);
- kmap = map__kmap(map);
- if (!kmap)
- return -1;
-
- kmap->kmaps = &machine->kmaps;
- maps__insert(&machine->kmaps, map);
-
+ maps__insert(&machine->kmaps, machine->vmlinux_map);
return 0;
}
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index cea05fc9595c..a08ca276098e 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -543,6 +543,16 @@ void maps__insert(struct maps *maps, struct map *map)
__maps__insert(maps, map);
++maps->nr_maps;
+ if (map->dso && map->dso->kernel) {
+ struct kmap *kmap = map__kmap(map);
+
+ if (kmap)
+ kmap->kmaps = maps;
+ else
+ pr_err("Internal error: kernel dso with non kernel map\n");
+ }
+
+
/*
* If we already performed some search by name, then we need to add the just
* inserted map and resort.
--
2.24.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [tip: perf/urgent] perf maps: Move kmap::kmaps setup to maps__insert()
2020-02-10 14:32 ` [PATCH 4/4] perf tools: Move kmap::kmaps setup to maps__insert Jiri Olsa
@ 2020-02-15 8:41 ` tip-bot2 for Jiri Olsa
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot2 for Jiri Olsa @ 2020-02-15 8:41 UTC (permalink / raw)
To: linux-tip-commits
Cc: Ravi Bangoria, Jiri Olsa, Kim Phillips, Alexander Shishkin,
Michael Petlan, Namhyung Kim, Peter Zijlstra,
Arnaldo Carvalho de Melo, x86, LKML
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: 484214f49bd0948d716832a94e4737ca4dd02c16
Gitweb: https://git.kernel.org/tip/484214f49bd0948d716832a94e4737ca4dd02c16
Author: Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 10 Feb 2020 15:32:18 +01:00
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Tue, 11 Feb 2020 16:41:49 -03:00
perf maps: Move kmap::kmaps setup to maps__insert()
So the kmaps pointer setup is centralized and we do not need to update
it in all those places (2 current places and few more missing) after
calling maps__insert().
Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Tested-by: Kim Phillips <kim.phillips@amd.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20200210143218.24948-5-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/machine.c | 13 +------------
tools/perf/util/map.c | 10 ++++++++++
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 0ad0265..fb5c2cd 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -981,7 +981,6 @@ int machine__create_extra_kernel_map(struct machine *machine,
kmap = map__kmap(map);
- kmap->kmaps = &machine->kmaps;
strlcpy(kmap->name, xm->name, KMAP_NAME_LEN);
maps__insert(&machine->kmaps, map);
@@ -1091,9 +1090,6 @@ int __weak machine__create_extra_kernel_maps(struct machine *machine __maybe_unu
static int
__machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
{
- struct kmap *kmap;
- struct map *map;
-
/* In case of renewal the kernel map, destroy previous one */
machine__destroy_kernel_maps(machine);
@@ -1102,14 +1098,7 @@ __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
return -1;
machine->vmlinux_map->map_ip = machine->vmlinux_map->unmap_ip = identity__map_ip;
- map = machine__kernel_map(machine);
- kmap = map__kmap(map);
- if (!kmap)
- return -1;
-
- kmap->kmaps = &machine->kmaps;
- maps__insert(&machine->kmaps, map);
-
+ maps__insert(&machine->kmaps, machine->vmlinux_map);
return 0;
}
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index cea05fc..a08ca27 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -543,6 +543,16 @@ void maps__insert(struct maps *maps, struct map *map)
__maps__insert(maps, map);
++maps->nr_maps;
+ if (map->dso && map->dso->kernel) {
+ struct kmap *kmap = map__kmap(map);
+
+ if (kmap)
+ kmap->kmaps = maps;
+ else
+ pr_err("Internal error: kernel dso with non kernel map\n");
+ }
+
+
/*
* If we already performed some search by name, then we need to add the just
* inserted map and resort.
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 0/4] perf tools: Fix kmap handling
2020-02-10 14:32 [PATCH 0/4] perf tools: Fix kmap handling Jiri Olsa
` (3 preceding siblings ...)
2020-02-10 14:32 ` [PATCH 4/4] perf tools: Move kmap::kmaps setup to maps__insert Jiri Olsa
@ 2020-02-10 16:47 ` Kim Phillips
2020-02-10 20:35 ` Jiri Olsa
4 siblings, 1 reply; 17+ messages in thread
From: Kim Phillips @ 2020-02-10 16:47 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan, Ravi Bangoria
On 2/10/20 8:32 AM, Jiri Olsa wrote:
> hi,
> Ravi Bangoria reported crash in perf top due to wrong kmap
> objects management, this patchset should fix that.
>
> thanks,
> jirka
>
>
> ---
> Jiri Olsa (4):
> perf tools: Mark modules dsos with kernel type
> perf tools: Mark ksymbol dsos with kernel type
> perf tools: Fix map__clone for struct kmap
> perf tools: Move kmap::kmaps setup to maps__insert
>
> tools/perf/util/machine.c | 24 ++++++++++--------------
> tools/perf/util/map.c | 17 ++++++++++++++++-
> 2 files changed, 26 insertions(+), 15 deletions(-)
>
This series fixes a segmentation fault I was seeing on a
couple of AMD systems, so:
Tested-by: Kim Phillips <kim.phillips@amd.com>
Thanks,
Kim
Thread 259 "perf" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fff3b7b6700 (LWP 13241)]
__map__is_kernel (map=0x7fffd80098d0) at util/map.c:244
244 return machine__kernel_map(map__kmaps((struct map *)map)->machine) == map;
(gdb) bt
#0 __map__is_kernel (map=0x7fffd80098d0) at util/map.c:244
#1 0x000055555575d756 in perf_event__process_sample (machine=<optimized out>, sample=0x7fff3b7b5710,
evsel=0x555555ef9dd0, event=<optimized out>, tool=0x7fffffff8660) at builtin-top.c:799
#2 deliver_event (qe=<optimized out>, qevent=<optimized out>) at builtin-top.c:1192
#3 0x0000555555831f81 in do_flush (show_progress=false, oe=0x7fffffff8960) at util/ordered-events.c:244
#4 __ordered_events__flush (oe=oe@entry=0x7fffffff8960, how=how@entry=OE_FLUSH__TOP, timestamp=timestamp@entry=0)
at util/ordered-events.c:323
#5 0x00005555558328df in __ordered_events__flush (timestamp=<optimized out>, how=<optimized out>, oe=<optimized out>)
at util/ordered-events.c:342
#6 ordered_events__flush (oe=oe@entry=0x7fffffff8960, how=how@entry=OE_FLUSH__TOP) at util/ordered-events.c:341
#7 0x000055555575d0f1 in process_thread (arg=0x7fffffff8660) at builtin-top.c:1104
#8 0x00007ffff7bbd6db in start_thread (arg=0x7fff3b7b6700) at pthread_create.c:463
#9 0x00007ffff5d9788f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
(gdb) p map
$1 = (const struct map *) 0x7fffd80098d0
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 0/4] perf tools: Fix kmap handling
2020-02-10 16:47 ` [PATCH 0/4] perf tools: Fix kmap handling Kim Phillips
@ 2020-02-10 20:35 ` Jiri Olsa
0 siblings, 0 replies; 17+ messages in thread
From: Jiri Olsa @ 2020-02-10 20:35 UTC (permalink / raw)
To: Kim Phillips
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Ingo Molnar,
Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
Ravi Bangoria
On Mon, Feb 10, 2020 at 10:47:11AM -0600, Kim Phillips wrote:
> On 2/10/20 8:32 AM, Jiri Olsa wrote:
> > hi,
> > Ravi Bangoria reported crash in perf top due to wrong kmap
> > objects management, this patchset should fix that.
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > Jiri Olsa (4):
> > perf tools: Mark modules dsos with kernel type
> > perf tools: Mark ksymbol dsos with kernel type
> > perf tools: Fix map__clone for struct kmap
> > perf tools: Move kmap::kmaps setup to maps__insert
> >
> > tools/perf/util/machine.c | 24 ++++++++++--------------
> > tools/perf/util/map.c | 17 ++++++++++++++++-
> > 2 files changed, 26 insertions(+), 15 deletions(-)
> >
>
> This series fixes a segmentation fault I was seeing on a
> couple of AMD systems, so:
>
> Tested-by: Kim Phillips <kim.phillips@amd.com>
great, thanks a lot for testing
jirka
^ permalink raw reply [flat|nested] 17+ messages in thread