linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf map: fix infinite loop on map_groups__fixup_overlappings
@ 2019-12-11 16:07 Matheus Marchini
  2019-12-11 18:13 ` Arnaldo Carvalho de Melo
  2019-12-14 11:16 ` Konstantin Khlebnikov
  0 siblings, 2 replies; 3+ messages in thread
From: Matheus Marchini @ 2019-12-11 16:07 UTC (permalink / raw)
  To: linux-perf-users
  Cc: jkoch, khlebnikov, Matheus Marchini, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Changbin Du, Song Liu, John Keeping,
	Andi Kleen, linux-kernel

In some cases, when using perf inject and there are JIT_CODE_MOVE
records in the jitdump file, perf will end up in an infinite loop on
map_groups__fixup_overlappings, which will keep allocating memory
indefinitely. This issue was observed on Node.js (with changes to
generate JIT_CODE_MOVE records) and on Java.

This issue started to occur after 6a9405b56c274 (perf map:
Optimize maps__fixup_overlappings()). To prevent it from happening,
partially revert those changes without losing the optimizations
introduced in it.

Signed-off-by: Matheus Marchini <mmarchini@netflix.com>
---
 tools/perf/util/map.c | 17 +++++++++++++++++
 tools/perf/util/map.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 744bfbaf35cf..8918fdb8ddab 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -781,6 +781,21 @@ static void __map_groups__insert(struct map_groups *mg, struct map *map)
 	__maps__insert(&mg->maps, map);
 }
 
