Messages in this thread | | | Date | Fri, 10 Oct 2008 03:23:00 -0400 | From | Mathieu Desnoyers <> | Subject | Re: [PATCH] Markers : fix check format with rcu callback race |
| |
* Lai Jiangshan (laijs@cn.fujitsu.com) wrote: > Mathieu Desnoyers wrote: > > The fix "markers: fix unchecked format" introduced an RCU callback race. This > > patch takes care of calling any pending RCU callback before set_format is > > called. > > > > marker_set_format() has this statement: > > if ((*entry)->rcu_pending) > rcu_barrier_sched(); >
True, therefore my fix is not needed.
Mathieu
> > > > * Lai Jiangshan (laijs@cn.fujitsu.com) wrote: > >> bit-field is not thread-safe nor smp-safe. > >> > >> struct marker_entry.rcu_pending is not protected by any lock > >> in rcu-callback free_old_closure(). > >> so we must turn it into a safe type. > >> > > > > All struct marker_entry.rcu_pending accesses are done with the > > markers_mutex held, except the one done in free_old_closure(). Normally, > > there should be a > > if (entry->rcu_pending) > > rcu_barrier_sched(); > > > > At the beginning of each markers_mutex section (just after get_marker()) > > to make sure any pending callback is executed at that point before any > > of rcu_pending or ptype are touched. > > > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> > > CC: Ingo Molnar <mingo@elte.hu> > > CC: Lai Jiangshan <laijs@cn.fujitsu.com> > > --- > > kernel/marker.c | 24 +++++++++++++----------- > > 1 file changed, 13 insertions(+), 11 deletions(-) > > > > Index: linux-2.6-lttng/kernel/marker.c > > =================================================================== > > --- linux-2.6-lttng.orig/kernel/marker.c 2008-10-10 01:35:32.000000000 -0400 > > +++ linux-2.6-lttng/kernel/marker.c 2008-10-10 01:35:32.000000000 -0400 > > @@ -657,21 +657,23 @@ int marker_probe_register(const char *na > > entry = add_marker(name, format); > > if (IS_ERR(entry)) > > ret = PTR_ERR(entry); > > - } else if (format) { > > - if (!entry->format) > > - ret = marker_set_format(&entry, format); > > - else if (strcmp(entry->format, format)) > > - ret = -EPERM; > > + } else { > > + /* > > + * If we detect that a call_rcu is pending for this marker, > > + * make sure it's executed now. > > + */ > > + if (entry->rcu_pending) > > + rcu_barrier_sched(); > > + if (format) { > > + if (!entry->format) > > + ret = marker_set_format(&entry, format); > > + else if (strcmp(entry->format, format)) > > + ret = -EPERM; > > + } > > } > > if (ret) > > goto end; > > > > - /* > > - * If we detect that a call_rcu is pending for this marker, > > - * make sure it's executed now. > > - */ > > - if (entry->rcu_pending) > > - rcu_barrier_sched(); > > old = marker_entry_add_probe(entry, probe, probe_private); > > if (IS_ERR(old)) { > > ret = PTR_ERR(old); > >
-- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
| |