Re: [patch 01/18] x86, bts: fix race when bts tracer is removed

[Ingo Molnar <mingo@elte.hu>]

* Metzger, Markus T <markus.t.metzger@intel.com> wrote:

> >-----Original Message-----
> >From: Ingo Molnar [mailto:mingo@elte.hu]
> >Sent: Thursday, April 02, 2009 8:45 PM
> >To: Metzger, Markus T
> >Cc: tglx@linutronix.de; hpa@zytor.com; markus.t.metzger@gmail.com; roland@redhat.com;
> >eranian@googlemail.com; oleg@redhat.com; Villacis, Juan; ak@linux.jf.intel.com; linux-
> >kernel@vger.kernel.org
> >Subject: Re: [patch 01/18] x86, bts: fix race when bts tracer is removed
> >
> >
> >* markus.t.metzger@intel.com <markus.t.metzger@intel.com> wrote:
> >
> >> +static inline void ds_take_timestamp(struct ds_context *context,
> >> +				     enum bts_qualifier qualifier,
> >> +				     struct task_struct *task)
> >> +{
> >> +	struct bts_tracer *tracer = context->bts_master;
> >> +	barrier();
> >
> >why the barrier()?
> 
> See http://lkml.org/lkml/2009/3/31/544
> 
> Oleg: "In theory, we need barrier() after reading ->bts_master.
> 
> (actually, I did see the bug reports when the compiler read the pointer
>  twice with the code like above)."

Please convert this piece of non-trivial information into a small 
two-sentence blurb and put it into a comment block.

> >struct bts_struct ts = {
> >	.qualifier		= qualifier,
> >	.variant.event.jiffies	= jiffies_64,
> >	.variant.event.pid	= task->pid
> >};
> >
> >Also, raw use of jiffies_64 is buggy and racy. Why does this use
> >jiffies to begin with - why not some finer grained time?
> 
> What would be a good time to use?

ktime_get() would be the primary candidate. (Or, perhaps, if 
performance is really an issue then trace_clock() or 
trace_clock_global().)

	Ingo