Hi zoran,

comments inline.

On Wednesday 16 March 2016 03:37 PM, Zoran Milinkovic wrote:
> Hi Neelakanta,
>
> Ack with a minor comment.
>
> Instead of calling clock_gettime(), it would be better if 
> osaf_clock_gettime() is called.
> Find the comment inline
When the helper function is moved to ImmModel osaf_clock_gettime will be 
used.

Regards,
Neel.
>
> -----Original Message-----
> From: [email protected] [mailto:[email protected]]
> Sent: Wednesday, March 16, 2016 8:36 AM
> To: Zoran Milinkovic; Anders Widell; Hung Duc Nguyen
> Cc: [email protected]
> Subject: [PATCH 1 of 1] imm: changing from system time to monotonic time 
> [#1617]
>
>   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);
>
> [Zoran] "IF" statement above can be replaced with:
>       osaf_clock_gettime(i_clk_id, &o_ts);
>
> Thanks,
> Zoran
>
> +     *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