Re: [Adeos-main] [PATCH 1/4] tracer: cleanup critical paths

[Jan Kiszka <jan.kiszka@domain.hid>]

Philippe Gerum wrote:
> On Thu, 2007-10-11 at 15:06 +0200, Jan Kiszka wrote:
>> 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:
> 
> diff --git a/kernel/ipipe/tracer.c b/kernel/ipipe/tracer.c
> index 2326706..8ecbe1a 100644
> --- a/kernel/ipipe/tracer.c
> +++ b/kernel/ipipe/tracer.c
> @@ -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) {

Makes sense.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux