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

[Ingo Molnar]

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

> >-----Original Message-----
> >From: Ingo Molnar [mailto:mingo@xxxxxxx]
> >Sent: Thursday, April 02, 2009 8:45 PM
> >To: Metzger, Markus T
> >Cc: tglx@xxxxxxxxxxxxx; hpa@xxxxxxxxx; markus.t.metzger@xxxxxxxxx; roland@xxxxxxxxxx;
> >eranian@xxxxxxxxxxxxxx; oleg@xxxxxxxxxx; Villacis, Juan; ak@xxxxxxxxxxxxxxxxxx; linux-
> >kernel@xxxxxxxxxxxxxxx
> >Subject: Re: [patch 01/18] x86, bts: fix race when bts tracer is removed
> >
> >
> >* markus.t.metzger@xxxxxxxxx <markus.t.metzger@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/