perf: Fix various cpu hotplug oddities

PATCH New Thomas Gleixner 2 participants 9 messages Feb 9, 2016 – Feb 17, 2016 perf events

Share & tools

Thomas Gleixner

author

2016-02-09 20:12 UTC #0 lore raw

cover perf: Fix various cpu hotplug oddities

The following series fixes a few oddities in the core perf code related to cpu hotplug namely memory leaks and pointless smp function calls to offline cpus.

Thanks,

tglx

Thomas Gleixner

author

2016-02-09 20:12 UTC #1 lore raw

PATCH 1/4 perf: Remove bogus UP_CANCELED hotplug state

If CPU_UP_PREPARE fails the perf hotplug code calls perf_event_exit_cpu(), which is a pointless exercise. The cpu is not online, so the smp function calls return -ENXIO. So the result is a list walk to call noops.

Remove it.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/events/core.c |    1 -
 1 file changed, 1 deletion(-)

kernel/events/core.c +0 -1

@@ -9286,7 +9286,6 @@ perf_cpu_notify(struct notifier_block *s
 		perf_event_init_cpu(cpu);
 		break;
 
-	case CPU_UP_CANCELED:
 	case CPU_DOWN_PREPARE:
 		perf_event_exit_cpu(cpu);
 		break;

tip-bot for Thomas Gleixner

merge bot

2016-02-17 12:27 UTC #2 lore raw

[tip:perf/core] perf/core: Remove bogus UP_CANCELED hotplug state

perf/core: Remove bogus UP_CANCELED hotplug state

Commit-ID:  b4f75d44bed1bdbb14ac704bfc38f62a3675e591
Gitweb:     http://git.kernel.org/tip/b4f75d44bed1bdbb14ac704bfc38f62a3675e591
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Tue, 9 Feb 2016 20:11:20 +0000
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 17 Feb 2016 10:37:28 +0100

perf/core: Remove bogus UP_CANCELED hotplug state

Remove it.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Vince Weaver <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/core.c | 1 -
 1 file changed, 1 deletion(-)

kernel/events/core.c +0 -1

index 5946460..474ffea 100644
@@ -9286,7 +9286,6 @@ perf_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu)
 		perf_event_init_cpu(cpu);
 		break;
 
-	case CPU_UP_CANCELED:
 	case CPU_DOWN_PREPARE:
 		perf_event_exit_cpu(cpu);
 		break;

Thomas Gleixner

author

2016-02-09 20:12 UTC #3 lore raw

PATCH 2/4 perf: Remove the bogus and dangerous CPU_DOWN_FAILED hotplug state

In reply to Thomas Gleixner

If CPU_DOWN_PREPARE fails the perf hotplug notifier is called for CPU_DOWN_FAILED and calls perf_event_init_cpu(), which checks whether the swhash is referenced. If yes it allocates a new hash and stores the pointer in the per cpu data structure.

But at this point the cpu is still online, so there must be a valid hash already. By overwriting the pointer the existing hash is not longer accessible.

Remove the CPU_DOWN_FAILED state, as there is nothing to (re)allocate.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/events/core.c |    1 -
 1 file changed, 1 deletion(-)

kernel/events/core.c +0 -1

