On Fri, 06 Sep 2013 21:37:58 +0900 Masami Hiramatsu <[email protected]> wrote:
> (2013/09/06 5:46), Steven Rostedt wrote: > > On Mon, 2 Sep 2013 22:52:19 -0500 > > Tom Zanussi <[email protected]> wrote: > >> extern void destroy_preds(struct ftrace_event_call *call); > >> diff --git a/kernel/trace/trace_events_trigger.c > >> b/kernel/trace/trace_events_trigger.c > >> index 85319cf..5388d55 100644 > >> --- a/kernel/trace/trace_events_trigger.c > >> +++ b/kernel/trace/trace_events_trigger.c > >> @@ -28,6 +28,13 @@ > >> static LIST_HEAD(trigger_commands); > >> static DEFINE_MUTEX(trigger_cmd_mutex); > >> > >> +static void > >> +trigger_data_free(struct event_trigger_data *data) > >> +{ > >> + synchronize_sched(); /* make sure current triggers exit before free */ > > > > Again, I think this can and should be synchronize_rcu(). > > > > As in the previous patch, event triggers called under preempt disabled. Yeah, my fault. I was thinking tracepoints used rcu_read_lock(). This is fine. > > > +void event_triggers_call(struct ftrace_event_file *file) > > +{ > > + struct event_trigger_data *data; > > + > > + if (list_empty(&file->triggers)) > > + return; > > + > > + preempt_disable_notrace(); > > + list_for_each_entry_rcu(data, &file->triggers, list) > > + data->ops->func(data); > > + preempt_enable_notrace(); > > +} > > In this case, I think synchronize_sched() is correct. Of course, > we need to discuss why it needs to disable preempt here. :) > Yeah, that is confusing. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
