Hi Hung, Neelakanta pointed that IMMD_EVT_ND2D_INTRO can be sent only to active IMMD. Ack from me.
Thanks, Zoran -----Original Message----- From: Zoran Milinkovic [mailto:zoran.milinko...@ericsson.com] Sent: den 5 juli 2016 16:01 To: Hung Duc Nguyen; reddy.neelaka...@oracle.com Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] imm: Fix the startup delay in IMMD [#1896] Hi Hung, All that is correct for called register function once. My point is when IMMD starts as standby IMMD. In this case, when will veteran_sync_lock be initialized ? In the patch, veteran_sync_lock is initialized only when IMMD starts as active IMMD: + if(cb->mScAbsenceAllowed && cb->ha_state == SA_AMF_HA_ACTIVE) { + /* Create the sel-obj before initializing MDS. + * This way, we can avoid an extra 'veteran_sync_awaited' + variable in IMMD_CB */ + if ((rc = m_NCS_LOCK_INIT(&immd_cb->veteran_sync_lock)) + != NCSCC_RC_SUCCESS) { + LOG_ER("Failed to get veteran_sync_lock lock"); + goto done; + } ... but immd_mds_rcv() is also called on standby IMMD. Will veteran_sync_lock be initialized in main() or initialize_for_assignment(), it does not matter, but veteran_sync_lock must be initialized in the beginning. Otherwise there might be a problem in immd_mds_rcv(). Thanks, Zoran From: Hung Nguyen [mailto:hung.d.ngu...@dektech.com.au] Sent: den 5 juli 2016 15:42 To: Zoran Milinkovic; reddy.neelaka...@oracle.com Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 1 of 1] imm: Fix the startup delay in IMMD [#1896] Hi Zoran, immd_mds_register() and immd_mbcsv_register() must be called once in IMMD. By putting the code near those functions, I expect that the code also executed only once for the entire lifetime of IMMD. If the immd_mds_register(), immd_mbcsv_register() and the code are executed more than once, there must be something wrong, and we need to fix that. immd_mds_rcv() is called only after we initialize MDS. If MDS invokes immd_mds_rcv() before initialzing MDS, that must be a bug in MDS. BR, Hung Nguyen - DEK Technologies -------------------------------------------------------------------------------- From: Zoran Milinkovic zoran.milinko...@ericsson.com<mailto:zoran.milinko...@ericsson.com> Sent: Tuesday, July 05, 2016 8:34PM To: Hung Nguyen, Neelakanta Reddy hung.d.ngu...@dektech.com.au<mailto:hung.d.ngu...@dektech.com.au>, reddy.neelaka...@oracle.com<mailto:reddy.neelaka...@oracle.com> Cc: Opensaf-devel opensaf-devel@lists.sourceforge.net<mailto:opensaf-devel@lists.sourceforge.net> Subject: RE: [PATCH 1 of 1] imm: Fix the startup delay in IMMD [#1896] Hi Hung, I see now that veteran_sync_lock init is guarded by fully_initialized. veteran_sync_lock still need to be initialized in the beginning regardless of ha_state. veteran_sync_lock is used in immd_mds_rcv(). BR, Zoran From: Hung Nguyen [mailto:hung.d.ngu...@dektech.com.au] Sent: den 5 juli 2016 14:37 To: Zoran Milinkovic; reddy.neelaka...@oracle.com<mailto:reddy.neelaka...@oracle.com> Cc: opensaf-devel@lists.sourceforge.net<mailto:opensaf-devel@lists.sourceforge.net> Subject: Re: [PATCH 1 of 1] imm: Fix the startup delay in IMMD [#1896] Hi Zoran, It was protected by "cb->fully_initialized". MDS/MBC is initialized only once, so veteran_sync_lock and veteran_sync_sel will only be created only once. If IMMD enters the code more than once then it must be wrong somewhere else. BR, Hung Nguyen - DEK Technologies -------------------------------------------------------------------------------- From: Zoran Milinkovic zoran.milinko...@ericsson.com<mailto:zoran.milinko...@ericsson.com> Sent: Tuesday, July 05, 2016 7:27PM To: Hung Nguyen, Neelakanta Reddy hung.d.ngu...@dektech.com.au<mailto:hung.d.ngu...@dektech.com.au>, reddy.neelaka...@oracle.com<mailto:reddy.neelaka...@oracle.com> Cc: Opensaf-devel opensaf-devel@lists.sourceforge.net<mailto:opensaf-devel@lists.sourceforge.net> Subject: RE: [PATCH 1 of 1] imm: Fix the startup delay in IMMD [#1896] Hi Hung, Find my comments regarding immd_cb->veteran_sync_lock inline. When the comment is fixed, you can push the code. No need to send the patch for another review. Ack from me. -----Original Message----- From: Hung Nguyen [mailto:hung.d.ngu...@dektech.com.au] Sent: den 28 juni 2016 05:50 To: Zoran Milinkovic; reddy.neelaka...@oracle.com<mailto:reddy.neelaka...@oracle.com> Cc: opensaf-devel@lists.sourceforge.net<mailto:opensaf-devel@lists.sourceforge.net> Subject: [PATCH 1 of 1] imm: Fix the startup delay in IMMD [#1896] osaf/services/saf/immsv/immd/immd_main.c | 60 ++++++++++++++++--------------- 1 files changed, 31 insertions(+), 29 deletions(-) Basically, the delay should take place after initializing MDS. This patch moves the code to initialize_for_assignment(). That way we can make sure that the delay is only started after MDS initialization, also we don't have to depend on the output of rda_get_role(). diff --git a/osaf/services/saf/immsv/immd/immd_main.c b/osaf/services/saf/immsv/immd/immd_main.c --- a/osaf/services/saf/immsv/immd/immd_main.c +++ b/osaf/services/saf/immsv/immd/immd_main.c @@ -200,6 +200,20 @@ uint32_t initialize_for_assignment(IMMD_ uint32_t rc = NCSCC_RC_SUCCESS; if (cb->fully_initialized || ha_state == SA_AMF_HA_QUIESCED) goto done; cb->ha_state = ha_state; + + if(cb->mScAbsenceAllowed && cb->ha_state == SA_AMF_HA_ACTIVE) { + /* Create the sel-obj before initializing MDS. + * This way, we can avoid an extra 'veteran_sync_awaited' + variable in IMMD_CB */ + if ((rc = m_NCS_LOCK_INIT(&immd_cb->veteran_sync_lock)) + != NCSCC_RC_SUCCESS) { + LOG_ER("Failed to get veteran_sync_lock lock"); + goto done; + } [Zoran] On each new HA_ACTIVE state, immd_cb->veteran_sync_lock is initialized again, but never destroyed. immd_cb->veteran_sync_lock can be initialized only once. + if ((rc = + m_NCS_SEL_OBJ_CREATE(&immd_cb->veteran_sync_sel)) != NCSCC_RC_SUCCESS) + { + LOG_ER("Failed to create veteran_sync_sel + sel_obj"); + goto done; + } + } + if ((rc = immd_mds_register(cb, ha_state)) != NCSCC_RC_SUCCESS) { LOG_ER("immd_mds_register FAILED %d", rc); goto done; @@ -217,6 +231,23 @@ uint32_t initialize_for_assignment(IMMD_ goto done; } cb->fully_initialized = true; + + if (cb->mScAbsenceAllowed && cb->ha_state == SA_AMF_HA_ACTIVE) { + /* If this IMMD has active role, wait for veteran payloads. + * Give up after 3 seconds if there's no veteran + payloads. */ + LOG_NO("Waiting 3 seconds to allow IMMND MDS attachments + to get +processed."); + + if + (osaf_poll_one_fd(m_GET_FD_FROM_SEL_OBJ(immd_cb->veteran_sync_sel), + 3000) != 1) { + TRACE("osaf_poll_one_fd on veteran_sync_sel + failed or timed out"); + } else { + LOG_NO("Received intro message from veteran + payload, stop waiting"); + } + + m_NCS_LOCK(&immd_cb->veteran_sync_lock, NCS_LOCK_WRITE); + m_NCS_SEL_OBJ_DESTROY(&immd_cb->veteran_sync_sel); + m_NCS_UNLOCK(&immd_cb->veteran_sync_lock, + NCS_LOCK_WRITE); + } + done: TRACE_LEAVE2("rc = %u", rc); return rc; @@ -275,19 +306,6 @@ int main(int argc, char *argv[]) } } - if(scAbsenceAllowed) { - /* Create the sel-obj before initializing MDS. - * This way, we can avoid an extra 'veteran_sync_awaited' variable in IMMD_CB */ - if (m_NCS_LOCK_INIT(&immd_cb->veteran_sync_lock) != NCSCC_RC_SUCCESS) { - LOG_ER("Failed to get veteran_sync_lock lock"); - goto done; - } [Zoran] The removed block above can remain in the code. immd_cb->veteran_sync_lock will be initialized only once. Thanks, Zoran - if (m_NCS_SEL_OBJ_CREATE(&immd_cb->veteran_sync_sel)!= NCSCC_RC_SUCCESS) { - LOG_ER("Failed to create veteran_sync_sel sel_obj"); - goto done; - } - } - immd_cb->mScAbsenceAllowed = scAbsenceAllowed; if (immd_initialize() != NCSCC_RC_SUCCESS) { @@ -298,22 +316,6 @@ int main(int argc, char *argv[]) if(scAbsenceAllowed) { LOG_NO("******* SC_ABSENCE_ALLOWED (Headless Hydra) is configured: %u ***********", scAbsenceAllowed); - - /* If this IMMD has active role, wait for veteran payloads. - * Give up after 3 seconds if there's no veteran payload. */ - if (immd_cb->ha_state == SA_AMF_HA_ACTIVE) { - LOG_NO("Waiting 3 seconds to allow IMMND MDS attachments to get processed."); - - if (osaf_poll_one_fd(m_GET_FD_FROM_SEL_OBJ(immd_cb->veteran_sync_sel), 3000) != 1) { - TRACE("osaf_poll_one_fd on veteran_sync_sel failed or timed out"); - } else { - LOG_NO("Received intro message from veteran payload, stop waiting"); - } - } - - m_NCS_LOCK(&immd_cb->veteran_sync_lock,NCS_LOCK_WRITE); - m_NCS_SEL_OBJ_DESTROY(&immd_cb->veteran_sync_sel); - m_NCS_UNLOCK(&immd_cb->veteran_sync_lock,NCS_LOCK_WRITE); } daemon_sigterm_install(&term_fd); ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
