From 80b64d6a30f884b345971332f831e6618907e9ae Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 23 May 2014 15:14:49 -0400 Subject: [PATCH 01/10] spinlocks update add epicsSpinMustCreate() Fix spinlock on RTEMS and vxWorks UP systems to disable task preemption. Don't use posix spinlocks when thread priorities are used. --- src/libCom/osi/Makefile | 1 + src/libCom/osi/epicsSpin.h | 3 ++- src/libCom/osi/os/RTEMS/osdSpin.c | 37 ++++++++++++++++++++++----- src/libCom/osi/os/default/osdSpin.c | 10 +++++++- src/libCom/osi/os/posix/osdSpin.c | 39 +++++++++++++++++++++++------ src/libCom/osi/os/vxWorks/osdSpin.c | 34 +++++++++++++++++++++---- 6 files changed, 104 insertions(+), 20 deletions(-) diff --git a/src/libCom/osi/Makefile b/src/libCom/osi/Makefile index 94fe92c61..96d4540f2 100644 --- a/src/libCom/osi/Makefile +++ b/src/libCom/osi/Makefile @@ -103,6 +103,7 @@ Com_SRCS += osdStdio.c #POSIX thread priority scheduling flag THREAD_CPPFLAGS_NO += -DDONT_USE_POSIX_THREAD_PRIORITY_SCHEDULING osdThread_CPPFLAGS += $(THREAD_CPPFLAGS_$(USE_POSIX_THREAD_PRIORITY_SCHEDULING)) +osdSpin_CPPFLAGS += $(THREAD_CPPFLAGS_$(USE_POSIX_THREAD_PRIORITY_SCHEDULING)) Com_SRCS += osdThread.c Com_SRCS += osdThreadExtra.c diff --git a/src/libCom/osi/epicsSpin.h b/src/libCom/osi/epicsSpin.h index 06bc44a09..22ce8ec8f 100644 --- a/src/libCom/osi/epicsSpin.h +++ b/src/libCom/osi/epicsSpin.h @@ -17,7 +17,8 @@ extern "C" { typedef struct epicsSpin *epicsSpinId; -epicsShareFunc epicsSpinId epicsSpinCreate(); +epicsShareFunc epicsSpinId epicsSpinCreate(void); +epicsShareFunc epicsSpinId epicsSpinMustCreate(void); epicsShareFunc void epicsSpinDestroy(epicsSpinId); epicsShareFunc void epicsSpinLock(epicsSpinId); diff --git a/src/libCom/osi/os/RTEMS/osdSpin.c b/src/libCom/osi/os/RTEMS/osdSpin.c index 78600e5f1..8e6a2096c 100644 --- a/src/libCom/osi/os/RTEMS/osdSpin.c +++ b/src/libCom/osi/os/RTEMS/osdSpin.c @@ -4,6 +4,8 @@ * Copyright (c) 2012 ITER Organization. * Copyright (c) 2013 UChicago Argonne LLC, as Operator of Argonne * National Laboratory. +* Copyright (c) 2013 Brookhaven Science Assoc. as Operator of Brookhaven +* National Laboratory. * EPICS BASE is distributed subject to a Software License Agreement found * in file LICENSE that is included with this distribution. \*************************************************************************/ @@ -11,37 +13,56 @@ /* * Authors: Ralph Lange * Andrew Johnson + * Michael Davidsaver * - * Based on epicsInterrupt.c (RTEMS implementation) by Eric Norum + * Inspired by Linux UP spinlocks implemention + * include/linux/spinlock_api_up.h */ /* * RTEMS (single CPU): LOCK INTERRUPT * * CAVEAT: - * This implementation is for UP architectures only. - * + * This implementation is intended for UP architectures only. */ +#define __RTEMS_VIOLATE_KERNEL_VISIBILITY__ 1 + #include #include +#include + #include "epicsSpin.h" typedef struct epicsSpin { rtems_interrupt_level level; + unsigned int locked; } epicsSpin; -epicsSpinId epicsSpinCreate() { +epicsSpinId epicsSpinCreate(void) { return calloc(1, sizeof(epicsSpin)); } +epicsSpinId epicsSpinMustCreate(void) +{ + epicsSpinId ret = epicsSpinCreate(); + if(!ret) + cantProceed("epicsSpinMustCreate fails"); + return ret; +} + void epicsSpinDestroy(epicsSpinId spin) { free(spin); } void epicsSpinLock(epicsSpinId spin) { - rtems_interrupt_disable(spin->level); + rtems_interrupt_level level; + rtems_interrupt_disable (level); + _Thread_Disable_dispatch(); + spin->level = level; + assert(!spin->locked); + spin->locked = 1; } int epicsSpinTryLock(epicsSpinId spin) { @@ -50,5 +71,9 @@ int epicsSpinTryLock(epicsSpinId spin) { } void epicsSpinUnlock(epicsSpinId spin) { - rtems_interrupt_enable(spin->level); + rtems_interrupt_level level = spin->level; + assert(spin->locked); + spin->level = spin->locked = 0; + rtems_interrupt_enable (level); + _Thread_Enable_dispatch(); } diff --git a/src/libCom/osi/os/default/osdSpin.c b/src/libCom/osi/os/default/osdSpin.c index 70d9fbace..f4caf4930 100644 --- a/src/libCom/osi/os/default/osdSpin.c +++ b/src/libCom/osi/os/default/osdSpin.c @@ -25,7 +25,7 @@ typedef struct epicsSpin { epicsMutexId lock; } epicsSpin; -epicsSpinId epicsSpinCreate() { +epicsSpinId epicsSpinCreate(void) { epicsSpin *spin; spin = calloc(1, sizeof(*spin)); @@ -43,6 +43,14 @@ fail: return NULL; } +epicsSpinId epicsSpinMustCreate(void) +{ + epicsSpinId ret = epicsSpinCreate(); + if(!ret) + cantProceed("epicsSpinMustCreate fails"); + return ret; +} + void epicsSpinDestroy(epicsSpinId spin) { epicsMutexDestroy(spin->lock); free(spin); diff --git a/src/libCom/osi/os/posix/osdSpin.c b/src/libCom/osi/os/posix/osdSpin.c index b8395dbb0..2b7c31c22 100644 --- a/src/libCom/osi/os/posix/osdSpin.c +++ b/src/libCom/osi/os/posix/osdSpin.c @@ -18,15 +18,27 @@ #define epicsExportSharedSymbols #include "errlog.h" +#include "cantProceed.h" #include "epicsSpin.h" +/* POSIX spinlocks may be subject to priority inversion + * and so can't be guaranteed safe in situations where + * threads have different priorities, and thread + * preemption can't be disabled. + */ +#if defined(DONT_USE_POSIX_THREAD_PRIORITY_SCHEDULING) +#if defined(_POSIX_SPIN_LOCKS) && (_POSIX_SPIN_LOCKS > 1) +# define USE_PSPIN +#endif +#endif + #define checkStatus(status,message) \ if ((status)) { \ errlogPrintf("epicsSpin %s failed: error %s\n", \ (message), strerror((status))); \ } -#if defined(_POSIX_SPIN_LOCKS) && (_POSIX_SPIN_LOCKS > 1) +#ifdef USE_PSPIN /* * POSIX SPIN LOCKS IMPLEMENTATION @@ -36,7 +48,7 @@ typedef struct epicsSpin { pthread_spinlock_t lock; } epicsSpin; -epicsSpinId epicsSpinCreate() { +epicsSpinId epicsSpinCreate(void) { epicsSpin *spin; int status; @@ -70,6 +82,8 @@ void epicsSpinLock(epicsSpinId spin) { status = pthread_spin_lock(&spin->lock); checkStatus(status, "pthread_spin_lock"); + if (status) + cantProceed("epicsSpinLock pspin"); } int epicsSpinTryLock(epicsSpinId spin) { @@ -79,7 +93,7 @@ int epicsSpinTryLock(epicsSpinId spin) { if (status == EBUSY) return 1; checkStatus(status, "pthread_spin_trylock"); - return 0; + return status; } void epicsSpinUnlock(epicsSpinId spin) { @@ -89,7 +103,7 @@ void epicsSpinUnlock(epicsSpinId spin) { checkStatus(status, "pthread_spin_unlock"); } -#else /* defined(_POSIX_SPIN_LOCKS) && (_POSIX_SPIN_LOCKS > 1) */ +#else /* USE_PSPIN */ /* * POSIX MUTEX IMPLEMENTATION @@ -99,7 +113,7 @@ typedef struct epicsSpin { pthread_mutex_t lock; } epicsSpin; -epicsSpinId epicsSpinCreate() { +epicsSpinId epicsSpinCreate(void) { epicsSpin *spin; int status; @@ -133,6 +147,8 @@ void epicsSpinLock(epicsSpinId spin) { status = pthread_mutex_lock(&spin->lock); checkStatus(status, "pthread_mutex_lock"); + if (status) + cantProceed("epicsSpinLock pmutex"); } int epicsSpinTryLock(epicsSpinId spin) { @@ -142,7 +158,7 @@ int epicsSpinTryLock(epicsSpinId spin) { if (status == EBUSY) return 1; checkStatus(status, "pthread_mutex_trylock"); - return 0; + return status; } void epicsSpinUnlock(epicsSpinId spin) { @@ -152,4 +168,13 @@ void epicsSpinUnlock(epicsSpinId spin) { checkStatus(status, "pthread_mutex_unlock"); } -#endif /* defined(_POSIX_SPIN_LOCKS) && (_POSIX_SPIN_LOCKS > 1) */ +#endif /* USE_PSPIN */ + + +epicsSpinId epicsSpinMustCreate(void) +{ + epicsSpinId ret = epicsSpinCreate(); + if(!ret) + cantProceed("epicsSpinMustCreate fails"); + return ret; +} diff --git a/src/libCom/osi/os/vxWorks/osdSpin.c b/src/libCom/osi/os/vxWorks/osdSpin.c index 9b545347d..990453124 100644 --- a/src/libCom/osi/os/vxWorks/osdSpin.c +++ b/src/libCom/osi/os/vxWorks/osdSpin.c @@ -2,18 +2,22 @@ * Copyright (c) 2012 Helmholtz-Zentrum Berlin * fuer Materialien und Energie GmbH. * Copyright (c) 2012 ITER Organization. +* Copyright (c) 2013 Brookhaven Science Assoc. as Operator of Brookhaven +* National Laboratory. * EPICS BASE is distributed subject to a Software License Agreement found * in file LICENSE that is included with this distribution. \*************************************************************************/ /* * Author: Ralph Lange + * Michael Davidsaver * - * based on epicsInterrupt.c (vxWorks implementation) by Marty Kraimer + * Inspired by Linux UP spinlocks implemention + * include/linux/spinlock_api_up.h */ /* - * vxWorks (single CPU): LOCK INTERRUPT + * vxWorks (single CPU): LOCK INTERRUPT and DISABLE PREEMPTION * * CAVEAT: * This implementation will not compile on vxWorks SMP architectures. @@ -23,23 +27,38 @@ #include #include +#include #include "epicsSpin.h" typedef struct epicsSpin { int key; + unsigned int locked; } epicsSpin; -epicsSpinId epicsSpinCreate() { +epicsSpinId epicsSpinCreate(void) { return calloc(1, sizeof(epicsSpin)); } +epicsSpinId epicsSpinMustCreate(void) +{ + epicsSpinId ret = epicsSpinCreate(); + if(!ret) + cantProceed("epicsSpinMustCreate fails"); + return ret; +} + void epicsSpinDestroy(epicsSpinId spin) { free(spin); } void epicsSpinLock(epicsSpinId spin) { - spin->key = intLock(); + int key = intLock(); + if(!intContext()) + taskLock(); + assert(!spin->locked); + spin->key = key; + spin->locked = 1; } int epicsSpinTryLock(epicsSpinId spin) { @@ -48,5 +67,10 @@ int epicsSpinTryLock(epicsSpinId spin) { } void epicsSpinUnlock(epicsSpinId spin) { - intUnlock(spin->key); + int key = spin->key; + assert(spin->locked); + spin->key = spin->locked = 0; + intUnlock(key); + if(!intContext()) + taskUnlock(); } From 3ba5bf943eef6dbc8b7120c97a3d73938fea654f Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 23 Jul 2014 18:40:08 -0400 Subject: [PATCH 02/10] epicsSpin: better error messages when mis-use is detected --- src/libCom/osi/os/RTEMS/osdSpin.c | 20 ++++++++++++++++---- src/libCom/osi/os/vxWorks/osdSpin.c | 15 +++++++++++++-- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/libCom/osi/os/RTEMS/osdSpin.c b/src/libCom/osi/os/RTEMS/osdSpin.c index 8e6a2096c..1d5a1d108 100644 --- a/src/libCom/osi/os/RTEMS/osdSpin.c +++ b/src/libCom/osi/os/RTEMS/osdSpin.c @@ -30,8 +30,7 @@ #include #include - -#include +#include #include "epicsSpin.h" @@ -61,7 +60,17 @@ void epicsSpinLock(epicsSpinId spin) { rtems_interrupt_disable (level); _Thread_Disable_dispatch(); spin->level = level; - assert(!spin->locked); + if(spin->locked) { + rtems_interrupt_enable (level); + _Thread_Enable_dispatch(); + if(!rtems_interrupt_is_in_progress()) { + printk("Deadlock in epicsSpinLock(%p). Either recursive lock, missing unlock, or locked by sleeping thread.", spin); + cantProceed(NULL); + } else { + printk("epicsSpinLock(%p) failure in ISR. Either recursive lock, missing unlock, or locked by sleeping thread.\n", spin); + } + return; + } spin->locked = 1; } @@ -72,7 +81,10 @@ int epicsSpinTryLock(epicsSpinId spin) { void epicsSpinUnlock(epicsSpinId spin) { rtems_interrupt_level level = spin->level; - assert(spin->locked); + if(!spin->locked) { + printk("epicsSpinUnlock(%p) failure. lock not taken\n", spin); + return; + } spin->level = spin->locked = 0; rtems_interrupt_enable (level); _Thread_Enable_dispatch(); diff --git a/src/libCom/osi/os/vxWorks/osdSpin.c b/src/libCom/osi/os/vxWorks/osdSpin.c index 990453124..06cb73c63 100644 --- a/src/libCom/osi/os/vxWorks/osdSpin.c +++ b/src/libCom/osi/os/vxWorks/osdSpin.c @@ -56,7 +56,16 @@ void epicsSpinLock(epicsSpinId spin) { int key = intLock(); if(!intContext()) taskLock(); - assert(!spin->locked); + if(spin->locked) { + intUnlock(key); + if(!intContext()) { + taskUnlock(); + cantProceed("Deadlock in epicsSpinLock(). Either recursive lock, missing unlock, or locked by sleeping thread."); + } else { + epicsInterruptContextMessage("epicsSpinLock() failure in ISR. Either recursive lock, missing unlock, or locked by sleeping thread.\n"); + } + return; + } spin->key = key; spin->locked = 1; } @@ -68,7 +77,9 @@ int epicsSpinTryLock(epicsSpinId spin) { void epicsSpinUnlock(epicsSpinId spin) { int key = spin->key; - assert(spin->locked); + if(!spin->locked) { + epicsInterruptContextMessage("epicsSpinUnlock() failure. lock not taken\n"); + } spin->key = spin->locked = 0; intUnlock(key); if(!intContext()) From 87a6688c17a83f8ddb28bbdf659550af22c2f609 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Thu, 24 Jul 2014 10:23:55 -0400 Subject: [PATCH 03/10] epicsSpin: remove redundant cantProceed() messages --- src/libCom/osi/os/posix/osdSpin.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libCom/osi/os/posix/osdSpin.c b/src/libCom/osi/os/posix/osdSpin.c index 2b7c31c22..6ebc9f785 100644 --- a/src/libCom/osi/os/posix/osdSpin.c +++ b/src/libCom/osi/os/posix/osdSpin.c @@ -83,7 +83,7 @@ void epicsSpinLock(epicsSpinId spin) { status = pthread_spin_lock(&spin->lock); checkStatus(status, "pthread_spin_lock"); if (status) - cantProceed("epicsSpinLock pspin"); + cantProceed(NULL); } int epicsSpinTryLock(epicsSpinId spin) { @@ -148,7 +148,7 @@ void epicsSpinLock(epicsSpinId spin) { status = pthread_mutex_lock(&spin->lock); checkStatus(status, "pthread_mutex_lock"); if (status) - cantProceed("epicsSpinLock pmutex"); + cantProceed(NULL); } int epicsSpinTryLock(epicsSpinId spin) { From 9b6e270b978d3f32a017faeb23c144c39102cf94 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Thu, 24 Jul 2014 16:33:35 -0500 Subject: [PATCH 04/10] Final spinlock tidying-up * Abort epicsSpinTest() if epicsSpinCreate() returns NULL * Adjust RELEASE_NOTES that describe the implementations. --- documentation/RELEASE_NOTES.html | 13 +++++++------ src/libCom/test/epicsSpinTest.c | 6 +++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/documentation/RELEASE_NOTES.html b/documentation/RELEASE_NOTES.html index d20d9bc65..f2f137e16 100644 --- a/documentation/RELEASE_NOTES.html +++ b/documentation/RELEASE_NOTES.html @@ -117,12 +117,13 @@ The basic rules which device support must follow are:

