Experimental Physics and
Industrial Control System

Ralph Lange <[email protected]> · Fri, 08 Aug 2014 21:08:25 +0200

On August 8, 2014 6:13:08 PM CEST, Andrew Johnson <[email protected]> wrote:
>Hi Ralph,
>
>On 08/08/2014 10:19 AM, Ralph Lange wrote:
>> I was looking at db/dbCaTest.c today, and I found that
>> 
>> dbcar() is looping through all record instances of all types, and
>taking
>> dbLockSetGblLock() when investigating a CA link.
>> dbcaStats() is looping through all record instances of all types, and
>> *NOT* taking dbLockSetGblLock() when investigating a CA link.
>> 
>> Why? Why not?
>> 
>> dbcar() is doing more CA investigation, and using the formatter
>in-between.
>> But other than that, dbcaStats() could also encounter another thread
>> changing the link type from CA to DB or CONSTANT.
>
>As the guilty party my reasoning probably went something like this:
>Links don't get changed very often; if a change does occur at the
>moment
>when we are counting a specific link, the result is that the statistics
>could be off by one that one time, but the consequence of that
>miss-count is effectively nil — the number was changing at the time
>anyway, so is really wrong? The user would only really notice if the
>number of CA links gets counted as less than the number of connected
>links, resulting in *pdiscon < 0.
>
>> Can anyone explain why these similar cases are handled differently?
>
>I didn't want to lock the global lock every time we redo the statistics
>count (which happens frequently if you're running devIocStats) as that
>could affect IOC performance.
>
>A better implementation might keep separate (atomic) running counter
>variables inside dbCa.c and have dbcaStats just return the values from
>those counters. dbcar could compare its counts with those (it still has
>to scan the whole database anyway). Using atomic counters wouldn't
>prevent dbcaStats from returning *pdiscon < 0 though, we'd need a
>single
>lock to cover both counters to prevent that.
>
>HTH,
>
>- Andrew

All agreed...

I am not afraid of miscounting at all...

My impression was dbcar() takes the lock to avoid a situation where another thread is changing the CA link into a DB or CONSTANT link while the investigation of CA properties is going on. The pointer to the cid would suddenly be invalid, and the ca_... functions that are dereferencing the pointer could easily segfault.

In any case I don't see the justification for handling the cases different.

@Michael: No intention to change anything. I found a case where the dbcar() code was cloned, but the locking calls were commented out, so I am trying to understand why they are in the original code in the first place.

Have a nice weekend,
~Ralph

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System