Messages in this thread | | | Date | Tue, 08 Jul 2008 16:37:45 -0400 | From | Masami Hiramatsu <> | Subject | Re: [RFC patch 01/12] Kernel Tracepoints |
| |
Hi Mathieu,
Masami Hiramatsu wrote: [...] >> +int tracepoint_probe_register(const char *name, void *probe) >> +{ >> + struct tracepoint_entry *entry; >> + int ret = 0; >> + void *old; >> + >> + mutex_lock(&tracepoints_mutex); >> + entry = get_tracepoint(name); >> + if (!entry) { >> + entry = add_tracepoint(name); >> + if (IS_ERR(entry)) { >> + ret = PTR_ERR(entry); >> + goto end; >> + } >> + } >> + /* >> + * If we detect that a call_rcu is pending for this tracepoint, >> + * make sure it's executed now. >> + */ >> + if (entry->rcu_pending) >> + rcu_barrier(); >> + old = tracepoint_entry_add_probe(entry, probe); >> + if (IS_ERR(old)) { >> + ret = PTR_ERR(old); >> + goto end; >> + } >> + mutex_unlock(&tracepoints_mutex); >> + tracepoint_update_probes(); /* may update entry */ >> + mutex_lock(&tracepoints_mutex); >> + entry = get_tracepoint(name); >> + WARN_ON(!entry);
As I said in another patch, you might have to check old != NULL here, because tracepoint_entry_add_probe() will return NULL when you add a first probe to the entry.
>> + entry->oldptr = old; >> + entry->rcu_pending = 1; >> + /* write rcu_pending before calling the RCU callback */ >> + smp_wmb(); >> +#ifdef CONFIG_PREEMPT_RCU >> + synchronize_sched(); /* Until we have the call_rcu_sched() */ >> +#endif >> + call_rcu(&entry->rcu, free_old_closure); >> +end: >> + mutex_unlock(&tracepoints_mutex); >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(tracepoint_probe_register); >> + >> +/** >> + * tracepoint_probe_unregister - Disconnect a probe from a tracepoint >> + * @name: tracepoint name >> + * @probe: probe function pointer >> + * >> + * We do not need to call a synchronize_sched to make sure the probes have >> + * finished running before doing a module unload, because the module unload >> + * itself uses stop_machine(), which insures that every preempt disabled section >> + * have finished. >> + */ >> +int tracepoint_probe_unregister(const char *name, void *probe) >> +{ >> + struct tracepoint_entry *entry; >> + void *old; >> + int ret = -ENOENT; >> + >> + mutex_lock(&tracepoints_mutex); >> + entry = get_tracepoint(name); >> + if (!entry) >> + goto end; >> + if (entry->rcu_pending) >> + rcu_barrier(); >> + old = tracepoint_entry_remove_probe(entry, probe); >> + mutex_unlock(&tracepoints_mutex); >> + tracepoint_update_probes(); /* may update entry */ >> + mutex_lock(&tracepoints_mutex); >> + entry = get_tracepoint(name); >> + if (!entry) >> + goto end; >> + entry->oldptr = old; >> + entry->rcu_pending = 1; >> + /* write rcu_pending before calling the RCU callback */ >> + smp_wmb(); >> +#ifdef CONFIG_PREEMPT_RCU >> + synchronize_sched(); /* Until we have the call_rcu_sched() */ >> +#endif >> + call_rcu(&entry->rcu, free_old_closure); >> + remove_tracepoint(name); /* Ignore busy error message */ >> + ret = 0; >> +end: >> + mutex_unlock(&tracepoints_mutex); >> + return ret; >> +} >> +EXPORT_SYMBOL_GPL(tracepoint_probe_unregister); >> +
On the other hand, tracepoint_entry_remove_probe() doesn't return NULL, however, I think it might be better to introduce tracepoint_entry_free_old() and simplify both of tracepoint_probe_register/unregister.
Thank you, -- Masami Hiramatsu
Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division
e-mail: mhiramat@redhat.com
| |