Experimental Physics and
Industrial Control System

"Mark Rivers" <[email protected]> · Wed, 10 Oct 2007 16:36:05 -0500

Hi Heinrich,

Thanks for finding this problem and suggesting one solution.

A recap of the problem for those who have not followed it:  if 2 or more
interrupts come in rapid sucession to asyn device support, it can result
in the the device support routine (called from scanIoRequest record
processing) calling the drivers read() routine when it should not do so.

It seems like there are 2 possible solutions.

Solution 1) (basically Heinrich's suggestion):
Ignore callbacks that happen before the previous callback has finished
executing processCommon in asyn device support.  The right way to do
this would be to add a mutex to protect access to the gotValue and data
fields.  This has the disadvantage of missing data when interrupts come
close together, even if the system could handle the average interrupt
rate.

Solution 2).
Rather than have the callback routine call scanIoRequest, have it
directly call dbScanLock and pr->process.  This way the callback and
record processing are done in the same thread, and there is no need for
a mutex.  The consequences of this solution are:

- There is no missed data, as long as the system can keep up with the
average data rate.

- The callback routine must not be called at interrupt level, because
the record will not be processed in a separate callback thread.
However, the asyn documentation is already explicit that the
pasynManager->interruptStart routine cannot be called at interrupt
level, so the callback routine cannot be running at interrupt level.

- The asyn driver must have already created a thread that the callback
will run in.  Under the new scheme this callback thread will not just
copy the data to the device support private, set a flag and call
scanIoRequest, but it will actually process the record.  It will also
process any records that this record forward links to.  This could
require more stack, and will take somewhat more time.

- The driver must implement some data queuing mechanism if data is not
to be lost.  For example, my ip330 ADC driver uses an epicsMessageQueue
to send the ADC data from the interrupt routine to the driver thread
that does callbacks to device support.  This queue must be large enough
to buffer the data when interrupts come close together and the callback
task is busy processing records.

I am leaning towards solution 2), but I would like to hear comments from
those who have an opinion.  Note that the solution must be applied to
all of the asyn device support, not just the asynInt32Array device
support Heinrich was working with.

Thanks,
Mark

> -----Original Message-----
> From: [email protected] 
> [mailto:[email protected]] On Behalf Of Heinrich du Toit
> Sent: Wednesday, October 10, 2007 3:19 AM
> To: TechTalk EPICS
> Subject: ASYN - calling read after interrupt - fix :)
> 
> Hi there
> 
> A while ago I posted a question asking why asyn calls my read 
> and write
> routines after interrupts. And it was said it shouldn't.
> 
> Well I think I figured it out.
> 
> The interrupt callback in asyn does pPvt->gotValue = 1.
> This is then suppose to let the process part just put the value in and
> not call read.
> 
> Well the problem is this:
> If you where to call interrupts very close to each other 
> (something not
> always avoidable) then gotvalue = 1 and scanIoRequest is 
> called once for
> each interrupt.
> This seems to mean process is called once for each interrupt.
> 
> Well the first process see gotvalue = 1 and skips the read part. This
> then works correctly and sets gotvalue = 0.
> Now the second and so on process execution finds gotvalue = 0 and then
> calls the read routine. 
> This offcourse only happens if the second interruptCallback executes
> before the queued process request can run.
> 
> The fix is actually pretty simple it seems:
> 
> in devAsynFloat64Array.c I changed the interruptCallbackInput to this:
> 
> 
> static void interruptCallbackInput(void *drvPvt, asynUser *pasynUser, 
>                 epicsFloat64 *value, size_t len)
> {
>     devAsynWfPvt *pPvt = (devAsynWfPvt *)drvPvt;
>     waveformRecord *pwf = (waveformRecord *)pPvt->pr;
>     int i;
>     epicsFloat64 *pfloat64 = (epicsFloat64 *)pwf->bptr;
> 
>     asynPrintIO(pPvt->pasynUser, ASYN_TRACEIO_DEVICE,
>         (char *)value, len*sizeof(epicsFloat64),
>         "%s devAsynFloat64Array::interruptCallbackInput\n",
>         pwf->name);
>     if (len > pwf->nelm) len = pwf->nelm;
>     for (i=0; i<len; i++) pfloat64[i] = value[i];
>     pPvt->nord = len;
>     if (pPvt->gotValue = 0){
>       pPvt->gotValue = 1;
>       scanIoRequest(pPvt->ioScanPvt);
>     }
> }
> 
> 
> (See the last 4 lines)
> 
> 
> I'm not sure if this is 100% bullet proof
> 
> As technically the interrupt can run while gotValue is still = 1 but
> pwf->nord = pPvt->nord is already executed in processCommon.
> 
> so to make this 100% bullet proof a mutex is needed I believe.
> 
> But I think doing this:
> 
> 
> 
> static long processCommon(dbCommon *pr)
> {
>     devAsynWfPvt *pPvt = (devAsynWfPvt *)pr->dpvt;
>     waveformRecord *pwf = (waveformRecord *)pr;
>     int status;
> 
>     if (!pPvt->gotValue && !pr->pact) {   /* This is an initial call
> from record */
>         if(pPvt->canBlock) pr->pact = 1;
>         status = pasynManager->queueRequest(pPvt->pasynUser, 0, 0);
>         if((status==asynSuccess) && pPvt->canBlock) return 0;
>         if(pPvt->canBlock) pr->pact = 0;
>         if (status != asynSuccess) {
>             asynPrint(pPvt->pasynUser, ASYN_TRACE_ERROR,
>                 "%s processCommon, error queuing request %s\n",
>                  pr->name, pPvt->pasynUser->errorMessage);
>             recGblSetSevr(pr, READ_ALARM, INVALID_ALARM);
>         }
>     }
>     if (pPvt->gotValue){
>       pPvt->gotValue = 0;
>       pwf->nord = pPvt->nord;
>     }
>     pPvt->gotValue = 0;
>     return 0;
> } 
> 
> Might improve things a little.
> 
> 
> I'm not sure if I got all my facts correct since I'm still 
> not 100% sure
> how the threading thing works inside ASYN.
> 
> But this seems atleast on my initial checks as if it stop the exstra
> read commands that seem to happen between bursts of interrupts :)
> 
> -Heinrich
> 
> 
> 
> 

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System