@@ -9282,7 +9282,6 @@ perf_cpu_notify(struct notifier_block *s
 	switch (action & ~CPU_TASKS_FROZEN) {
 
 	case CPU_UP_PREPARE:
-	case CPU_DOWN_FAILED:
 		perf_event_init_cpu(cpu);
 		break;

tip-bot for Thomas Gleixner

merge bot

2016-02-17 12:27 UTC #4 lore raw

[tip:perf/core] perf/core: Remove the bogus and dangerous CPU_DOWN_FAILED hotplug state

perf/core: Remove the bogus and dangerous CPU_DOWN_FAILED hotplug state

Commit-ID:  27ca9236c96f4a21b72a2b4f08260efeab951bd0
Gitweb:     http://git.kernel.org/tip/27ca9236c96f4a21b72a2b4f08260efeab951bd0
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Tue, 9 Feb 2016 20:11:26 +0000
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 17 Feb 2016 10:37:29 +0100

perf/core: Remove the bogus and dangerous CPU_DOWN_FAILED hotplug state

But at this point the cpu is still online, so there must be a valid hash already. By overwriting the pointer the existing hash is not longer accessible.

Remove the CPU_DOWN_FAILED state, as there is nothing to (re)allocate.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Vince Weaver <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/core.c | 1 -
 1 file changed, 1 deletion(-)

kernel/events/core.c +0 -1

index 474ffea..4aa64a8 100644
@@ -9282,7 +9282,6 @@ perf_cpu_notify(struct notifier_block *self, unsigned long action, void *hcpu)
 	switch (action & ~CPU_TASKS_FROZEN) {
 
 	case CPU_UP_PREPARE:
-	case CPU_DOWN_FAILED:
 		perf_event_init_cpu(cpu);
 		break;

Thomas Gleixner

author

2016-02-09 20:12 UTC #5 lore raw

PATCH 3/4 perf: Plug potential memory leak in CPU_UP_PREPARE

In reply to Thomas Gleixner

If CPU_UP_PREPARE is called it is not guaranteed, that a previously allocated and assigned hash has been freed already, but perf_event_init_cpu() unconditionally allocates and assignes a new hash if the swhash is referenced. By overwriting the pointer the existing hash is not longer accessible.

Verify that there is no hash assigned on this cpu before allocating and assigning a new one.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/events/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/events/core.c +1 -1

@@ -9206,7 +9206,7 @@ static void perf_event_init_cpu(int cpu)
 	struct swevent_htable *swhash = &per_cpu(swevent_htable, cpu);
 
 	mutex_lock(&swhash->hlist_mutex);
-	if (swhash->hlist_refcount > 0) {
+	if (swhash->hlist_refcount > 0 && !swevent_hlist_deref(swhash)) {
 		struct swevent_hlist *hlist;
 
 		hlist = kzalloc_node(sizeof(*hlist), GFP_KERNEL, cpu_to_node(cpu));

tip-bot for Thomas Gleixner

merge bot

2016-02-17 12:28 UTC #6 lore raw

[tip:perf/core] perf/core: Plug potential memory leak in CPU_UP_PREPARE

perf/core: Plug potential memory leak in CPU_UP_PREPARE

Commit-ID:  059fcd8cd16622da6513804a7a3e826d152c6c96
Gitweb:     http://git.kernel.org/tip/059fcd8cd16622da6513804a7a3e826d152c6c96
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Tue, 9 Feb 2016 20:11:34 +0000
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 17 Feb 2016 10:37:30 +0100

perf/core: Plug potential memory leak in CPU_UP_PREPARE

Verify that there is no hash assigned on this cpu before allocating and assigning a new one.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Vince Weaver <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/events/core.c +1 -1

index 4aa64a8..0d58522 100644
@@ -9206,7 +9206,7 @@ static void perf_event_init_cpu(int cpu)
 	struct swevent_htable *swhash = &per_cpu(swevent_htable, cpu);
 
 	mutex_lock(&swhash->hlist_mutex);
-	if (swhash->hlist_refcount > 0) {
+	if (swhash->hlist_refcount > 0 && !swevent_hlist_deref(swhash)) {
 		struct swevent_hlist *hlist;
 
 		hlist = kzalloc_node(sizeof(*hlist), GFP_KERNEL, cpu_to_node(cpu));

Thomas Gleixner

author

2016-02-09 20:12 UTC #7 lore raw

PATCH 4/4 perf: Remove unused arguments from a bunch of functions

In reply to Thomas Gleixner

No functional change, just less confusing to read.

Signed-off-by Thomas Gleixner <[email protected]>

---
 kernel/events/core.c |   21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

kernel/events/core.c +10 ~~-11~~

@@ -6710,7 +6710,7 @@ static void swevent_hlist_release(struct
 	kfree_rcu(hlist, rcu_head);
 }
 
-static void swevent_hlist_put_cpu(struct perf_event *event, int cpu)
+static void swevent_hlist_put_cpu(int cpu)
 {
 	struct swevent_htable *swhash = &per_cpu(swevent_htable, cpu);

@@ -6722,15 +6722,15 @@ static void swevent_hlist_put_cpu(struct
 	mutex_unlock(&swhash->hlist_mutex);
 }
 
-static void swevent_hlist_put(struct perf_event *event)
+static void swevent_hlist_put(void)
 {
 	int cpu;
 
 	for_each_possible_cpu(cpu)
-		swevent_hlist_put_cpu(event, cpu);
+		swevent_hlist_put_cpu(cpu);
 }
 
-static int swevent_hlist_get_cpu(struct perf_event *event, int cpu)
+static int swevent_hlist_get_cpu(int cpu)
 {
 	struct swevent_htable *swhash = &per_cpu(swevent_htable, cpu);
 	int err = 0;

@@ -6753,14 +6753,13 @@ static int swevent_hlist_get_cpu(struct
 	return err;
 }
 
-static int swevent_hlist_get(struct perf_event *event)
+static int swevent_hlist_get(void)
 {
-	int err;
-	int cpu, failed_cpu;
+	int err, cpu, failed_cpu;
 
 	get_online_cpus();
 	for_each_possible_cpu(cpu) {
-		err = swevent_hlist_get_cpu(event, cpu);
+		err = swevent_hlist_get_cpu(cpu);
 		if (err) {
 			failed_cpu = cpu;
 			goto fail;

@@ -6773,7 +6772,7 @@ static int swevent_hlist_get(struct perf
 	for_each_possible_cpu(cpu) {
 		if (cpu == failed_cpu)
 			break;
-		swevent_hlist_put_cpu(event, cpu);
+		swevent_hlist_put_cpu(cpu);
 	}
 
 	put_online_cpus();

@@ -6789,7 +6788,7 @@ static void sw_perf_event_destroy(struct
 	WARN_ON(event->parent);
 
 	static_key_slow_dec(&perf_swevent_enabled[event_id]);
-	swevent_hlist_put(event);
+	swevent_hlist_put();
 }
 
 static int perf_swevent_init(struct perf_event *event)

@@ -6820,7 +6819,7 @@ static int perf_swevent_init(struct perf
 	if (!event->parent) {
 		int err;
 
-		err = swevent_hlist_get(event);
+		err = swevent_hlist_get();
 		if (err)
 			return err;

tip-bot for Thomas Gleixner

merge bot

2016-02-17 12:27 UTC #8 lore raw

[tip:perf/core] perf/core: Remove unused arguments from a bunch of functions

perf/core: Remove unused arguments from a bunch of functions

Commit-ID:  3b364d7b587db0f0eeafde0f271e0698187de776
Gitweb:     http://git.kernel.org/tip/3b364d7b587db0f0eeafde0f271e0698187de776
Author:     Thomas Gleixner <[email protected]>
AuthorDate: Tue, 9 Feb 2016 20:11:40 +0000
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 17 Feb 2016 10:37:48 +0100

perf/core: Remove unused arguments from a bunch of functions

No functional change, just less confusing to read.

Signed-off-by Thomas Gleixner <[email protected]>

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Vince Weaver <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/events/core.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

kernel/events/core.c +10 ~~-11~~

index 0d58522..94c47e3 100644
@@ -6710,7 +6710,7 @@ static void swevent_hlist_release(struct swevent_htable *swhash)
 	kfree_rcu(hlist, rcu_head);
 }
 
-static void swevent_hlist_put_cpu(struct perf_event *event, int cpu)
+static void swevent_hlist_put_cpu(int cpu)
 {
 	struct swevent_htable *swhash = &per_cpu(swevent_htable, cpu);

@@ -6722,15 +6722,15 @@ static void swevent_hlist_put_cpu(struct perf_event *event, int cpu)
 	mutex_unlock(&swhash->hlist_mutex);
 }
 
-static void swevent_hlist_put(struct perf_event *event)
+static void swevent_hlist_put(void)
 {
 	int cpu;
 
 	for_each_possible_cpu(cpu)
-		swevent_hlist_put_cpu(event, cpu);
+		swevent_hlist_put_cpu(cpu);
 }
 
-static int swevent_hlist_get_cpu(struct perf_event *event, int cpu)
+static int swevent_hlist_get_cpu(int cpu)
 {
 	struct swevent_htable *swhash = &per_cpu(swevent_htable, cpu);
 	int err = 0;

@@ -6753,14 +6753,13 @@ exit:
 	return err;
 }
 
-static int swevent_hlist_get(struct perf_event *event)
+static int swevent_hlist_get(void)
 {
-	int err;
-	int cpu, failed_cpu;
+	int err, cpu, failed_cpu;
 
 	get_online_cpus();
 	for_each_possible_cpu(cpu) {
-		err = swevent_hlist_get_cpu(event, cpu);
+		err = swevent_hlist_get_cpu(cpu);
 		if (err) {
 			failed_cpu = cpu;
 			goto fail;

@@ -6773,7 +6772,7 @@ fail:
 	for_each_possible_cpu(cpu) {
 		if (cpu == failed_cpu)
 			break;
-		swevent_hlist_put_cpu(event, cpu);
+		swevent_hlist_put_cpu(cpu);
 	}
 
 	put_online_cpus();

@@ -6789,7 +6788,7 @@ static void sw_perf_event_destroy(struct perf_event *event)
 	WARN_ON(event->parent);
 
 	static_key_slow_dec(&perf_swevent_enabled[event_id]);
-	swevent_hlist_put(event);
+	swevent_hlist_put();
 }
 
 static int perf_swevent_init(struct perf_event *event)

@@ -6820,7 +6819,7 @@ static int perf_swevent_init(struct perf_event *event)
 	if (!event->parent) {
 		int err;
 
-		err = swevent_hlist_get(event);
+		err = swevent_hlist_get();
 		if (err)
 			return err;