From 8880ec672eeb8f57c023938074d1336dfc82e2ed Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Sat, 23 Sep 2017 11:17:47 -0500 Subject: Style cleanups and misc. bugfixes discovered during full SWMR development. --- src/H5ACprivate.h | 3 +-- src/H5C.c | 47 +++++++++++++++++++++++++++++------------------ src/H5F.c | 6 +++++- src/H5Fint.c | 8 ++++++-- src/H5Fpkg.h | 30 +++++++++++++++--------------- src/H5Fsuper.c | 3 +++ src/H5MF.c | 1 + src/H5MFdbg.c | 2 +- src/H5Oefl.c | 2 +- src/H5Otest.c | 2 +- src/H5PB.c | 4 ++-- test/ohdr.c | 5 +++-- tools/lib/h5tools_ref.c | 3 ++- 13 files changed, 70 insertions(+), 46 deletions(-) diff --git a/src/H5ACprivate.h b/src/H5ACprivate.h index b9e2a60..083ee5b 100644 --- a/src/H5ACprivate.h +++ b/src/H5ACprivate.h @@ -230,7 +230,7 @@ typedef struct H5AC_proxy_entry_t { /* (Note that this currently duplicates some cache functionality) */ } H5AC_proxy_entry_t; - +/* Name of property for ring info in DXPL */ #define H5AC_RING_NAME "H5AC_ring_type" /* Dataset transfer property lists for metadata calls */ @@ -247,7 +247,6 @@ H5_DLLVAR hid_t H5AC_noio_dxpl_id; H5_DLLVAR hid_t H5AC_rawdata_dxpl_id; /* Default cache configuration. */ - #define H5AC__DEFAULT_METADATA_WRITE_STRATEGY \ H5AC_METADATA_WRITE_STRATEGY__DISTRIBUTED diff --git a/src/H5C.c b/src/H5C.c index 371fc90..926d3fd 100644 --- a/src/H5C.c +++ b/src/H5C.c @@ -4062,8 +4062,7 @@ H5C_destroy_flush_dependency(void *parent_thing, void * child_thing) child_entry->flush_dep_parent_nalloc = 0; } /* end if */ else if(child_entry->flush_dep_parent_nalloc > H5C_FLUSH_DEP_PARENT_INIT - && child_entry->flush_dep_nparents - <= (child_entry->flush_dep_parent_nalloc / 4)) { + && child_entry->flush_dep_nparents <= (child_entry->flush_dep_parent_nalloc / 4)) { if(NULL == (child_entry->flush_dep_parent = (H5C_cache_entry_t **)H5FL_BLK_REALLOC(parent, child_entry->flush_dep_parent, (child_entry->flush_dep_parent_nalloc / 4) * sizeof(H5C_cache_entry_t *)))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for flush dependency parent list") child_entry->flush_dep_parent_nalloc /= 4; @@ -6428,7 +6427,7 @@ H5C__flush_single_entry(H5F_t *f, hid_t dxpl_id, H5C_cache_entry_t *entry_ptr, HDassert(entry_ptr->flush_dep_ndirty_children == 0); if(entry_ptr->flush_dep_nparents > 0) if(H5C__mark_flush_dep_clean(entry_ptr) < 0) - HGOTO_ERROR(H5E_CACHE, H5E_CANTMARKDIRTY, FAIL, "Can't propagate flush dep clean flag") + HGOTO_ERROR(H5E_CACHE, H5E_CANTMARKCLEAN, FAIL, "Can't propagate flush dep clean flag") } /* end if */ } /* end else */ @@ -7989,7 +7988,7 @@ done: static herr_t H5C__mark_flush_dep_clean(H5C_cache_entry_t * entry) { - unsigned u; /* Local index variable */ + int i; /* Local index variable */ herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_STATIC @@ -7998,16 +7997,19 @@ H5C__mark_flush_dep_clean(H5C_cache_entry_t * entry) HDassert(entry); /* Iterate over the parent entries, if any */ - for(u = 0; u < entry->flush_dep_nparents; u++) { + /* Note reverse iteration order, in case the callback removes the flush + * dependency - QAK, 2017/08/12 + */ + for(i = ((int)entry->flush_dep_nparents) - 1; i >= 0; i--) { /* Sanity check */ - HDassert(entry->flush_dep_parent[u]->flush_dep_ndirty_children > 0); + HDassert(entry->flush_dep_parent[i]->flush_dep_ndirty_children > 0); /* Adjust the parent's number of dirty children */ - entry->flush_dep_parent[u]->flush_dep_ndirty_children--; + entry->flush_dep_parent[i]->flush_dep_ndirty_children--; /* If the parent has a 'notify' callback, send a 'child entry cleaned' notice */ - if(entry->flush_dep_parent[u]->type->notify && - (entry->flush_dep_parent[u]->type->notify)(H5C_NOTIFY_ACTION_CHILD_CLEANED, entry->flush_dep_parent[u]) < 0) + if(entry->flush_dep_parent[i]->type->notify && + (entry->flush_dep_parent[i]->type->notify)(H5C_NOTIFY_ACTION_CHILD_CLEANED, entry->flush_dep_parent[i]) < 0) HGOTO_ERROR(H5E_CACHE, H5E_CANTNOTIFY, FAIL, "can't notify parent about child entry dirty flag reset") } /* end for */ @@ -8033,7 +8035,7 @@ done: herr_t H5C__mark_flush_dep_serialized(H5C_cache_entry_t * entry_ptr) { - unsigned u; /* Local index variable */ + int i; /* Local index variable */ herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_STATIC @@ -8042,18 +8044,21 @@ H5C__mark_flush_dep_serialized(H5C_cache_entry_t * entry_ptr) HDassert(entry_ptr); /* Iterate over the parent entries, if any */ - for(u = 0; u < entry_ptr->flush_dep_nparents; u++) { - + /* Note reverse iteration order, in case the callback removes the flush + * dependency - QAK, 2017/08/12 + */ + for(i = ((int)entry_ptr->flush_dep_nparents) - 1; i >= 0; i--) { + /* Sanity checks */ HDassert(entry_ptr->flush_dep_parent); - HDassert(entry_ptr->flush_dep_parent[u]->magic == H5C__H5C_CACHE_ENTRY_T_MAGIC); - HDassert(entry_ptr->flush_dep_parent[u]->flush_dep_nunser_children > 0); + HDassert(entry_ptr->flush_dep_parent[i]->magic == H5C__H5C_CACHE_ENTRY_T_MAGIC); + HDassert(entry_ptr->flush_dep_parent[i]->flush_dep_nunser_children > 0); /* decrement the parents number of unserialized children */ - entry_ptr->flush_dep_parent[u]->flush_dep_nunser_children--; + entry_ptr->flush_dep_parent[i]->flush_dep_nunser_children--; /* If the parent has a 'notify' callback, send a 'child entry serialized' notice */ - if(entry_ptr->flush_dep_parent[u]->type->notify && - (entry_ptr->flush_dep_parent[u]->type->notify)(H5C_NOTIFY_ACTION_CHILD_SERIALIZED, entry_ptr->flush_dep_parent[u]) < 0) + if(entry_ptr->flush_dep_parent[i]->type->notify && + (entry_ptr->flush_dep_parent[i]->type->notify)(H5C_NOTIFY_ACTION_CHILD_SERIALIZED, entry_ptr->flush_dep_parent[i]) < 0) HGOTO_ERROR(H5E_CACHE, H5E_CANTNOTIFY, FAIL, "can't notify parent about child entry serialized flag set") } /* end for */ @@ -8065,7 +8070,7 @@ done: /*------------------------------------------------------------------------- * Function: H5C__mark_flush_dep_unserialized() * - * Purpose: Decrement the flush_dep_nunser_children fields of all the + * Purpose: Increment the flush_dep_nunser_children fields of all the * target entry's flush dependency parents in response to * the target entry becoming unserialized. * @@ -8872,6 +8877,12 @@ H5C_remove_entry(void *_entry) HGOTO_ERROR(H5E_CACHE, H5E_CANTREMOVE, FAIL, "can't remove protected entry from cache") if(entry->is_pinned) HGOTO_ERROR(H5E_CACHE, H5E_CANTREMOVE, FAIL, "can't remove pinned entry from cache") + /* NOTE: If these two errors are getting tripped because the entry is + * in a flush dependency with a freedspace entry, move the checks + * after the "before evict" message is sent, and add the + * "child being evicted" message to the "before evict" notify + * section below. QAK - 2017/08/03 + */ if(entry->flush_dep_nparents > 0) HGOTO_ERROR(H5E_CACHE, H5E_CANTREMOVE, FAIL, "can't remove entry with flush dependency parents from cache") if(entry->flush_dep_nchildren > 0) diff --git a/src/H5F.c b/src/H5F.c index 78fce2a..211f1aa 100644 --- a/src/H5F.c +++ b/src/H5F.c @@ -1633,7 +1633,7 @@ H5Fstart_swmr_write(hid_t file_id) if(file->shared->sblock->super_vers < HDF5_SUPERBLOCK_VERSION_3) HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "file superblock version should be at least 3") - HDassert(file->shared->latest_flags == H5F_LATEST_ALL_FLAGS); + HDassert((file->shared->latest_flags | H5F_LATEST_LAYOUT_MSG) > 0); /* Should not be marked for SWMR writing mode already */ if(file->shared->sblock->status_flags & H5F_SUPER_SWMR_WRITE_ACCESS) @@ -1647,6 +1647,10 @@ H5Fstart_swmr_write(hid_t file_id) if(ci_load || ci_write ) HGOTO_ERROR(H5E_FILE, H5E_UNSUPPORTED, FAIL, "can't have both SWMR and MDC cache image") + /* Flush the superblock extension */ + if(H5F_flush_tagged_metadata(file, file->shared->sblock->ext_addr, H5AC_ind_read_dxpl_id) < 0) + HGOTO_ERROR(H5E_FILE, H5E_CANTFLUSH, NULL, "unable to flush superblock extension") + /* Flush data buffers */ if(H5F__flush(file, H5AC_ind_read_dxpl_id, H5AC_rawdata_dxpl_id, FALSE) < 0) HGOTO_ERROR(H5E_FILE, H5E_CANTFLUSH, FAIL, "unable to flush file's cached information") diff --git a/src/H5Fint.c b/src/H5Fint.c index e52d539..4c5e00e 100644 --- a/src/H5Fint.c +++ b/src/H5Fint.c @@ -678,8 +678,10 @@ H5F_new(H5F_file_t *shared, unsigned flags, hid_t fcpl_id, hid_t fapl_id, H5FD_t if(H5P_get(plist, H5F_ACS_LATEST_FORMAT_NAME, &latest_format) < 0) HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, NULL, "can't get 'latest format' flag") /* For latest format or SWMR_WRITE, activate all latest version support */ - if(latest_format || (H5F_INTENT(f) & H5F_ACC_SWMR_WRITE)) + if(latest_format) f->shared->latest_flags |= H5F_LATEST_ALL_FLAGS; + else if(H5F_INTENT(f) & H5F_ACC_SWMR_WRITE) + f->shared->latest_flags |= H5F_LATEST_LAYOUT_MSG; if(H5P_get(plist, H5F_ACS_USE_MDC_LOGGING_NAME, &(f->shared->use_mdc_logging)) < 0) HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, NULL, "can't get 'use mdc logging' flag") if(H5P_get(plist, H5F_ACS_START_MDC_LOG_ON_ACCESS_NAME, &(f->shared->start_mdc_log_on_access)) < 0) @@ -1456,11 +1458,13 @@ H5F_open(const char *name, unsigned flags, hid_t fcpl_id, hid_t fapl_id, if(H5F_INTENT(file) & H5F_ACC_SWMR_WRITE) file->shared->sblock->status_flags |= H5F_SUPER_SWMR_WRITE_ACCESS; - /* Flush the superblock */ + /* Flush the superblock & superblock extension */ if(H5F_super_dirty(file) < 0) HGOTO_ERROR(H5E_FILE, H5E_CANTMARKDIRTY, NULL, "unable to mark superblock as dirty") if(H5F_flush_tagged_metadata(file, H5AC__SUPERBLOCK_TAG, meta_dxpl_id) < 0) HGOTO_ERROR(H5E_FILE, H5E_CANTFLUSH, NULL, "unable to flush superblock") + if(H5F_flush_tagged_metadata(file, file->shared->sblock->ext_addr, meta_dxpl_id) < 0) + HGOTO_ERROR(H5E_FILE, H5E_CANTFLUSH, NULL, "unable to flush superblock extension") /* Remove the file lock for SWMR_WRITE */ if(use_file_locking && (H5F_INTENT(file) & H5F_ACC_SWMR_WRITE)) { diff --git a/src/H5Fpkg.h b/src/H5Fpkg.h index 7a5c126..d702506 100644 --- a/src/H5Fpkg.h +++ b/src/H5Fpkg.h @@ -309,16 +309,16 @@ struct H5F_file_t { H5UC_t *grp_btree_shared; /* Ref-counted group B-tree node info */ /* File space allocation information */ - H5F_fspace_strategy_t fs_strategy; /* File space handling strategy */ + H5F_fspace_strategy_t fs_strategy; /* File space handling strategy */ hsize_t fs_threshold; /* Free space section threshold */ - hbool_t fs_persist; /* Free-space persist or not */ + hbool_t fs_persist; /* Free-space persist or not */ hbool_t use_tmp_space; /* Whether temp. file space allocation is allowed */ haddr_t tmp_addr; /* Next address to use for temp. space in the file */ - hbool_t point_of_no_return; /* flag to indicate that we can't go back and delete a freespace header when it's used up */ + hbool_t point_of_no_return; /* Flag to indicate that we can't go back and delete a freespace header when it's used up */ H5F_fs_state_t fs_state[H5F_MEM_PAGE_NTYPES]; /* State of free space manager for each type */ - haddr_t fs_addr[H5F_MEM_PAGE_NTYPES]; /* Address of free space manager info for each type */ - H5FS_t *fs_man[H5F_MEM_PAGE_NTYPES]; /* Free space manager for each file space type */ + haddr_t fs_addr[H5F_MEM_PAGE_NTYPES]; /* Address of free space manager info for each type */ + H5FS_t *fs_man[H5F_MEM_PAGE_NTYPES]; /* Free space manager for each file space type */ hbool_t first_alloc_dealloc; /* TRUE iff free space managers */ /* are persistant and have not */ /* been used accessed for either */ @@ -333,25 +333,25 @@ struct H5F_file_t { /* HADDR_UNDEF if no cache image. */ /* Free-space aggregation info */ - unsigned fs_aggr_merge[H5FD_MEM_NTYPES]; /* Flags for whether free space can merge with aggregator(s) */ - H5FD_mem_t fs_type_map[H5FD_MEM_NTYPES]; /* Mapping of "real" file space type into tracked type */ - H5F_blk_aggr_t meta_aggr; /* Metadata aggregation info (if aggregating metadata allocations) */ - H5F_blk_aggr_t sdata_aggr; /* "Small data" aggregation info (if aggregating "small data" allocations) */ + unsigned fs_aggr_merge[H5FD_MEM_NTYPES]; /* Flags for whether free space can merge with aggregator(s) */ + H5FD_mem_t fs_type_map[H5FD_MEM_NTYPES]; /* Mapping of "real" file space type into tracked type */ + H5F_blk_aggr_t meta_aggr; /* Metadata aggregation info (if aggregating metadata allocations) */ + H5F_blk_aggr_t sdata_aggr; /* "Small data" aggregation info (if aggregating "small data" allocations) */ /* Paged aggregation info */ - hsize_t fs_page_size; /* File space page size */ - size_t pgend_meta_thres; /* Do not track page end meta section <= this threshold */ + hsize_t fs_page_size; /* File space page size */ + size_t pgend_meta_thres; /* Do not track page end meta section <= this threshold */ /* Metadata accumulator information */ - H5F_meta_accum_t accum; /* Metadata accumulator info */ + H5F_meta_accum_t accum; /* Metadata accumulator info */ /* Metadata retry info */ - unsigned read_attempts; /* The # of reads to try when reading metadata with checksum */ - unsigned retries_nbins; /* # of bins for each retries[] */ + unsigned read_attempts; /* The # of reads to try when reading metadata with checksum */ + unsigned retries_nbins; /* # of bins for each retries[] */ uint32_t *retries[H5AC_NTYPES]; /* Track # of read retries for metdata items with checksum */ /* Object flush info */ - H5F_object_flush_t object_flush; /* Information for object flush callback */ + H5F_object_flush_t object_flush; /* Information for object flush callback */ }; /* diff --git a/src/H5Fsuper.c b/src/H5Fsuper.c index 7c70a64..942a7ed 100644 --- a/src/H5Fsuper.c +++ b/src/H5Fsuper.c @@ -974,6 +974,9 @@ H5F__super_init(H5F_t *f, hid_t dxpl_id) /* Bump superblock version if latest superblock version support is enabled */ if(H5F_USE_LATEST_FLAGS(f, H5F_LATEST_SUPERBLOCK)) super_vers = HDF5_SUPERBLOCK_VERSION_LATEST; + /* Bump superblock version to use version 3 superblock for SWMR writing */ + else if((H5F_INTENT(f) & H5F_ACC_SWMR_WRITE)) + super_vers = HDF5_SUPERBLOCK_VERSION_3; /* Bump superblock version to create superblock extension for SOHM info */ else if(f->shared->sohm_nindexes > 0) super_vers = HDF5_SUPERBLOCK_VERSION_2; diff --git a/src/H5MF.c b/src/H5MF.c index d7af56a..49c7b77 100644 --- a/src/H5MF.c +++ b/src/H5MF.c @@ -115,6 +115,7 @@ hbool_t H5_PKG_INIT_VAR = FALSE; /* Local Variables */ /*******************/ + /*------------------------------------------------------------------------- * Function: H5MF_init_merge_flags diff --git a/src/H5MFdbg.c b/src/H5MFdbg.c index 1ae902f..0b300ba 100644 --- a/src/H5MFdbg.c +++ b/src/H5MFdbg.c @@ -155,8 +155,8 @@ done: herr_t H5MF_sects_debug(H5F_t *f, hid_t dxpl_id, haddr_t fs_addr, FILE *stream, int indent, int fwidth) { - herr_t ret_value = SUCCEED; /* Return value */ H5F_mem_page_t type; /* Memory type for iteration */ + herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_TAG(dxpl_id, H5AC__FREESPACE_TAG, FAIL) diff --git a/src/H5Oefl.c b/src/H5Oefl.c index 49c442f..ba7a6ee 100644 --- a/src/H5Oefl.c +++ b/src/H5Oefl.c @@ -151,7 +151,7 @@ H5O_efl_decode(H5F_t *f, hid_t dxpl_id, H5O_t H5_ATTR_UNUSED *open_oh, if((s = (const char *)H5HL_offset_into(heap, mesg->slot[u].name_offset)) == NULL) HGOTO_ERROR(H5E_SYM, H5E_CANTGET, NULL, "unable to get external file name") - if(*s == (char)NULL) + if(*s == (char)'\0') HGOTO_ERROR(H5E_SYM, H5E_CANTGET, NULL, "invalid external file name") mesg->slot[u].name = H5MM_xstrdup (s); HDassert(mesg->slot[u].name); diff --git a/src/H5Otest.c b/src/H5Otest.c index f0deade..68462a1 100644 --- a/src/H5Otest.c +++ b/src/H5Otest.c @@ -731,7 +731,7 @@ H5O_msg_move_to_new_chunk_test(hid_t oid, unsigned msg_type) /* Allocate and initialize new chunk in the file, moving the found message */ /* (*new_idx returned from this routine is unused here) */ - if(H5O__alloc_chunk(loc->file, H5AC_ind_read_dxpl_id, oh, 40, oh->nmesgs, &found_msg, &new_idx) < 0) + if(H5O__alloc_chunk(loc->file, H5AC_ind_read_dxpl_id, oh, (curr_msg->raw_size + H5O_SIZEOF_MSGHDR_OH(oh)), oh->nmesgs, &found_msg, &new_idx) < 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTALLOC, FAIL, "can't allocate new object header chunk") /* Break out of loop, the message was found */ diff --git a/src/H5PB.c b/src/H5PB.c index 52576f8..63e5e7a 100644 --- a/src/H5PB.c +++ b/src/H5PB.c @@ -1506,7 +1506,7 @@ H5PB__write_entry(const H5F_io_info2_t *fio_info, H5PB_entry_t *page_entry) HDassert(page_entry); /* Retrieve the 'eoa' for the file */ - if(HADDR_UNDEF == (eoa = H5F_get_eoa(fio_info->f, page_entry->type))) + if(HADDR_UNDEF == (eoa = H5F_get_eoa(fio_info->f, (H5FD_mem_t)page_entry->type))) HGOTO_ERROR(H5E_PAGEBUF, H5E_CANTGET, FAIL, "driver get_eoa request failed") /* If the starting address of the page is larger than @@ -1525,7 +1525,7 @@ H5PB__write_entry(const H5F_io_info2_t *fio_info, H5PB_entry_t *page_entry) fdio_info.meta_dxpl = fio_info->meta_dxpl; fdio_info.raw_dxpl = fio_info->raw_dxpl; - if(H5FD_write(&fdio_info, page_entry->type, page_entry->addr, page_size, page_entry->page_buf_ptr) < 0) + if(H5FD_write(&fdio_info, (H5FD_mem_t)page_entry->type, page_entry->addr, page_size, page_entry->page_buf_ptr) < 0) HGOTO_ERROR(H5E_PAGEBUF, H5E_WRITEERROR, FAIL, "file write failed") } /* end if */ diff --git a/test/ohdr.c b/test/ohdr.c index faec835..3915b38 100644 --- a/test/ohdr.c +++ b/test/ohdr.c @@ -392,8 +392,9 @@ test_ohdr_swmr(hbool_t new_format) if(H5Oget_info(did, &obj_info) < 0) FAIL_STACK_ERROR - if(obj_info.hdr.version != OBJ_VERSION_LATEST) - FAIL_STACK_ERROR + if(new_format) + if(obj_info.hdr.version != OBJ_VERSION_LATEST) + FAIL_STACK_ERROR /* The size of object header should be greater than the speculative read size of H5O_SPEC_READ_SIZE */ /* This will exercise the coding for the re-read of the object header for SWMR access */ diff --git a/tools/lib/h5tools_ref.c b/tools/lib/h5tools_ref.c index 6153f0c..d6e5f01 100644 --- a/tools/lib/h5tools_ref.c +++ b/tools/lib/h5tools_ref.c @@ -166,7 +166,8 @@ haddr_t ref_path_table_lookup(const char *thepath) { H5O_info_t oi; - if((HDstrlen(thepath) == 0) || (thepath == NULL)) + + if((thepath == NULL) || (HDstrlen(thepath) == 0)) return HADDR_UNDEF; /* Allow lookups on the root group, even though it doesn't have any link info */ if(HDstrcmp(thepath, "/")) { -- cgit v0.12