From ee297dc55879e71dd963d79fce0d9566df68f07a Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Tue, 24 Mar 2015 14:18:11 -0400 Subject: [PATCH] dbLock: use new backref tracking --- src/ioc/db/dbAccess.c | 2 +- src/ioc/db/dbLock.c | 80 ++++++++++------------------------ src/ioc/db/dbLockPvt.h | 9 ---- src/ioc/db/test/dbStressLock.c | 33 ++++++++++++-- src/ioc/misc/iocInit.c | 7 ++- 5 files changed, 58 insertions(+), 73 deletions(-) diff --git a/src/ioc/db/dbAccess.c b/src/ioc/db/dbAccess.c index b24739d2c..732e783dc 100644 --- a/src/ioc/db/dbAccess.c +++ b/src/ioc/db/dbAccess.c @@ -1052,7 +1052,7 @@ static long dbPutFieldLink(DBADDR *paddr, switch (plink->type) { /* Old link type */ case DB_LINK: case CA_LINK: - dbRemoveLink(&locker, precord, plink); + dbRemoveLink(&locker, precord, plink); /* link type becomes PV_LINK */ break; case PV_LINK: diff --git a/src/ioc/db/dbLock.c b/src/ioc/db/dbLock.c index a2fb4da28..2be1f5f82 100644 --- a/src/ioc/db/dbLock.c +++ b/src/ioc/db/dbLock.c @@ -135,11 +135,18 @@ void dbLockDecRef(lockSet *ls) if(cnt) return; + /* not necessary as no one else holds a reference, + * but lock anyway to quiet valgrind + */ + epicsMutexMustLock(ls->lock); + if(ellCount(&ls->lockRecordList)!=0) { errlogPrintf("dbLockDecRef(%p) would free lockSet with %d records\n", ls, ellCount(&ls->lockRecordList)); cantProceed(NULL); } + epicsMutexUnlock(ls->lock); + epicsMutexMustLock(lockSetsGuard); ellDelete(&lockSetsActive, &ls->node); #ifndef LOCKSET_FREE @@ -491,13 +498,11 @@ done: static int createLockRecord(void* junk, DBENTRY* pdbentry) { dbCommon *prec = pdbentry->precnode->precord; - size_t i, no_links=prec->rdes->no_links; lockRecord *lrec; - size_t arrsize=(no_links-1)*sizeof(linkRef); - assert(no_links>=1); /* dbCommon has TSEL */ assert(!prec->lset); - lrec = callocMustSucceed(1, sizeof(*lrec)+arrsize, "lockRecord"); + /* TODO: one allocation for all records? */ + lrec = callocMustSucceed(1, sizeof(*lrec), "lockRecord"); if(!lrec) cantProceed("no memory for lockRecord"); lrec->spin = epicsSpinCreate(); @@ -505,11 +510,6 @@ static int createLockRecord(void* junk, DBENTRY* pdbentry) cantProceed("no memory for spinlock in lockRecord"); lrec->precord = prec; - ellInit(&lrec->backlinks); - - for(i=0; ilinks[i].psrc = lrec; - } prec->lset = lrec; return 0; @@ -524,7 +524,6 @@ static int initPVLinks(void* junk, DBENTRY* pdbentry) /* for each link originating from this record */ for(i=0; ino_links; i++) { - linkRef *bref = &prec->lset->links[i]; DBADDR *paddr; dbFldDes *pdesc = rtype->papFldDes[rtype->link_ind[i]]; DBLINK *plink = (DBLINK*)((char*)prec + pdesc->offset); @@ -559,10 +558,6 @@ static int initPVLinks(void* junk, DBENTRY* pdbentry) dbLockIncRef(B); ellAdd(&B->lockRecordList, &prec->lset->node); } - - /* initialize backward link tracking */ - bref->ptarget = paddr->precord->lset; - ellAdd(&bref->ptarget->backlinks, &bref->backlinksnode); } return 0; } @@ -637,40 +632,6 @@ void dbLockCleanupRecords(dbBase *pdbbase) #endif } -/* update backwards link tracking */ -static void updateBackRefs(dbCommon *prec) -{ - size_t i; - /* for each link */ - for(i=0; irdes->no_links; i++) { - linkRef *bref = &prec->lset->links[i]; - dbFldDes *pdesc = prec->rdes->papFldDes[prec->rdes->link_ind[i]]; - DBLINK *plink = (DBLINK*)((char*)prec + pdesc->offset); - - if(plink->type!=DB_LINK && bref->ptarget) - { - /* removed link */ - ellDelete(&bref->ptarget->backlinks, &bref->backlinksnode); - bref->ptarget = NULL; - } else if(plink->type==DB_LINK) - { - DBADDR *paddr = (DBADDR*)plink->value.pv_link.pvt; - - if(paddr->precord->lset != bref->ptarget) { - /* changed link */ - if(bref->ptarget) { - /* clear old */ - ellDelete(&bref->ptarget->backlinks, &bref->backlinksnode); - bref->ptarget = NULL; - } - - bref->ptarget = paddr->precord->lset; - ellAdd(&bref->ptarget->backlinks, &bref->backlinksnode); - } - } - } -} - /* Caller must lock both pfirst and psecond. * Assumes that pfirst has been modified * to link to psecond. @@ -705,8 +666,6 @@ void dbLockSetMerge(dbLocker *locker, dbCommon *pfirst, dbCommon *psecond) if(A==B) return; /* already in the same lockSet */ - updateBackRefs(pfirst); /* not required */ - Nb = ellCount(&B->lockRecordList); assert(Nb>0); @@ -791,7 +750,6 @@ void dbLockSetSplit(dbLocker *locker, dbCommon *pfirst, dbCommon *psecond) cantProceed(NULL); } - updateBackRefs(pfirst); if(pfirst==psecond) return; @@ -829,8 +787,16 @@ void dbLockSetSplit(dbLocker *locker, dbCommon *pfirst, dbCommon *psecond) /* Visit all the links originating from prec */ for(i=0; ino_links; i++) { - linkRef *bref=&lr->links[i]; - lockRecord *lr = bref->ptarget; + dbFldDes *pdesc = rtype->papFldDes[rtype->link_ind[i]]; + DBLINK *plink = (DBLINK*)((char*)prec + pdesc->offset); + DBADDR *ptarget; + lockRecord *lr; + + if(plink->type!=DB_LINK) + continue; + + ptarget = plink->value.pv_link.pvt; + lr = ptarget->precord->lset; if(!lr) continue; /* not DB_LINK */ @@ -851,10 +817,12 @@ void dbLockSetSplit(dbLocker *locker, dbCommon *pfirst, dbCommon *psecond) } /* Visit all links terminating at prec */ - for(bcur=ellFirst(&lr->backlinks); bcur; bcur=ellNext(bcur)) + for(bcur=ellFirst(&prec->bklnk); bcur; bcur=ellNext(bcur)) { - linkRef *bref=CONTAINER(bcur, linkRef, backlinksnode); - lockRecord *lr = bref->psrc; + struct pv_link *plink1 = CONTAINER(bcur, struct pv_link, backlinknode); + union value *plink2 = CONTAINER(plink1, union value, pv_link); + DBLINK *plink = CONTAINER(plink2, DBLINK, value); + lockRecord *lr = plink->value.pv_link.precord->lset; if(lr->precord==pfirst) { goto nosplit; diff --git a/src/ioc/db/dbLockPvt.h b/src/ioc/db/dbLockPvt.h index 4a5646dd7..4913ac719 100644 --- a/src/ioc/db/dbLockPvt.h +++ b/src/ioc/db/dbLockPvt.h @@ -37,12 +37,6 @@ typedef struct dbLockSet { struct lockRecord; -typedef struct { - ELLNODE backlinksnode; - struct lockRecord *psrc; - struct lockRecord *ptarget; -} linkRef; - /* dbCommon.LSET is a plockRecord. * Except for spin and plockSet, all members of lockRecord are guarded * by the present lockset lock. @@ -62,9 +56,6 @@ typedef struct lockRecord { /* temp used during lockset split */ ELLNODE compnode; unsigned int compflag; - - ELLLIST backlinks; - linkRef links[1]; /* actual size based on no_links from dbRecordType */ } lockRecord; typedef struct { diff --git a/src/ioc/db/test/dbStressLock.c b/src/ioc/db/test/dbStressLock.c index c4cb5b2fb..7aa33100c 100644 --- a/src/ioc/db/test/dbStressLock.c +++ b/src/ioc/db/test/dbStressLock.c @@ -39,6 +39,9 @@ #include "xRecord.h" +#define testIntOk1(A, OP, B) testOk((A) OP (B), "%s (%d) %s %s (%d)", #A, A, #OP, #B, B); +#define testPtrOk1(A, OP, B) testOk((A) OP (B), "%s (%p) %s %s (%p)", #A, A, #OP, #B, B); + void dbTestIoc_registerRecordDeviceDriver(struct dbBase *); /* number of seconds for the test to run */ @@ -136,7 +139,7 @@ void doreTarget(workerPriv *p) if(ret) testAbort("bad record name? %ld", ret); - if(action<0.25) { + if(action<=0.6) { scratchdst[0] = '\0'; } else { strcpy(scratchdst, ptarg->name); @@ -197,7 +200,7 @@ MAIN(dbStressTest) char *nwork=getenv("NWORK"); struct timespec seed; - testPlan(0); + testPlan(95); clock_gettime(CLOCK_REALTIME, &seed); srand(seed.tv_nsec); @@ -272,7 +275,7 @@ MAIN(dbStressTest) &worker, &priv[i]); } - testDiag("All started"); + testDiag("All started. Will run for %f sec", runningtime); epicsThreadSleep(runningtime); @@ -289,6 +292,30 @@ MAIN(dbStressTest) testDiag("All stopped"); + testDiag("Validate lockSet ref counts"); + dbInitEntry(pdbbase, &ent); + for(status = dbFirstRecordType(&ent); + !status; + status = dbNextRecordType(&ent)) + { + for(status = dbFirstRecord(&ent); + !status; + status = dbNextRecord(&ent)) + { + dbCommon *prec = ent.precnode->precord; + lockSet *ls; + if(ent.precnode->flags&DBRN_FLAGS_ISALIAS) + continue; + ls = prec->lset->plockSet; + testOk(ellCount(&ls->lockRecordList)==ls->refcount, "%s only lockRecords hold refs. %d == %d", + prec->name,ellCount(&ls->lockRecordList),ls->refcount); + testOk1(ls->ownerlocker==NULL); + } + + } + dbFinishEntry(&ent); + + testDiag("Statistics"); for(i=0; ino_links; j++) { dbFldDes *pdbFldDes = papFldDes[link_ind[j]]; - DBLINK *plink = (DBLINK *)((char *)precord + pdbFldDes->offset); if (ellCount(&precord->rdes->devList) > 0 && pdbFldDes->isDevLink) { devSup *pdevSup = dbDTYPtoDevSup(pdbRecordType, precord->dtyp); @@ -631,13 +630,14 @@ static void doCloseLinks(dbRecordType *pdbRecordType, dbCommon *precord, locked = 1; } dbCaRemoveLink(plink); - plink->type = CONSTANT; + plink->type = PV_LINK; } else if (plink->type == DB_LINK) { /* free link, but don't split lockset like dbDbRemoveLink() */ free(plink->value.pv_link.pvt); - plink->type = CONSTANT; + plink->type = PV_LINK; } + dbFreeLinkContents(plink); } if (precord->dset && @@ -672,7 +672,6 @@ static void doFreeRecord(dbRecordType *pdbRecordType, dbCommon *precord, pdbRecordType->papFldDes[pdbRecordType->link_ind[j]]; DBLINK *plink = (DBLINK *)((char *)precord + pdbFldDes->offset); - dbFreeLinkContents(plink); } }