+int map__overlap(struct map *l, struct map *r)
+{
+	if (l->start > r->start) {
+		struct map *t = l;
+
+		l = r;
+		r = t;
+	}
+
+	if (l->end > r->start)
+		return 1;
+
+	return 0;
+}
+
 int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, FILE *fp)
 {
 	struct maps *maps = &mg->maps;
@@ -821,6 +836,8 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, FILE
 		 */
 		if (pos->start >= map->end)
 			break;
+		if (!map__overlap(map, pos))
+			continue;
 
 		if (verbose >= 2) {
 
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 5e8899883231..1383571437aa 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -132,6 +132,7 @@ static inline void __map__zput(struct map **map)
 
 #define map__zput(map) __map__zput(&map)
 
+int map__overlap(struct map *l, struct map *r);
 size_t map__fprintf(struct map *map, FILE *fp);
 size_t map__fprintf_dsoname(struct map *map, FILE *fp);
 char *map__srcline(struct map *map, u64 addr, struct symbol *sym);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] perf map: fix infinite loop on map_groups__fixup_overlappings
  2019-12-11 16:07 [PATCH] perf map: fix infinite loop on map_groups__fixup_overlappings Matheus Marchini
@ 2019-12-11 18:13 ` Arnaldo Carvalho de Melo
  2019-12-14 11:16 ` Konstantin Khlebnikov
  1 sibling, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-12-11 18:13 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: Matheus Marchini, linux-perf-users, jkoch, khlebnikov,
	Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Changbin Du, Song Liu, John Keeping,
	Andi Kleen, linux-kernel

Em Wed, Dec 11, 2019 at 08:07:31AM -0800, Matheus Marchini escreveu:
> In some cases, when using perf inject and there are JIT_CODE_MOVE
> records in the jitdump file, perf will end up in an infinite loop on
> map_groups__fixup_overlappings, which will keep allocating memory
> indefinitely. This issue was observed on Node.js (with changes to
> generate JIT_CODE_MOVE records) and on Java.
> 
> This issue started to occur after 6a9405b56c274 (perf map:
> Optimize maps__fixup_overlappings()). To prevent it from happening,
> partially revert those changes without losing the optimizations
> introduced in it.

Konstantin, can you please take a look and provide your Acked-by or
Reviewed-by?

- Arnaldo
 
> Signed-off-by: Matheus Marchini <mmarchini@netflix.com>
> ---
>  tools/perf/util/map.c | 17 +++++++++++++++++
>  tools/perf/util/map.h |  1 +
>  2 files changed, 18 insertions(+)
> 
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 744bfbaf35cf..8918fdb8ddab 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -781,6 +781,21 @@ static void __map_groups__insert(struct map_groups *mg, struct map *map)
>  	__maps__insert(&mg->maps, map);
>  }
>  
> +int map__overlap(struct map *l, struct map *r)
> +{
> +	if (l->start > r->start) {
> +		struct map *t = l;
> +
> +		l = r;
> +		r = t;
> +	}
> +
> +	if (l->end > r->start)
> +		return 1;
> +
> +	return 0;
> +}
> +
>  int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, FILE *fp)
>  {
>  	struct maps *maps = &mg->maps;
> @@ -821,6 +836,8 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, FILE
>  		 */
>  		if (pos->start >= map->end)
>  			break;
> +		if (!map__overlap(map, pos))
> +			continue;
>  
>  		if (verbose >= 2) {
>  
> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> index 5e8899883231..1383571437aa 100644
> --- a/tools/perf/util/map.h
> +++ b/tools/perf/util/map.h
> @@ -132,6 +132,7 @@ static inline void __map__zput(struct map **map)
>  
>  #define map__zput(map) __map__zput(&map)
>  
> +int map__overlap(struct map *l, struct map *r);
>  size_t map__fprintf(struct map *map, FILE *fp);
>  size_t map__fprintf_dsoname(struct map *map, FILE *fp);
>  char *map__srcline(struct map *map, u64 addr, struct symbol *sym);
> -- 
> 2.17.1

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] perf map: fix infinite loop on map_groups__fixup_overlappings
  2019-12-11 16:07 [PATCH] perf map: fix infinite loop on map_groups__fixup_overlappings Matheus Marchini
  2019-12-11 18:13 ` Arnaldo Carvalho de Melo
@ 2019-12-14 11:16 ` Konstantin Khlebnikov
  1 sibling, 0 replies; 3+ messages in thread
From: Konstantin Khlebnikov @ 2019-12-14 11:16 UTC (permalink / raw)
  To: Matheus Marchini
  Cc: linux-perf-users, jkoch,
	Константин
	Геннадьевич
	Хлебников,
	Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Changbin Du, Song Liu, John Keeping, Andi Kleen,
	Linux Kernel Mailing List

On Wed, Dec 11, 2019 at 7:10 PM Matheus Marchini <mmarchini@netflix.com> wrote:
>t
> In some cases, when using perf inject and there are JIT_CODE_MOVE
> records in the jitdump file, perf will end up in an infinite loop on
> map_groups__fixup_overlappings, which will keep allocating memory
> indefinitely. This issue was observed on Node.js (with changes to
> generate JIT_CODE_MOVE records) and on Java.

Could you show what it prints with -vv ?
I suppose map tree could be broken, like there is zero (or negative) size maps.

This should help to catch that

--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -848,13 +848,18 @@ static void __maps__insert(struct maps *maps,
struct map *map)
        const u64 ip = map->start;
        struct map *m;

+       assert((map->start < map->end) || !map->end);
+
        while (*p != NULL) {
                parent = *p;
                m = rb_entry(parent, struct map, rb_node);
-               if (ip < m->start)
+               if (ip < m->start) {
+                       assert(map->end <= m->start);
                        p = &(*p)->rb_left;
-               else
+               } else {
+                       assert(m->end <= map->start);
                        p = &(*p)->rb_right;
+               }
        }

        rb_link_node(&map->rb_node, parent, p);


>
> This issue started to occur after 6a9405b56c274 (perf map:
> Optimize maps__fixup_overlappings()). To prevent it from happening,
> partially revert those changes without losing the optimizations
> introduced in it.
>
> Signed-off-by: Matheus Marchini <mmarchini@netflix.com>
> ---
>  tools/perf/util/map.c | 17 +++++++++++++++++
>  tools/perf/util/map.h |  1 +
>  2 files changed, 18 insertions(+)
>
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index 744bfbaf35cf..8918fdb8ddab 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -781,6 +781,21 @@ static void __map_groups__insert(struct map_groups *mg, struct map *map)
>         __maps__insert(&mg->maps, map);
>  }
>
> +int map__overlap(struct map *l, struct map *r)
> +{
> +       if (l->start > r->start) {
> +               struct map *t = l;
> +
> +               l = r;
> +               r = t;
> +       }
> +
> +       if (l->end > r->start)
> +               return 1;
> +
> +       return 0;
> +}
> +
>  int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, FILE *fp)
>  {
>         struct maps *maps = &mg->maps;
> @@ -821,6 +836,8 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, FILE
>                  */
>                 if (pos->start >= map->end)
>                         break;
> +               if (!map__overlap(map, pos))
> +                       continue;
>
>                 if (verbose >= 2) {
>
> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> index 5e8899883231..1383571437aa 100644
> --- a/tools/perf/util/map.h
> +++ b/tools/perf/util/map.h
> @@ -132,6 +132,7 @@ static inline void __map__zput(struct map **map)
>
>  #define map__zput(map) __map__zput(&map)
>
> +int map__overlap(struct map *l, struct map *r);
>  size_t map__fprintf(struct map *map, FILE *fp);
>  size_t map__fprintf_dsoname(struct map *map, FILE *fp);
>  char *map__srcline(struct map *map, u64 addr, struct symbol *sym);
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-12-14 11:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-11 16:07 [PATCH] perf map: fix infinite loop on map_groups__fixup_overlappings Matheus Marchini
2019-12-11 18:13 ` Arnaldo Carvalho de Melo
2019-12-14 11:16 ` Konstantin Khlebnikov

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).