From: Matthew Dillon Date: Thu, 10 Jul 2003 18:23:24 +0000 (+0000) Subject: MP Implmentation 3A/4: Cleanup MP lock operations to allow the MP lock to X-Git-Tag: v2.0.1~13306 X-Git-Url: https://gitweb.dragonflybsd.org/dragonfly.git/commitdiff_plain/a5934754f853376de97575b84dff7b9e2c92c7a2 MP Implmentation 3A/4: Cleanup MP lock operations to allow the MP lock to occassionally be out of synch from td_mpcount, so we don't have to disable interrupts and so we can call try_mplock() from the middle of a switch function. --- diff --git a/sys/i386/i386/mplock.s b/sys/i386/i386/mplock.s index d9072c38de..5ed0988927 100644 --- a/sys/i386/i386/mplock.s +++ b/sys/i386/i386/mplock.s @@ -7,7 +7,7 @@ * ---------------------------------------------------------------------------- * * $FreeBSD: src/sys/i386/i386/mplock.s,v 1.29.2.2 2000/05/16 06:58:06 dillon Exp $ - * $DragonFly: src/sys/i386/i386/Attic/mplock.s,v 1.6 2003/07/10 04:47:53 dillon Exp $ + * $DragonFly: src/sys/i386/i386/Attic/mplock.s,v 1.7 2003/07/10 18:23:23 dillon Exp $ * * Functions for locking between CPUs in a SMP system. * @@ -56,8 +56,7 @@ NON_GPROF_ENTRY(cpu_get_initial_mplock) /* * cpu_try_mplock() returns non-zero on success, 0 on failure. It - * only adjusts mp_lock. It does not touch td_mpcount, and it - * must be called from inside a critical section. + * only adjusts mp_lock. It does not touch td_mpcount. */ NON_GPROF_ENTRY(cpu_try_mplock) movl PCPU(cpuid),%ecx @@ -73,97 +72,84 @@ NON_GPROF_ENTRY(cpu_try_mplock) movl $0,%eax NON_GPROF_RET + /* + * get_mplock() Obtains the MP lock and may switch away if it cannot + * get it. Note that td_mpcount may not be synchronized with the + * actual state of the MP lock. This situation occurs when + * get_mplock() or try_mplock() is indirectly called from the + * lwkt_switch() code, or from a preemption (though, truthfully, + * only try_mplock() should ever be called in this fashion). If + * we cannot get the MP lock we pre-dispose TD_MPCOUNT and call + * lwkt_swich(). The MP lock will be held on return. + * + * Note that both get_mplock() and try_mplock() must pre-dispose + * mpcount before attempting to get the lock, in case we get + * preempted. This allows us to avoid expensive interrupt + * disablement instructions and allows us to be called from outside + * a critical section. + */ NON_GPROF_ENTRY(get_mplock) + movl PCPU(cpuid),%ecx movl PCPU(curthread),%edx - cmpl $0,TD_MPCOUNT(%edx) - je 1f - incl TD_MPCOUNT(%edx) /* already have it, just ++mpcount */ -#ifdef INVARIANTS - movl PCPU(cpuid),%eax /* failure */ - cmpl %eax,mp_lock - jne 4f -#endif + cmpl %ecx,mp_lock + jne 1f + incl TD_MPCOUNT(%edx) NON_GPROF_RET 1: - pushfl - cli - movl $1,TD_MPCOUNT(%edx) - movl PCPU(cpuid),%ecx + incl TD_MPCOUNT(%edx) movl $-1,%eax - lock cmpxchgl %ecx,mp_lock /* ecx<->mem & JZ if eax matches */ + lock cmpxchgl %ecx,mp_lock jnz 2f #ifdef PARANOID_INVLTLB movl %cr3,%eax; movl %eax,%cr3 /* YYY check and remove */ #endif - popfl /* success */ NON_GPROF_RET 2: -#ifdef INVARIANTS - movl PCPU(cpuid),%eax /* failure */ - cmpl %eax,mp_lock - je 3f -#endif - addl $TDPRI_CRIT,TD_PRI(%edx) - popfl call lwkt_switch /* will be correct on return */ #ifdef INVARIANTS movl PCPU(cpuid),%eax /* failure */ cmpl %eax,mp_lock jne 4f #endif - movl PCPU(curthread),%edx - subl $TDPRI_CRIT,TD_PRI(%edx) - NON_GPROF_RET -3: - cmpl $0,panicstr /* don't double panic */ - je badmp_get - popfl NON_GPROF_RET - 4: cmpl $0,panicstr /* don't double panic */ je badmp_get2 NON_GPROF_RET + /* + * try_mplock() attempts to obtain the MP lock and will not switch + * away if it cannot get it. Note that td_mpcoutn may not be + * synchronized with the actual state of the MP lock. + */ NON_GPROF_ENTRY(try_mplock) + movl PCPU(cpuid),%ecx movl PCPU(curthread),%edx - cmpl $0,TD_MPCOUNT(%edx) - je 1f - incl TD_MPCOUNT(%edx) /* already have it, just ++mpcount */ -#ifdef INVARIANTS - movl PCPU(cpuid),%eax /* failure */ - cmpl %eax,mp_lock - jne 4b -#endif + cmpl %ecx,mp_lock + jne 1f + incl TD_MPCOUNT(%edx) movl $1,%eax NON_GPROF_RET 1: - pushfl - cli - movl PCPU(cpuid),%ecx + incl TD_MPCOUNT(%edx) /* pre-dispose */ movl $-1,%eax - lock cmpxchgl %ecx,mp_lock /* ecx<->mem & JZ if eax matches */ + lock cmpxchgl %ecx,mp_lock jnz 2f - movl $1,TD_MPCOUNT(%edx) #ifdef PARANOID_INVLTLB movl %cr3,%eax; movl %eax,%cr3 /* YYY check and remove */ #endif - popfl /* success */ - movl $1,%eax NON_GPROF_RET 2: -#ifdef INVARIANTS - cmpl $0,panicstr - jnz 3f - movl PCPU(cpuid),%eax /* failure */ - cmpl %eax,mp_lock - je badmp_get -3: -#endif - popfl - movl $0,%eax + decl TD_MPCOUNT(%edx) /* un-dispose */ + subl %eax,%eax NON_GPROF_RET + /* + * rel_mplock() release the MP lock. The MP lock MUST be held, + * td_mpcount must NOT be out of synch with the lock. It is allowed + * for the physical lock to be released prior to setting the count + * to 0, preemptions will deal with the case (see lwkt_thread.c). + */ NON_GPROF_ENTRY(rel_mplock) movl PCPU(curthread),%edx movl TD_MPCOUNT(%edx),%eax @@ -177,16 +163,13 @@ NON_GPROF_ENTRY(rel_mplock) movl %eax,TD_MPCOUNT(%edx) NON_GPROF_RET 1: - pushfl - cli #ifdef INVARIANTS movl PCPU(cpuid),%ecx cmpl %ecx,mp_lock jne badmp_rel2 #endif - movl $0,TD_MPCOUNT(%edx) movl $MP_FREE_LOCK,mp_lock - popfl + movl $0,TD_MPCOUNT(%edx) NON_GPROF_RET #ifdef INVARIANTS diff --git a/sys/kern/kern_switch.c b/sys/kern/kern_switch.c index 751e7e0b54..36535a8d99 100644 --- a/sys/kern/kern_switch.c +++ b/sys/kern/kern_switch.c @@ -24,12 +24,13 @@ * SUCH DAMAGE. * * $FreeBSD: src/sys/kern/kern_switch.c,v 1.3.2.1 2000/05/16 06:58:12 dillon Exp $ - * $DragonFly: src/sys/kern/Attic/kern_switch.c,v 1.5 2003/07/10 04:47:54 dillon Exp $ + * $DragonFly: src/sys/kern/Attic/kern_switch.c,v 1.6 2003/07/10 18:23:24 dillon Exp $ */ #include #include #include +#include #include #include #include @@ -278,8 +279,15 @@ remrunqueue(struct proc *p) /* * Release the P_CURPROC designation on the CURRENT process only. This - * will allow another userland process to be scheduled and places our - * process back on the userland scheduling queue. + * will allow another userland process to be scheduled. If we do not + * have or cannot get the MP lock we just wakeup the scheduler thread for + * this cpu. + * + * WARNING! The MP lock may be in an unsynchronized state due to the + * way get_mplock() works and the fact that this function may be called + * from a passive release during a lwkt_switch(). try_mplock() will deal + * with this for us but you should be aware that td_mpcount may not be + * useable. */ void release_curproc(struct proc *p) @@ -296,13 +304,20 @@ release_curproc(struct proc *p) p->p_flag |= P_CP_RELEASED; if (p->p_flag & P_CURPROC) { p->p_flag &= ~P_CURPROC; - KKASSERT(curprocmask & (1 << cpuid)); - if ((np = chooseproc()) != NULL) { - np->p_flag |= P_CURPROC; - lwkt_acquire(np->p_thread); - lwkt_schedule(np->p_thread); + if (try_mplock()) { + KKASSERT(curprocmask & (1 << cpuid)); + if ((np = chooseproc()) != NULL) { + np->p_flag |= P_CURPROC; + lwkt_acquire(np->p_thread); + lwkt_schedule(np->p_thread); + } else { + curprocmask &= ~(1 << cpuid); + } + rel_mplock(); } else { curprocmask &= ~(1 << cpuid); + if (rdyprocmask & (1 << cpuid)) + lwkt_schedule(&globaldata_find(cpuid)->gd_schedthread); } } crit_exit(); diff --git a/sys/kern/lwkt_thread.c b/sys/kern/lwkt_thread.c index 50c2a60901..761c40f033 100644 --- a/sys/kern/lwkt_thread.c +++ b/sys/kern/lwkt_thread.c @@ -28,7 +28,7 @@ * to use a critical section to avoid problems. Foreign thread * scheduling is queued via (async) IPIs. * - * $DragonFly: src/sys/kern/lwkt_thread.c,v 1.18 2003/07/10 04:47:54 dillon Exp $ + * $DragonFly: src/sys/kern/lwkt_thread.c,v 1.19 2003/07/10 18:23:24 dillon Exp $ */ #include @@ -512,17 +512,17 @@ lwkt_preempt(thread_t ntd, int critpri) #ifdef SMP /* * note: an interrupt might have occured just as we were transitioning - * to the MP lock, with the lock held but our mpcount still 0. We have - * to be sure we restore the same condition when the preemption returns. + * to the MP lock. In this case td_mpcount will be pre-disposed but + * not actually synchronized with the actual state of the lock. We + * can use it to imply an MP lock requirement for the preemption but + * we cannot use it to test whether we hold the MP lock or not. */ - mpheld = MP_LOCK_HELD(); /* 0 or 1 */ + mpheld = MP_LOCK_HELD(); if (mpheld && td->td_mpcount == 0) panic("lwkt_preempt(): held and no count"); savecnt = td->td_mpcount; - td->td_mpcount += mpheld; ntd->td_mpcount += td->td_mpcount; if (mpheld == 0 && ntd->td_mpcount && !cpu_try_mplock()) { - td->td_mpcount -= mpheld; ntd->td_mpcount -= td->td_mpcount; ++preempt_miss; need_resched(); @@ -536,7 +536,6 @@ lwkt_preempt(thread_t ntd, int critpri) td->td_switch(ntd); KKASSERT(ntd->td_preempted && (td->td_flags & TDF_PREEMPT_DONE)); #ifdef SMP - td->td_mpcount -= mpheld; KKASSERT(savecnt == td->td_mpcount); if (mpheld == 0 && MP_LOCK_HELD()) cpu_rel_mplock(); diff --git a/sys/platform/pc32/i386/mplock.s b/sys/platform/pc32/i386/mplock.s index f45bf7bfc1..837f8b5ec5 100644 --- a/sys/platform/pc32/i386/mplock.s +++ b/sys/platform/pc32/i386/mplock.s @@ -7,7 +7,7 @@ * ---------------------------------------------------------------------------- * * $FreeBSD: src/sys/i386/i386/mplock.s,v 1.29.2.2 2000/05/16 06:58:06 dillon Exp $ - * $DragonFly: src/sys/platform/pc32/i386/mplock.s,v 1.6 2003/07/10 04:47:53 dillon Exp $ + * $DragonFly: src/sys/platform/pc32/i386/mplock.s,v 1.7 2003/07/10 18:23:23 dillon Exp $ * * Functions for locking between CPUs in a SMP system. * @@ -56,8 +56,7 @@ NON_GPROF_ENTRY(cpu_get_initial_mplock) /* * cpu_try_mplock() returns non-zero on success, 0 on failure. It - * only adjusts mp_lock. It does not touch td_mpcount, and it - * must be called from inside a critical section. + * only adjusts mp_lock. It does not touch td_mpcount. */ NON_GPROF_ENTRY(cpu_try_mplock) movl PCPU(cpuid),%ecx @@ -73,97 +72,84 @@ NON_GPROF_ENTRY(cpu_try_mplock) movl $0,%eax NON_GPROF_RET + /* + * get_mplock() Obtains the MP lock and may switch away if it cannot + * get it. Note that td_mpcount may not be synchronized with the + * actual state of the MP lock. This situation occurs when + * get_mplock() or try_mplock() is indirectly called from the + * lwkt_switch() code, or from a preemption (though, truthfully, + * only try_mplock() should ever be called in this fashion). If + * we cannot get the MP lock we pre-dispose TD_MPCOUNT and call + * lwkt_swich(). The MP lock will be held on return. + * + * Note that both get_mplock() and try_mplock() must pre-dispose + * mpcount before attempting to get the lock, in case we get + * preempted. This allows us to avoid expensive interrupt + * disablement instructions and allows us to be called from outside + * a critical section. + */ NON_GPROF_ENTRY(get_mplock) + movl PCPU(cpuid),%ecx movl PCPU(curthread),%edx - cmpl $0,TD_MPCOUNT(%edx) - je 1f - incl TD_MPCOUNT(%edx) /* already have it, just ++mpcount */ -#ifdef INVARIANTS - movl PCPU(cpuid),%eax /* failure */ - cmpl %eax,mp_lock - jne 4f -#endif + cmpl %ecx,mp_lock + jne 1f + incl TD_MPCOUNT(%edx) NON_GPROF_RET 1: - pushfl - cli - movl $1,TD_MPCOUNT(%edx) - movl PCPU(cpuid),%ecx + incl TD_MPCOUNT(%edx) movl $-1,%eax - lock cmpxchgl %ecx,mp_lock /* ecx<->mem & JZ if eax matches */ + lock cmpxchgl %ecx,mp_lock jnz 2f #ifdef PARANOID_INVLTLB movl %cr3,%eax; movl %eax,%cr3 /* YYY check and remove */ #endif - popfl /* success */ NON_GPROF_RET 2: -#ifdef INVARIANTS - movl PCPU(cpuid),%eax /* failure */ - cmpl %eax,mp_lock - je 3f -#endif - addl $TDPRI_CRIT,TD_PRI(%edx) - popfl call lwkt_switch /* will be correct on return */ #ifdef INVARIANTS movl PCPU(cpuid),%eax /* failure */ cmpl %eax,mp_lock jne 4f #endif - movl PCPU(curthread),%edx - subl $TDPRI_CRIT,TD_PRI(%edx) - NON_GPROF_RET -3: - cmpl $0,panicstr /* don't double panic */ - je badmp_get - popfl NON_GPROF_RET - 4: cmpl $0,panicstr /* don't double panic */ je badmp_get2 NON_GPROF_RET + /* + * try_mplock() attempts to obtain the MP lock and will not switch + * away if it cannot get it. Note that td_mpcoutn may not be + * synchronized with the actual state of the MP lock. + */ NON_GPROF_ENTRY(try_mplock) + movl PCPU(cpuid),%ecx movl PCPU(curthread),%edx - cmpl $0,TD_MPCOUNT(%edx) - je 1f - incl TD_MPCOUNT(%edx) /* already have it, just ++mpcount */ -#ifdef INVARIANTS - movl PCPU(cpuid),%eax /* failure */ - cmpl %eax,mp_lock - jne 4b -#endif + cmpl %ecx,mp_lock + jne 1f + incl TD_MPCOUNT(%edx) movl $1,%eax NON_GPROF_RET 1: - pushfl - cli - movl PCPU(cpuid),%ecx + incl TD_MPCOUNT(%edx) /* pre-dispose */ movl $-1,%eax - lock cmpxchgl %ecx,mp_lock /* ecx<->mem & JZ if eax matches */ + lock cmpxchgl %ecx,mp_lock jnz 2f - movl $1,TD_MPCOUNT(%edx) #ifdef PARANOID_INVLTLB movl %cr3,%eax; movl %eax,%cr3 /* YYY check and remove */ #endif - popfl /* success */ - movl $1,%eax NON_GPROF_RET 2: -#ifdef INVARIANTS - cmpl $0,panicstr - jnz 3f - movl PCPU(cpuid),%eax /* failure */ - cmpl %eax,mp_lock - je badmp_get -3: -#endif - popfl - movl $0,%eax + decl TD_MPCOUNT(%edx) /* un-dispose */ + subl %eax,%eax NON_GPROF_RET + /* + * rel_mplock() release the MP lock. The MP lock MUST be held, + * td_mpcount must NOT be out of synch with the lock. It is allowed + * for the physical lock to be released prior to setting the count + * to 0, preemptions will deal with the case (see lwkt_thread.c). + */ NON_GPROF_ENTRY(rel_mplock) movl PCPU(curthread),%edx movl TD_MPCOUNT(%edx),%eax @@ -177,16 +163,13 @@ NON_GPROF_ENTRY(rel_mplock) movl %eax,TD_MPCOUNT(%edx) NON_GPROF_RET 1: - pushfl - cli #ifdef INVARIANTS movl PCPU(cpuid),%ecx cmpl %ecx,mp_lock jne badmp_rel2 #endif - movl $0,TD_MPCOUNT(%edx) movl $MP_FREE_LOCK,mp_lock - popfl + movl $0,TD_MPCOUNT(%edx) NON_GPROF_RET #ifdef INVARIANTS