From 2d14ba29e46207fdcc52c0a8fb6573bfd9fa81c6 Mon Sep 17 00:00:00 2001 From: dlg Date: Fri, 17 Apr 2015 12:38:54 +0000 Subject: [PATCH] while trying to reproduce lockups on mp alpha i hit an MUTEX_ASSERT_UNLOCKED, but it turns out alpha mutexes arent very friendly to diagnostics on smp systems. alpha mutexes contained an mtx_lock member. when 0 the mutex was unlocked, and when 1 it was locked. the MUTEX_ASSERT_UNLOCKED checked if mtx_lock was 1 to see if the current cpu owned the mutex, but in an mp system another cpu may have set mtx_lock to 1, which causes the assert to fire. this changes alpha mutexes so they record which cpu owns the lock rather than just if the lock is held or not. the diagnostics compare the owner to the current cpus curcpu() address so they can actually tell if the current cpu holds the lock instead of whether any cpu holds the lock. instead of using custom asm to implement a cas this uses atomic_cas_ptr, which on alpha uses gcc cas code. miod says he has far more confidence in the gcc cas than the code that was there before. while im here i also shuffled the code. on MULTIPROCESSOR systems instead of duplicating code between mtx_enter and mtx_enter_try, mtx_enter simply loops on mtx_enter_try until it succeeds. this also provides an alternative implementation of mutexes on !MULTIPROCESSOR systems that avoids interlocking opcodes. mutexes wont contend on UP boxes, theyre basically wrappers around spls. we can just do the splraise, stash the owner as a guard value for DIAGNOSTIC and return. similarly, mtx_enter_try on UP will never fail, so we can just call mtx_enter and return 1. ok miod@ --- sys/arch/alpha/alpha/mutex.c | 114 +++++++++++++++------------------ sys/arch/alpha/include/mutex.h | 19 ++---- 2 files changed, 55 insertions(+), 78 deletions(-) diff --git a/sys/arch/alpha/alpha/mutex.c b/sys/arch/alpha/alpha/mutex.c index 9f204d6a288..345b94853bc 100644 --- a/sys/arch/alpha/alpha/mutex.c +++ b/sys/arch/alpha/alpha/mutex.c @@ -1,4 +1,4 @@ -/* $OpenBSD: mutex.c,v 1.13 2015/02/11 01:15:06 dlg Exp $ */ +/* $OpenBSD: mutex.c,v 1.14 2015/04/17 12:38:54 dlg Exp $ */ /* * Copyright (c) 2004 Artur Grabowski @@ -28,114 +28,102 @@ #include #include #include +#include #include #include #include -static inline int -try_lock(struct mutex *mtx) -{ -#ifdef MULTIPROCESSOR - unsigned long t0, v0; - - __asm volatile( - "1: ldl_l %0, %3 \n" /* t0 = mtx->mtx_lock */ - " bne %0, 2f \n" - " bis $31, 1, %0 \n" /* t0 = 1 */ - " stl_c %0, %2 \n" /* mtx->mtx_lock = 1 */ - " beq %0, 3f \n" - " mb \n" - " bis $31, 1, %1 \n" /* v0 = 1 */ - " br 4f \n" - "3: br 1b \n" /* update failed */ - "2: bis $31, $31, %1 \n" /* v0 = 0 */ - "4: \n" - : "=&r" (t0), "=r" (v0), "=m" (mtx->mtx_lock) - : "m" (mtx->mtx_lock) - : "memory"); - - return (v0 != 0); -#else - mtx->mtx_lock = 1; - return 1; -#endif -} - void __mtx_init(struct mutex *mtx, int wantipl) { + mtx->mtx_owner = NULL; mtx->mtx_oldipl = IPL_NONE; mtx->mtx_wantipl = wantipl; - mtx->mtx_lock = 0; -#ifdef MULTIPROCESSOR - mtx->mtx_owner = NULL; -#endif } +#ifdef MULTIPROCESSOR void mtx_enter(struct mutex *mtx) { - int s; - - for (;;) { - if (mtx->mtx_wantipl != IPL_NONE) - s = _splraise(mtx->mtx_wantipl); - if (try_lock(mtx)) { - if (mtx->mtx_wantipl != IPL_NONE) - mtx->mtx_oldipl = s; - mtx->mtx_owner = curcpu(); -#ifdef DIAGNOSTIC - curcpu()->ci_mutex_level++; -#endif - return; - } - if (mtx->mtx_wantipl != IPL_NONE) - splx(s); - -#ifdef MULTIPROCESSOR + while (mtx_enter_try(mtx) == 0) SPINLOCK_SPIN_HOOK; -#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); - if (try_lock(mtx)) { + + 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) { if (mtx->mtx_wantipl != IPL_NONE) mtx->mtx_oldipl = s; - mtx->mtx_owner = curcpu(); #ifdef DIAGNOSTIC - curcpu()->ci_mutex_level++; + ci->ci_mutex_level++; #endif - return 1; + membar_enter(); + return (1); } + if (mtx->mtx_wantipl != IPL_NONE) splx(s); - return 0; + + 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 MULTIPROCESSOR + membar_exit(); +#endif #ifdef DIAGNOSTIC curcpu()->ci_mutex_level--; #endif + s = mtx->mtx_oldipl; mtx->mtx_owner = NULL; - mtx->mtx_lock = 0; -#ifdef MULTIPROCESSOR - alpha_wmb(); -#endif if (mtx->mtx_wantipl != IPL_NONE) splx(s); } diff --git a/sys/arch/alpha/include/mutex.h b/sys/arch/alpha/include/mutex.h index a4aeca3c63d..c5509881c5a 100644 --- a/sys/arch/alpha/include/mutex.h +++ b/sys/arch/alpha/include/mutex.h @@ -1,4 +1,4 @@ -/* $OpenBSD: mutex.h,v 1.6 2014/03/18 21:09:28 kettenis Exp $ */ +/* $OpenBSD: mutex.h,v 1.7 2015/04/17 12:38:54 dlg Exp $ */ /* * Copyright (c) 2004 Artur Grabowski @@ -29,10 +29,9 @@ #define _MACHINE_MUTEX_H_ struct mutex { - int mtx_lock; + void *mtx_owner; int mtx_wantipl; int mtx_oldipl; - void *mtx_owner; }; /* @@ -49,29 +48,19 @@ struct mutex { #define __MUTEX_IPL(ipl) (ipl) #endif -#define MUTEX_INITIALIZER(ipl) { 0, __MUTEX_IPL((ipl)), IPL_NONE, NULL } +#define MUTEX_INITIALIZER(ipl) { NULL, __MUTEX_IPL((ipl)), IPL_NONE } void __mtx_init(struct mutex *, int); #define mtx_init(mtx, ipl) __mtx_init((mtx), __MUTEX_IPL((ipl))) #ifdef DIAGNOSTIC -#ifdef MULTIPROCESSOR #define MUTEX_ASSERT_LOCKED(mtx) do { \ - if ((mtx)->mtx_lock == 0) \ - panic("mutex %p not held in %s", (mtx), __func__); \ if ((mtx)->mtx_owner != curcpu()) \ - panic("mutex %p held by cpu %p in %s", \ - (mtx), (mtx)->mtx_owner, __func__); \ -} while (0) -#else -#define MUTEX_ASSERT_LOCKED(mtx) do { \ - if ((mtx)->mtx_lock == 0) \ panic("mutex %p not held in %s", (mtx), __func__); \ } while (0) -#endif #define MUTEX_ASSERT_UNLOCKED(mtx) do { \ - if ((mtx)->mtx_lock != 0) \ + if ((mtx)->mtx_owner == curcpu()) \ panic("mutex %p held in %s", (mtx), __func__); \ } while (0) #else -- 2.20.1