Hi!

One comment here: wouldn't it be better to change the type of 
mCreateTime to "struct timespec"? By using time_t we get a very low 
resolution of time measurements. A timeout of six seconds could result 
in an actual timeout of only five seconds, if we happen to read the 
system clock just before the second dial moves to the next second. By 
using struct timespec instead of time_t, you don't have to introduce new 
helper functions and the resolution will be nanoseconds instead of seconds.

Another thing to note is that I have introduced C++ helper functions for 
dealing with struct timespec in ticket [#836]. These include operators 
for arithmetic operations and comparisons. Another option in C++ is to 
use std::chrono::steady_clock from the standard library, if you think 
that is easier.

regards,
Anders Widell

On 03/16/2016 08:36 AM, [email protected] wrote:
>   osaf/libs/core/common/include/osaf_time.h    |  20 +++++++++++++++++++
>   osaf/services/saf/immsv/immnd/ImmModel.cc    |  29 
> +++++++++++++++------------
>   osaf/services/saf/immsv/immnd/ImmSearchOp.hh |   3 +-
>   osaf/services/saf/immsv/immnd/immnd_evt.c    |   7 +++--
>   osaf/services/saf/immsv/immnd/immnd_proc.c   |   6 +++-
>   5 files changed, 46 insertions(+), 19 deletions(-)
>
>
> diff --git a/osaf/libs/core/common/include/osaf_time.h 
> b/osaf/libs/core/common/include/osaf_time.h
> --- a/osaf/libs/core/common/include/osaf_time.h
> +++ b/osaf/libs/core/common/include/osaf_time.h
> @@ -89,6 +89,18 @@ extern void osaf_nanosleep(const struct
>   static inline void osaf_clock_gettime(clockid_t i_clk_id,
>       struct timespec* o_ts);
>   
> +
> +/**
> + * @brief Get the time in seconds
> + *
> + * This is a convenience function that behaves exactly like the POSIX 
> function
> + * clock_gettime(3P), except that it will abort the process instead of 
> returning
> + * an error code in case of a failure. The Output vlaue passed will be in 
> seconds.
> + */
> +static inline void osaf_clock_gettime_sec(clockid_t i_clk_id,
> +        time_t* o_t);
> +
> +
>   /**
>    * @brief Normalize a timespec structure.
>    *
> @@ -257,6 +269,14 @@ static inline void osaf_clock_gettime(cl
>       if (clock_gettime(i_clk_id, o_ts) != 0) osaf_abort(i_clk_id);
>   }
>   
> +static inline void osaf_clock_gettime_sec(clockid_t i_clk_id,
> +        time_t* o_t)
> +{
> +     struct timespec o_ts;
> +     if (clock_gettime(i_clk_id, &o_ts) != 0) osaf_abort(i_clk_id);
> +     *o_t = o_ts.tv_sec;
> +}
> +
>   static inline void osaf_normalize_timespec(const struct timespec* i_ts,
>       struct timespec* o_nrm)
>   {
> diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc 
> b/osaf/services/saf/immsv/immnd/ImmModel.cc
> --- a/osaf/services/saf/immsv/immnd/ImmModel.cc
> +++ b/osaf/services/saf/immsv/immnd/ImmModel.cc
> @@ -27,6 +27,7 @@
>   #include "immnd.h"
>   #include "osaf_unicode.h"
>   #include "osaf_extended_name.h"
> +#include "osaf_time.h"
>   
>   // Local types
>   #define DEFAULT_TIMEOUT_SEC 6 /* Should be saImmOiTimeout in SaImmMngt */
> @@ -44,8 +45,7 @@ struct ContinuationInfo2
>       ContinuationInfo2():mCreateTime(0), mConn(0), mTimeout(0), mImplId(0){}
>       ContinuationInfo2(SaUint32T conn, SaUint32T timeout):mConn(conn), 
> mTimeout(timeout),
>            mImplId(0)
> -        {mCreateTime = time(NULL);osafassert(mCreateTime >= ((time_t) 0));}
> -
> +         {osaf_clock_gettime_sec(CLOCK_MONOTONIC, 
> &mCreateTime);osafassert(mCreateTime >= ((time_t) 0));}
>       time_t  mCreateTime;
>       SaUint32T mConn;
>       SaUint32T mTimeout; //0=> no timeout. Otherwise timeout in SECONDS.
> @@ -1121,7 +1121,8 @@ immModel_cleanTheBasement(IMMND_CB *cb,
>           osafassert(ix==(*pbePrtoReqArrSize));
>       }
>   
> -    time_t now = time(NULL);
> +    time_t now;
> +    osaf_clock_gettime_sec(CLOCK_MONOTONIC, &now);
>       osafassert(now >= ((time_t) 0));
>       time_t nextSearch = 0;
>       time_t opSearchTime;
> @@ -5416,7 +5417,7 @@ ImmModel::ccbApply(SaUint32T ccbId,
>                   implAssoc->mWaitForImplAck = true;
>                   implAssoc->mContinuationId = sLastContinuationId;/* 
> incremented above */
>                   if(ccb->mWaitStartTime == 0) {
> -                    ccb->mWaitStartTime = time(NULL);
> +                    osaf_clock_gettime_sec(CLOCK_MONOTONIC, 
> &ccb->mWaitStartTime);
>                       osafassert(ccb->mWaitStartTime >= ((time_t) 0));
>                       TRACE("Wait timer for completed started for ccb:%u",
>                           ccb->mId);
> @@ -6263,7 +6264,7 @@ ImmModel::ccbTerminate(SaUint32T ccbId)
>           /*  Retain the ccb info to allow ccb result recovery. */
>   
>           if(ccb->mWaitStartTime == 0)  {
> -            ccb->mWaitStartTime = time(NULL);
> +            osaf_clock_gettime_sec(CLOCK_MONOTONIC, &ccb->mWaitStartTime);
>               osafassert(ccb->mWaitStartTime >= ((time_t) 0));
>               TRACE_5("Ccb Wait-time for GC set. State: %u/%s", ccb->mState,
>                   (ccb->mState == IMM_CCB_COMMITTED)?"COMMITTED":
> @@ -8037,7 +8038,7 @@ SaAisErrorT ImmModel::ccbObjectCreate(Im
>                   TRACE_5("THERE IS AN IMPLEMENTER %u conn:%u node:%x 
> name:%s\n",
>                       object->mImplementer->mId, *implConn, *implNodeId,
>                       object->mImplementer->mImplementerName.c_str());
> -                ccb->mWaitStartTime = time(NULL);
> +                osaf_clock_gettime_sec(CLOCK_MONOTONIC, 
> &ccb->mWaitStartTime);
>                   osafassert(ccb->mWaitStartTime >= ((time_t) 0));
>               } else if(className == immMngtClass) {
>                   if(sImmNodeState == IMM_NODE_LOADING) {
> @@ -9239,7 +9240,7 @@ ImmModel::ccbObjectModify(const ImmsvOmC
>                   object->mImplementer->mId, *implConn, *implNodeId,
>                   object->mImplementer->mImplementerName.c_str());
>               
> -            ccb->mWaitStartTime = time(NULL);
> +            osaf_clock_gettime_sec(CLOCK_MONOTONIC, &ccb->mWaitStartTime);
>               osafassert(ccb->mWaitStartTime >= ((time_t) 0));
>           } else if(ccb->mCcbFlags & SA_IMM_CCB_REGISTERED_OI) {
>               if((object->mImplementer == NULL) &&
> @@ -9808,7 +9809,7 @@ ImmModel::deleteObject(ObjectMap::iterat
>   
>               SaUint32T implConn = oi->second->mImplementer->mConn;
>               
> -            ccb->mWaitStartTime = time(NULL);
> +            osaf_clock_gettime_sec(CLOCK_MONOTONIC, &ccb->mWaitStartTime);
>               osafassert(ccb->mWaitStartTime >= ((time_t) 0));
>               /* TODO: Resetting the ccb timer for each deleted object here.
>                  Not so efficient. Should set it only when all objects
> @@ -10150,7 +10151,7 @@ ImmModel::ccbWaitForCompletedAck(SaUint3
>                  objects write locked by the ccb) until we know the outcome.
>                  Restart the timer to catch ccbs hung waiting on PBE.
>               */
> -             ccb->mWaitStartTime = time(NULL);
> +             osaf_clock_gettime_sec(CLOCK_MONOTONIC, &ccb->mWaitStartTime);
>                osafassert(ccb->mWaitStartTime >= ((time_t) 0));
>               return true; /* Wait for PBE commit*/
>           } else {
> @@ -12825,7 +12826,8 @@ ImmModel::getOldCriticalCcbs(IdVector& c
>       *pbeConnPtr = 0;
>       *pbeIdPtr = 0;
>       CcbVector::iterator i;
> -    time_t now = time(NULL);
> +    time_t now;
> +    osaf_clock_gettime_sec(CLOCK_MONOTONIC, &now);
>       osafassert(now >= ((time_t) 0));
>       for(i=sCcbVector.begin(); i!=sCcbVector.end(); ++i) {
>           if((*i)->mState == IMM_CCB_CRITICAL &&
> @@ -13046,7 +13048,8 @@ ImmModel::cleanTheBasement(InvocVector&
>       InvocVector& searchReqs, IdVector& ccbs, IdVector& pbePrtoReqs,
>       bool iAmCoord)
>   {
> -    time_t now = time(NULL);
> +    time_t now;
> +    osaf_clock_gettime_sec(CLOCK_MONOTONIC, &now);
>       osafassert(now >= ((time_t) 0));
>       ContinuationMap2::iterator ci2;
>       ImplementerEvtMap::iterator iem;
> @@ -13455,7 +13458,7 @@ ImmModel::implementerSet(const IMMSV_OCT
>                                   ccb->mImplementers[info->mId] = 
> oldImplAssoc;
>                                   TRACE_7("Replaced implid %u with %u", 
> oldImplId, info->mId);
>                                   ccb->mPbeRestartId = info->mId;
> -                                ccb->mWaitStartTime = time(NULL);/*Reset 
> timer on new impl*/
> +                                osaf_clock_gettime_sec(CLOCK_MONOTONIC, 
> &ccb->mWaitStartTime);/*Reset timer on new impl*/
>                                   osafassert(ccb->mWaitStartTime >= ((time_t) 
> 0));
>                                   /* Can only be one PBE impl asoc*/
>                                   break;  /* out of for(isi = 
> ccb->mImplementers....*/
> @@ -17877,7 +17880,7 @@ ImmModel::finalizeSync(ImmsvOmFinalizeSy
>                   newCcb->mOriginatingConn = 0;
>                   newCcb->mVeto = SA_AIS_OK;
>                   newCcb->mState = (ImmCcbState) (prt45allowed ? ol->ccbState 
> : (ol->ccbState + 2));
> -                newCcb->mWaitStartTime = time(NULL);
> +                osaf_clock_gettime_sec(CLOCK_MONOTONIC, 
> &newCcb->mWaitStartTime);
>                   if(newCcb->mWaitStartTime < ((time_t) 0)) {
>                       LOG_ER("newCcb->mWaitStartTime < 0");
>                       err = SA_AIS_ERR_FAILED_OPERATION;
> diff --git a/osaf/services/saf/immsv/immnd/ImmSearchOp.hh 
> b/osaf/services/saf/immsv/immnd/ImmSearchOp.hh
> --- a/osaf/services/saf/immsv/immnd/ImmSearchOp.hh
> +++ b/osaf/services/saf/immsv/immnd/ImmSearchOp.hh
> @@ -23,6 +23,7 @@
>   #include <string>
>   #include <list>
>   #include <time.h>
> +#include "osaf_time.h"
>   
>   
>   struct SearchAttribute
> @@ -85,7 +86,7 @@ public:
>       bool          isAccessor() {return mIsAccessor;}
>       bool          isNonExtendedNameSet() {return mNonExtendedName;}
>       time_t        getLastSearchTime() { return mLastSearch; }
> -    void          updateSearchTime() { mLastSearch = time(NULL); }
> +    void          updateSearchTime() { 
> osaf_clock_gettime_sec(CLOCK_MONOTONIC, &mLastSearch); }
>       void*         syncOsi;
>       void*         attrNameList;
>       void*         classInfo;
> diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c 
> b/osaf/services/saf/immsv/immnd/immnd_evt.c
> --- a/osaf/services/saf/immsv/immnd/immnd_evt.c
> +++ b/osaf/services/saf/immsv/immnd/immnd_evt.c
> @@ -33,6 +33,7 @@
>   #include "ncssysf_mem.h"
>   #include "mds_papi.h"
>   #include "osaf_extended_name.h"
> +#include "osaf_time.h"
>   
>   /* Adjust to 90% of MDS_DIRECT_BUF_MAXSIZE  */
>   #define IMMND_SEARCH_BUNDLE_SIZE ((MDS_DIRECT_BUF_MAXSIZE / 100) * 90)
> @@ -8384,7 +8385,7 @@ uint32_t immnd_evt_proc_abort_sync(IMMND
>                       cb->mState = IMM_SERVER_LOADING_PENDING;
>                       LOG_WA("SERVER STATE: IMM_SERVER_SYNC_CLIENT --> 
> IMM_SERVER_LOADING_PENDING (sync aborted)");
>                       cb->mStep = 0;
> -                     cb->mJobStart = time(NULL);
> +                     osaf_clock_gettime_sec(CLOCK_MONOTONIC, &cb->mJobStart);
>                       osafassert(cb->mJobStart >= ((time_t) 0));
>                       cb->mMyEpoch = 0;
>                       cb->mSync = false;
> @@ -8519,7 +8520,7 @@ static uint32_t immnd_evt_proc_start_syn
>               immModel_setScAbsenceAllowed(cb);
>       } else if ((cb->mState == IMM_SERVER_SYNC_CLIENT) && 
> (immnd_syncComplete(cb, SA_FALSE, cb->mStep))) {
>               cb->mStep = 0;
> -             cb->mJobStart = time(NULL);
> +             osaf_clock_gettime_sec(CLOCK_MONOTONIC, &cb->mJobStart);
>               osafassert(cb->mJobStart >= ((time_t) 0));
>               cb->mState = IMM_SERVER_READY;
>               immnd_ackToNid(NCSCC_RC_SUCCESS);
> @@ -8653,7 +8654,7 @@ static uint32_t immnd_evt_proc_loading_o
>                       LOG_NO("SERVER STATE: IMM_SERVER_LOADING_PENDING --> 
> IMM_SERVER_LOADING_CLIENT (materialized by proc_loading_ok)");
>                       cb->mState = IMM_SERVER_LOADING_CLIENT;
>                       cb->mStep = 0;
> -                     cb->mJobStart = time(NULL);
> +                     osaf_clock_gettime_sec(CLOCK_MONOTONIC, &cb->mJobStart);
>                       osafassert(cb->mJobStart >= ((time_t) 0));
>                       cb->mAccepted = true;
>                       if(cb->mCanBeCoord) {cb->mIsOtherScUp = true;}
> diff --git a/osaf/services/saf/immsv/immnd/immnd_proc.c 
> b/osaf/services/saf/immsv/immnd/immnd_proc.c
> --- a/osaf/services/saf/immsv/immnd/immnd_proc.c
> +++ b/osaf/services/saf/immsv/immnd/immnd_proc.c
> @@ -35,6 +35,7 @@
>   #include "immnd.h"
>   #include "immsv_api.h"
>   #include "immnd_init.h"
> +#include "osaf_time.h"
>   
>   static const char *loaderBase = "osafimmloadd";
>   static const char *pbeBase = "osafimmpbed";
> @@ -597,7 +598,7 @@ void immnd_announceDump(IMMND_CB *cb)
>               /* Reset jobStart timer to delay potential start of sync.
>                  Reduces risk of epoch race with dump.
>                */
> -             cb->mJobStart = time(NULL);
> +             osaf_clock_gettime_sec(CLOCK_MONOTONIC, &cb->mJobStart);
>               osafassert(cb->mJobStart >= ((time_t) 0));
>       }
>   }
> @@ -1648,7 +1649,8 @@ uint32_t immnd_proc_server(uint32_t *tim
>       uint32_t rc = NCSCC_RC_SUCCESS;
>       int32_t coord, newEpoch;
>       int32_t printFrq = (*timeout > 100) ? 5 : 50;
> -     time_t now = time(NULL);
> +     time_t now;
> +     osaf_clock_gettime_sec(CLOCK_MONOTONIC, &now);
>       osafassert(now >= ((time_t) 0));
>       uint32_t jobDuration = (uint32_t) now - cb->mJobStart;
>       SaBoolT pbeImmndDeadlock=SA_FALSE;


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785231&iu=/4140
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to