Re: contention on profile_lock

[Dipankar Sarma <dipankar@in.ibm.com>]

On Thu, Nov 04, 2004 at 01:55:27PM -0800, Jesse Barnes wrote:
> On Thursday, November 4, 2004 12:49 pm, Jesse Barnes wrote:
> > John pointed out that this breaks modules.  Would registering and
>  
>  static void timer_stop(void)
>  {
> -	unregister_profile_notifier(&timer_notifier);
> +	/* Tear down the callback pointer after sync_kernel */
> +	synchronize_kernel();
> +	oprofile_timer_notify = NULL;
>  }
>  
>  
> +/* Oprofile timer tick hook */
> +int (*oprofile_timer_notify)(struct pt_regs *);
> +
>  static atomic_t *prof_buffer;
>  static unsigned long prof_len, prof_shift;
>  static int prof_on;
> @@ -168,38 +172,6 @@
>  	return err;
>  }
>  


> -void profile_hook(struct pt_regs * regs)
> -{
> -	read_lock(&profile_lock);
> -	notifier_call_chain(&profile_listeners, 0, regs);
> -	read_unlock(&profile_lock);
> -}
> -
> -EXPORT_SYMBOL_GPL(register_profile_notifier);
> -EXPORT_SYMBOL_GPL(unregister_profile_notifier);
>  EXPORT_SYMBOL_GPL(task_handoff_register);
>  EXPORT_SYMBOL_GPL(task_handoff_unregister);
>  
> @@ -394,8 +366,8 @@
>  
>  void profile_tick(int type, struct pt_regs *regs)
>  {
> -	if (type == CPU_PROFILING)
> -		profile_hook(regs);
> +	if (type == CPU_PROFILING && oprofile_timer_notify)
> +		oprofile_timer_notify(regs);
>  	if (!user_mode(regs) && cpu_isset(smp_processor_id(), prof_cpu_mask))
>  		profile_hit(type, (void *)profile_pc(regs));
>  }


Or you could just do -

void profile_tick(int type, struct pt_regs *regs)
{
	int (*timer_notify)(struct pt_regs *);

	timer_notify = oprofile_timer_notify;
	smp_read_barrier_depends();
	if (type == CPU_PROFILING && timer_notify) {
		timer_notify(regs);
	}
	if (!user_mode(regs) && cpu_isset(smp_processor_id(), prof_cpu_mask))
		profile_hit(type, (void *)profile_pc(regs));
}

And avoid the synchronize_kernel(). But I think the synchronize_kernel()
thing is cleaner.

I am looking at the notifier chain mechanism itself and its
lack of proper locking. That is a different story.

Thanks
Dipankar