Diff below moves amd64 and i386 mutex to the common C implementation. The differences are: - membar_enter_after_atomic(9) instead of membar_enter(9), and - membar_exit_before_atomic(9) instead of membar_exit(9)
I'd appreciate any performance test to know if the performance degradation is acceptable with these barriers. Index: amd64/amd64/mutex.c =================================================================== RCS file: amd64/amd64/mutex.c diff -N amd64/amd64/mutex.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ amd64/amd64/mutex.c 14 Dec 2017 14:49:59 -0000 @@ -0,0 +1,151 @@ +/* $OpenBSD: mutex.c,v 1.19 2017/09/11 09:52:15 mpi Exp $ */ + +/* + * Copyright (c) 2004 Artur Grabowski <[email protected]> + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY + * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; + * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR + * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include <sys/param.h> +#include <sys/mutex.h> +#include <sys/systm.h> +#include <sys/atomic.h> + +#include <machine/intr.h> + +#include <ddb/db_output.h> + +void +__mtx_init(struct mutex *mtx, int wantipl) +{ + mtx->mtx_owner = NULL; + mtx->mtx_wantipl = wantipl; + mtx->mtx_oldipl = IPL_NONE; +} + +#ifdef MULTIPROCESSOR +#ifdef MP_LOCKDEBUG +#ifndef DDB +#error "MP_LOCKDEBUG requires DDB" +#endif + +/* CPU-dependent timing, needs this to be settable from ddb. */ +extern int __mp_lock_spinout; +#endif + +void +__mtx_enter(struct mutex *mtx) +{ +#ifdef MP_LOCKDEBUG + int nticks = __mp_lock_spinout; +#endif + + while (__mtx_enter_try(mtx) == 0) { + CPU_BUSY_CYCLE(); + +#ifdef MP_LOCKDEBUG + if (--nticks == 0) { + db_printf("%s: %p lock spun out", __func__, mtx); + db_enter(); + nticks = __mp_lock_spinout; + } +#endif + } +} + +int +__mtx_enter_try(struct mutex *mtx) +{ + struct cpu_info *owner, *ci = curcpu(); + int s; + + if (mtx->mtx_wantipl != IPL_NONE) + s = splraise(mtx->mtx_wantipl); + + owner = atomic_cas_ptr(&mtx->mtx_owner, NULL, ci); +#ifdef DIAGNOSTIC + if (__predict_false(owner == ci)) + panic("mtx %p: locking against myself", mtx); +#endif + if (owner == NULL) { + membar_enter_after_atomic(); + if (mtx->mtx_wantipl != IPL_NONE) + mtx->mtx_oldipl = s; +#ifdef DIAGNOSTIC + ci->ci_mutex_level++; +#endif + return (1); + } + + if (mtx->mtx_wantipl != IPL_NONE) + splx(s); + + return (0); +} +#else +void +__mtx_enter(struct mutex *mtx) +{ + struct cpu_info *ci = curcpu(); + +#ifdef DIAGNOSTIC + if (__predict_false(mtx->mtx_owner == ci)) + panic("mtx %p: locking against myself", mtx); +#endif + + if (mtx->mtx_wantipl != IPL_NONE) + mtx->mtx_oldipl = splraise(mtx->mtx_wantipl); + + mtx->mtx_owner = ci; + +#ifdef DIAGNOSTIC + ci->ci_mutex_level++; +#endif +} + +int +__mtx_enter_try(struct mutex *mtx) +{ + __mtx_enter(mtx); + return (1); +} +#endif + +void +__mtx_leave(struct mutex *mtx) +{ + int s; + + MUTEX_ASSERT_LOCKED(mtx); + +#ifdef DIAGNOSTIC + curcpu()->ci_mutex_level--; +#endif + + s = mtx->mtx_oldipl; +#ifdef MULTIPROCESSOR + membar_exit_before_atomic(); +#endif + mtx->mtx_owner = NULL; + if (mtx->mtx_wantipl != IPL_NONE) + splx(s); +} Index: amd64/conf/files.amd64 =================================================================== RCS file: /cvs/src/sys/arch/amd64/conf/files.amd64,v retrieving revision 1.91 diff -u -p -r1.91 files.amd64 --- amd64/conf/files.amd64 17 Oct 2017 14:25:35 -0000 1.91 +++ amd64/conf/files.amd64 14 Dec 2017 14:51:20 -0000 @@ -28,7 +28,7 @@ file arch/amd64/amd64/fpu.c file arch/amd64/amd64/softintr.c file arch/amd64/amd64/i8259.c file arch/amd64/amd64/cacheinfo.c -file arch/amd64/amd64/mutex.S +file arch/amd64/amd64/mutex.c file arch/amd64/amd64/vector.S file arch/amd64/amd64/copy.S file arch/amd64/amd64/spl.S Index: i386/conf/files.i386 =================================================================== RCS file: /cvs/src/sys/arch/i386/conf/files.i386,v retrieving revision 1.235 diff -u -p -r1.235 files.i386 --- i386/conf/files.i386 17 Oct 2017 14:25:35 -0000 1.235 +++ i386/conf/files.i386 14 Dec 2017 14:50:43 -0000 @@ -21,6 +21,7 @@ file arch/i386/i386/est.c !small_kernel file arch/i386/i386/gdt.c file arch/i386/i386/in_cksum.s file arch/i386/i386/machdep.c +file arch/i386/i386/mutex.c file arch/i386/i386/hibernate_machdep.c hibernate file arch/i386/i386/via.c file arch/i386/i386/locore.s Index: i386/i386/locore.s =================================================================== RCS file: /cvs/src/sys/arch/i386/i386/locore.s,v retrieving revision 1.180 diff -u -p -r1.180 locore.s --- i386/i386/locore.s 25 Aug 2017 19:28:48 -0000 1.180 +++ i386/i386/locore.s 14 Dec 2017 14:45:36 -0000 @@ -1319,8 +1319,6 @@ ENTRY(cpu_paenable) #include <i386/i386/apicvec.s> #endif -#include <i386/i386/mutex.S> - .section .rodata .globl _C_LABEL(_stac) _C_LABEL(_stac): Index: i386/i386/mutex.c =================================================================== RCS file: i386/i386/mutex.c diff -N i386/i386/mutex.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ i386/i386/mutex.c 14 Dec 2017 14:47:29 -0000 @@ -0,0 +1,151 @@ +/* $OpenBSD: mutex.c,v 1.19 2017/09/11 09:52:15 mpi Exp $ */ + +/* + * Copyright (c) 2004 Artur Grabowski <[email protected]> + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY + * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; + * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR + * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include <sys/param.h> +#include <sys/mutex.h> +#include <sys/systm.h> +#include <sys/atomic.h> + +#include <machine/intr.h> + +#include <ddb/db_output.h> + +void +__mtx_init(struct mutex *mtx, int wantipl) +{ + mtx->mtx_owner = NULL; + mtx->mtx_wantipl = wantipl; + mtx->mtx_oldipl = IPL_NONE; +} + +#ifdef MULTIPROCESSOR +#ifdef MP_LOCKDEBUG +#ifndef DDB +#error "MP_LOCKDEBUG requires DDB" +#endif + +/* CPU-dependent timing, needs this to be settable from ddb. */ +extern int __mp_lock_spinout; +#endif + +void +__mtx_enter(struct mutex *mtx) +{ +#ifdef MP_LOCKDEBUG + int nticks = __mp_lock_spinout; +#endif + + while (__mtx_enter_try(mtx) == 0) { + CPU_BUSY_CYCLE(); + +#ifdef MP_LOCKDEBUG + if (--nticks == 0) { + db_printf("%s: %p lock spun out", __func__, mtx); + db_enter(); + nticks = __mp_lock_spinout; + } +#endif + } +} + +int +__mtx_enter_try(struct mutex *mtx) +{ + struct cpu_info *owner, *ci = curcpu(); + int s; + + if (mtx->mtx_wantipl != IPL_NONE) + s = splraise(mtx->mtx_wantipl); + + owner = atomic_cas_ptr(&mtx->mtx_owner, NULL, ci); +#ifdef DIAGNOSTIC + if (__predict_false(owner == ci)) + panic("mtx %p: locking against myself", mtx); +#endif + if (owner == NULL) { + membar_enter_after_atomic(); + if (mtx->mtx_wantipl != IPL_NONE) + mtx->mtx_oldipl = s; +#ifdef DIAGNOSTIC + ci->ci_mutex_level++; +#endif + return (1); + } + + if (mtx->mtx_wantipl != IPL_NONE) + splx(s); + + return (0); +} +#else +void +__mtx_enter(struct mutex *mtx) +{ + struct cpu_info *ci = curcpu(); + +#ifdef DIAGNOSTIC + if (__predict_false(mtx->mtx_owner == ci)) + panic("mtx %p: locking against myself", mtx); +#endif + + if (mtx->mtx_wantipl != IPL_NONE) + mtx->mtx_oldipl = splraise(mtx->mtx_wantipl); + + mtx->mtx_owner = ci; + +#ifdef DIAGNOSTIC + ci->ci_mutex_level++; +#endif +} + +int +__mtx_enter_try(struct mutex *mtx) +{ + __mtx_enter(mtx); + return (1); +} +#endif + +void +__mtx_leave(struct mutex *mtx) +{ + int s; + + MUTEX_ASSERT_LOCKED(mtx); + +#ifdef DIAGNOSTIC + curcpu()->ci_mutex_level--; +#endif + + s = mtx->mtx_oldipl; +#ifdef MULTIPROCESSOR + membar_exit_before_atomic(); +#endif + mtx->mtx_owner = NULL; + if (mtx->mtx_wantipl != IPL_NONE) + splx(s); +}
