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>