Messages in this thread Patch in this message | | | Date | Tue, 31 Aug 2004 22:20:51 +0200 | From | Ingo Molnar <> | Subject | Re: [patch] voluntary-preempt-2.6.9-rc1-bk4-Q5 |
| |
* Ingo Molnar <mingo@elte.hu> wrote:
> > * Lee Revell <rlrevell@joe-job.com> wrote: > > > File under boot-time stuff, I guess. This could be bad if X crashes, > > but I can't remember the last time this happened to me, and I use xorg > > CVS. > > but the first wbinvd() within prepare_set() seems completely unnecessary > - we can flush the cache after disabling the cache just fine.
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.
another thing is that interrupts are not disabled (although the Intel docs suggest so). It is best to disable interrupts because any handler executing in this window will perform extremely slowly (because caches are disabled), and might even interfere with MTRR setting. Best disable IRQs.
so ... could you try the patch below - does it work and how does the latency look like now? (ontop of an unmodified generic.c)
Ingo
--- linux/arch/i386/kernel/cpu/mtrr/generic.c.orig +++ linux/arch/i386/kernel/cpu/mtrr/generic.c @@ -240,11 +240,14 @@ static void prepare_set(void) /* Note that this is not ideal, since the cache is only flushed/disabled for this CPU while the MTRRs are changed, but changing this requires more invasive changes to the way the kernel boots */ - spin_lock(&set_atomicity_lock); + /* + * Since we are disabling the cache dont allow any interrupts - they + * would run extremely slow and would only increase the pain: + */ + spin_lock_irq(&set_atomicity_lock); /* Enter the no-fill (CD=1, NW=0) cache mode and flush caches. */ cr0 = read_cr0() | 0x40000000; /* set CD flag */ - wbinvd(); write_cr0(cr0); wbinvd(); @@ -266,8 +269,7 @@ static void prepare_set(void) static void post_set(void) { - /* Flush caches and TLBs */ - wbinvd(); + /* Flush TLBs (no need to flush caches - they are disabled) */ __flush_tlb(); /* Intel (P6) standard MTRRs */ @@ -279,7 +281,7 @@ static void post_set(void) /* Restore value of CR4 */ if ( cpu_has_pge ) write_cr4(cr4); - spin_unlock(&set_atomicity_lock); + spin_unlock_irq(&set_atomicity_lock); } static void generic_set_all(void) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |