Skip to contentSkip to conversation

tracer: cleanup critical paths

PATCH In review Jan Kiszka 2 participants 4 messages Oct 11, 2007 – Oct 15, 2007

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/4 tracer: cleanup critical paths

for_each_online_cpu is not free of function calls on SMP, thus causes false NMI noise in the tracer log (due to tracer recursions). Switch to some plain loop instead.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
---
 kernel/ipipe/tracer.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/ipipe/tracer.c +2 -2
Index: linux-2.6.22.9/kernel/ipipe/tracer.c
===================================================================
@@ -252,7 +252,7 @@ __ipipe_trace_freeze(int cpu_id, struct 
 	active = __ipipe_get_free_trace_path(active, cpu_id);
 
 	/* check if this is the first frozen path */
-	for_each_online_cpu(i) {
+	for (i = 0; i < NR_CPUS; i++) {
 		if ((i != cpu_id) &&
 		    (trace_paths[i][frozen_path[i]].end >= 0))
 			tp->end = -1;

@@ -515,7 +515,7 @@ int ipipe_trace_max_reset(void)
 
 	flags = __ipipe_global_path_lock();
 
-	for_each_online_cpu(cpu_id) {
+	for (cpu_id = 0; cpu_id < NR_CPUS; cpu_id++) {
 		path = &trace_paths[cpu_id][max_path[cpu_id]];
 
 		if (path->dump_lock) {
#1 lore raw
Quoted 3 lines · Jan Kiszka

for_each_online_cpu is not free of function calls on SMP, thus causes false NMI noise in the tracer log (due to tracer recursions). Switch to some plain loop instead.

Series merged, thanks.

Signature · 1 line
Philippe.
#2 lore raw
In reply to Jan Kiszka
Quoted 3 lines · Jan Kiszka

for_each_online_cpu is not free of function calls on SMP, thus causes false NMI noise in the tracer log (due to tracer recursions). Switch to some plain loop instead.

This one breaks badly on SMP, where NR_CPUS is likely greater than the highest numbered CPU from the possible CPU map (at least if you don't lower the former value at config time), so in the TRACE_VMALLOC case, you end up dereferencing NULL trace_paths[] pointers.

Since we may not want to mark __next_cpu() and find_next_bit() as notrace routines, we have to check the boundaries:

kernel/ipipe/tracer.c +2 -2
index 2326706..8ecbe1a 100644
@@ -255,7 +255,7 @@ __ipipe_trace_freeze(int cpu_id, struct ipipe_trace_path *tp, int pos)
 	active = __ipipe_get_free_trace_path(active, cpu_id);
 
 	/* check if this is the first frozen path */
-	for (i = 0; i < NR_CPUS; i++) {
+	for (i = 0; i < NR_CPUS && trace_paths[i] != NULL; i++) {
 		if ((i != cpu_id) &&
 		    (trace_paths[i][frozen_path[i]].end >= 0))
 			tp->end = -1;

@@ -520,7 +520,7 @@ int ipipe_trace_max_reset(void)
 
 	flags = __ipipe_global_path_lock();
 
-	for (cpu_id = 0; cpu_id < NR_CPUS; cpu_id++) {
+	for (cpu_id = 0; cpu_id < NR_CPUS && trace_paths[cpu_id] != NULL; cpu_id++) {
 		path = &trace_paths[cpu_id][max_path[cpu_id]];
 
 		if (path->dump_lock) {
Signature · 1 line
Philippe.
author
#3 lore raw
Quoted 27 lines · Philippe Gerum
Quoted 3 lines · Jan Kiszka

for_each_online_cpu is not free of function calls on SMP, thus causes false NMI noise in the tracer log (due to tracer recursions). Switch to some plain loop instead.

This one breaks badly on SMP, where NR_CPUS is likely greater than the highest numbered CPU from the possible CPU map (at least if you don't lower the former value at config time), so in the TRACE_VMALLOC case, you end up dereferencing NULL trace_paths[] pointers.

Since we may not want to mark __next_cpu() and find_next_bit() as notrace routines, we have to check the boundaries:

kernel/ipipe/tracer.c +1 -1
index 2326706..8ecbe1a 100644
@@ -255,7 +255,7 @@ __ipipe_trace_freeze(int cpu_id, struct ipipe_trace_path *tp, int pos)
 	active = __ipipe_get_free_trace_path(active, cpu_id);
 
 	/* check if this is the first frozen path */
-	for (i = 0; i < NR_CPUS; i++) {
+	for (i = 0; i < NR_CPUS && trace_paths[i] != NULL; i++) {
 		if ((i != cpu_id) &&
 		    (trace_paths[i][frozen_path[i]].end >= 0))
 			tp->end = -1;

kernel/ipipe/tracer.c int ipipe_trace_max_reset(void) · line 526

Quoted 8 lines
kernel/ipipe/tracer.c +1 -1
@@ -520,7 +520,7 @@ int ipipe_trace_max_reset(void)
 
 	flags = __ipipe_global_path_lock();
 
-	for (cpu_id = 0; cpu_id < NR_CPUS; cpu_id++) {
+	for (cpu_id = 0; cpu_id < NR_CPUS && trace_paths[cpu_id] != NULL; cpu_id++) {
 		path = &trace_paths[cpu_id][max_path[cpu_id]];
 
 		if (path->dump_lock) {

Makes sense.

Thanks,
Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux