Re: [RFC][PATCH] linux-2.6.0-test2_mtrr-race-fix_A0

[Mark Haverkamp <markh@osdl.org>]

On Wed, 2003-08-06 at 10:15, john stultz wrote:
> On Wed, 2003-08-06 at 01:52, Mathias Fröhlich wrote:
> > Hi,
> > 
> > You should not remove the barrier past mtrr change. If you do that, it is 
> > possible that cpu's run with inconsistent mtrrs. This can have bad 
> > sideeffects since at least the cache snooping protocol used by intel uses 
> > assumptions about the cachability of memory regions. Those information about 
> > the cachability is also taken from the mtrrs as far as I remember.
> > This intel cpu developer manual, which documented the early PII and PPro 
> > chips, recommended this algorithm. Since actual intel cpus use the same old 
> > cpu to chipset bus protocol, this old documentation most propably still 
> > applies.
> 
> Hmm. I should dig up that doc. Its a little hazy in my mind, but I think
> I understand your description. I'm glad you caught this, as I can't
> imagine the subtle bugs that might have popped up. 
> 
> Well, let me look at it again and see if I can come up with a proper
> fix. 
> 
> Thanks for the knowledgeable feedback and sanity checking!
> -john


I added an extra sync up from the caller after the last gate change so
it is the last one to touch the automatic data.

Mark.


===== arch/i386/kernel/cpu/mtrr/main.c 1.29 vs edited =====
--- 1.29/arch/i386/kernel/cpu/mtrr/main.c	Tue Jul 15 10:08:48 2003
+++ edited/arch/i386/kernel/cpu/mtrr/main.c	Wed Aug  6 10:46:00 2003
@@ -169,6 +169,7 @@
 		cpu_relax();
 		barrier();
 	}
+	atomic_dec(&data->count);
 	local_irq_restore(flags);
 }
 
@@ -256,8 +257,18 @@
 		cpu_relax();
 		barrier();
 	}
-	local_irq_restore(flags);
+	atomic_set(&data.count, num_booting_cpus() - 1);
 	atomic_set(&data.gate,0);
+
+	/* 
+	 * Wait here for everyone to have seen the gate change
+	 * So we're the last ones to touch 'data'
+	 */
+	while(atomic_read(&data.count)) {
+		cpu_relax();
+		barrier();
+	}
+	local_irq_restore(flags);
 }
 
 /**


-- 
Mark Haverkamp <markh@osdl.org>