Date: Wed, 20 Feb 2002 23:47:58 -0500 (EST) From: John Baldwin <jhb@FreeBSD.org> To: Matthew Dillon <dillon@apollo.backplane.com> Cc: current@FreeBSD.org Subject: Re: RE: Patch to improve mutex collision performance Message-ID: <XFMail.020220234758.jhb@FreeBSD.org> In-Reply-To: <200202210102.g1L12MR91194@apollo.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 21-Feb-02 Matthew Dillon wrote:
> This sounds better but why do we need a 'pause' at all? I don't
> think spinning in this case will have any effect on power
> consumption.
The pause is extra stuff mostly needed for the HyperThreading stuff on the
Pentium4 and also it helps improve cache performance on the P4. Talk to Peter
about it. It's really a separate deal but this was a quick cut 'n paste. The
actual kernel preemption code will reduce the number of context switches back
and forth on mutex release since it only preempts the kernel for real-time (i.e.
interrupt) kernel threads. Currently we will preempt one non-RT kthread for
another. When I talked to the guy who worked on Irix's ithread stuff at
Usenix, he said that adaptive locks actually ended up hurting performance, so I
would prefer to not put this in until after preemption is in and then do some
testing to see if adaptive mutexes help or hurt more.
> -Matt
> Matthew Dillon
> <dillon@backplane.com>
>
>:I would rather make the locks adaptive like so: (untested)
>:
>:--- //depot/projects/smpng/sys/conf/options 2002/02/08 13:19:07
>:+++ //depot/user/jhb/lock/conf/options 2002/02/08 13:50:54
>:@@ -56,6 +56,7 @@
>: # mapped I/O
>:
>: # Miscellaneous options.
>:+ADAPTIVE_MUTEXES
>: BLEED
>: COMPAT_43 opt_compat.h
>: COMPAT_SUNOS opt_compat.h
>:--- //depot/projects/smpng/sys/i386/include/cpufunc.h 2001/12/18 13:07:32
>:+++ //depot/user/jhb/lock/i386/include/cpufunc.h 2001/12/20 15:54:32
>:@@ -550,6 +550,12 @@
>: __asm __volatile("movl %0,%%dr7" : : "r" (sel));
>: }
>:
>:+static __inline void
>:+cpu_pause(void)
>:+{
>:+ __asm __volatile("pause");
>:+}
>:+
>: static __inline critical_t
>: cpu_critical_enter(void)
>: {
>:--- //depot/projects/smpng/sys/kern/kern_mutex.c 2002/02/08 14:19:21
>:+++ //depot/user/jhb/lock/kern/kern_mutex.c 2002/02/08 13:50:54
>:@@ -34,6 +34,7 @@
>: * Machine independent bits of mutex implementation.
>: */
>:
>:+#include "opt_adaptive_mutexes.h"
>: #include "opt_ddb.h"
>:
>: #include <sys/param.h>
>:@@ -345,7 +354,22 @@
>: continue;
>: }
>:
>:+#if defined(SMP) && defined(ADAPTIVE_MUTEXES)
>: /*
>:+ * If the current owner of the lock is executing on another
>:+ * CPU, spin instead of blocking.
>:+ */
>:+ if (((struct thread *)(v & MTX_FLAGMASK)->td_kse->kse_oncpu
>:!=
>:+ NOCPU) {
>:+ mtx_unlock_spin(&sched_lock);
>:+#ifdef __i386__
>:+ cpu_pause();
>:+#endif
>:+ continue;
>:+ }
>:+#endif /* SMP && ADAPTIVE_MUTEXES */
>:+
>:+ /*
>: * We deffinately must sleep for this lock.
>: */
>: mtx_assert(m, MA_NOTOWNED);
>:@@ -433,6 +457,9 @@
>: /* Give interrupts a chance while we spin. */
>: critical_exit();
>: while (m->mtx_lock != MTX_UNOWNED) {
>:+#ifdef __i386__
>:+ cpu_pause();
>:+#endif
>: if (i++ < 10000000)
>: continue;
>: if (i++ < 60000000)
>:
>:This is more a specific problem with Giant and I don't think it will be a
>:problem with other mutexes, so I'd prefer a solution not quite so tailored to
>:this particular behavior of Giant.
>:
>:--
>:
>:John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
>:"Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.020220234758.jhb>