Argonne National Laboratory

Experimental Physics and
Industrial Control System

2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  <2017 Index 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  <2017
<== Date ==> <== Thread ==>

Subject: Re: Concerns about link-support-2 branch
From: Andrew Johnson <anj@aps.anl.gov>
To: Michael Davidsaver <mdavidsaver@gmail.com>
Cc: EPICS core-talk <core-talk@aps.anl.gov>
Date: Wed, 8 Mar 2017 16:44:57 -0600
Hi Michael,

On 03/07/2017 08:57 AM, Michael Davidsaver wrote:
> Maybe I was rambling too much.  Let me be clear.  I see no regression in
> this change.

Ok, *now* I think I understand what you're after.

> I don't like the existing behavior of the dbGet*() calls.  I've realized
> that the code I've been writing is open to race conditions w/ CA_LINK.
> I want to see this "fixed".  I agree that fixing dbCa.c is out of scope
> for your branch.  However, I think that such a fix will be complicated
> by introducing another layer of public API.

Maybe not.

Does the attached patch add sufficient functionality for your needs? I
haven't tested it yet, but I think the basic functionality is right.

You pass a callback routine to dbLinkDoLocked(plink, ...) and (assuming
the new lset::doLocked() method is supported by the link type) your
routine then gets called while any mutex associated with the link is
locked. dbLinkDoLocked() also passes down a void *private pointer for
context information, and the callback may then make multiple calls to
dbLink.h routines against this link while that mutex is locked. The
return status gets passed back up to the original caller. Note that
disconnected dbCa links will return -1 without calling the routine.

I prefer using a callback to providing direct access to a link's
lock/unlock capabilities, it's harder for user code to leave a link
locked this way. Link types that provide an lset::doLocked() method must
use a recursive mutex for locking.

- Andrew

-- 
Arguing for surveillance because you have nothing to hide is no
different than making the claim, "I don't care about freedom of
speech because I have nothing to say." -- Edward Snowdon
=== modified file 'src/ioc/db/dbLink.h'
--- src/ioc/db/dbLink.h	2016-09-04 00:21:11 +0000
+++ src/ioc/db/dbLink.h	2017-03-08 21:35:45 +0000
@@ -27,6 +27,8 @@
 
 struct dbLocker;
 
+typedef long (*dbLinkUserCallback)(struct link *plink, void *priv);
+
 typedef struct lset {
     /* Characteristics of the link type */
     const unsigned isConstant:1;
@@ -71,6 +73,9 @@
 
     /* Process */
     void (*scanForward)(struct link *plink);
+
+    /* Atomicity */
+    long (*doLocked)(struct link *plink, dbLinkUserCallback rtn, void *priv);
 } lset;
 
 #define dbGetSevr(link, sevr) \
@@ -117,6 +122,9 @@
         const void *pbuffer, long nRequest);
 epicsShareFunc void dbScanFwdLink(struct link *plink);
 
+epicsShareFunc void dbLinkDoLocked(struct link *plink, dbLinkUserCallback rtn,
+        void *priv);
+
 epicsShareFunc long dbLoadLinkLS(struct link *plink, char *pbuffer,
         epicsUInt32 size, epicsUInt32 *plen);
 epicsShareFunc long dbGetLinkLS(struct link *plink, char *pbuffer,

=== modified file 'src/ioc/db/dbLink.c'
--- src/ioc/db/dbLink.c	2016-09-07 05:47:51 +0000
+++ src/ioc/db/dbLink.c	2017-03-08 21:45:18 +0000
@@ -414,6 +414,18 @@
         plset->scanForward(plink);
 }
 
+long dbLinkDoLocked(struct link *plink, dbLinkUserCallback rtn,
+        void *priv)
+{
+    lset *plset = plink->lset;
+
+    if (!rtn || !plset || !plset->doLocked)
+        return S_db_noLSET;
+
+    return plset->doLocked(plink, rtn, priv);
+}
+
+
 /* Helper functions for long string support */
 
 long dbGetLinkLS(struct link *plink, char *pbuffer, epicsUInt32 size,

=== modified file 'src/ioc/db/dbCa.c'
--- src/ioc/db/dbCa.c	2016-09-04 00:21:11 +0000
+++ src/ioc/db/dbCa.c	2017-03-08 21:46:49 +0000
@@ -684,6 +684,17 @@
     return gotAttributes ? 0 : -1;
 }
 
