Date: Tue, 24 Dec 1996 10:41:11 -0800 From: Erich Boleyn <erich@uruk.org> To: smp@freebsd.org Subject: Re: I think we have the culprit!! (was -> Re: Eureka (maybe...) (was -> Re: P6 problem idea ) ) Message-ID: <E0vcbmp-0004zg-00@uruk.org> In-Reply-To: Your message of "Mon, 23 Dec 1996 23:39:18 PST." <E0vcRSI-0003hW-00@uruk.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Erich Boleyn <erich@uruk.org> writes: > I'll generate a cvs diff for the tree I have tomorrow morning (it > conditionally compiles the Page Global stuff on not having SMP > enabled, and adds the waiting of the other CPUs to the TLB shootdown... > it doesn't synchronize the other CPUs *before* the page tables are > changed, but that seems to be a much rarer problem, so getting > this in as is is probably worthwhile). I must go to bed for now. I figured I'd send unified diffs of what I have, which works on my machine without crashing or corruption that I could detect, but there was a snag... The diff included below comments out the TLB shootdown rendevous because either the broadcast doesn't work quite right or one of the CPUs have blocked interrupts in some way. In any case, somewhere along the way it hangs. Breaking into the kernel debugger invariably showed a bit in the "smp_invalidate_needed" variable still set. A variant where I simply set "smp_invalidate_needed" to 1 on the sending CPU and to 0 in the IPI handler routine (i.e. the first CPU receiving the interrupt will release the sender) seemed to work OK, but in general I figured it was a band-aid that might fail if all the other CPUs don't get a message (a 4-CPU machine like mine might be OK, but a 2-CPU machine might choke before too long). So, this is indicative of some kind of problem with TLB invalidates not getting sent to or received by all CPUs (or maybe my "lock" instruction wasn't assembled correctly and the CPUs didn't do locked bus cycles??? I'm just not sure). ----------------------(start cvs diff -u)----------------------- Index: sys/i386/i386/locore.s =================================================================== RCS file: /usr/cvssup/sys/i386/i386/locore.s,v retrieving revision 1.34 diff -u -r1.34 locore.s --- locore.s 1996/12/03 05:51:09 1.34 +++ locore.s 1996/12/24 06:30:02 @@ -725,12 +725,14 @@ create_pagetables: +#ifndef SMP testl $CPUID_PGE, R(_cpu_feature) jz 1f movl %cr4, %eax orl $CR4_PGE, %eax movl %eax, %cr4 1: +#endif /* Find end of kernel image (rounded up to a page boundary). */ movl $R(_end),%esi @@ -792,9 +794,11 @@ jne map_read_write #endif xorl %edx,%edx +#ifndef SMP testl $CPUID_PGE, R(_cpu_feature) jz 2f orl $PG_G,%edx +#endif 2: movl $R(_etext),%ecx addl $PAGE_MASK,%ecx @@ -807,9 +811,11 @@ andl $~PAGE_MASK, %eax map_read_write: movl $PG_RW,%edx +#ifndef SMP testl $CPUID_PGE, R(_cpu_feature) jz 1f orl $PG_G,%edx +#endif 1: movl R(_KERNend),%ecx subl %eax,%ecx Index: sys/i386/i386/mp_machdep.c =================================================================== RCS file: /usr/cvssup/sys/i386/i386/mp_machdep.c,v retrieving revision 1.35 diff -u -r1.35 mp_machdep.c --- mp_machdep.c 1996/12/12 08:43:52 1.35 +++ mp_machdep.c 1996/12/24 17:08:02 @@ -1545,9 +1545,23 @@ /* * Flush the TLB on all other CPU's * - * XXX: Needs to handshake and wait for completion before proceding. + * XXX: Needs to handshake and wait for completion before proceeding. + * + * XXX: -- Erich Boleyn <erich@uruk.org> + * The code inside the "WORKING_INVALIDATE_RENDEVOUS" defines implements a + * handshake waiting for all the other CPUs to complete their invalidate. + * "WORKING_INVALIDATE_RENDEVOUS" is undefined because the "allButSelfIPI" + * doesn't always seem to be received by all CPUs. This could be a + * problem where one of other CPUs' interrupt controllers are pending + * on a higher priority interrupt or some such thing... I really don't + * know at this point, but I figured I'd leave the code in since it + * works fine otherwise. */ +#ifdef WORKING_INVALIDATE_RENDEVOUS +volatile unsigned smp_invalidate_needed = 0; +#endif + void smp_invltlb() { @@ -1558,8 +1572,15 @@ serial_putc('A' + cpunumber()); } #endif - if (invldebug & 2) + if (invldebug & 2) { +#ifdef WORKING_INVALIDATE_RENDEVOUS + smp_invalidate_needed = ((1 << mp_ncpus) - 1) & ~(1 << cpunumber()); +#endif allButSelfIPI(ICU_OFFSET+27); +#ifdef WORKING_INVALIDATE_RENDEVOUS + while (smp_invalidate_needed); +#endif + } } } @@ -1606,6 +1627,15 @@ __asm __volatile("movl %%cr3, %0; movl %0, %%cr3" : "=r" (temp) : : "memory"); +#ifdef WORKING_INVALIDATE_RENDEVOUS + /* + * This is an atomic bit reset, to declare to the world that + * the invalidate for this CPU has been performed. + */ + temp = cpunumber(); + __asm __volatile("lock ; btr %0, _smp_invalidate_needed" : : + "r" (temp) : "memory"); +#endif } } #endif /* SMP_INVLTLB */ Index: sys/i386/i386/pmap.c =================================================================== RCS file: /usr/cvssup/sys/i386/i386/pmap.c,v retrieving revision 1.32 diff -u -r1.32 pmap.c --- pmap.c 1996/12/12 08:43:53 1.32 +++ pmap.c 1996/12/24 17:15:21 @@ -157,7 +157,9 @@ vm_offset_t virtual_end; /* VA of last avail page (end of kernel AS) */ static boolean_t pmap_initialized = FALSE; /* Has pmap_init completed? */ static vm_offset_t vm_first_phys; +#ifndef SMP static int pgeflag; /* PG_G or-in */ +#endif static int nkpt; static vm_page_t nkpg; @@ -354,10 +356,12 @@ invltlb(); +#ifndef SMP if (cpu_feature & CPUID_PGE) pgeflag = PG_G; else pgeflag = 0; +#endif } #if defined(SMP) || defined(APIC_IO) @@ -647,7 +651,11 @@ for (i = 0; i < count; i++) { vm_offset_t tva = va + i * PAGE_SIZE; +#ifndef SMP unsigned npte = VM_PAGE_TO_PHYS(m[i]) | PG_RW | PG_V | pgeflag; +#else + unsigned npte = VM_PAGE_TO_PHYS(m[i]) | PG_RW | PG_V; +#endif unsigned opte; pte = (unsigned *)vtopte(tva); opte = *pte; @@ -690,7 +698,11 @@ register unsigned *pte; unsigned npte, opte; +#ifndef SMP npte = pa | PG_RW | PG_V | pgeflag; +#else + npte = pa | PG_RW | PG_V; +#endif pte = (unsigned *)vtopte(va); opte = *pte; *pte = npte; @@ -1658,8 +1670,10 @@ * Machines that don't support invlpg, also don't support * PG_G. */ +#ifndef SMP if (oldpte & PG_G) invlpg(va); +#endif pmap->pm_stats.resident_count -= 1; if (oldpte & PG_MANAGED) { ppv = pa_to_pvh(oldpte); @@ -2088,8 +2102,10 @@ newpte |= PG_W; if (va < UPT_MIN_ADDRESS) newpte |= PG_U; +#ifndef SMP if (pmap == kernel_pmap) newpte |= pgeflag; +#endif /* * if the mapping or permission bits are different, we need ----------------------(end cvs diff -u)----------------------- -- Erich Stefan Boleyn \_ E-mail (preferred): <erich@uruk.org> Mad Genius wanna-be, CyberMuffin \__ (finger me for other stats) Web: http://www.uruk.org/~erich/ Motto: "I'll live forever or die trying"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E0vcbmp-0004zg-00>