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

Ingo Molnar
2009-04-03T11:29:26Z

* 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