Re: [PATCH] ftrace memory barriers

[Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>]

(CC'ed LKML)

* Steven Rostedt (rostedt@goodmis.org) wrote:
> 
> On Mon, 14 Jul 2008, Mathieu Desnoyers wrote:
> >
> > Thanks a lot Steve! By the way, you might be interested in looking at
> > the extra tracing enable/disabled tests you have in ftrace : it should
> > be enough to register/unregister the probes to activate/deactivate
> > tracing. I don't see why you would need if() checks in the probe itself.
> >
> 
> That was actually one of the causes of the bugs I had. We had two markers,
> one tracing schedule switches, the other wake ups. As soon as one is
> enabled, tracing starts, and you will miss the tracing of the other one.
> 
> The result was a strange trace were things were waking up then the waker
> was being scheduled in??
> 
> I need to enable all markers at the same time, which can be done with the
> if () check. Not to mention, I like to be able to shut of tracing on oops
> without needing to do anyting drastic (like deactivating probes).
> 
> -- Steve
> 

I see the reason why you need this, but beware that the activation
boolean you are using in your check won't be seen as active at the same
time by all CPUs because of out-of-order read/writes. So while the
update is per se atomic, the replication of the updated value to all
processors isn't. However, when one CPU sees the new version, this
version will stay valid.

This is one of the reasons why I don't aim to provide atomic activation
of multiple markers : thinking that the state will be atomically changed
on all CPUs without using the proper memory barriers is a bit bogus
anyway. Does ftrace expect such atomic propagation of the change across
all CPUs or does it deal with the information it receives on a per-cpu
basis ?

In any case, you should also make sure that you call synchronize_sched()
after the probe registration to make sure the quiescent state is reached
and that all probes are connected (so that no marker still see the old
data structures). This should be done before you activate your boolean
in the probes.

I agree that the extra check is very well suited to the OOPS case.

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68