Messages in this thread | | | Date | Fri, 5 Nov 2004 03:46:55 +0530 | From | Dipankar Sarma <> | Subject | Re: 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/
| |