Project

General

Profile

Bug #1364

Review use of volatile variables in the runtime

Added by Sam White over 2 years ago. Updated about 1 year ago.

Status:
New
Priority:
Normal
Assignee:
Category:
SMP
Target version:
-
Start date:
01/18/2017
Due date:
% Done:

0%


Description

We've seen instances of volatile being used in places in the runtime where memory fences/barriers are actually needed.

Here is a list of volatile variables used in the runtime (in code that we care about that is not inline-assembly):

Core:
GONE src/ck-core/ckarrayreductionmgr.h:    volatile int ctorDoneFlag;
GONE src/conv-core/conv-interoperate.c:    static volatile int interopCommThdExit = 0;

Machine Layers:
src/arch/util/machine-common-core.c:  volatile int commThdExit = 0;
src/arch/util/pcqueue.h:              struct CircQueueStruct * CMK_SMP_volatile next;
src/arch/util/pcqueue.h:              CircQueue CMK_SMP_volatile tail;
src/arch/util/machine-smp.c:          static volatile HANDLE barrier_mutex;
src/arch/util/machine-smp.c:          static volatile int    barrier_wait[2] = {0,0};
src/arch/util/machine-smp.c:          static volatile int    barrier_which = 0;
src/arch/util/machine-smp.c:          static unsigned int volatile level = 0;
src/arch/util/machine-smp.h:          volatile int isSleeping; /*Are we asleep in this cond?*/
src/arch/util/machine-smp.h:          volatile int hasMessages; /*Is there a message waiting?*/
src/arch/util/machine-smp.h:          volatile int isSleeping; /*Are we asleep in this cond?*/
src/arch/util/machine-pxshm.c:        volatile int flagSender;
src/arch/util/machine-pxshm.c:        volatile int flagReceiver;
src/arch/util/machine-pxshm.c:        volatile int turn;
src/arch/util/machine-xpmem.c:        volatile int flagSender;
src/arch/util/machine-xpmem.c:        volatile int flagReceiver;
src/arch/util/machine-xpmem.c:        volatile int turn;
src/arch/netlrts/machine.c:           static volatile int memflag=0;
src/arch/netlrts/machine.c:           static volatile int comm_flag=0;
src/arch/verbs/machine.c:             static volatile int memflag=0;
src/arch/verbs/machine.c:             static volatile int comm_flag=0;
src/arch/netlrts/machine-dgram.c:     static int CMK_SMP_volatile writeableAcks,writeableDgrams; /*Write-queue counts (to know when to sleep)*/
src/arch/netlrts/machine-dgram.c:     struct ExplicitDgramStruct CMK_SMP_volatile *next;
src/arch/netlrts/machine-dgram.c:     struct ImplicitDgramStruct CMK_SMP_volatile *next;
src/arch/netlrts/machine-dgram.c:     ImplicitDgram CMK_SMP_volatile           send_queue_h; /* head of send queue */
src/arch/netlrts/machine-dgram.c:     ImplicitDgram CMK_SMP_volatile           send_queue_t; /* tail of send queue */
src/arch/netlrts/machine-dgram.c:     unsigned int  CMK_SMP_volatile           send_next;    /* next seqno to go into queue */
src/arch/pamilrts/lrtsqueue.h:        volatile uint64_t Consumer;    // not used atomically
src/arch/pamilrts/lrtsqueue.h:        volatile uint64_t Producer;
src/arch/pamilrts/lrtsqueue.h:        volatile uint64_t UpperBound;
src/arch/pamilrts/lrtsqueue.h:        volatile uint64_t Flush;    // contents not used
src/arch/pamilrts/lrtsqueue.h:        volatile void * volatile    * _array;
src/arch/pamilrts/lrtsqueue.h:        volatile void *e = NULL;
src/arch/pamilrts/machine.c:          volatile int msgQueueLen [MAX_NUM_CONTEXTS];
src/arch/pamilrts/machine.c:          volatile int outstanding_recvs [MAX_NUM_CONTEXTS];
src/arch/pamilrts/machine.c:          volatile int msgQueueLen;
src/arch/pamilrts/machine.c:          volatile int outstanding_recvs;
src/arch/pamilrts/machine.c:          volatile int pami_barrier_flag = 0;
src/arch/pamilrts/machine.c:          volatile int x;

CkLoop:
src/libs/ck-libs/ckloop/CkLoop.C:     static volatile int gCrtCnt = 0;
src/libs/ck-libs/ckloop/CkLoop.C:     static volatile int exitFlag = 0;
GONE src/libs/ck-libs/ckloop/CkLoop.h:     volatile int curChunkIdx;
src/libs/ck-libs/ckloop/CkLoop.h:     volatile int finishFlag;

History

#1 Updated by Sam White over 2 years ago

The one in ckarrayreductionmgr was unused, so removed here: https://charm.cs.illinois.edu/gerrit/#/c/2132/

#2 Updated by Phil Miller over 2 years ago

  • Description updated (diff)

#3 Updated by Sam White over 2 years ago

  • Category set to SMP

#4 Updated by Phil Miller over 2 years ago

interopCommThdExit is removed (except for BG XLC) here: https://charm.cs.illinois.edu/gerrit/#/c/2145/2

#5 Updated by Phil Miller over 2 years ago

  • Description updated (diff)

#6 Updated by Phil Miller over 2 years ago

CkLoop's curChunkIdx cleaned out here: https://charm.cs.illinois.edu/gerrit/2229
It's just a simple atomic counter.

#7 Updated by Phil Miller over 2 years ago

  • Description updated (diff)

#8 Updated by Eric Bohm over 2 years ago

  • Assignee set to Nitin Bhat

#9 Updated by Sam White about 1 year ago

Convert volatile int commThdExit + a CmiNodeLock to std::atomic<int>: https://charm.cs.illinois.edu/gerrit/#/c/charm/+/4103/

Also available in: Atom PDF