lkml.org 
[lkml]   [2004]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: contention on profile_lock
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:07    [W:0.068 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site