Messages in this thread | | | Date | Wed, 23 Nov 2011 08:06:53 -0800 | From | Tejun Heo <> | Subject | Re: [PATCH 1/2] trace_events_filter: use rcu_assign_pointer() when setting ftrace_event_call->filter |
| |
On Wed, Nov 23, 2011 at 10:16:47AM -0500, Steven Rostedt wrote: > [ Added Paul to Cc ] > > On Tue, 2011-11-22 at 17:46 -0800, Tejun Heo wrote: > > ftrace_event_call->filter is sched RCU protected but didn't use > > rcu_assign_pointer(). Fix it. > > Is it really needed? Maybe just for documentation but I'm not sure this > use is required because all use cases have synchronize_sched() used, > which is a big hammer compared to the rcu_assign_pointer().
Oh yeah, I think we do. synchronize_sched() is to drain users of the old pointer. Whether synchronize_sched() or call_rcu() is used is irrelevant to the synchronization of new pointer.
rcu_assign_pointer() is to synchronize against rcu_dereference() dereferencing the new one. More specifically, we need it for the writer memory barrier, so that it matches the data dependency read barrier in rcu_dereference() when it access the new pointer; otherwise, it may fetch the old values from before the new area is initialized on archs where data dependency barrier isn't noop.
> We update filter here, and then call synchronize_sched() before we free > the filter_item->filter. ... > Again you can see that synchronize_sched() is called here.
So, synchronized_sched() being called after isn't relevant. We want smp_wmb() between data structure initialization and assignment of the new pointer.
Thank you.
-- tejun
| |