Re: [devel] [PATCH 1/1] amfd: put sync jobs into queue if IMM is busy [#2863]

Wed, 04 Jul 2018 16:46:32 -0700 

Hi Gary,

ack with a minor comment with [M]

Thanks,

Minh


On 04/07/18 11:15, Gary Lee wrote:

---
  src/amf/amfd/cb.h   |  3 ++-
  src/amf/amfd/clm.cc |  4 ++--
  src/amf/amfd/clm.h  |  4 ++--
  src/amf/amfd/imm.cc | 33 ++++++++++++++++++++++++---------
  src/amf/amfd/imm.h  | 18 +++++++++---------
  src/amf/amfd/ntf.cc |  2 +-
  6 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/src/amf/amfd/cb.h b/src/amf/amfd/cb.h
index 60bb554de..3b7e6d13f 100644
--- a/src/amf/amfd/cb.h
+++ b/src/amf/amfd/cb.h
@@ -63,7 +63,8 @@ typedef enum {
    AVD_IMM_INIT_BASE = 1,
    AVD_IMM_INIT_ONGOING = 2,
    AVD_IMM_INIT_DONE = 3,
-  AVD_IMM_TERMINATING = 4,
+  AVD_IMM_BUSY = 4,
+  AVD_IMM_TERMINATING = 5,
  } AVD_IMM_INIT_STATUS;
  /*
   * Sync state of the Standby.
diff --git a/src/amf/amfd/clm.cc b/src/amf/amfd/clm.cc
index 9d317892a..e113a65f9 100644
--- a/src/amf/amfd/clm.cc
+++ b/src/amf/amfd/clm.cc
@@ -627,7 +627,7 @@ SaAisErrorT avd_start_clm_init_bg(void) {
    return SA_AIS_OK;
  }
-AvdJobDequeueResultT ClmTrackStart::exec(const AVD_CL_CB* cb) { 
+AvdJobDequeueResultT ClmTrackStart::exec(AVD_CL_CB* cb) {
    AvdJobDequeueResultT res;
    TRACE_ENTER();
@@ -648,7 +648,7 @@ AvdJobDequeueResultT ClmTrackStart::exec(const AVD_CL_CB* cb) { 
    return res;
  }
-AvdJobDequeueResultT ClmTrackStop::exec(const AVD_CL_CB* cb) { 
+AvdJobDequeueResultT ClmTrackStop::exec(AVD_CL_CB* cb) {
    AvdJobDequeueResultT res;
    TRACE_ENTER();
diff --git a/src/amf/amfd/clm.h b/src/amf/amfd/clm.h 
index 2bbe320f7..f4399c62e 100644
--- a/src/amf/amfd/clm.h
+++ b/src/amf/amfd/clm.h
@@ -40,14 +40,14 @@ public:
  class ClmTrackStart : public ClmJob {
   public:
    ClmTrackStart() : ClmJob(){};
-  AvdJobDequeueResultT exec(const struct cl_cb_tag *cb);
+  AvdJobDequeueResultT exec(struct cl_cb_tag *cb);
    ~ClmTrackStart() {}
  };
class ClmTrackStop : public ClmJob { 
   public:
    ClmTrackStop() : ClmJob(){};
-  AvdJobDequeueResultT exec(const struct cl_cb_tag *cb);
+  AvdJobDequeueResultT exec(struct cl_cb_tag *cb);
    ~ClmTrackStop() {}
  };
diff --git a/src/amf/amfd/imm.cc b/src/amf/amfd/imm.cc 
index 60a997943..3c1a93729 100644
--- a/src/amf/amfd/imm.cc
+++ b/src/amf/amfd/imm.cc
@@ -133,7 +133,7 @@ Job::~Job() {}
// TODO: Make isImmServiceReady as static to limit its scope 
  // This function should belong to AVD_CB class as a method
-static bool isImmServiceReady(const AVD_CL_CB *cb) {
+static bool isImmServiceReady(const AVD_CL_CB *cb, bool ignore_busy = false) {
    TRACE_ENTER();
    bool rc = true;
@@ -149,16 +149,21 @@ static bool isImmServiceReady(const AVD_CL_CB *cb) { 
      TRACE("Already IMM init is going, try again after sometime");
      rc = false;
    }
+  if (avd_cb->avd_imm_status == AVD_IMM_BUSY &&
+    ignore_busy == false) {
+    TRACE("IMM returned TRY_AGAIN. Postponing synchronous calls");
+    rc = false;
+  }
    TRACE_LEAVE2("%u:", rc);
    return rc;
  }
// 
  bool ImmJob::isRunnable(const AVD_CL_CB *cb) {
-  return isImmServiceReady(cb);
+  return isImmServiceReady(cb, true);
  }
  //
-AvdJobDequeueResultT ImmObjCreate::exec(const AVD_CL_CB *cb) {
+AvdJobDequeueResultT ImmObjCreate::exec(AVD_CL_CB *cb) {
    SaAisErrorT rc;
    AvdJobDequeueResultT res;
    const SaImmOiHandleT immOiHandle = cb->immOiHandle;
@@ -173,6 +178,7 @@ AvdJobDequeueResultT ImmObjCreate::exec(const AVD_CL_CB 
*cb) {
    }
    rc = saImmOiRtObjectCreate_2(immOiHandle, className_, parent_name,
                                 attrValues_);
+  cb->avd_imm_status = AVD_IMM_INIT_DONE;
if ((rc == SA_AIS_OK) || (rc == SA_AIS_ERR_EXIST)) { 
      delete Fifo::dequeue();
@@ -180,6 +186,7 @@ AvdJobDequeueResultT ImmObjCreate::exec(const AVD_CL_CB 
*cb) {
    } else if (rc == SA_AIS_ERR_TRY_AGAIN) {
      TRACE("TRY-AGAIN");
      res = JOB_ETRYAGAIN;
+    cb->avd_imm_status = AVD_IMM_BUSY;
    } else if (rc == SA_AIS_ERR_TIMEOUT) {
      TRACE("TIMEOUT");
      res = JOB_ETRYAGAIN;
@@ -228,7 +235,7 @@ ImmObjCreate::~ImmObjCreate() {
  }
// 
-AvdJobDequeueResultT ImmObjUpdate::exec(const AVD_CL_CB *cb) {
+AvdJobDequeueResultT ImmObjUpdate::exec(AVD_CL_CB *cb) {
    SaAisErrorT rc;
    AvdJobDequeueResultT res;
    const SaImmOiHandleT immOiHandle = cb->immOiHandle;
@@ -252,6 +259,7 @@ AvdJobDequeueResultT ImmObjUpdate::exec(const AVD_CL_CB 
*cb) {
    attrMod.modAttr.attrValues = attrValues;
rc = saImmOiRtObjectUpdate_o3(immOiHandle, dn.c_str(), attrMods); 
+  cb->avd_imm_status = AVD_IMM_INIT_DONE;
if ((rc == SA_AIS_OK) || (rc == SA_AIS_ERR_NOT_EXIST)) { 
      delete Fifo::dequeue();
@@ -259,6 +267,7 @@ AvdJobDequeueResultT ImmObjUpdate::exec(const AVD_CL_CB 
*cb) {
    } else if (rc == SA_AIS_ERR_TRY_AGAIN) {
      TRACE("TRY-AGAIN");
      res = JOB_ETRYAGAIN;
+    cb->avd_imm_status = AVD_IMM_BUSY;
    } else if (rc == SA_AIS_ERR_TIMEOUT) {
      TRACE("TIMEOUT");
      res = JOB_ETRYAGAIN;
@@ -286,7 +295,7 @@ ImmObjUpdate::~ImmObjUpdate() {
  }
// 
-AvdJobDequeueResultT ImmObjDelete::exec(const AVD_CL_CB *cb) {
+AvdJobDequeueResultT ImmObjDelete::exec(AVD_CL_CB *cb) {
    SaAisErrorT rc;
    AvdJobDequeueResultT res;
    const SaImmOiHandleT immOiHandle = cb->immOiHandle;
@@ -300,6 +309,7 @@ AvdJobDequeueResultT ImmObjDelete::exec(const AVD_CL_CB 
*cb) {
      goto done;
    }
    rc = saImmOiRtObjectDelete_o3(immOiHandle, dn.c_str());
+  cb->avd_imm_status = AVD_IMM_INIT_DONE;
if ((rc == SA_AIS_OK) || (rc == SA_AIS_ERR_NOT_EXIST)) { 
      delete Fifo::dequeue();
@@ -307,6 +317,7 @@ AvdJobDequeueResultT ImmObjDelete::exec(const AVD_CL_CB 
*cb) {
    } else if (rc == SA_AIS_ERR_TRY_AGAIN) {
      TRACE("TRY-AGAIN");
      res = JOB_ETRYAGAIN;
+    cb->avd_imm_status = AVD_IMM_BUSY;
    } else if (rc == SA_AIS_ERR_TIMEOUT) {
      TRACE("TIMEOUT");
      res = JOB_ETRYAGAIN;
@@ -327,7 +338,7 @@ done:
  ImmObjDelete::~ImmObjDelete() {}
// 
-AvdJobDequeueResultT ImmAdminResponse::exec(const AVD_CL_CB *cb) {
+AvdJobDequeueResultT ImmAdminResponse::exec(AVD_CL_CB *cb) {
    SaAisErrorT rc;
    AvdJobDequeueResultT res;
    const SaImmOiHandleT handle = cb->immOiHandle;
@@ -335,6 +346,7 @@ AvdJobDequeueResultT ImmAdminResponse::exec(const AVD_CL_CB 
*cb) {
    TRACE_ENTER2("Admin resp inv:%llu res:%u", invocation_, result_);
rc = saImmOiAdminOperationResult(handle, invocation_, result_); 
+  cb->avd_imm_status = AVD_IMM_INIT_DONE;
switch (rc) { 
      case SA_AIS_OK:
@@ -347,6 +359,7 @@ AvdJobDequeueResultT ImmAdminResponse::exec(const AVD_CL_CB 
*cb) {
      case SA_AIS_ERR_TRY_AGAIN:
        TRACE("TRY-AGAIN");
        res = JOB_ETRYAGAIN;
+      cb->avd_imm_status = AVD_IMM_BUSY;
        break;
      case SA_AIS_ERR_BAD_HANDLE:
        // there is no need to reattempt reply,
@@ -404,7 +417,7 @@ void ckpt_job_queue_size() {
  }
// 
-AvdJobDequeueResultT Fifo::execute(const AVD_CL_CB *cb) {
+AvdJobDequeueResultT Fifo::execute(AVD_CL_CB *cb) {
    Job *ajob;
    AvdJobDequeueResultT ret;
@@ -428,7 +441,7 @@ AvdJobDequeueResultT Fifo::execute(const AVD_CL_CB *cb) { 
    return ret;
  }
-AvdJobDequeueResultT Fifo::executeAll(const AVD_CL_CB *cb, AvdJobTypeT job_type) { 
+AvdJobDequeueResultT Fifo::executeAll(AVD_CL_CB *cb, AvdJobTypeT job_type) {
Job *ajob, *firstjob; 
    AvdJobDequeueResultT ret = JOB_EXECUTED;
@@ -489,7 +502,7 @@ void Fifo::remove(const AVD_CL_CB *cb, AvdJobTypeT 
job_type) {
    TRACE_LEAVE();
  }
-AvdJobDequeueResultT Fifo::executeAdminResp(const AVD_CL_CB *cb) { 
+AvdJobDequeueResultT Fifo::executeAdminResp(AVD_CL_CB *cb) {
    Job *ajob;
    AvdJobDequeueResultT ret = JOB_EXECUTED;
@@ -2175,6 +2188,8 @@ static void *avd_imm_reinit_bg_thread(void *_cb) { 
    status = m_NCS_IPC_SEND(&avd_cb->avd_mbx, evt, NCS_IPC_PRIORITY_VERY_HIGH);
    osafassert(status == NCSCC_RC_SUCCESS);
+ cb->avd_imm_status = AVD_IMM_INIT_DONE; 
+
[M]: Is it ok to set @avd_imm_status here in this thread? it's currently set in main thread after receiving event AVD_IMM_REINITIALIZED. 
    LOG_NO("Finished re-initializing with IMM");
    /* Release mutex taken.*/
    osaf_mutex_unlock_ordie(&imm_reinit_mutex);
diff --git a/src/amf/amfd/imm.h b/src/amf/amfd/imm.h
index 83c56869b..1778d27ee 100644
--- a/src/amf/amfd/imm.h
+++ b/src/amf/amfd/imm.h
@@ -64,7 +64,7 @@ typedef enum {
  // @todo move this into job.h
  class Job {
   public:
-  virtual AvdJobDequeueResultT exec(const AVD_CL_CB *cb) = 0;
+  virtual AvdJobDequeueResultT exec(AVD_CL_CB *cb) = 0;
    virtual ~Job() = 0;
    virtual AvdJobTypeT getJobType() = 0;
    virtual bool isRunnable(const AVD_CL_CB *cb) = 0;
@@ -87,7 +87,7 @@ class ImmObjCreate : public ImmJob {
ImmObjCreate() : ImmJob(){}; 
    bool immobj_update_required();
-  AvdJobDequeueResultT exec(const AVD_CL_CB *cb);
+  AvdJobDequeueResultT exec(AVD_CL_CB *cb);
~ImmObjCreate(); 
  };
@@ -102,7 +102,7 @@ class ImmObjUpdate : public ImmJob {
ImmObjUpdate() : ImmJob(){}; 
    bool immobj_update_required();
-  AvdJobDequeueResultT exec(const AVD_CL_CB *cb);
+  AvdJobDequeueResultT exec(AVD_CL_CB *cb);
    bool si_get_attr_value();
    bool siass_get_attr_value();
    bool csiass_get_attr_value();
@@ -122,7 +122,7 @@ class ImmObjDelete : public ImmJob {
    bool immobj_update_required();
    bool is_csiass_exist();
    bool is_siass_exist();
-  AvdJobDequeueResultT exec(const AVD_CL_CB *cb);
+  AvdJobDequeueResultT exec(AVD_CL_CB *cb);
~ImmObjDelete(); 
  };
@@ -133,7 +133,7 @@ class ImmAdminResponse : public ImmJob {
      this->invocation_ = invocation;
      this->result_ = result;
    }
-  AvdJobDequeueResultT exec(const AVD_CL_CB *cb);
+  AvdJobDequeueResultT exec(AVD_CL_CB *cb);
~ImmAdminResponse() {} @@ -146,7 +146,7 @@ class ImmAdminResponse : public ImmJob { 
  class NtfSend : public Job {
   public:
    NtfSend() : already_sent(false) {}
-  AvdJobDequeueResultT exec(const AVD_CL_CB *cb);
+  AvdJobDequeueResultT exec(AVD_CL_CB *cb);
    AvdJobTypeT getJobType() { return JOB_TYPE_NTF; }
    bool isRunnable(const AVD_CL_CB *cb) { return true;}
    SaNtfNotificationsT myntf;
@@ -164,12 +164,12 @@ class Fifo {
static Job *dequeue(); - static AvdJobDequeueResultT execute(const AVD_CL_CB *cb); 
-  static AvdJobDequeueResultT executeAll(const AVD_CL_CB *cb,
+  static AvdJobDequeueResultT execute(AVD_CL_CB *cb);
+  static AvdJobDequeueResultT executeAll(AVD_CL_CB *cb,
        AvdJobTypeT job_type = JOB_TYPE_ANY);
    static void remove(const AVD_CL_CB *cb,
        AvdJobTypeT job_type = JOB_TYPE_ANY);
-  static AvdJobDequeueResultT executeAdminResp(const AVD_CL_CB *cb);
+  static AvdJobDequeueResultT executeAdminResp(AVD_CL_CB *cb);
static void empty(); diff --git a/src/amf/amfd/ntf.cc b/src/amf/amfd/ntf.cc 
index 500bc7046..b8bed1a8f 100644
--- a/src/amf/amfd/ntf.cc
+++ b/src/amf/amfd/ntf.cc
@@ -797,7 +797,7 @@ SaAisErrorT avd_start_ntf_init_bg(void) {
    return SA_AIS_OK;
  }
-AvdJobDequeueResultT NtfSend::exec(const AVD_CL_CB* cb) { 
+AvdJobDequeueResultT NtfSend::exec(AVD_CL_CB* cb) {
    AvdJobDequeueResultT res;
    TRACE_ENTER2("Ntf Type:%x, sent status:%u", myntf.notificationType,
                 already_sent);



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to