From 59ec8d897dec4e836c2a1582e5933d3e392ba025 Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Fri, 10 Aug 2018 10:17:13 -0400 Subject: [PATCH 1/7] Expose callback queue status This allows tools like iocStats to monitor the queue status of the callback queues. This fixes lp:1786540. --- src/ioc/db/callback.c | 51 ++++++++++++++++++++++++++-- src/ioc/db/callback.h | 9 +++++ src/ioc/db/dbIocRegister.c | 24 +++++++++++++ src/ioc/db/dbScan.c | 37 ++++++++++++++++++++ src/ioc/db/dbScan.h | 9 +++++ src/libCom/ring/epicsRingBytes.c | 29 ++++++++++++++-- src/libCom/ring/epicsRingBytes.h | 3 ++ src/libCom/ring/epicsRingPointer.cpp | 12 +++++++ src/libCom/ring/epicsRingPointer.h | 38 +++++++++++++++++++-- 9 files changed, 205 insertions(+), 7 deletions(-) diff --git a/src/ioc/db/callback.c b/src/ioc/db/callback.c index ae074141c..2b95cc5af 100644 --- a/src/ioc/db/callback.c +++ b/src/ioc/db/callback.c @@ -54,6 +54,7 @@ typedef struct cbQueueSet { epicsEventId semWakeUp; epicsRingPointerId queue; int queueOverflow; + int queueOverflows; int shutdown; int threadsConfigured; int threadsRunning; @@ -99,10 +100,55 @@ int callbackSetQueueSize(int size) fprintf(stderr, "Callback system already initialized\n"); return -1; } - callbackQueueSize = size; + epicsAtomicSetIntT(&callbackQueueSize, size); return 0; } +int callbackQueueStatus(const int reset, callbackQueueStats *result) +{ + if (!callbackIsInit) return -1; + int ret; + if (result) { + result->size = epicsAtomicGetIntT(&callbackQueueSize); + int prio; + for(prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) { + epicsRingPointerId qId = callbackQueue[prio].queue; + result->numUsed[prio] = epicsRingPointerGetUsed(qId); + result->maxUsed[prio] = epicsRingPointerGetHighWaterMark(qId); + result->numOverflow[prio] = epicsAtomicGetIntT(&callbackQueue[prio].queueOverflows); + } + ret = 0; + } else { + ret = -2; + } + if (reset) { + int prio; + for(prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) { + epicsRingPointerResetHighWaterMark(callbackQueue[prio].queue); + } + } + return ret; +} + +void callbackQueuePrintStatus(const int reset) +{ + callbackQueueStats stats; + if (callbackQueueStatus(reset, &stats) == -1) { + fprintf(stderr, "Callback system not initialized, yet. Please run " + "iocInit before using this command.\n"); + return; + } + + printf("PRIORITY HIGH-WATER MARK ITEMS IN Q Q SIZE %% USED Q OVERFLOWS\n"); + int prio; + for (prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) { + double qusage = 100.0 * stats.numUsed[prio] / stats.size; + printf("%8s %15d %10d %6d %6.1f %11d\n", threadNamePrefix[prio], + stats.maxUsed[prio], stats.numUsed[prio], stats.size, + qusage, stats.numOverflow[prio]); + } +} + int callbackParallelThreads(int count, const char *prio) { if (callbackIsInit) { @@ -240,7 +286,7 @@ void callbackInit(void) epicsThreadId tid; callbackQueue[i].semWakeUp = epicsEventMustCreate(epicsEventEmpty); - callbackQueue[i].queue = epicsRingPointerLockedCreate(callbackQueueSize); + callbackQueue[i].queue = epicsRingPointerLockedCreate(epicsAtomicGetIntT(&callbackQueueSize)); if (callbackQueue[i].queue == 0) cantProceed("epicsRingPointerLockedCreate failed for %s\n", threadNamePrefix[i]); @@ -290,6 +336,7 @@ int callbackRequest(CALLBACK *pcallback) if (!pushOK) { epicsInterruptContextMessage(fullMessage[priority]); mySet->queueOverflow = TRUE; + epicsAtomicIncrIntT(&mySet->queueOverflows); return S_db_bufFull; } epicsEventSignal(mySet->semWakeUp); diff --git a/src/ioc/db/callback.h b/src/ioc/db/callback.h index fa626d1d0..dd13cdb81 100644 --- a/src/ioc/db/callback.h +++ b/src/ioc/db/callback.h @@ -48,6 +48,13 @@ typedef epicsCallback CALLBACK; typedef void (*CALLBACKFUNC)(struct callbackPvt*); +typedef struct callbackQueueStats { + int size; + int numUsed[NUM_CALLBACK_PRIORITIES]; + int maxUsed[NUM_CALLBACK_PRIORITIES]; + int numOverflow[NUM_CALLBACK_PRIORITIES]; +} callbackQueueStats; + #define callbackSetCallback(PFUN, PCALLBACK) \ ( (PCALLBACK)->callback = (PFUN) ) #define callbackSetPriority(PRIORITY, PCALLBACK) \ @@ -73,6 +80,8 @@ epicsShareFunc void callbackCancelDelayed(CALLBACK *pcallback); epicsShareFunc void callbackRequestProcessCallbackDelayed( CALLBACK *pCallback, int Priority, void *pRec, double seconds); epicsShareFunc int callbackSetQueueSize(int size); +epicsShareFunc int callbackQueueStatus(const int reset, callbackQueueStats *result); +void callbackQueuePrintStatus(const int reset); epicsShareFunc int callbackParallelThreads(int count, const char *prio); #ifdef __cplusplus diff --git a/src/ioc/db/dbIocRegister.c b/src/ioc/db/dbIocRegister.c index 4d0b88cd9..cecf75608 100644 --- a/src/ioc/db/dbIocRegister.c +++ b/src/ioc/db/dbIocRegister.c @@ -296,6 +296,17 @@ static void scanOnceSetQueueSizeCallFunc(const iocshArgBuf *args) scanOnceSetQueueSize(args[0].ival); } +/* scanOnceQueueStatus */ +static const iocshArg scanOnceQueueStatusArg0 = { "reset",iocshArgInt}; +static const iocshArg * const scanOnceQueueStatusArgs[1] = + {&scanOnceQueueStatusArg0}; +static const iocshFuncDef scanOnceQueueStatusFuncDef = + {"scanOnceQueueStatus",1,scanOnceQueueStatusArgs}; +static void scanOnceQueueStatusCallFunc(const iocshArgBuf *args) +{ + scanOnceQueuePrintStatus(args[0].ival); +} + /* scanppl */ static const iocshArg scanpplArg0 = { "rate",iocshArgDouble}; static const iocshArg * const scanpplArgs[1] = {&scanpplArg0}; @@ -335,6 +346,17 @@ static void callbackSetQueueSizeCallFunc(const iocshArgBuf *args) callbackSetQueueSize(args[0].ival); } +/* callbackQueueStatus */ +static const iocshArg callbackQueueStatusArg0 = { "reset", iocshArgInt}; +static const iocshArg * const callbackQueueStatusArgs[1] = + {&callbackQueueStatusArg0}; +static const iocshFuncDef callbackQueueStatusFuncDef = + {"callbackQueueStatus",1,callbackQueueStatusArgs}; +static void callbackQueueStatusCallFunc(const iocshArgBuf *args) +{ + callbackQueuePrintStatus(args[0].ival); +} + /* callbackParallelThreads */ static const iocshArg callbackParallelThreadsArg0 = { "no of threads", iocshArgInt}; static const iocshArg callbackParallelThreadsArg1 = { "priority", iocshArgString}; @@ -441,12 +463,14 @@ void dbIocRegister(void) iocshRegister(&dbLockShowLockedFuncDef,dbLockShowLockedCallFunc); iocshRegister(&scanOnceSetQueueSizeFuncDef,scanOnceSetQueueSizeCallFunc); + iocshRegister(&scanOnceQueueStatusFuncDef,scanOnceQueueStatusCallFunc); iocshRegister(&scanpplFuncDef,scanpplCallFunc); iocshRegister(&scanpelFuncDef,scanpelCallFunc); iocshRegister(&postEventFuncDef,postEventCallFunc); iocshRegister(&scanpiolFuncDef,scanpiolCallFunc); iocshRegister(&callbackSetQueueSizeFuncDef,callbackSetQueueSizeCallFunc); + iocshRegister(&callbackQueueStatusFuncDef,callbackQueueStatusCallFunc); iocshRegister(&callbackParallelThreadsFuncDef,callbackParallelThreadsCallFunc); /* Needed before callback system is initialized */ diff --git a/src/ioc/db/dbScan.c b/src/ioc/db/dbScan.c index e5c78fea1..c94632794 100644 --- a/src/ioc/db/dbScan.c +++ b/src/ioc/db/dbScan.c @@ -24,6 +24,7 @@ #include "cantProceed.h" #include "dbDefs.h" #include "ellLib.h" +#include "epicsAtomic.h" #include "epicsEvent.h" #include "epicsMutex.h" #include "epicsPrint.h" @@ -63,6 +64,7 @@ static volatile enum ctl scanCtl; static int onceQueueSize = 1000; static epicsEventId onceSem; static epicsRingBytesId onceQ; +static int onceQOverruns = 0; static epicsThreadId onceTaskId; static void *exitOnce; @@ -676,6 +678,7 @@ int scanOnceCallback(struct dbCommon *precord, once_complete cb, void *usr) if (!pushOK) { if (newOverflow) errlogPrintf("scanOnce: Ring buffer overflow\n"); newOverflow = FALSE; + epicsAtomicIncrIntT(&onceQOverruns); } else { newOverflow = TRUE; } @@ -722,6 +725,40 @@ int scanOnceSetQueueSize(int size) return 0; } +int scanOnceQueueStatus(const int reset, scanOnceQueueStats *result) +{ + if (!onceQ) return -1; + int ret; + if (result) { + result->size = epicsRingBytesSize(onceQ) / sizeof(onceEntry); + result->numUsed = epicsRingBytesUsedBytes(onceQ) / sizeof(onceEntry); + result->maxUsed = epicsRingBytesHighWaterMark(onceQ) / sizeof(onceEntry); + result->numOverflow = epicsAtomicGetIntT(&onceQOverruns); + ret = 0; + } else { + ret = -2; + } + if (reset) { + epicsRingBytesResetHighWaterMark(onceQ); + } + return ret; +} + +void scanOnceQueuePrintStatus(const int reset) +{ + scanOnceQueueStats stats; + if (scanOnceQueueStatus(reset, &stats) == -1) { + fprintf(stderr, "scanOnce system not initialized, yet. Please run " + "iocInit before using this command.\n"); + return; + } + + printf("PRIORITY HIGH-WATER MARK ITEMS IN Q Q SIZE %% USED Q OVERFLOWS\n"); + double qusage = 100.0 * stats.numUsed / stats.size; + printf("%8s %15d %10d %6d %6.1f %11d\n", "scanOnce", stats.maxUsed, stats.numUsed, stats.size, qusage, + epicsAtomicGetIntT(&onceQOverruns)); +} + static void initOnce(void) { if ((onceQ = epicsRingBytesLockedCreate(sizeof(onceEntry)*onceQueueSize)) == NULL) { diff --git a/src/ioc/db/dbScan.h b/src/ioc/db/dbScan.h index d483a0c30..830d3a8dc 100644 --- a/src/ioc/db/dbScan.h +++ b/src/ioc/db/dbScan.h @@ -42,6 +42,13 @@ struct dbCommon; typedef void (*io_scan_complete)(void *usr, IOSCANPVT, int prio); typedef void (*once_complete)(void *usr, struct dbCommon*); +typedef struct scanOnceQueueStats { + int size; + int numUsed; + int maxUsed; + int numOverflow; +} scanOnceQueueStats; + epicsShareFunc long scanInit(void); epicsShareFunc void scanRun(void); epicsShareFunc void scanPause(void); @@ -57,6 +64,8 @@ epicsShareFunc double scanPeriod(int scan); epicsShareFunc int scanOnce(struct dbCommon *); epicsShareFunc int scanOnceCallback(struct dbCommon *, once_complete cb, void *usr); epicsShareFunc int scanOnceSetQueueSize(int size); +epicsShareFunc int scanOnceQueueStatus(const int reset, scanOnceQueueStats *result); +void scanOnceQueuePrintStatus(const int reset); /*print periodic lists*/ epicsShareFunc int scanppl(double rate); diff --git a/src/libCom/ring/epicsRingBytes.c b/src/libCom/ring/epicsRingBytes.c index cb7e52e83..a976d8412 100644 --- a/src/libCom/ring/epicsRingBytes.c +++ b/src/libCom/ring/epicsRingBytes.c @@ -21,6 +21,7 @@ #include #define epicsExportSharedSymbols +#include "epicsAtomic.h" #include "epicsSpin.h" #include "dbDefs.h" #include "epicsRingBytes.h" @@ -38,6 +39,7 @@ typedef struct ringPvt { volatile int nextPut; volatile int nextGet; int size; + int highWaterMark; volatile char buffer[1]; /* actually larger */ }ringPvt; @@ -47,6 +49,7 @@ epicsShareFunc epicsRingBytesId epicsShareAPI epicsRingBytesCreate(int size) if(!pring) return NULL; pring->size = size + SLOP; + pring->highWaterMark = 0; pring->nextGet = 0; pring->nextPut = 0; pring->lock = 0; @@ -131,8 +134,13 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( if (pring->lock) epicsSpinUnlock(pring->lock); return 0; } - if (nbytes) + if (nbytes) { memcpy ((void *)&pring->buffer[nextPut], value, nbytes); + int curUsed = pring->size - SLOP - freeCount; + if (curUsed > epicsAtomicGetIntT(&pring->highWaterMark)) { + epicsAtomicSetIntT(&pring->highWaterMark, curUsed); + } + } nextPut += nbytes; } else { @@ -143,8 +151,13 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( } topCount = size - nextPut; copyCount = (nbytes > topCount) ? topCount : nbytes; - if (copyCount) + if (copyCount) { memcpy ((void *)&pring->buffer[nextPut], value, copyCount); + int curUsed = pring->size - SLOP - freeCount; + if (curUsed > epicsAtomicGetIntT(&pring->highWaterMark)) { + epicsAtomicSetIntT(&pring->highWaterMark, curUsed); + } + } nextPut += copyCount; if (nextPut == size) { int nLeft = nbytes - copyCount; @@ -224,3 +237,15 @@ epicsShareFunc int epicsShareAPI epicsRingBytesIsFull(epicsRingBytesId id) { return (epicsRingBytesFreeBytes(id) <= 0); } + +epicsShareFunc int epicsShareAPI epicsRingBytesHighWaterMark(epicsRingBytesIdConst id) +{ + ringPvt *pring = (ringPvt *)id; + return epicsAtomicGetIntT(&pring->highWaterMark); +} + +epicsShareFunc void epicsShareAPI epicsRingBytesResetHighWaterMark(epicsRingBytesId id) +{ + ringPvt *pring = (ringPvt *)id; + epicsAtomicSetIntT(&pring->highWaterMark, epicsRingBytesUsedBytes(id)); +} diff --git a/src/libCom/ring/epicsRingBytes.h b/src/libCom/ring/epicsRingBytes.h index 011829bfe..3dc0081ad 100644 --- a/src/libCom/ring/epicsRingBytes.h +++ b/src/libCom/ring/epicsRingBytes.h @@ -24,6 +24,7 @@ extern "C" { #include "shareLib.h" typedef void *epicsRingBytesId; +typedef void const *epicsRingBytesIdConst; epicsShareFunc epicsRingBytesId epicsShareAPI epicsRingBytesCreate(int nbytes); /* Same, but secured by a spinlock */ @@ -39,6 +40,8 @@ epicsShareFunc int epicsShareAPI epicsRingBytesUsedBytes(epicsRingBytesId id); epicsShareFunc int epicsShareAPI epicsRingBytesSize(epicsRingBytesId id); epicsShareFunc int epicsShareAPI epicsRingBytesIsEmpty(epicsRingBytesId id); epicsShareFunc int epicsShareAPI epicsRingBytesIsFull(epicsRingBytesId id); +epicsShareFunc int epicsShareAPI epicsRingBytesHighWaterMark(epicsRingBytesIdConst id); +epicsShareFunc void epicsShareAPI epicsRingBytesResetHighWaterMark(epicsRingBytesId id); #ifdef __cplusplus } diff --git a/src/libCom/ring/epicsRingPointer.cpp b/src/libCom/ring/epicsRingPointer.cpp index 9c144cec1..709ab6509 100644 --- a/src/libCom/ring/epicsRingPointer.cpp +++ b/src/libCom/ring/epicsRingPointer.cpp @@ -90,3 +90,15 @@ epicsShareFunc int epicsShareAPI epicsRingPointerIsFull(epicsRingPointerId id) voidPointer *pvoidPointer = reinterpret_cast(id); return((pvoidPointer->isFull()) ? 1 : 0); } + +epicsShareFunc int epicsShareAPI epicsRingPointerGetHighWaterMark(epicsRingPointerIdConst id) +{ + voidPointer const *pvoidPointer = reinterpret_cast(id); + return(pvoidPointer->getHighWaterMark()); +} + +epicsShareFunc void epicsShareAPI epicsRingPointerResetHighWaterMark(epicsRingPointerId id) +{ + voidPointer *pvoidPointer = reinterpret_cast(id); + pvoidPointer->resetHighWaterMark(); +} diff --git a/src/libCom/ring/epicsRingPointer.h b/src/libCom/ring/epicsRingPointer.h index 48d62036d..b62923883 100644 --- a/src/libCom/ring/epicsRingPointer.h +++ b/src/libCom/ring/epicsRingPointer.h @@ -23,6 +23,7 @@ * epicsRingPointerLocked uses a spinlock. */ +#include "epicsAtomic.h" #include "epicsSpin.h" #include "shareLib.h" @@ -40,18 +41,22 @@ public: /* Functions */ int getSize() const; bool isEmpty() const; bool isFull() const; + int getHighWaterMark() const; + void resetHighWaterMark(); private: /* Prevent compiler-generated member functions */ /* default constructor, copy constructor, assignment operator */ epicsRingPointer(); epicsRingPointer(const epicsRingPointer &); epicsRingPointer& operator=(const epicsRingPointer &); + int getUsedNoLock() const; private: /* Data */ epicsSpinId lock; volatile int nextPush; volatile int nextPop; int size; + int highWaterMark; T * volatile * buffer; }; @@ -59,6 +64,7 @@ extern "C" { #endif /*__cplusplus */ typedef void *epicsRingPointerId; +typedef void const *epicsRingPointerIdConst; epicsShareFunc epicsRingPointerId epicsShareAPI epicsRingPointerCreate(int size); /* Same, but secured by a spinlock */ @@ -74,6 +80,8 @@ epicsShareFunc int epicsShareAPI epicsRingPointerGetUsed(epicsRingPointerId id) epicsShareFunc int epicsShareAPI epicsRingPointerGetSize(epicsRingPointerId id); epicsShareFunc int epicsShareAPI epicsRingPointerIsEmpty(epicsRingPointerId id); epicsShareFunc int epicsShareAPI epicsRingPointerIsFull(epicsRingPointerId id); +epicsShareFunc int epicsShareAPI epicsRingPointerGetHighWaterMark(epicsRingPointerIdConst id); +epicsShareFunc void epicsShareAPI epicsRingPointerResetHighWaterMark(epicsRingPointerId id); /* This routine was incorrectly named in previous releases */ #define epicsRingPointerSize epicsRingPointerGetSize @@ -95,7 +103,8 @@ epicsShareFunc int epicsShareAPI epicsRingPointerIsFull(epicsRingPointerId id); template inline epicsRingPointer::epicsRingPointer(int sz, bool locked) : - lock(0), nextPush(0), nextPop(0), size(sz+1), buffer(new T* [sz+1]) + lock(0), nextPush(0), nextPop(0), size(sz+1), highWaterMark(0), + buffer(new T* [sz+1]) { if (locked) lock = epicsSpinCreate(); @@ -121,6 +130,10 @@ inline bool epicsRingPointer::push(T *p) } buffer[next] = p; nextPush = newNext; + int used = getUsedNoLock(); + if (used > epicsAtomicGetIntT(&highWaterMark)) { + epicsAtomicSetIntT(&highWaterMark, used); + } if (lock) epicsSpinUnlock(lock); return(true); } @@ -161,12 +174,19 @@ inline int epicsRingPointer::getFree() const return n; } +template +inline int epicsRingPointer::getUsedNoLock() const +{ + int n = nextPush - nextPop; + if (n < 0) n += size; + return n; +} + template inline int epicsRingPointer::getUsed() const { if (lock) epicsSpinLock(lock); - int n = nextPush - nextPop; - if (n < 0) n += size; + int n = getUsedNoLock(); if (lock) epicsSpinUnlock(lock); return n; } @@ -196,6 +216,18 @@ inline bool epicsRingPointer::isFull() const return((count == 0) || (count == size)); } +template +inline int epicsRingPointer::getHighWaterMark() const +{ + return epicsAtomicGetIntT(&highWaterMark); +} + +template +inline void epicsRingPointer::resetHighWaterMark() +{ + epicsAtomicSetIntT(&highWaterMark, getUsed()); +} + #endif /* __cplusplus */ #endif /* INCepicsRingPointerh */ From 0f16977caf20c5b7f21746a11daea7c5c86f236c Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Tue, 27 Nov 2018 10:30:35 -0500 Subject: [PATCH 2/7] Make code compatible with ISO C90 --- src/ioc/db/callback.c | 24 ++++++++++++------------ src/ioc/db/dbScan.c | 14 +++++++------- src/libCom/ring/epicsRingBytes.c | 4 ++-- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/ioc/db/callback.c b/src/ioc/db/callback.c index 2b95cc5af..253d2b86b 100644 --- a/src/ioc/db/callback.c +++ b/src/ioc/db/callback.c @@ -106,11 +106,11 @@ int callbackSetQueueSize(int size) int callbackQueueStatus(const int reset, callbackQueueStats *result) { - if (!callbackIsInit) return -1; int ret; + if (!callbackIsInit) return -1; if (result) { - result->size = epicsAtomicGetIntT(&callbackQueueSize); int prio; + result->size = epicsAtomicGetIntT(&callbackQueueSize); for(prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) { epicsRingPointerId qId = callbackQueue[prio].queue; result->numUsed[prio] = epicsRingPointerGetUsed(qId); @@ -136,16 +136,16 @@ void callbackQueuePrintStatus(const int reset) if (callbackQueueStatus(reset, &stats) == -1) { fprintf(stderr, "Callback system not initialized, yet. Please run " "iocInit before using this command.\n"); - return; - } - - printf("PRIORITY HIGH-WATER MARK ITEMS IN Q Q SIZE %% USED Q OVERFLOWS\n"); - int prio; - for (prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) { - double qusage = 100.0 * stats.numUsed[prio] / stats.size; - printf("%8s %15d %10d %6d %6.1f %11d\n", threadNamePrefix[prio], - stats.maxUsed[prio], stats.numUsed[prio], stats.size, - qusage, stats.numOverflow[prio]); + } else { + int prio; + printf("PRIORITY HIGH-WATER MARK ITEMS IN Q Q SIZE %% USED Q OVERFLOWS\n"); + for (prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) { + double qusage = 100.0 * stats.numUsed[prio] / stats.size; + printf("%8s %15d %10d %6d %6.1f %11d\n", + threadNamePrefix[prio], stats.maxUsed[prio], + stats.numUsed[prio], stats.size, qusage, + stats.numOverflow[prio]); + } } } diff --git a/src/ioc/db/dbScan.c b/src/ioc/db/dbScan.c index c94632794..e0e14e0d2 100644 --- a/src/ioc/db/dbScan.c +++ b/src/ioc/db/dbScan.c @@ -727,8 +727,8 @@ int scanOnceSetQueueSize(int size) int scanOnceQueueStatus(const int reset, scanOnceQueueStats *result) { - if (!onceQ) return -1; int ret; + if (!onceQ) return -1; if (result) { result->size = epicsRingBytesSize(onceQ) / sizeof(onceEntry); result->numUsed = epicsRingBytesUsedBytes(onceQ) / sizeof(onceEntry); @@ -750,13 +750,13 @@ void scanOnceQueuePrintStatus(const int reset) if (scanOnceQueueStatus(reset, &stats) == -1) { fprintf(stderr, "scanOnce system not initialized, yet. Please run " "iocInit before using this command.\n"); - return; + } else { + double qusage = 100.0 * stats.numUsed / stats.size; + printf("PRIORITY HIGH-WATER MARK ITEMS IN Q Q SIZE %% USED Q OVERFLOWS\n"); + printf("%8s %15d %10d %6d %6.1f %11d\n", "scanOnce", stats.maxUsed, + stats.numUsed, stats.size, qusage, + epicsAtomicGetIntT(&onceQOverruns)); } - - printf("PRIORITY HIGH-WATER MARK ITEMS IN Q Q SIZE %% USED Q OVERFLOWS\n"); - double qusage = 100.0 * stats.numUsed / stats.size; - printf("%8s %15d %10d %6d %6.1f %11d\n", "scanOnce", stats.maxUsed, stats.numUsed, stats.size, qusage, - epicsAtomicGetIntT(&onceQOverruns)); } static void initOnce(void) diff --git a/src/libCom/ring/epicsRingBytes.c b/src/libCom/ring/epicsRingBytes.c index a976d8412..c38e32b77 100644 --- a/src/libCom/ring/epicsRingBytes.c +++ b/src/libCom/ring/epicsRingBytes.c @@ -135,8 +135,8 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( return 0; } if (nbytes) { - memcpy ((void *)&pring->buffer[nextPut], value, nbytes); int curUsed = pring->size - SLOP - freeCount; + memcpy ((void *)&pring->buffer[nextPut], value, nbytes); if (curUsed > epicsAtomicGetIntT(&pring->highWaterMark)) { epicsAtomicSetIntT(&pring->highWaterMark, curUsed); } @@ -152,8 +152,8 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( topCount = size - nextPut; copyCount = (nbytes > topCount) ? topCount : nbytes; if (copyCount) { - memcpy ((void *)&pring->buffer[nextPut], value, copyCount); int curUsed = pring->size - SLOP - freeCount; + memcpy ((void *)&pring->buffer[nextPut], value, copyCount); if (curUsed > epicsAtomicGetIntT(&pring->highWaterMark)) { epicsAtomicSetIntT(&pring->highWaterMark, curUsed); } From 040f9013f461346e924ba10df04a723dd7c55f3b Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Tue, 27 Nov 2018 10:45:35 -0500 Subject: [PATCH 3/7] Remove unneeded epicsAtomic from callbackQueueSize --- src/ioc/db/callback.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ioc/db/callback.c b/src/ioc/db/callback.c index 253d2b86b..1b6c24906 100644 --- a/src/ioc/db/callback.c +++ b/src/ioc/db/callback.c @@ -100,7 +100,7 @@ int callbackSetQueueSize(int size) fprintf(stderr, "Callback system already initialized\n"); return -1; } - epicsAtomicSetIntT(&callbackQueueSize, size); + callbackQueueSize = size; return 0; } @@ -110,7 +110,7 @@ int callbackQueueStatus(const int reset, callbackQueueStats *result) if (!callbackIsInit) return -1; if (result) { int prio; - result->size = epicsAtomicGetIntT(&callbackQueueSize); + result->size = callbackQueueSize; for(prio = 0; prio < NUM_CALLBACK_PRIORITIES; prio++) { epicsRingPointerId qId = callbackQueue[prio].queue; result->numUsed[prio] = epicsRingPointerGetUsed(qId); @@ -286,7 +286,7 @@ void callbackInit(void) epicsThreadId tid; callbackQueue[i].semWakeUp = epicsEventMustCreate(epicsEventEmpty); - callbackQueue[i].queue = epicsRingPointerLockedCreate(epicsAtomicGetIntT(&callbackQueueSize)); + callbackQueue[i].queue = epicsRingPointerLockedCreate(callbackQueueSize); if (callbackQueue[i].queue == 0) cantProceed("epicsRingPointerLockedCreate failed for %s\n", threadNamePrefix[i]); From d436561cb22fd847d187956178d78effd690adf7 Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Thu, 29 Nov 2018 19:12:48 -0500 Subject: [PATCH 4/7] Add tests for highWaterMark feature --- src/libCom/test/ringBytesTest.c | 34 +++++++++++++++++++------------ src/libCom/test/ringPointerTest.c | 9 +++++++- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/libCom/test/ringBytesTest.c b/src/libCom/test/ringBytesTest.c index 6cef93334..bb91d0201 100644 --- a/src/libCom/test/ringBytesTest.c +++ b/src/libCom/test/ringBytesTest.c @@ -30,7 +30,8 @@ typedef struct info { epicsRingBytesId ring; }info; -static void check(epicsRingBytesId ring, int expectedFree) +static void check(epicsRingBytesId ring, int expectedFree, + int expectedHighWaterMark) { int expectedUsed = RINGSIZE - expectedFree; int expectedEmpty = (expectedUsed == 0); @@ -39,11 +40,14 @@ static void check(epicsRingBytesId ring, int expectedFree) int nUsed = epicsRingBytesUsedBytes(ring); int isEmpty = epicsRingBytesIsEmpty(ring); int isFull = epicsRingBytesIsFull(ring); + int highWaterMark = epicsRingBytesHighWaterMark(ring); testOk(nFree == expectedFree, "Free: %d == %d", nFree, expectedFree); testOk(nUsed == expectedUsed, "Used: %d == %d", nUsed, expectedUsed); testOk(isEmpty == expectedEmpty, "Empty: %d == %d", isEmpty, expectedEmpty); testOk(isFull == expectedFull, "Full: %d == %d", isFull, expectedFull); + testOk(highWaterMark == expectedHighWaterMark, "HighWaterMark: %d == %d", + highWaterMark, expectedHighWaterMark); } MAIN(ringBytesTest) @@ -55,7 +59,7 @@ MAIN(ringBytesTest) char get[RINGSIZE+1]; epicsRingBytesId ring; - testPlan(245); + testPlan(292); pinfo = calloc(1,sizeof(info)); if (!pinfo) { @@ -70,50 +74,54 @@ MAIN(ringBytesTest) if (!ring) { testAbort("epicsRingBytesCreate failed"); } - check(ring, RINGSIZE); + check(ring, RINGSIZE, 0); for (i = 0 ; i < sizeof(put) ; i++) put[i] = i; for(i = 0 ; i < RINGSIZE ; i++) { n = epicsRingBytesPut(ring, put, i); testOk(n==i, "ring put %d", i); - check(ring, RINGSIZE-i); + check(ring, RINGSIZE-i, i); n = epicsRingBytesGet(ring, get, i); testOk(n==i, "ring get %d", i); - check(ring, RINGSIZE); + check(ring, RINGSIZE, i); testOk(memcmp(put,get,i)==0, "get matches write"); } + epicsRingBytesResetHighWaterMark(ring); + for(i = 0 ; i < RINGSIZE ; i++) { n = epicsRingBytesPut(ring, put+i, 1); testOk(n==1, "ring put 1, %d", i); - check(ring, RINGSIZE-1-i); + check(ring, RINGSIZE-1-i, i + 1); } n = epicsRingBytesPut(ring, put+RINGSIZE, 1); testOk(n==0, "put to full ring"); - check(ring, 0); + check(ring, 0, RINGSIZE); for(i = 0 ; i < RINGSIZE ; i++) { n = epicsRingBytesGet(ring, get+i, 1); testOk(n==1, "ring get 1, %d", i); - check(ring, 1+i); + check(ring, 1+i, RINGSIZE); } testOk(memcmp(put,get,RINGSIZE)==0, "get matches write"); n = epicsRingBytesGet(ring, get+RINGSIZE, 1); testOk(n==0, "get from empty ring"); - check(ring, RINGSIZE); + check(ring, RINGSIZE, RINGSIZE); + + epicsRingBytesResetHighWaterMark(ring); n = epicsRingBytesPut(ring, put, RINGSIZE+1); testOk(n==0, "ring put beyond ring capacity (%d, expected 0)",n); - check(ring, RINGSIZE); + check(ring, RINGSIZE, 0); n = epicsRingBytesPut(ring, put, 1); testOk(n==1, "ring put %d", 1); - check(ring, RINGSIZE-1); + check(ring, RINGSIZE-1, 1); n = epicsRingBytesPut(ring, put, RINGSIZE); testOk(n==0, "ring put beyond ring capacity (%d, expected 0)",n); - check(ring, RINGSIZE-1); + check(ring, RINGSIZE-1, 1); n = epicsRingBytesGet(ring, get, 1); testOk(n==1, "ring get %d", 1); - check(ring, RINGSIZE); + check(ring, RINGSIZE, 1); epicsRingBytesDelete(ring); epicsEventDestroy(consumerEvent); diff --git a/src/libCom/test/ringPointerTest.c b/src/libCom/test/ringPointerTest.c index 65a349489..d351708b5 100644 --- a/src/libCom/test/ringPointerTest.c +++ b/src/libCom/test/ringPointerTest.c @@ -64,6 +64,7 @@ static void testSingle(void) testOk1(epicsRingPointerGetFree(ring)==rsize); testOk1(epicsRingPointerGetSize(ring)==rsize); testOk1(epicsRingPointerGetUsed(ring)==0); + testOk1(epicsRingPointerGetHighWaterMark(ring)==0); testOk1(epicsRingPointerPop(ring)==NULL); @@ -75,6 +76,10 @@ static void testSingle(void) testOk1(epicsRingPointerGetFree(ring)==rsize-1); testOk1(epicsRingPointerGetSize(ring)==rsize); testOk1(epicsRingPointerGetUsed(ring)==1); + testOk1(epicsRingPointerGetHighWaterMark(ring)==1); + + epicsRingPointerResetHighWaterMark(ring); + testOk1(epicsRingPointerGetHighWaterMark(ring)==1); testDiag("Fill it up"); for(i=2; i<2*rsize; i++) { @@ -92,6 +97,7 @@ static void testSingle(void) testOk1(epicsRingPointerGetFree(ring)==0); testOk1(epicsRingPointerGetSize(ring)==rsize); testOk1(epicsRingPointerGetUsed(ring)==rsize); + testOk1(epicsRingPointerGetHighWaterMark(ring)==rsize); testDiag("Drain it out"); for(i=1; i<2*rsize; i++) { @@ -108,6 +114,7 @@ static void testSingle(void) testOk1(epicsRingPointerGetFree(ring)==rsize); testOk1(epicsRingPointerGetSize(ring)==rsize); testOk1(epicsRingPointerGetUsed(ring)==0); + testOk1(epicsRingPointerGetHighWaterMark(ring)==rsize); testDiag("Fill it up again"); for(i=2; i<2*rsize; i++) { @@ -236,7 +243,7 @@ MAIN(ringPointerTest) { int prio = epicsThreadGetPrioritySelf(); - testPlan(37); + testPlan(42); testSingle(); if (prio) epicsThreadSetPriority(epicsThreadGetIdSelf(), epicsThreadPriorityScanLow); From 10d951e2d79cd48144ce32bcac2c99d44d156f09 Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Thu, 29 Nov 2018 19:14:55 -0500 Subject: [PATCH 5/7] Fix incorrect value for highWaterMark in epicsRingBytes --- src/libCom/ring/epicsRingBytes.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/libCom/ring/epicsRingBytes.c b/src/libCom/ring/epicsRingBytes.c index c38e32b77..a9b13b990 100644 --- a/src/libCom/ring/epicsRingBytes.c +++ b/src/libCom/ring/epicsRingBytes.c @@ -121,7 +121,7 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( { ringPvt *pring = (ringPvt *)id; int nextGet, nextPut, size; - int freeCount, copyCount, topCount; + int freeCount, copyCount, topCount, used; if (pring->lock) epicsSpinLock(pring->lock); nextGet = pring->nextGet; @@ -135,11 +135,7 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( return 0; } if (nbytes) { - int curUsed = pring->size - SLOP - freeCount; memcpy ((void *)&pring->buffer[nextPut], value, nbytes); - if (curUsed > epicsAtomicGetIntT(&pring->highWaterMark)) { - epicsAtomicSetIntT(&pring->highWaterMark, curUsed); - } } nextPut += nbytes; } @@ -152,11 +148,7 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( topCount = size - nextPut; copyCount = (nbytes > topCount) ? topCount : nbytes; if (copyCount) { - int curUsed = pring->size - SLOP - freeCount; memcpy ((void *)&pring->buffer[nextPut], value, copyCount); - if (curUsed > epicsAtomicGetIntT(&pring->highWaterMark)) { - epicsAtomicSetIntT(&pring->highWaterMark, curUsed); - } } nextPut += copyCount; if (nextPut == size) { @@ -168,6 +160,12 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( } pring->nextPut = nextPut; + used = nextPut - nextGet; + if (used < 0) used += pring->size; + if (used > epicsAtomicGetIntT(&pring->highWaterMark)) { + epicsAtomicSetIntT(&pring->highWaterMark, used); + } + if (pring->lock) epicsSpinUnlock(pring->lock); return nbytes; } From 87761ebf290dfa853b2a3fb6b99f8c0da7fe998f Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Mon, 3 Dec 2018 12:01:47 -0500 Subject: [PATCH 6/7] Prevent data race between resetHighWaterMark() and put() --- src/libCom/ring/epicsRingBytes.c | 6 +++--- src/libCom/ring/epicsRingPointer.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libCom/ring/epicsRingBytes.c b/src/libCom/ring/epicsRingBytes.c index a9b13b990..ec9e5094d 100644 --- a/src/libCom/ring/epicsRingBytes.c +++ b/src/libCom/ring/epicsRingBytes.c @@ -121,7 +121,7 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( { ringPvt *pring = (ringPvt *)id; int nextGet, nextPut, size; - int freeCount, copyCount, topCount, used; + int freeCount, copyCount, topCount, used, oldHWM; if (pring->lock) epicsSpinLock(pring->lock); nextGet = pring->nextGet; @@ -162,8 +162,8 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( used = nextPut - nextGet; if (used < 0) used += pring->size; - if (used > epicsAtomicGetIntT(&pring->highWaterMark)) { - epicsAtomicSetIntT(&pring->highWaterMark, used); + while(oldHWM = epicsAtomicGetIntT(&pring->highWaterMark), oldHWM < used) { + epicsAtomicCmpAndSwapIntT(&pring->highWaterMark, oldHWM, used); } if (pring->lock) epicsSpinUnlock(pring->lock); diff --git a/src/libCom/ring/epicsRingPointer.h b/src/libCom/ring/epicsRingPointer.h index b62923883..2b4f1b172 100644 --- a/src/libCom/ring/epicsRingPointer.h +++ b/src/libCom/ring/epicsRingPointer.h @@ -131,8 +131,8 @@ inline bool epicsRingPointer::push(T *p) buffer[next] = p; nextPush = newNext; int used = getUsedNoLock(); - if (used > epicsAtomicGetIntT(&highWaterMark)) { - epicsAtomicSetIntT(&highWaterMark, used); + while(int oldHWM = epicsAtomicGetIntT(&highWaterMark), oldHWM < used) { + epicsAtomicCmpAndSwapIntT(&highWaterMark, oldHWM, used); } if (lock) epicsSpinUnlock(lock); return(true); From 6f919c3991bc264e91a313415a402250bab463ac Mon Sep 17 00:00:00 2001 From: Martin Konrad Date: Tue, 4 Dec 2018 12:00:26 -0500 Subject: [PATCH 7/7] Simpler implementation using spin lock rather than atomics --- src/libCom/ring/epicsRingBytes.c | 16 +++++++++------- src/libCom/ring/epicsRingPointer.h | 11 +++++------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/libCom/ring/epicsRingBytes.c b/src/libCom/ring/epicsRingBytes.c index ec9e5094d..ab048e467 100644 --- a/src/libCom/ring/epicsRingBytes.c +++ b/src/libCom/ring/epicsRingBytes.c @@ -21,7 +21,6 @@ #include #define epicsExportSharedSymbols -#include "epicsAtomic.h" #include "epicsSpin.h" #include "dbDefs.h" #include "epicsRingBytes.h" @@ -121,7 +120,7 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( { ringPvt *pring = (ringPvt *)id; int nextGet, nextPut, size; - int freeCount, copyCount, topCount, used, oldHWM; + int freeCount, copyCount, topCount, used; if (pring->lock) epicsSpinLock(pring->lock); nextGet = pring->nextGet; @@ -162,9 +161,7 @@ epicsShareFunc int epicsShareAPI epicsRingBytesPut( used = nextPut - nextGet; if (used < 0) used += pring->size; - while(oldHWM = epicsAtomicGetIntT(&pring->highWaterMark), oldHWM < used) { - epicsAtomicCmpAndSwapIntT(&pring->highWaterMark, oldHWM, used); - } + if (used > pring->highWaterMark) pring->highWaterMark = used; if (pring->lock) epicsSpinUnlock(pring->lock); return nbytes; @@ -239,11 +236,16 @@ epicsShareFunc int epicsShareAPI epicsRingBytesIsFull(epicsRingBytesId id) epicsShareFunc int epicsShareAPI epicsRingBytesHighWaterMark(epicsRingBytesIdConst id) { ringPvt *pring = (ringPvt *)id; - return epicsAtomicGetIntT(&pring->highWaterMark); + return pring->highWaterMark; } epicsShareFunc void epicsShareAPI epicsRingBytesResetHighWaterMark(epicsRingBytesId id) { ringPvt *pring = (ringPvt *)id; - epicsAtomicSetIntT(&pring->highWaterMark, epicsRingBytesUsedBytes(id)); + int used; + if (pring->lock) epicsSpinLock(pring->lock); + used = pring->nextGet - pring->nextPut; + if (used < 0) used += pring->size; + pring->highWaterMark = used; + if (pring->lock) epicsSpinUnlock(pring->lock); } diff --git a/src/libCom/ring/epicsRingPointer.h b/src/libCom/ring/epicsRingPointer.h index 2b4f1b172..68bf8f5a6 100644 --- a/src/libCom/ring/epicsRingPointer.h +++ b/src/libCom/ring/epicsRingPointer.h @@ -23,7 +23,6 @@ * epicsRingPointerLocked uses a spinlock. */ -#include "epicsAtomic.h" #include "epicsSpin.h" #include "shareLib.h" @@ -131,9 +130,7 @@ inline bool epicsRingPointer::push(T *p) buffer[next] = p; nextPush = newNext; int used = getUsedNoLock(); - while(int oldHWM = epicsAtomicGetIntT(&highWaterMark), oldHWM < used) { - epicsAtomicCmpAndSwapIntT(&highWaterMark, oldHWM, used); - } + if (used > highWaterMark) highWaterMark = used; if (lock) epicsSpinUnlock(lock); return(true); } @@ -219,13 +216,15 @@ inline bool epicsRingPointer::isFull() const template inline int epicsRingPointer::getHighWaterMark() const { - return epicsAtomicGetIntT(&highWaterMark); + return highWaterMark; } template inline void epicsRingPointer::resetHighWaterMark() { - epicsAtomicSetIntT(&highWaterMark, getUsed()); + if (lock) epicsSpinLock(lock); + highWaterMark = getUsedNoLock(); + if (lock) epicsSpinUnlock(lock); } #endif /* __cplusplus */