Re: [patch] voluntary-preempt-2.6.9-rc1-bk4-Q5

[Ingo Molnar <mingo@elte.hu>]

* Andi Kleen <ak@muc.de> wrote:

> > the third wbinvd() in post_set() seems unnecessary too - what kind of
> > cache do we expect to flush, we've disabled caching in the CPU ... But
> > the Intel pseudocode does it too - this is a thinko i think.
> 
> The multiple steps are needed, otherwise there can be problems with
> hyperthreading (the first x86-64 didn't do it in all cases, and it
> causes occassional problens with Intel CPUs)

i'm quite sure the first one is unnecessary - any interrupt could
already come inbetween and generate dirty cachelines, so it has zero
guaranteed effect.

the third one _seems_ unnecessary - what dirty cachelines can there be
after we've disabled the cache?

the hyperthreading question is valid but no way does the flush solve the
HT problems this code already has! The only proper way i can see to
_guarantee_ zero cache effects is to do a 'catch CPUs', 'disable IRQs,
all caches and flush them', and 'set MTRR's on all CPUs', 'enable all
caches', 'continue CPUs' sequence, SMP-synchronized at every point.
Otherwise you can always get some dirty cache state due to HT races and
whatever data corruption there might occur. I believe the MTRR code is
quite incorrectly coded as-is.

> Also repeated calls of this are relatively cheap because at the second
> time there is not much to flush anymore.

the trace shows that the first one is 300 usecs, the second and third 
one is 150 usecs each, so it's not exactly cheap.

	Ingo