diff --git a/src/sde_lib/sde_lib.c b/src/sde_lib/sde_lib.c index 868a57bfa..814a687fb 100644 --- a/src/sde_lib/sde_lib.c +++ b/src/sde_lib/sde_lib.c @@ -211,36 +211,30 @@ papi_sde_enable( papi_handle_t handle ){ int papi_sde_shutdown( papi_handle_t handle ){ papisde_library_desc_t *lib_handle, *tmp_lib, *next_lib, *prev_lib; - papisde_list_entry_t *list_head, *curr; - int i, ret_val=SDE_OK; + int i; lib_handle = (papisde_library_desc_t *) handle; papisde_control_t *gctl = _papisde_global_control; if( (NULL==lib_handle) || lib_handle->disabled || (NULL==gctl) || gctl->disabled) return SDE_OK; + SDEDBG("papi_sde_shutdown(): for library '%s'.\n", lib_handle->libraryName); + sde_lock(); - for(i=0; ilib_counters[i]); - if( NULL == list_head ) - continue; - if(NULL != list_head->item){ - sdei_free_counter(list_head->item); - } + sde_counter_t *all_lib_counters; + int item_cnt = ht_to_array(lib_handle->lib_counters, &all_lib_counters); - for(curr = list_head->next; NULL != curr; curr=list_head->next){ - if(NULL == curr->item){ // This can only legally happen for the head of the list. - SDE_ERROR("papi_sde_shutdown(): the counter hash table is clobbered."); - ret_val = SDE_EINVAL; - goto fn_exit; - } - sdei_free_counter(curr->item); - list_head->next = curr->next; - free(curr); - } + for(i=0; inext = next_lib @@ -263,9 +257,8 @@ papi_sde_shutdown( papi_handle_t handle ){ free(lib_handle->libraryName); free(lib_handle); -fn_exit: sde_unlock(); - return ret_val; + return SDE_OK; } @@ -353,7 +346,7 @@ papi_sde_unregister_counter( papi_handle_t handle, const char *event_name) char *full_event_name; int ret_val; - SDEDBG("Preparing to unregister counter\n"); + SDEDBG("Preparing to unregister counter: '%s'.\n",event_name); lib_handle = (papisde_library_desc_t *) handle; papisde_control_t *gctl = _papisde_global_control; @@ -525,7 +518,6 @@ papi_sde_add_counter_to_group(papi_handle_t handle, const char *event_name, cons (void)ht_insert(gctl->all_reg_counters, ht_hash_id(cntr_group_uniq_id), tmp_group); }else{ - // should the following branch ever be true? Why do we already have a group registered if it's empty? if( NULL == tmp_group->u.cntr_group.group_head ){ SDE_ERROR("papi_sde_add_counter_to_group(): Found an empty counter group: '%s'. This might indicate that a cleanup routine is not doing its job.", group_name); } @@ -544,6 +536,9 @@ papi_sde_add_counter_to_group(papi_handle_t handle, const char *event_name, cons new_head->item = tmp_item; new_head->next = tmp_group->u.cntr_group.group_head; tmp_group->u.cntr_group.group_head = new_head; + if( SDE_OK != sdei_inc_ref_count(tmp_item) ){ + SDE_ERROR("papi_sde_add_counter_to_group(): Error while adding counter '%s' to counter group: '%s'.", tmp_item->name, group_name); + } free(full_group_name); ret_val = SDE_OK; diff --git a/src/sde_lib/sde_lib.h b/src/sde_lib/sde_lib.h index f33b56d89..6bb93a911 100644 --- a/src/sde_lib/sde_lib.h +++ b/src/sde_lib/sde_lib.h @@ -41,6 +41,7 @@ #define SDE_ENOMEM -2 /**< Insufficient memory */ #define SDE_ECMP -4 /**< Not supported by component */ #define SDE_ENOEVNT -7 /**< Event does not exist */ +#define SDE_EMISC -14 /**< Unknown error code */ #define register_fp_counter register_counter_cb #define papi_sde_register_fp_counter papi_sde_register_counter_cb diff --git a/src/sde_lib/sde_lib_internal.h b/src/sde_lib/sde_lib_internal.h index abc36ed54..dc31e5841 100644 --- a/src/sde_lib/sde_lib_internal.h +++ b/src/sde_lib/sde_lib_internal.h @@ -139,6 +139,7 @@ struct sde_counter_s { int overflow; int cntr_type; int cntr_mode; + int ref_count; papisde_library_desc_t *which_lib; }; @@ -187,7 +188,8 @@ struct papisde_control_s { int sdei_setup_counter_internals( papi_handle_t handle, const char *event_name, int cntr_mode, int cntr_type, enum CNTR_CLASS cntr_class, cntr_class_specific_t cntr_union ); int sdei_delete_counter(papisde_library_desc_t* lib_handle, const char *name); -int sdei_free_counter(sde_counter_t *counter); +int sdei_inc_ref_count(sde_counter_t *counter); +int sdei_free_counter_resources(sde_counter_t *counter); int sdei_read_counter_group( sde_counter_t *counter, long long int *rslt_ptr ); void sdei_counting_set_to_list( void *cset_handle, cset_list_object_t **list_head ); int sdei_read_and_update_data_value( sde_counter_t *counter, long long int previous_value, long long int *rslt_ptr ); diff --git a/src/sde_lib/sde_lib_misc.c b/src/sde_lib/sde_lib_misc.c index 7a6b78d45..d96a85786 100644 --- a/src/sde_lib/sde_lib_misc.c +++ b/src/sde_lib/sde_lib_misc.c @@ -232,82 +232,160 @@ int sdei_setup_counter_internals( papi_handle_t handle, const char *event_name, return ret_val; } -int sdei_delete_counter(papisde_library_desc_t* lib_handle, const char* name) { +int sdei_inc_ref_count(sde_counter_t *counter){ + papisde_list_entry_t *curr; + if( NULL == counter ) + return SDE_OK; + + // If the counter is a group, recursivelly increment the ref_count of all its children. + if(CNTR_CLASS_GROUP == counter->cntr_class){ + curr = counter->u.cntr_group.group_head; + do{ + sde_counter_t *tmp_cntr = curr->item; + // recursively increment the ref_count of all the elements in the group. + int ret_val = sdei_inc_ref_count(tmp_cntr); + if( SDE_OK != ret_val ) + return ret_val; + curr = curr->next; + }while(NULL != curr); + } + // Increment the ref_count of the counter itself, INCLUDING the case where the counter is a group. + (counter->ref_count)++; + + return SDE_OK; +} + +int sdei_delete_counter(papisde_library_desc_t* lib_handle, const char* name) { sde_counter_t *tmp_item; papisde_control_t *gctl; uint32_t item_uniq_id; + int ret_val = SDE_OK; gctl = sdei_get_global_struct(); // Look for the counter entry in the hash-table of the library tmp_item = ht_lookup_by_name(lib_handle->lib_counters, name); - if( NULL == tmp_item ) - return 1; - - item_uniq_id = tmp_item->glb_uniq_id; - - // Delete the entry from the library hash-table (which hashes by name) - tmp_item = ht_delete(lib_handle->lib_counters, ht_hash_name(name), item_uniq_id); if( NULL == tmp_item ){ - return 1; + ret_val = SDE_EINVAL; + goto fn_exit; } - // Delete the entry from the global hash-table (which hashes by id) and free the memory - // occupied by the counter (not the hash-table entry 'papisde_list_entry_t', the 'sde_counter_t') - tmp_item = ht_delete(gctl->all_reg_counters, ht_hash_id(item_uniq_id), item_uniq_id); - if( NULL == tmp_item ){ - return 1; + if( CNTR_CLASS_GROUP == tmp_item->cntr_class ){ + papisde_list_entry_t *curr, *prev; + // If we are dealing with a goup, then we need to recurse down all its children and + // delete them (this might mean free them, or just decrement their ref_count). + curr = tmp_item->u.cntr_group.group_head; + prev = curr; + while(NULL != curr){ + int counter_is_dead = 0; + sde_counter_t *tmp_cntr = curr->item; + if( NULL == tmp_cntr ){ + ret_val = SDE_EMISC; + goto fn_exit; + } + + // If this counter is going to be freed, we need to remove it from this group. + if( 0 == tmp_cntr->ref_count ) + counter_is_dead = 1; + + // recursively delete all the elements of the group. + int ret_val = sdei_delete_counter(lib_handle, tmp_cntr->name); + if( SDE_OK != ret_val ) + goto fn_exit; + + if( counter_is_dead ){ + if( curr == tmp_item->u.cntr_group.group_head ){ + // if we were removing with the head, change the head, we can't free() it. + tmp_item->u.cntr_group.group_head = curr->next; + prev = curr->next; + curr = curr->next; + }else{ + // if we are removing an element, first bridge the previous to the next. + prev->next = curr->next; + free(curr); + curr = prev->next; + } + }else{ + // if we are not removing anything, just move the pointers. + prev = curr; + curr = curr->next; + } + } } - // We free the counter only once, although it is in two hash-tables, - // because it is the same structure that is pointed to by both hash-tables. - sdei_free_counter(tmp_item); + item_uniq_id = tmp_item->glb_uniq_id; + + // If the reference count is not zero, then we don't remove it from the hash tables + if( 0 == tmp_item->ref_count ){ + // Delete the entry from the library hash-table (which hashes by name) + tmp_item = ht_delete(lib_handle->lib_counters, ht_hash_name(name), item_uniq_id); + if( NULL == tmp_item ){ + ret_val = SDE_EMISC; + goto fn_exit; + } - // Decrement the number of live events. - gctl->num_live_events--; + // Delete the entry from the global hash-table (which hashes by id) and free the memory + // occupied by the counter (not the hash-table entry 'papisde_list_entry_t', the 'sde_counter_t') + tmp_item = ht_delete(gctl->all_reg_counters, ht_hash_id(item_uniq_id), item_uniq_id); + if( NULL == tmp_item ){ + ret_val = SDE_EMISC; + goto fn_exit; + } + + // We free the counter only once, although it is in two hash-tables, + // because it is the same structure that is pointed to by both hash-tables. + // This function will check internally if the reference count is zero, because it + // needs to recursivelly access all the children of groups. + sdei_free_counter_resources(tmp_item); - return 0; + // Decrement the number of live events. + (gctl->num_live_events)--; + }else{ + (tmp_item->ref_count)--; + } + +fn_exit: + return ret_val; } -int sdei_free_counter(sde_counter_t *counter){ +int sdei_free_counter_resources(sde_counter_t *counter){ int i, ret_val = SDE_OK; - papisde_list_entry_t *curr; if( NULL == counter ) return SDE_OK; - free(counter->name); - free(counter->description); + if( counter->ref_count < 0 ){ + SDE_ERROR("sdei_free_counter_resources(): Counter has a negative ref_count.\n"); + return SDE_EINVAL; + } - switch(counter->cntr_class){ - case CNTR_CLASS_CREATED: - free(counter->u.cntr_basic.data); - break; - case CNTR_CLASS_RECORDER: - free(counter->u.cntr_recorder.data->sorted_buffer); - for(i=0; iu.cntr_recorder.data->ptr_array[i]); - } - free(counter->u.cntr_recorder.data); - break; - case CNTR_CLASS_CSET: - ret_val = cset_delete(counter->u.cntr_cset.data); - break; - case CNTR_CLASS_GROUP: - curr = counter->u.cntr_group.group_head; - do{ - sde_counter_t *tmp_cntr = curr->item; - // recursively free all the elements of the group. - ret_val = sdei_free_counter(tmp_cntr); - if( SDE_OK != ret_val ) - return ret_val; - curr = curr->next; - }while(NULL != curr); - break; + if( 0 == counter->ref_count ){ + switch(counter->cntr_class){ + case CNTR_CLASS_CREATED: + SDEDBG(" + Freeing Created Counter Data.\n"); + free(counter->u.cntr_basic.data); + break; + case CNTR_CLASS_RECORDER: + SDEDBG(" + Freeing Recorder Data.\n"); + free(counter->u.cntr_recorder.data->sorted_buffer); + for(i=0; iu.cntr_recorder.data->ptr_array[i]); + } + free(counter->u.cntr_recorder.data); + break; + case CNTR_CLASS_CSET: + SDEDBG(" + Freeing CountingSet Data.\n"); + ret_val = cset_delete(counter->u.cntr_cset.data); + break; + } + + SDEDBG(" -> Freeing Counter '%s'.\n",counter->name); + free(counter->name); + free(counter->description); + free(counter); } - free(counter); return ret_val; }