Let't not become sentimental about old broke code. As noted, this
mechanism is currently broken and doesn't do anything useful in its
current form. I doubt that it has ever worked during the history of the
OpenSAF project. And if it was really needed, it is doubtful why it
would only be needed in the timer implementation and not anywhere else.
regards,
Anders Widell
On 01/25/2017 05:22 AM, ramesh betham wrote:
> Hi Anders,
>
> Quick comments.
>
> The purpose of having ncslpg_<...>() functions in sysf_tmr is to
> make sure to go through gracefull shutdown process.
> sysfTmrDestroy() is suppose to wait till all threads using sysfTmr
> to complete their tasks. Observed that ncslpg_destroy() should
> have been called in sysfTmrDestroy() to accomplish the defined
> purpose.
>
> Regards,
> Ramesh.
>
> On 1/24/2017 7:24 PM, Anders Widell wrote:
>> src/base/hj_hdl.c | 96
>> +-------------------------------------------------
>> src/base/ncs_hdl_pub.h | 22 +----------
>> src/base/sysf_tmr.c | 34 ++---------------
>> 3 files changed, 6 insertions(+), 146 deletions(-)
>>
>>
>> Remove the "local persistence guard" API since it is only referenced by the
>> OpenSAF timer implementation, and in there it servces no purpose since the
>> function ncslpg_destroy() is never called.
>>
>> diff --git a/src/base/hj_hdl.c b/src/base/hj_hdl.c
>> --- a/src/base/hj_hdl.c
>> +++ b/src/base/hj_hdl.c
>> @@ -1,6 +1,7 @@
>> /* -*- OpenSAF -*-
>> *
>> * (C) Copyright 2008 The OpenSAF Foundation
>> + * Copyright Ericsson AB 2012, 2017 - All Rights Reserved.
>> *
>> * This program is distributed in the hope that it will be useful, but
>> * WITHOUT ANY WARRANTY; without even the implied warranty of
>> MERCHANTABILITY
>> @@ -25,15 +26,6 @@
>> See ncs_hdl.h for brief write-up of issues/capabilities. See the
>> ncshm_ calls.
>>
>> - This file also contains an implementation for a 'Local Persistence Guard',
>> - which is a cheap (in CPU cycles) persistence guard scheme (for read
>> - only object access, like the handle manager) that can be used when an
>> object
>> - that is known to exist needs a guard for some object that hangs off of
>> this
>> - object. For example, the LEAP sysfpool has a global struct that always
>> exists.
>> - What we need to know and be assured of is that when we access memory, that
>> - the structures managed will persist (destroy() could be called after all)
>> - while we are in the middle of an operation. See the ncslpg_ calls.
>> -
>>
>> ******************************************************************************
>> */
>>
>> @@ -718,89 +710,3 @@ void hm_unblock_him(HM_CELL *cell)
>> int rc = sem_post((sem_t*)cell->data); /* unblock that destroy thread
>> */
>> osafassert(rc == 0);
>> }
>> -
>> -/***************************************************************************
>> - *
>> - *
>> - *
>> - * P u b l i c L o c a l P e r s i s t e n c e G u a r d
>> - *
>> - * A P I s (prefix 'ncslpg_')
>> - *
>> - *
>> - *
>> -
>> ***************************************************************************/
>> -
>> -/*****************************************************************************
>> -
>> - PROCEDURE NAME: ncslpg_take
>> -
>> - DESCRIPTION: If all validation stuff is in order return true, which
>> - means this thread can enter this object.
>> -
>> -*****************************************************************************/
>> -
>> -bool ncslpg_take(NCSLPG_OBJ *pg)
>> -{
>> - m_NCS_OS_ATOMIC_INC(&(pg->inhere)); /* set first, ask later.. to
>> beat 'closing' */
>> - if (pg->open == true)
>> - return true; /* its open, lets go in */
>> - else
>> - m_NCS_OS_ATOMIC_DEC(&(pg->inhere));
>> - return false; /* its closed */
>> -}
>> -
>> -/*****************************************************************************
>> -
>> - PROCEDURE NAME: ncslpg_give
>> -
>> - DESCRIPTION: decriment the refcount, as this thread is leaving now.
>> -
>> -*****************************************************************************/
>> -
>> -uint32_t ncslpg_give(NCSLPG_OBJ *pg, uint32_t ret)
>> -{
>> - m_NCS_OS_ATOMIC_DEC(&(pg->inhere));
>> - return ret;
>> -}
>> -
>> -/*****************************************************************************
>> -
>> - PROCEDURE NAME: ncslpg_create
>> -
>> - DESCRIPTION: Put the passed NCSLPG_OBJ in start state. If its
>> already
>> - in start state, return FAILURE.
>> -
>> -*****************************************************************************/
>> -
>> -uint32_t ncslpg_create(NCSLPG_OBJ *pg)
>> -{
>> - if (pg->open == true)
>> - m_LEAP_DBG_SINK_VOID;
>> - pg->open = true;
>> - pg->inhere = 0;
>> - return NCSCC_RC_SUCCESS;
>> -}
>> -
>> -/*****************************************************************************
>> -
>> - PROCEDURE NAME: ncslpg_destroy
>> -
>> - DESCRIPTION: Close this LPG. Wait for all other threads to leave
>> before
>> - returning to the invoker, allowing her to proceed. Note
>> - that if this object is already closed, this function
>> - returns false (invoker should not proceed, as the
>> object is
>> - already destroyed or being destoyed.
>> -
>> -*****************************************************************************/
>> -
>> -bool ncslpg_destroy(NCSLPG_OBJ *pg)
>> -{
>> - if (pg->open == false)
>> - return false; /* already closed */
>> - pg->open = false; /* stop others from entering */
>> - while (pg->inhere != 0) /* Anybody inhere?? */
>> - m_NCS_TASK_SLEEP(1); /* OK, I'll wait; could do semaphore I
>> suppose */
>> -
>> - return true; /* Invoker can proceed to get rid of protected
>> thing */
>> -}
>> diff --git a/src/base/ncs_hdl_pub.h b/src/base/ncs_hdl_pub.h
>> --- a/src/base/ncs_hdl_pub.h
>> +++ b/src/base/ncs_hdl_pub.h
>> @@ -1,6 +1,7 @@
>> /* -*- OpenSAF -*-
>> *
>> * (C) Copyright 2008 The OpenSAF Foundation
>> + * Copyright Ericsson AB 2016, 2017 - All Rights Reserved.
>> *
>> * This program is distributed in the hope that it will be useful, but
>> * WITHOUT ANY WARRANTY; without even the implied warranty of
>> MERCHANTABILITY
>> @@ -94,27 +95,6 @@ NCSCONTEXT ncshm_take_hdl(NCS_SERVICE_ID
>>
>> void ncshm_give_hdl(uint32_t hdl);
>>
>> -/************************************************************************/
>> -/* NCSLPG_OBJ - this structure is embedded in known, persistent thing */
>> -/************************************************************************/
>> -
>> -typedef struct ncslpg_obj {
>> - bool open; /* Is the object (still) open/available */
>> - uint32_t inhere; /* use-count of clients 'inside' object now */
>> -
>> -} NCSLPG_OBJ; /* Local Persistence Guard */
>> -
>> -/***************************************************************************
>> - *
>> - * P u b l i c L o c a l P e r s i s t e n c e G u a r d A P I s
>> - *
>> -
>> ***************************************************************************/
>> -
>> -bool ncslpg_take(NCSLPG_OBJ *pg);
>> -uint32_t ncslpg_give(NCSLPG_OBJ *pg, uint32_t ret);
>> -uint32_t ncslpg_create(NCSLPG_OBJ *pg);
>> -bool ncslpg_destroy(NCSLPG_OBJ *pg);
>> -
>> #ifdef __cplusplus
>> }
>> #endif
>> diff --git a/src/base/sysf_tmr.c b/src/base/sysf_tmr.c
>> --- a/src/base/sysf_tmr.c
>> +++ b/src/base/sysf_tmr.c
>> @@ -1,6 +1,7 @@
>> /* -*- OpenSAF -*-
>> *
>> * (C) Copyright 2008 The OpenSAF Foundation
>> + * Copyright Ericsson AB 2009, 2017 - All Rights Reserved.
>> *
>> * This program is distributed in the hope that it will be useful, but
>> * WITHOUT ANY WARRANTY; without even the implied warranty of
>> MERCHANTABILITY
>> @@ -160,7 +161,6 @@ typedef struct tmr_safe {
>> typedef struct sysf_tmr_cb {
>> uint32_t tick; /* Times TmrExpiry has been called */
>>
>> - NCSLPG_OBJ persist; /* guard against fleeting destruction */
>> TMR_SAFE safe; /* critical region stuff */
>> NCS_PATRICIA_TREE tmr_pat_tree;
>> TMR_STATS stats;
>> @@ -239,17 +239,10 @@ static bool sysfTmrExpiry(SYSF_TMR_PAT_N
>> /* get these guys one behind to start as well */
>> dead_tmr->next = NULL;
>>
>> - /* Confirm and secure tmr service is/will persist */
>> - if (ncslpg_take(&gl_tcb.persist) == false)
>> - return false; /* going or gone away.. Lets leave */
>> -
>> if (tmr_destroying == true) {
>> /* Raise An indication */
>> m_NCS_SEL_OBJ_IND(&tmr_destroy_syn_obj);
>>
>> - /*If thread canceled here, It had no effect on timer thread
>> destroy */
>> - ncslpg_give(&gl_tcb.persist, 0);
>> -
>> /* returns true if thread is going to be destroyed otherwise
>> return false(normal flow) */
>> return true;
>> }
>> @@ -308,7 +301,6 @@ static bool sysfTmrExpiry(SYSF_TMR_PAT_N
>> ncs_patricia_tree_del(&gl_tcb.tmr_pat_tree, (NCS_PATRICIA_NODE *)tmp);
>> m_NCS_MEM_FREE(tmp, NCS_MEM_REGION_PERSISTENT, NCS_SERVICE_ID_LEAP_TMR,
>> 0);
>>
>> - ncslpg_give(&gl_tcb.persist, 0);
>> return false;
>> }
>>
>> @@ -485,9 +477,6 @@ bool sysfTmrCreate(void)
>> /* Empty Timer Service control block. */
>> memset(&gl_tcb, '\0', sizeof(SYSF_TMR_CB));
>>
>> - /* put local persistent guard in start state */
>> - ncslpg_create(&gl_tcb.persist);
>> -
>> /* Initialize the locks */
>> m_NCS_LOCK_INIT(&gl_tcb.safe.enter_lock);
>> m_NCS_LOCK_INIT(&gl_tcb.safe.free_lock);
>> @@ -641,9 +630,6 @@ tmr_t ncs_tmr_alloc(char *file, uint32_t
>> if (tmr_destroying == true)
>> return NULL;
>>
>> - if (ncslpg_take(&gl_tcb.persist) == false) /* guarentee
>> persistence */
>> - return NULL;
>> -
>> m_NCS_LOCK(&gl_tcb.safe.free_lock, NCS_LOCK_WRITE);
>>
>> back = &gl_tcb.safe.dmy_free; /* see if we have a free one */
>> @@ -681,7 +667,6 @@ tmr_t ncs_tmr_alloc(char *file, uint32_t
>> TMR_DBG_SET(tmr->dbg, file, line);
>> }
>>
>> - ncslpg_give(&gl_tcb.persist, 0);
>> return (tmr_t)tmr;
>> }
>>
>> @@ -707,13 +692,9 @@ tmr_t ncs_tmr_start(tmr_t tid, int64_t t
>>
>> TMR_DBG_ASSERT_STATE(tmr, (TMR_STATE_DORMANT | TMR_STATE_CREATE));
>>
>> - if (ncslpg_take(&gl_tcb.persist) == false) /* guarentee
>> persistence */
>> - return NULL;
>> -
>> if (TMR_TEST_STATE(tmr, TMR_STATE_DORMANT)) { /* If client is
>> re-using timer */
>> m_NCS_TMR_CREATE(new_tmr, tmrDelay, tmrCB, tmrUarg); /* get
>> a new one */
>> if (new_tmr == NULL) {
>> - ncslpg_give(&gl_tcb.persist, 0);
>> return NULL;
>> }
>>
>> @@ -738,7 +719,6 @@ tmr_t ncs_tmr_start(tmr_t tid, int64_t t
>> if (rc == NCSCC_RC_FAILURE) {
>> /* Free the timer created */
>> m_NCS_UNLOCK(&gl_tcb.safe.enter_lock, NCS_LOCK_WRITE);
>> - ncslpg_give(&gl_tcb.persist, 0);
>> return NULL;
>> }
>> #if ENABLE_SYSLOG_TMR_STATS
>> @@ -779,8 +759,6 @@ tmr_t ncs_tmr_start(tmr_t tid, int64_t t
>> m_NCS_UNLOCK(&gl_tcb.safe.enter_lock, NCS_LOCK_WRITE);
>> TMR_DBG_SET(tmr->dbg, file, line);
>>
>> - ncslpg_give(&gl_tcb.persist, 0);
>> -
>> return tmr;
>> }
>>
>> @@ -915,13 +893,9 @@ int64_t ncs_tmr_remaining(tmr_t tmrID, i
>> *p_tleft = 0;
>> TMR_DBG_ASSERT_ISA(tmr->dbg); /* confirm that its timer memory */
>>
>> - if (ncslpg_take(&gl_tcb.persist) == false) /* guarentee
>> persistence */
>> - return NCSCC_RC_FAILURE;
>> -
>> m_NCS_LOCK(&gl_tcb.safe.enter_lock, NCS_LOCK_WRITE); /* critical
>> region START */
>> if (!TMR_TEST_STATE(tmr, TMR_STATE_START)) {
>> m_NCS_UNLOCK(&gl_tcb.safe.enter_lock, NCS_LOCK_WRITE); /*
>> critical region START */
>> - ncslpg_give(&gl_tcb.persist, 0);
>> return NCSCC_RC_FAILURE;
>> }
>> m_NCS_UNLOCK(&gl_tcb.safe.enter_lock, NCS_LOCK_WRITE); /* critical
>> region START */
>> @@ -931,7 +905,7 @@ int64_t ncs_tmr_remaining(tmr_t tmrID, i
>>
>> *p_tleft = total_ticks_left * NCS_MILLISECONDS_PER_TICK;
>>
>> - return ncslpg_give(&gl_tcb.persist, NCSCC_RC_SUCCESS);
>> + return NCSCC_RC_SUCCESS;
>> }
>>
>>
>> /****************************************************************************
>> @@ -969,7 +943,7 @@ uint32_t ncs_tmr_whatsout(void)
>> printf("| #| | line| | | |\n");
>> printf("|---|----+-----+-----------+------------+----------------|\n");
>>
>> - if ((ncslpg_take(&gl_tcb.persist) == false) || (tmr_destroying ==
>> true)) {
>> + if (tmr_destroying == true) {
>> printf("< . . . TMR SVC DESTROYED: .CLEANUP ALREADY DONE..>\n");
>> return NCSCC_RC_FAILURE; /* going or gone away.. Lets
>> leave */
>> }
>> @@ -990,7 +964,7 @@ uint32_t ncs_tmr_whatsout(void)
>> free = free->keep;
>> }
>> m_NCS_UNLOCK(&gl_tcb.safe.free_lock, NCS_LOCK_WRITE); /* critical
>> region END */
>> - return ncslpg_give(&gl_tcb.persist, NCSCC_RC_SUCCESS);
>> + return NCSCC_RC_SUCCESS;
>> }
>>
>>
>> /****************************************************************************
>
------------------------------------------------------------------------------
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