+static long doLocked(struct link *plink, dbLinkUserCallback rtn, void *priv)
+{
+    caLink *pca;
+    long status;
+
+    pcaGetCheck
+    status = rtn(plink, priv);
+    epicsMutexUnlock(pca->lock);
+    return status;
+}
+
 static void scanComplete(void *raw, dbCommon *prec)
 {
     caLink *pca = raw;
@@ -727,7 +738,7 @@
     getPrecision, getUnits,
     getAlarm, getTimeStamp,
     dbCaPutLink, dbCaPutAsync,
-    scanForward
+    scanForward, doLocked
 };
 
 static void connectionCallback(struct connection_handler_args arg)

=== modified file 'src/ioc/db/dbDbLink.c'
--- src/ioc/db/dbDbLink.c	2016-09-04 00:21:11 +0000
+++ src/ioc/db/dbDbLink.c	2017-03-08 21:48:52 +0000
@@ -337,6 +337,11 @@
     dbScanPassive(precord, paddr->precord);
 }
 
+static long doLocked(struct link *plink, dbLinkUserCallback rtn, void *priv)
+{
+    return rtn(plink, priv);
+}
+
 static lset dbDb_lset = {
     0, 0, /* not Constant, not Volatile */
     NULL, dbDbRemoveLink,
@@ -348,6 +353,5 @@
     dbDbGetPrecision, dbDbGetUnits,
     dbDbGetAlarm, dbDbGetTimeStamp,
     dbDbPutValue, NULL,
-    dbDbScanFwdLink
+    dbDbScanFwdLink, doLocked
 };
-

=== modified file 'src/ioc/db/dbConstLink.c'
--- src/ioc/db/dbConstLink.c	2016-09-04 00:21:11 +0000
+++ src/ioc/db/dbConstLink.c	2017-03-08 21:50:15 +0000
@@ -135,6 +135,5 @@
     NULL, NULL,
     NULL, NULL,
     NULL, NULL,
-    NULL
+    NULL, NULL
 };
-

=== modified file 'src/std/link/lnkCalc.c'
--- src/std/link/lnkCalc.c	2016-09-07 03:28:36 +0000
+++ src/std/link/lnkCalc.c	2017-03-08 21:52:50 +0000
@@ -597,6 +597,11 @@
     return 0;
 }
 
+static long doLocked(struct link *plink, dbLinkUserCallback rtn, void *priv)
+{
+    return rtn(plink, priv);
+}
+
 
 /************************* Interface Tables *************************/
 
@@ -610,7 +615,7 @@
     lnkCalc_getPrecision, lnkCalc_getUnits,
     lnkCalc_getAlarm, NULL,
     NULL, NULL,
-    NULL
+    NULL, doLocked
 };
 
 static jlif lnkCalcIf = {

=== modified file 'src/std/link/lnkConst.c'
--- src/std/link/lnkConst.c	2016-09-05 19:25:33 +0000
+++ src/std/link/lnkConst.c	2017-03-08 21:51:44 +0000
@@ -551,7 +551,7 @@
     NULL, NULL,
     NULL, NULL,
     NULL, NULL,
-    NULL
+    NULL, NULL
 };
 
 static jlif lnkConstIf = {

=== modified file 'src/ioc/db/test/jlinkz.c'
--- src/ioc/db/test/jlinkz.c	2017-03-03 18:23:36 +0000
+++ src/ioc/db/test/jlinkz.c	2017-03-08 22:38:30 +0000
@@ -146,6 +146,7 @@
     &z_putval,
     NULL, /* putasync */
     NULL, /* forward */
+    NULL, /* doLocked */
 };
 
 static

=== modified file 'src/ioc/db/test/xLink.c'
--- src/ioc/db/test/xLink.c	2016-09-07 05:50:40 +0000
+++ src/ioc/db/test/xLink.c	2017-03-08 22:38:53 +0000
@@ -73,7 +73,7 @@
     NULL, NULL,
     NULL, NULL,
     NULL, NULL,
-    NULL
+    NULL, NULL
 };
 
 static jlif xlinkIf = {


Replies:
Re: Concerns about link-support-2 branch Michael Davidsaver
References:
Concerns about link-support-2 branch Andrew Johnson
Re: Concerns about link-support-2 branch Michael Davidsaver
Re: Concerns about link-support-2 branch Johnson, Andrew N.
Re: Concerns about link-support-2 branch Michael Davidsaver

Navigate by Date:
Prev: Jenkins build is back to normal : epics-base-3.16-mac-test #71 APS Jenkins
Next: Re: Concerns about link-support-2 branch Michael Davidsaver
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  <2017
Navigate by Thread:
Prev: Re: Concerns about link-support-2 branch Michael Davidsaver
Next: Re: Concerns about link-support-2 branch Michael Davidsaver
Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  <2017
ANJ, 08 Mar 2017 Valid HTML 4.01! · Home · News · About · Base · Modules · Extensions · Distributions · Download ·
· EPICS V4 · IRMIS · Talk · Bugs · Documents · Links · Licensing ·