ran cppcheck on this patch it added two comments. otherwise ACK from me.
On 06/14/2016 01:26 PM, reddy.neelaka...@oracle.com wrote: > osaf/services/saf/smfsv/smfd/smfd.h | 4 + > osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc | 19 ++++- > osaf/services/saf/smfsv/smfd/smfd_cb.h | 1 + > osaf/services/saf/smfsv/smfd/smfd_main.c | 86 > ++++++++++++++++++++++- > 4 files changed, 104 insertions(+), 6 deletions(-) > > > mutex is introduced, when accessing IMM OI handle, between campaign thread > and main thread > > diff --git a/osaf/services/saf/smfsv/smfd/smfd.h > b/osaf/services/saf/smfsv/smfd/smfd.h > --- a/osaf/services/saf/smfsv/smfd/smfd.h > +++ b/osaf/services/saf/smfsv/smfd/smfd.h > @@ -98,6 +98,10 @@ extern "C" { > > void smfd_cb_lock(); > void smfd_cb_unlock(); > + void smfd_imm_lock(); > + void smfd_imm_unlock(); > + int smfd_imm_trylock(); > + > > #ifdef __cplusplus > } > diff --git a/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc > b/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc > --- a/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc > +++ b/osaf/services/saf/smfsv/smfd/smfd_campaign_oi.cc > @@ -743,11 +743,26 @@ uint32_t create_campaign_objects(smfd_cb > */ > uint32_t updateImmAttr(const char *dn, SaImmAttrNameT attributeName, > SaImmValueTypeT attrValueType, void *value) > { > + smfd_imm_lock(); > SaAisErrorT rc = immutil_update_one_rattr(smfd_cb->campaignOiHandle, > dn, attributeName, attrValueType, value); > + smfd_imm_unlock(); > + > if (rc != SA_AIS_OK) { > - LOG_ER("updateImmAttr(): immutil_update_one_rattr FAILED, rc = > %d, going to assert", (int)rc); > - osafassert(0); > + if (rc == SA_AIS_ERR_BAD_HANDLE){ > + /* If there is IMMND restart in the middle of campign, > there is chance that > + * the camiaign thread with Rt-update will get > timeout, because the update > + * has been sent to IMMND, before the reply is sent > back to OI(SMFD) the IMMND > + * is restarted and BAD_HANDLE is returned after API > timout. > + */ > + > + LOG_WA("updateImmAttr(): immutil_update_one_rattr > FAILED, rc = %d," > + "OI handle will be resurrected", > (int)rc); > + } else { > + LOG_ER("updateImmAttr(): immutil_update_one_rattr > FAILED, rc = %d, going to assert", (int)rc); > + osafassert(0); > + } > } > + > return NCSCC_RC_SUCCESS; > } > > diff --git a/osaf/services/saf/smfsv/smfd/smfd_cb.h > b/osaf/services/saf/smfsv/smfd/smfd_cb.h > --- a/osaf/services/saf/smfsv/smfd/smfd_cb.h > +++ b/osaf/services/saf/smfsv/smfd/smfd_cb.h > @@ -68,6 +68,7 @@ typedef struct smfd_cb { > SMFD_SMFND_ADEST_INVID_MAP *smfnd_list; /* SMFNDs need to respond > to the callback. */ > uint32_t no_of_smfnd; > pthread_mutex_t lock; /* Used by smfd_cb_t > lock/unlock functions */ > + pthread_mutex_t imm_lock; /* Used when IMM OI handle > is shared between campaign thread and main thread*/ > uint32_t maxDnLength; /* Max DN length */ > uint32_t procExecutionMode; /* Control the procedure > execution modes > SMF_STANDARD 0 > diff --git a/osaf/services/saf/smfsv/smfd/smfd_main.c > b/osaf/services/saf/smfsv/smfd/smfd_main.c > --- a/osaf/services/saf/smfsv/smfd/smfd_main.c > +++ b/osaf/services/saf/smfsv/smfd/smfd_main.c > @@ -96,6 +96,50 @@ void smfd_cb_unlock() > } > } > > +int smfd_imm_trylock() > +{ > + TRACE_ENTER(); > + int err ; > + > + err = pthread_mutex_trylock(&(smfd_cb->imm_lock)); > + if (err != 0) { > + if ( err == EBUSY ){ > + LOG_WA("Lock failed eith EBUSY pthread_mutex_trylock > for imm %d", err); > + return 1; > + } else { > + LOG_ER("Lock failed pthread_mutex_trylock for imm %d", > err); > + abort(); > + } > + } > + return 0; [rafael] this return is never reached > + TRACE_LEAVE(); > +} > + > +void smfd_imm_lock() > +{ > + TRACE_ENTER(); > + int errno; [rafael] errno is never used > + > + if (pthread_mutex_lock(&(smfd_cb->imm_lock)) != 0) { > + LOG_ER("Lock failed pthread_mutex_lock for imm"); > + abort(); > + } > + TRACE_LEAVE(); > +} > + > + > +void smfd_imm_unlock() > +{ > + TRACE_ENTER(); > + if (pthread_mutex_unlock(&(smfd_cb->imm_lock)) != 0) { > + LOG_ER("Unlock failed pthread_mutex_unlock for imm"); > + abort(); > + } > + TRACE_LEAVE(); > +} > + > + > + > > /**************************************************************************** > * Name : smfd_cb_init > * > @@ -112,7 +156,7 @@ uint32_t smfd_cb_init(smfd_cb_t * smfd_c > { > TRACE_ENTER(); > > - pthread_mutexattr_t mutex_attr; > + pthread_mutexattr_t mutex_attr, mutex_attr1; > > smfd_cb->amfSelectionObject = -1; > smfd_cb->campaignSelectionObject = -1; > @@ -159,6 +203,28 @@ uint32_t smfd_cb_init(smfd_cb_t * smfd_c > LOG_ER("Failed pthread_mutexattr_destroy"); > return NCSCC_RC_FAILURE; > } > + if (pthread_mutexattr_init(&mutex_attr1) != 0) { > + LOG_ER("Failed pthread_mutexattr_init used for imm"); > + return NCSCC_RC_FAILURE; > + } > + > + if (pthread_mutexattr_settype(&mutex_attr1, PTHREAD_MUTEX_RECURSIVE) != > 0) { > + LOG_ER("Failed pthread_mutexattr_settype used for imm"); > + pthread_mutexattr_destroy(&mutex_attr1); > + return NCSCC_RC_FAILURE; > + } > + > + if (pthread_mutex_init(&(smfd_cb->imm_lock), &mutex_attr1) != 0) { > + LOG_ER("Failed pthread_mutex_init used for imm"); > + pthread_mutexattr_destroy(&mutex_attr1); > + return NCSCC_RC_FAILURE; > + } > + > + if (pthread_mutexattr_destroy(&mutex_attr1) != 0) { > + LOG_ER("Failed pthread_mutexattr_destroy used for imm"); > + return NCSCC_RC_FAILURE; > + } > + > > TRACE_LEAVE(); > return NCSCC_RC_SUCCESS; > @@ -335,9 +401,21 @@ static void main_process(void) > > /* Process all the Imm callback events */ > if (fds[SMFD_COI_FD].revents & POLLIN) { > - if ((error = > - saImmOiDispatch(smfd_cb->campaignOiHandle, > - SA_DISPATCH_ALL)) != SA_AIS_OK) { > + if ( smfd_imm_trylock() == 0 ) { > + > + /* In the case of IMMND restart, If trylock > succedes then the oiDispatch > + * is called and IMM OI will get resurrected. > Otherwise, the campign > + * thread has taken the imm_lock. Campign > thread will resurrect the IMM > + * OI handle. trylock is used to unlock the > main thread, if the > + * immlock is taken by campaign thread. > + */ > + > + error = > saImmOiDispatch(smfd_cb->campaignOiHandle, > + SA_DISPATCH_ALL); > + smfd_imm_unlock(); > + } > + > + if (error != SA_AIS_OK) { > /* > ** BAD_HANDLE is interpreted as an IMM service > restart. Try > ** reinitialize the IMM OI API in a background > thread and let ------------------------------------------------------------------------------ What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports. http://pubads.g.doubleclick.net/gampad/clk?id=1444514421&iu=/41014381 _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel