Re: [RFC PATCH][3.0] Tracepoint: dissociate from module mutex

Mathieu Desnoyers
2011-08-10T19:16:59Z

* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Wed, 2011-08-10 at 13:41 -0400, Mathieu Desnoyers wrote:
[...]
> > void tracepoint_update_probe_range(struct tracepoint * const *begin,
> > struct tracepoint * const *end)
>
> Can we make this static?

will do, thanks!

Mathieu

>
> Otherwise, looks good.
>
> -- Steve
>
> > @@ -302,7 +308,6 @@ void tracepoint_update_probe_range(struc
> > if (!begin)
> > return;
> >
> > - mutex_lock(&tracepoints_mutex);
> > for (iter = begin; iter < end; iter++) {
> > mark_entry = get_tracepoint((*iter)->name);
> > if (mark_entry) {
> > @@ -312,11 +317,27 @@ void tracepoint_update_probe_range(struc
> > disable_tracepoint(*iter);
> > }
> > }
> > - mutex_unlock(&tracepoints_mutex);
> > }
> >
> > +#ifdef CONFIG_MODULES
> > +void module_update_tracepoints(void)
> > +{
> > + struct tp_module *tp_mod;
> > +
> > + list_for_each_entry(tp_mod, &tracepoint_module_list, list)
> > + tracepoint_update_probe_range(tp_mod->tracepoints_ptrs,
> > + tp_mod->tracepoints_ptrs + tp_mod->num_tracepoints);
> > +}
> > +#else /* CONFIG_MODULES */
> > +void module_update_tracepoints(void)
> > +{
> > +}
> > +#endif /* CONFIG_MODULES */
> > +
> > +
> > /*
> > * Update probes, removing the faulty probes.
> > + * Called with tracepoints_mutex held.
> > */
> > static void tracepoint_update_probes(void)
> > {
> > @@ -359,11 +380,12 @@ int tracepoint_probe_register(const char
> >
> > mutex_lock(&tracepoints_mutex);
> > old = tracepoint_add_probe(name, probe, data);
> > - mutex_unlock(&tracepoints_mutex);
> > - if (IS_ERR(old))
> > + if (IS_ERR(old)) {
> > + mutex_unlock(&tracepoints_mutex);
> > return PTR_ERR(old);
> > -
> > + }
> > tracepoint_update_probes(); /* may update entry */
> > + mutex_unlock(&tracepoints_mutex);
> > release_probes(old);
> > return 0;
> > }
> > @@ -402,11 +424,12 @@ int tracepoint_probe_unregister(const ch
> >
> > mutex_lock(&tracepoints_mutex);
> > old = tracepoint_remove_probe(name, probe, data);
> > - mutex_unlock(&tracepoints_mutex);
> > - if (IS_ERR(old))
> > + if (IS_ERR(old)) {
> > + mutex_unlock(&tracepoints_mutex);
> > return PTR_ERR(old);
> > -
> > + }
> > tracepoint_update_probes(); /* may update entry */
> > + mutex_unlock(&tracepoints_mutex);
> > release_probes(old);
> > return 0;
> > }
> > @@ -489,9 +512,8 @@ void tracepoint_probe_update_all(void)
> > if (!list_empty(&old_probes))
> > list_replace_init(&old_probes, &release_probes);
> > need_update = 0;
> > - mutex_unlock(&tracepoints_mutex);
> > -
> > tracepoint_update_probes();
> > + mutex_unlock(&tracepoints_mutex);
> > list_for_each_entry_safe(pos, next, &release_probes, u.list) {
> > list_del(&pos->u.list);
> > call_rcu_sched(&pos->u.rcu, rcu_free_old_probes);
> > @@ -509,7 +531,7 @@ EXPORT_SYMBOL_GPL(tracepoint_probe_updat
> > * Will return the first tracepoint in the range if the input tracepoint is
> > * NULL.
> > */
> > -int tracepoint_get_iter_range(struct tracepoint * const **tracepoint,
> > +static int tracepoint_get_iter_range(struct tracepoint * const **tracepoint,
> > struct tracepoint * const *begin, struct tracepoint * const *end)
> > {
> > if (!*tracepoint && begin != end) {
> > @@ -520,11 +542,12 @@ int tracepoint_get_iter_range(struct tra
> > return 1;
> > return 0;
> > }
> > -EXPORT_SYMBOL_GPL(tracepoint_get_iter_range);
> >
> > +#ifdef CONFIG_MODULES
> > static void tracepoint_get_iter(struct tracepoint_iter *iter)
> > {
> > int found = 0;
> > + struct tp_module *iter_mod;
> >
> > /* Core kernel tracepoints */
> > if (!iter->module) {
> > @@ -534,12 +557,43 @@ static void tracepoint_get_iter(struct t
> > if (found)
> > goto end;
> > }
> > - /* tracepoints in modules. */
> > - found = module_get_iter_tracepoints(iter);
> > + /* Tracepoints in modules */
> > + mutex_lock(&tracepoints_mutex);
> > + list_for_each_entry(iter_mod, &tracepoint_module_list, list) {
> > + /*
> > + * Sorted module list
> > + */
> > + if (iter_mod < iter->module)
> > + continue;
> > + else if (iter_mod > iter->module)
> > + iter->tracepoint = NULL;
> > + found = tracepoint_get_iter_range(&iter->tracepoint,
> > + iter_mod->tracepoints_ptrs,
> > + iter_mod->tracepoints_ptrs
> > + + iter_mod->num_tracepoints);
> > + if (found) {
> > + iter->module = iter_mod;
> > + break;
> > + }
> > + }
> > + mutex_unlock(&tracepoints_mutex);
> > end:
> > if (!found)
> > tracepoint_iter_reset(iter);
> > }
> > +#else /* CONFIG_MODULES */
> > +static void tracepoint_get_iter(struct tracepoint_iter *iter)
> > +{
> > + int found = 0;
> > +
> > + /* Core kernel tracepoints */
> > + found = tracepoint_get_iter_range(&iter->tracepoint,
> > + __start___tracepoints_ptrs,
> > + __stop___tracepoints_ptrs);
> > + if (!found)
> > + tracepoint_iter_reset(iter);
> > +}
> > +#endif /* CONFIG_MODULES */
> >
> > void tracepoint_iter_start(struct tracepoint_iter *iter)
> > {
> > @@ -566,26 +620,98 @@ EXPORT_SYMBOL_GPL(tracepoint_iter_stop);
> >
> > void tracepoint_iter_reset(struct tracepoint_iter *iter)
> > {
> > +#ifdef CONFIG_MODULES
> > iter->module = NULL;
> > +#endif /* CONFIG_MODULES */
> > iter->tracepoint = NULL;
> > }
> > EXPORT_SYMBOL_GPL(tracepoint_iter_reset);
> >
> > #ifdef CONFIG_MODULES
> > +static int tracepoint_module_coming(struct module *mod)
> > +{
> > + struct tp_module *tp_mod, *iter;
> > + int ret = 0;
> > +
> > + /*
> > + * We skip modules that tain the kernel, especially those with different
> > + * module header (for forced load), to make sure we don't cause a crash.
> > + */
> > + if (mod->taints)
> > + return 0;
> > + mutex_lock(&tracepoints_mutex);
> > + tp_mod = kmalloc(sizeof(struct tp_module), GFP_KERNEL);
> > + if (!tp_mod) {
> > + ret = -ENOMEM;
> > + goto end;
> > + }
> > + tp_mod->num_tracepoints = mod->num_tracepoints;
> > + tp_mod->tracepoints_ptrs = mod->tracepoints_ptrs;
> > +
> > + /*
> > + * tracepoint_module_list is kept sorted by struct module pointer
> > + * address for iteration on tracepoints from a seq_file that can release
> > + * the mutex between calls.
> > + */
> > + list_for_each_entry_reverse(iter, &tracepoint_module_list, list) {
> > + BUG_ON(iter == tp_mod); /* Should never be in the list twice */
> > + if (iter < tp_mod) {
> > + /* We belong to the location right after iter. */
> > + list_add(&tp_mod->list, &iter->list);
> > + goto module_added;
> > + }
> > + }
> > + /* We belong to the beginning of the list */
> > + list_add(&tp_mod->list, &tracepoint_module_list);
> > +module_added:
> > + tracepoint_update_probe_range(mod->tracepoints_ptrs,
> > + mod->tracepoints_ptrs + mod->num_tracepoints);
> > +end:
> > + mutex_unlock(&tracepoints_mutex);
> > + return ret;
> > +}
> > +
> > +static int tracepoint_module_going(struct module *mod)
> > +{
> > + struct tp_module *pos;
> > +
> > + mutex_lock(&tracepoints_mutex);
> > + tracepoint_update_probe_range(mod->tracepoints_ptrs,
> > + mod->tracepoints_ptrs + mod->num_tracepoints);
> > + list_for_each_entry(pos, &tracepoint_module_list, list) {
> > + if (pos->tracepoints_ptrs == mod->tracepoints_ptrs) {
> > + list_del(&pos->list);
> > + kfree(pos);
> > + break;
> > + }
> > + }
> > + /*
> > + * In the case of modules that were tainted at "coming", we'll simply
> > + * walk through the list without finding it. We cannot use the "tainted"
> > + * flag on "going", in case a module taints the kernel only after being
> > + * loaded.
> > + */
> > + mutex_unlock(&tracepoints_mutex);
> > + return 0;
> > +}
> >
> > int tracepoint_module_notify(struct notifier_block *self,
> > unsigned long val, void *data)
> > {
> > struct module *mod = data;
> > + int ret = 0;
> >
> > switch (val) {
> > case MODULE_STATE_COMING:
> > + ret = tracepoint_module_coming(mod);
> > + break;
> > + case MODULE_STATE_LIVE:
> > + break;
> > case MODULE_STATE_GOING:
> > - tracepoint_update_probe_range(mod->tracepoints_ptrs,
> > - mod->tracepoints_ptrs + mod->num_tracepoints);
> > + ret = tracepoint_module_going(mod);
> > break;
> > }
> > - return 0;
> > + return ret;
> > }
> >
> > struct notifier_block tracepoint_module_nb = {
> > @@ -598,7 +724,6 @@ static int init_tracepoints(void)
> > return register_module_notifier(&tracepoint_module_nb);
> > }
> > __initcall(init_tracepoints);
> > -
> > #endif /* CONFIG_MODULES */
> >
> > #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
> > Index: linux-2.6-lttng/include/linux/module.h
> > ===================================================================
> > --- linux-2.6-lttng.orig/include/linux/module.h
> > +++ linux-2.6-lttng/include/linux/module.h
> > @@ -578,9 +578,6 @@ int unregister_module_notifier(struct no
> >
> > extern void print_modules(void);
> >
> > -extern void module_update_tracepoints(void);
> > -extern int module_get_iter_tracepoints(struct tracepoint_iter *iter);
> > -
> > #else /* !CONFIG_MODULES... */
> > #define EXPORT_SYMBOL(sym)
> > #define EXPORT_SYMBOL_GPL(sym)
> > @@ -696,15 +693,6 @@ static inline int unregister_module_noti
> > static inline void print_modules(void)
> > {
> > }
> > -
> > -static inline void module_update_tracepoints(void)
> > -{
> > -}
> > -
> > -static inline int module_get_iter_tracepoints(struct tracepoint_iter *iter)
> > -{
> > - return 0;
> > -}
> > #endif /* CONFIG_MODULES */
> >
> > #ifdef CONFIG_SYSFS
> >