Messages in this thread | | | Date | Wed, 1 Apr 2009 01:48:23 +0200 | From | Oleg Nesterov <> | Subject | Re: [patch 1/21] x86, bts: fix race when bts tracer is removed |
| |
On 03/31, Markus Metzger wrote: > > Read the tracer once during a context switch. > ... > @@ -1044,36 +1051,39 @@ void ds_switch_to(struct task_struct *pr > { > struct ds_context *prev_ctx = prev->thread.ds_ctx; > struct ds_context *next_ctx = next->thread.ds_ctx; > + unsigned long debugctlmsr = next->thread.debugctlmsr; > > if (prev_ctx) { > + struct bts_tracer *tracer = prev_ctx->bts_master; > + > update_debugctlmsr(0); > > - if (prev_ctx->bts_master && > - (prev_ctx->bts_master->trace.ds.flags & BTS_TIMESTAMPS)) { > + if (tracer && (tracer->flags & BTS_TIMESTAMPS)) {
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).
Off-topic, but afaics modulo bts_task_departs/bts_task_arrives we have the identical code for prev_ctx/next_ctx, perhaps it makes sense to make a helper which calls bts_write().
To clarify, even _if_ I am right and _if_ you agree, we can do this later, I am not suggesting to change this patch right now.
Oleg.
| |