Re: [patch 1/21] x86, bts: fix race when bts tracer is removed

[Ingo Molnar]

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

> >-----Original Message-----
> >From: Oleg Nesterov [mailto:oleg@xxxxxxxxxx]
> >Sent: Wednesday, April 01, 2009 1:48 AM
> >To: Metzger, Markus T
> >Cc: linux-kernel@xxxxxxxxxxxxxxx; mingo@xxxxxxx; tglx@xxxxxxxxxxxxx; hpa@xxxxxxxxx;
> >markus.t.metzger@xxxxxxxxx; roland@xxxxxxxxxx; eranian@xxxxxxxxxxxxxx; Villacis, Juan;
> >ak@xxxxxxxxxxxxxxxxxx
> >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).
> 
> I guess the same is true for prev_ctx, next_ctx, and debugctlmsr, then.
> 
> Ingo,
> would it be OK to resend this one patch with the barrier()s added?

Sure - but note that i have put the series on hold until you get 
broad Ack's from Oleg for the ptrace bits. Please fix the review 
feedback from Oleg and propagate his acks into the commit logs as 
well. Oleg is finding bugs we missed in the past so his review work 
is very valuable.

Also - minor patch submission technicality observation: currently 
each of your mails goes into a separate discussion thread, making it 
hard to review them as a group.

The preferred way to send such series is to use "git format-patch" + 
"git send-email". (That will give a nice 0/21 mail and a properly 
threaded discussion with proper References header lines.)

Thanks,

	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/