Hi Rafael,
I will update the comments, while pushing the patch.
/Neel.
On 2016/06/16 04:14 PM, Rafael Odzakow wrote:
> 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