The new header file epicsSpin.h adds a portable spin-locks API which is intended for locking very short sections of code (typically one or two lines of C or C++) to provide a critical section that protects against race conditions. -On Posix platforms this uses the pthread_spinlock_t type if it's available but -falls back to a pthread_mutex_t if not; on the UP VxWorks and RTEMS platforms -the implementations lock out the CPU interrupts; the default implementation -(used where no better implementation is available for the platform) uses an -epicsMutex. Spin-locks may not be taken recursively, and the code inside the -critical section should always be short and deterministic.

+On Posix platforms this uses the pthread_spinlock_t type if it's available and +the build is not configured to use Posix thread priorities, but otherwise it +falls back to a pthread_mutex_t. On the UP VxWorks and RTEMS platforms the +implementations lock out CPU interrupts and disable task preemption while a +spin-lock is held. The default implementation (used when no other implementation +is provided) uses an epicsMutex. Spin-locks may not be taken recursively, and +the code inside the critical section should be short and deterministic.

Improvements to aToIPAddr()

diff --git a/src/libCom/test/epicsSpinTest.c b/src/libCom/test/epicsSpinTest.c index 707345b89..ff2a89c85 100644 --- a/src/libCom/test/epicsSpinTest.c +++ b/src/libCom/test/epicsSpinTest.c @@ -94,6 +94,8 @@ void epicsSpinPerformance () /* Initialize spinlock */ spin = epicsSpinCreate(); + if (!spin) + testAbort("epicsSpinCreate() returned NULL"); /* test a single lock pair */ epicsTimeGetCurrent(&begin); @@ -126,7 +128,7 @@ static void verifyTryLock() { struct verifyTryLock verify; - verify.spin = epicsSpinCreate(); + verify.spin = epicsSpinMustCreate(); verify.done = epicsEventMustCreate(epicsEventEmpty); testOk1(epicsSpinTryLock(verify.spin) == 0); @@ -159,6 +161,8 @@ MAIN(epicsSpinTest) verifyTryLock(); spin = epicsSpinCreate(); + if (!spin) + testAbort("epicsSpinCreate() returned NULL"); id = (epicsThreadId *) calloc(nthreads, sizeof(epicsThreadId)); name = (char **) calloc(nthreads, sizeof(char *)); From 5824f989729996adad8a5b81b79949fd224525de Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 25 Jul 2014 15:11:54 -0400 Subject: [PATCH 05/10] epicsSpinTest: fix verifyTryLock() avoid sleeping with a spinlock held. Now test only works on SMP systems. --- src/libCom/test/epicsSpinTest.c | 67 ++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/src/libCom/test/epicsSpinTest.c b/src/libCom/test/epicsSpinTest.c index ff2a89c85..b9b27aa06 100644 --- a/src/libCom/test/epicsSpinTest.c +++ b/src/libCom/test/epicsSpinTest.c @@ -22,6 +22,7 @@ #include "epicsTime.h" #include "epicsThread.h" +#include "epicsAtomic.h" #include "epicsSpin.h" #include "epicsEvent.h" #include "errlog.h" @@ -110,38 +111,78 @@ void epicsSpinPerformance () testDiag("lock()*1/unlock()*1 takes %f microseconds", delay); } +struct verifyTryLock; + +struct verifyTryLockEnt { + epicsEventId done; + struct verifyTryLock *main; +}; + struct verifyTryLock { epicsSpinId spin; - epicsEventId done; + int flag; + struct verifyTryLockEnt *ents; }; static void verifyTryLockThread(void *pArg) { - struct verifyTryLock *pVerify = - (struct verifyTryLock *) pArg; + struct verifyTryLockEnt *pVerify = + (struct verifyTryLockEnt *) pArg; + + while(epicsAtomicGetIntT(&pVerify->main->flag)==0) { + int ret = epicsSpinTryLock(pVerify->main->spin); + if(ret!=0) { + epicsAtomicCmpAndSwapIntT(&pVerify->main->flag, 0, ret); + break; + } else + epicsSpinUnlock(pVerify->main->spin); + } - testOk1(epicsSpinTryLock(pVerify->spin)); epicsEventSignal(pVerify->done); } +/* Start one thread per CPU which will all try lock + * the same spinlock. They break as soon as one + * fails to take the lock. + */ static void verifyTryLock() { + int N, i; struct verifyTryLock verify; + N = epicsThreadGetCPUs(); + if(N==1) { + testSkip(1, "verifyTryLock() only for SMP systems"); + return; + } + + verify.flag = 0; verify.spin = epicsSpinMustCreate(); - verify.done = epicsEventMustCreate(epicsEventEmpty); - testOk1(epicsSpinTryLock(verify.spin) == 0); + testDiag("Starting %d spinners", N); - epicsThreadCreate("verifyTryLockThread", 40, - epicsThreadGetStackSize(epicsThreadStackSmall), - verifyTryLockThread, &verify); + verify.ents = calloc(N, sizeof(*verify.ents)); + for(i=0; i Date: Fri, 25 Jul 2014 15:12:02 -0400 Subject: [PATCH 06/10] epicsSpin: try lock return non-blocking Avoid cantProceed() in try lock, even for undefined behavior. --- src/libCom/osi/os/RTEMS/osdSpin.c | 13 +++++++++++-- src/libCom/osi/os/vxWorks/osdSpin.c | 12 +++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/libCom/osi/os/RTEMS/osdSpin.c b/src/libCom/osi/os/RTEMS/osdSpin.c index 1d5a1d108..f2587f76d 100644 --- a/src/libCom/osi/os/RTEMS/osdSpin.c +++ b/src/libCom/osi/os/RTEMS/osdSpin.c @@ -59,7 +59,6 @@ void epicsSpinLock(epicsSpinId spin) { rtems_interrupt_level level; rtems_interrupt_disable (level); _Thread_Disable_dispatch(); - spin->level = level; if(spin->locked) { rtems_interrupt_enable (level); _Thread_Enable_dispatch(); @@ -71,11 +70,21 @@ void epicsSpinLock(epicsSpinId spin) { } return; } + spin->level = level; spin->locked = 1; } int epicsSpinTryLock(epicsSpinId spin) { - epicsSpinLock(spin); + rtems_interrupt_level level; + rtems_interrupt_disable (level); + _Thread_Disable_dispatch(); + if(spin->locked) { + rtems_interrupt_enable (level); + _Thread_Enable_dispatch(); + return 1; + } + spin->level = level; + spin->locked = 1; return 0; } diff --git a/src/libCom/osi/os/vxWorks/osdSpin.c b/src/libCom/osi/os/vxWorks/osdSpin.c index 06cb73c63..97d490916 100644 --- a/src/libCom/osi/os/vxWorks/osdSpin.c +++ b/src/libCom/osi/os/vxWorks/osdSpin.c @@ -71,7 +71,17 @@ void epicsSpinLock(epicsSpinId spin) { } int epicsSpinTryLock(epicsSpinId spin) { - epicsSpinLock(spin); + int key = intLock(); + if(!intContext()) + taskLock(); + if(spin->locked) { + intUnlock(key); + if(!intContext()) + taskUnlock(); + return 1; + } + spin->key = key; + spin->locked = 1; return 0; } From 752549d1c866bd289ed43e87381911bca0e9cbba Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Fri, 25 Jul 2014 15:05:58 -0500 Subject: [PATCH 07/10] Fix epicsAtomic headers when used from C code Several C++ and C99-isms crept in. --- src/libCom/osi/epicsAtomicDefault.h | 46 ++++++++++++++++------ src/libCom/osi/os/posix/epicsAtomicOSD.h | 2 +- src/libCom/osi/os/solaris/epicsAtomicOSD.h | 2 +- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/libCom/osi/epicsAtomicDefault.h b/src/libCom/osi/epicsAtomicDefault.h index ec0975daf..26115ce63 100644 --- a/src/libCom/osi/epicsAtomicDefault.h +++ b/src/libCom/osi/epicsAtomicDefault.h @@ -34,9 +34,11 @@ extern "C" { #ifndef EPICS_ATOMIC_INCR_INTT EPICS_ATOMIC_INLINE int epicsAtomicIncrIntT ( int * pTarget ) { - EpicsAtomicLockKey key; + EpicsAtomicLockKey key; + int result; + epicsAtomicLock ( & key ); - const int result = ++(*pTarget); + result = ++(*pTarget); epicsAtomicUnlock ( & key ); return result; } @@ -46,8 +48,10 @@ EPICS_ATOMIC_INLINE int epicsAtomicIncrIntT ( int * pTarget ) EPICS_ATOMIC_INLINE size_t epicsAtomicIncrSizeT ( size_t * pTarget ) { EpicsAtomicLockKey key; + size_t result; + epicsAtomicLock ( & key ); - const size_t result = ++(*pTarget); + result = ++(*pTarget); epicsAtomicUnlock ( & key ); return result; } @@ -60,8 +64,10 @@ EPICS_ATOMIC_INLINE size_t epicsAtomicIncrSizeT ( size_t * pTarget ) EPICS_ATOMIC_INLINE int epicsAtomicDecrIntT ( int * pTarget ) { EpicsAtomicLockKey key; + int result; + epicsAtomicLock ( & key ); - const int result = --(*pTarget); + result = --(*pTarget); epicsAtomicUnlock ( & key ); return result; } @@ -71,8 +77,10 @@ EPICS_ATOMIC_INLINE int epicsAtomicDecrIntT ( int * pTarget ) EPICS_ATOMIC_INLINE size_t epicsAtomicDecrSizeT ( size_t * pTarget ) { EpicsAtomicLockKey key; + size_t result; + epicsAtomicLock ( & key ); - const size_t result = --(*pTarget); + result = --(*pTarget); epicsAtomicUnlock ( & key ); return result; } @@ -85,8 +93,10 @@ EPICS_ATOMIC_INLINE size_t epicsAtomicDecrSizeT ( size_t * pTarget ) EPICS_ATOMIC_INLINE int epicsAtomicAddIntT ( int * pTarget, int delta ) { EpicsAtomicLockKey key; + int result; + epicsAtomicLock ( & key ); - const int result = *pTarget += delta; + result = *pTarget += delta; epicsAtomicUnlock ( & key ); return result; } @@ -96,8 +106,10 @@ EPICS_ATOMIC_INLINE int epicsAtomicAddIntT ( int * pTarget, int delta ) EPICS_ATOMIC_INLINE size_t epicsAtomicAddSizeT ( size_t * pTarget, size_t delta ) { EpicsAtomicLockKey key; + size_t result; + epicsAtomicLock ( & key ); - const size_t result = *pTarget += delta; + result = *pTarget += delta; epicsAtomicUnlock ( & key ); return result; } @@ -107,8 +119,10 @@ EPICS_ATOMIC_INLINE size_t epicsAtomicAddSizeT ( size_t * pTarget, size_t delta EPICS_ATOMIC_INLINE size_t epicsAtomicSubSizeT ( size_t * pTarget, size_t delta ) { EpicsAtomicLockKey key; + size_t result; + epicsAtomicLock ( & key ); - const size_t result = *pTarget -= delta; + result = *pTarget -= delta; epicsAtomicUnlock ( & key ); return result; } @@ -176,9 +190,11 @@ EPICS_ATOMIC_INLINE EpicsAtomicPtrT #ifndef EPICS_ATOMIC_CAS_INTT EPICS_ATOMIC_INLINE int epicsAtomicCmpAndSwapIntT ( int * pTarget, int oldval, int newval ) { - EpicsAtomicLockKey key; + EpicsAtomicLockKey key; + int cur; + epicsAtomicLock ( & key ); - const int cur = *pTarget; + cur = *pTarget; if ( cur == oldval ) { *pTarget = newval; } @@ -191,9 +207,11 @@ EPICS_ATOMIC_INLINE int epicsAtomicCmpAndSwapIntT ( int * pTarget, int oldval, i EPICS_ATOMIC_INLINE size_t epicsAtomicCmpAndSwapSizeT ( size_t * pTarget, size_t oldval, size_t newval ) { - EpicsAtomicLockKey key; + EpicsAtomicLockKey key; + size_t cur; + epicsAtomicLock ( & key ); - const size_t cur = *pTarget; + cur = *pTarget; if ( cur == oldval ) { *pTarget = newval; } @@ -208,8 +226,10 @@ EPICS_ATOMIC_INLINE EpicsAtomicPtrT epicsAtomicCmpAndSwapPtrT ( EpicsAtomicPtrT oldval, EpicsAtomicPtrT newval ) { EpicsAtomicLockKey key; + EpicsAtomicPtrT cur; + epicsAtomicLock ( & key ); - const EpicsAtomicPtrT cur = *pTarget; + cur = *pTarget; if ( cur == oldval ) { *pTarget = newval; } diff --git a/src/libCom/osi/os/posix/epicsAtomicOSD.h b/src/libCom/osi/os/posix/epicsAtomicOSD.h index 76c70cd54..ff972016d 100644 --- a/src/libCom/osi/os/posix/epicsAtomicOSD.h +++ b/src/libCom/osi/os/posix/epicsAtomicOSD.h @@ -16,7 +16,7 @@ #ifndef epicsAtomicOSD_h #define epicsAtomicOSD_h -struct EpicsAtomicLockKey {}; +typedef struct EpicsAtomicLockKey {} EpicsAtomicLockKey; #ifdef __cplusplus extern "C" { diff --git a/src/libCom/osi/os/solaris/epicsAtomicOSD.h b/src/libCom/osi/os/solaris/epicsAtomicOSD.h index 5845b6b39..9c7808a49 100644 --- a/src/libCom/osi/os/solaris/epicsAtomicOSD.h +++ b/src/libCom/osi/os/solaris/epicsAtomicOSD.h @@ -159,7 +159,7 @@ EPICS_ATOMIC_INLINE size_t epicsAtomicSubSizeT ( size_t * pTarget, #endif /* ifdef __SunOS_5_10 */ -struct EpicsAtomicLockKey {}; +typedef struct EpicsAtomicLockKey {} EpicsAtomicLockKey; #ifdef __cplusplus extern "C" { From 2a9d05248f193471fd8b2b871a7477fa08096764 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Fri, 25 Jul 2014 16:21:03 -0500 Subject: [PATCH 08/10] Fix epicsSpinTest.c spinThread tests Runs many more rounds, without blocking with the lock held. --- src/libCom/test/epicsSpinTest.c | 45 ++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/libCom/test/epicsSpinTest.c b/src/libCom/test/epicsSpinTest.c index b9b27aa06..21d70e6aa 100644 --- a/src/libCom/test/epicsSpinTest.c +++ b/src/libCom/test/epicsSpinTest.c @@ -32,22 +32,26 @@ typedef struct info { int threadnum; epicsSpinId spin; - int quit; + int *counter; + int rounds; + epicsEventId done; } info; +#define spinDelay 0.016667 + void spinThread(void *arg) { info *pinfo = (info *) arg; testDiag("spinThread %d starting", pinfo->threadnum); - while (pinfo->quit--) { + epicsThreadSleep(0.1); /* Try to align threads */ + while (pinfo->rounds--) { + epicsThreadSleep(spinDelay); epicsSpinLock(pinfo->spin); - testPass("spinThread %d epicsSpinLock taken", pinfo->threadnum); - epicsThreadSleep(.1); + pinfo->counter[0]++; epicsSpinUnlock(pinfo->spin); - epicsThreadSleep(.9); } testDiag("spinThread %d exiting", pinfo->threadnum); - return; + epicsEventSignal(pinfo->done); } static void lockPair(struct epicsSpin *spin) @@ -188,16 +192,15 @@ static void verifyTryLock() MAIN(epicsSpinTest) { const int nthreads = 3; - const int nrounds = 5; + const int nrounds = 500; unsigned int stackSize; epicsThreadId *id; - int i; + int i, counter; char **name; - void **arg; info **pinfo; epicsSpinId spin; - testPlan(1 + nthreads * nrounds); + testPlan(2); verifyTryLock(); @@ -207,22 +210,34 @@ MAIN(epicsSpinTest) id = (epicsThreadId *) calloc(nthreads, sizeof(epicsThreadId)); name = (char **) calloc(nthreads, sizeof(char *)); - arg = (void **) calloc(nthreads, sizeof(void *)); pinfo = (info **) calloc(nthreads, sizeof(info *)); stackSize = epicsThreadGetStackSize(epicsThreadStackSmall); + counter = 0; for (i = 0; i < nthreads; i++) { name[i] = (char *) calloc(10, sizeof(char)); sprintf(name[i],"task%d",i); pinfo[i] = (info *) calloc(1, sizeof(info)); pinfo[i]->threadnum = i; pinfo[i]->spin = spin; - pinfo[i]->quit = nrounds; - arg[i] = pinfo[i]; + pinfo[i]->counter = &counter; + pinfo[i]->rounds = nrounds; + pinfo[i]->done = epicsEventMustCreate(epicsEventEmpty); id[i] = epicsThreadCreate(name[i], 40, stackSize, spinThread, - arg[i]); + pinfo[i]); } - epicsThreadSleep(2.0 + nrounds); + for (i = 0; i < nthreads; i++) { + epicsEventMustWait(pinfo[i]->done); + epicsEventDestroy(pinfo[i]->done); + free(name[i]); + } + testOk(counter == nthreads * nrounds, "Loops run = %d (expecting %d)", + counter, nthreads * nrounds); + + free(pinfo); + free(name); + free(id); + epicsSpinDestroy(spin); epicsSpinPerformance(); From 9cb65e5408e2be25f109a8a676b5681e8c3d9109 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 25 Jul 2014 18:17:02 -0400 Subject: [PATCH 09/10] epicsSpinTest: plug some leaks --- src/libCom/test/epicsSpinTest.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libCom/test/epicsSpinTest.c b/src/libCom/test/epicsSpinTest.c index 21d70e6aa..c45348beb 100644 --- a/src/libCom/test/epicsSpinTest.c +++ b/src/libCom/test/epicsSpinTest.c @@ -113,6 +113,7 @@ void epicsSpinPerformance () delay /= N * 100u; /* convert to delay per lock pair */ delay *= 1e6; /* convert to micro seconds */ testDiag("lock()*1/unlock()*1 takes %f microseconds", delay); + epicsSpinDestroy(spin); } struct verifyTryLock; @@ -230,6 +231,7 @@ MAIN(epicsSpinTest) epicsEventMustWait(pinfo[i]->done); epicsEventDestroy(pinfo[i]->done); free(name[i]); + free(pinfo[i]); } testOk(counter == nthreads * nrounds, "Loops run = %d (expecting %d)", counter, nthreads * nrounds); From c4a1208d6ee46c245c6d9106668e75fd11a3c391 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Fri, 25 Jul 2014 18:17:27 -0400 Subject: [PATCH 10/10] epicsSpinTest: add to libCom test harness --- src/libCom/test/epicsRunLibComTests.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libCom/test/epicsRunLibComTests.c b/src/libCom/test/epicsRunLibComTests.c index f77821199..5b1f69492 100644 --- a/src/libCom/test/epicsRunLibComTests.c +++ b/src/libCom/test/epicsRunLibComTests.c @@ -18,6 +18,7 @@ int epicsThreadTest(void); int epicsTimerTest(void); +int epicsSpinTest(void); int epicsAlgorithm(void); int epicsEllTest(void); int epicsEnvTest(void); @@ -60,6 +61,8 @@ void epicsRunLibComTests(void) */ runTest(epicsTimerTest); + runTest(epicsSpinTest); + runTest(epicsAlgorithm); runTest(epicsEllTest);