ACK. No comment from me.
-----Original Message-----
From: Thanh Nguyen <thanh.ngu...@dektech.com.au>
Sent: Monday, April 12, 2021 9:58 AM
To: Thang Duc Nguyen <thang.d.ngu...@dektech.com.au>; Minh Hon Chau
<minh.c...@dektech.com.au>; Thien Minh Huynh <thien.m.hu...@dektech.com.au>
Cc: opensaf-devel@lists.sourceforge.net; Thanh Nguyen
<thanh.ngu...@dektech.com.au>
Subject: [PATCH 1/1] smf: Enhance handling of global variables [#2726]
Appropriate global variables in smfnd are placed under mutex
protection: smfnd_cb_t::smfd_dest, smfnd_cb_t::agent_cnt
---
src/smf/agent/smfa_mds.c | 20 ++++++++++++++++----
src/smf/smfnd/smfnd.h | 1 +
src/smf/smfnd/smfnd_evt.c | 7 +++++++
src/smf/smfnd/smfnd_mds.c | 11 +++++++++--
4 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/src/smf/agent/smfa_mds.c b/src/smf/agent/smfa_mds.c index
3d288b5eb..6e38c769b 100644
--- a/src/smf/agent/smfa_mds.c
+++ b/src/smf/agent/smfa_mds.c
@@ -185,6 +185,7 @@ uint32_t smfa_mds_callback(struct ncsmds_callback_info
*info) uint32_t smfa_mds_svc_evt_cbk(MDS_CALLBACK_SVC_EVENT_INFO *svc_evt) {
SMFA_CB *cb = &_smfa_cb;
+ uint32_t rc = NCSCC_RC_SUCCESS;
/* Agent subscribes for only local ND UP/DOWN.*/
if (NCSMDS_SVC_ID_SMFND != svc_evt->i_svc_id) { @@ -197,25 +198,36 @@
uint32_t smfa_mds_svc_evt_cbk(MDS_CALLBACK_SVC_EVENT_INFO *svc_evt)
/* Catch the adest of SMFND*/
if (!m_MDS_DEST_IS_AN_ADEST(svc_evt->i_dest))
return NCSCC_RC_SUCCESS;
- /* TODO: No lock is taken. This might be dangerous.*/
+ if (m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) !=
+ NCSCC_RC_SUCCESS) {
+ rc = NCSCC_RC_FAILURE;
+ goto lock_fail;
+ }
+
cb->is_smfnd_up = true;
cb->smfnd_adest = svc_evt->i_dest;
+ m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE);
break;
case NCSMDS_DOWN:
if (!m_MDS_DEST_IS_AN_ADEST(svc_evt->i_dest))
return NCSCC_RC_SUCCESS;
- /* TODO: No lock is taken. This might be dangerous.*/
+ if (m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) !=
+ NCSCC_RC_SUCCESS) {
+ rc = NCSCC_RC_FAILURE;
+ goto lock_fail;
+ }
cb->is_smfnd_up = false;
cb->smfnd_adest = 0;
+ m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE);
break;
default:
LOG_NO("SMFA: Got the svc evt: %d for SMFND",
svc_evt->i_change);
}
-
- return NCSCC_RC_SUCCESS;
+lock_fail:
+ return rc;
}
/***************************************************************************
diff --git a/src/smf/smfnd/smfnd.h b/src/smf/smfnd/smfnd.h index
947745a40..d5c4ac81b 100644
--- a/src/smf/smfnd/smfnd.h
+++ b/src/smf/smfnd/smfnd.h
@@ -66,6 +66,7 @@ typedef struct {
*cbk_list; /* Mapping between inv_id and all the agents */
uint32_t agent_cnt; /* Count of SMF Agents */
MDS_DEST smfd_dest; /* MDS DEST of SMFD */
+ pthread_mutex_t cb_lock; /* Used by smfnd_cb_t lock/unlock functions
+ */
} smfnd_cb_t;
extern smfnd_cb_t *smfnd_cb;
diff --git a/src/smf/smfnd/smfnd_evt.c b/src/smf/smfnd/smfnd_evt.c index
552e11a37..48015fd5b 100644
--- a/src/smf/smfnd/smfnd_evt.c
+++ b/src/smf/smfnd/smfnd_evt.c
@@ -22,6 +22,7 @@
#include "smfnd.h"
#include "smf/common/smfsv_defs.h"
#include "smf/common/smfsv_evt.h"
+#include "base/osaf_utility.h"
/* This function is called in another threads context so be
careful with what you do here */
@@ -417,10 +418,12 @@ uint32_t smfnd_cbk_resp_err_proc(smfnd_cb_t *cb,
SaInvocationT inv_id)
resp_evt.info.smfd.event.cbk_rsp.evt.resp_evt
.err = SA_AIS_ERR_FAILED_OPERATION;
+ osaf_mutex_lock_ordie(&cb->cb_lock);
rc = smfsv_mds_msg_send(
cb->mds_handle, NCSMDS_SVC_ID_SMFD,
cb->smfd_dest, NCSMDS_SVC_ID_SMFND,
&resp_evt);
+ osaf_mutex_unlock_ordie(&cb->cb_lock);
}
break;
}
@@ -474,10 +477,12 @@ uint32_t smfnd_cbk_resp_ok_proc(smfnd_cb_t *cb,
SaInvocationT inv_id,
resp_evt.info.smfd.event.cbk_rsp.evt
.resp_evt.err = resp;
+ osaf_mutex_lock_ordie(&cb->cb_lock);
rc = smfsv_mds_msg_send(
cb->mds_handle, NCSMDS_SVC_ID_SMFD,
cb->smfd_dest, NCSMDS_SVC_ID_SMFND,
&resp_evt);
+ osaf_mutex_unlock_ordie(&cb->cb_lock);
}
/* Send resp to SMFD and break.*/
break;
@@ -536,7 +541,9 @@ static void proc_cbk_req_rsp(smfnd_cb_t *cb, SMFSV_EVT *evt)
TRACE_ENTER();
switch (evt->info.smfnd.event.cbk_req_rsp.evt_type) {
case SMF_CLBK_EVT: {
+ osaf_mutex_lock_ordie(&cb->cb_lock);
smfnd_cbk_req_proc(cb, evt);
+ osaf_mutex_unlock_ordie(&cb->cb_lock);
break;
}
case SMF_RSP_EVT: {
diff --git a/src/smf/smfnd/smfnd_mds.c b/src/smf/smfnd/smfnd_mds.c index
b49f5f1bd..5f8ebb59b 100644
--- a/src/smf/smfnd/smfnd_mds.c
+++ b/src/smf/smfnd/smfnd_mds.c
@@ -19,6 +19,7 @@
#include "smfnd.h"
#include "smf/common/smfsv_evt.h"
+#include "base/osaf_utility.h"
uint32_t mds_register(smfnd_cb_t *cb);
void mds_unregister(smfnd_cb_t *cb);
@@ -240,16 +241,19 @@ static uint32_t mds_svc_event(struct ncsmds_callback_info
*info)
case NCSMDS_NEW_ACTIVE:
LOG_NO("MDS %s: NCSMDS_NEW_ACTIVE", __FUNCTION__);
case NCSMDS_UP:
- /* TODO: No lock is taken. This might be dangerous.*/
if (NCSMDS_SVC_ID_SMFA == svc_evt->i_svc_id) {
+ osaf_mutex_lock_ordie(&cb->cb_lock);
cb->agent_cnt++;
+ osaf_mutex_unlock_ordie(&cb->cb_lock);
TRACE("Count of agents incremeted to : %d",
cb->agent_cnt);
} else if (NCSMDS_SVC_ID_SMFD == svc_evt->i_svc_id) {
/* Catch the vdest of SMFD*/
if (m_MDS_DEST_IS_AN_ADEST(svc_evt->i_dest))
return NCSCC_RC_SUCCESS;
+ osaf_mutex_lock_ordie(&cb->cb_lock);
cb->smfd_dest = svc_evt->i_dest;
+ osaf_mutex_unlock_ordie(&cb->cb_lock);
LOG_NO("MDS %s: NCSMDS_SVC_ID_SMFD "
"dest = 0x%" PRIx64,
__FUNCTION__, svc_evt->i_dest);
@@ -257,19 +261,22 @@ static uint32_t mds_svc_event(struct ncsmds_callback_info
*info)
break;
case NCSMDS_DOWN:
- /* TODO: No lock is taken. This might be dangerous.*/
/* TODO: Need to clean up the cb->cbk_list, otherwise there will
be memory leak. For the time being we are storing only count of
agents, not the adest of agents and hence it is not possible to
clean up cbk_list.*/
if (NCSMDS_SVC_ID_SMFA == svc_evt->i_svc_id) {
+ osaf_mutex_lock_ordie(&cb->cb_lock);
cb->agent_cnt--;
+ osaf_mutex_unlock_ordie(&cb->cb_lock);
TRACE("Count of agents decremeted to : %d",
cb->agent_cnt);
} else if (NCSMDS_SVC_ID_SMFD == svc_evt->i_svc_id) {
if (m_MDS_DEST_IS_AN_ADEST(svc_evt->i_dest))
return NCSCC_RC_SUCCESS;
+ osaf_mutex_lock_ordie(&cb->cb_lock);
cb->smfd_dest = 0;
+ osaf_mutex_unlock_ordie(&cb->cb_lock);
LOG_NO("MDS %s: NCSMDS_DOWN smfd_dest = 0",
__FUNCTION__);
}
--
2.25.0
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
