From 0b7490e7e4f2121b15fe01daa1cce318dab36ad9 Mon Sep 17 00:00:00 2001 From: Neil Fortner Date: Fri, 4 Jan 2013 15:32:18 -0500 Subject: [svn-r23135] Purpose: Fix bug in SWMR object header code Descriptions: When removing object header messages, it is possible for object header continuation messages to move to a different chunk. When this happens, flush dependencies need to be updated to reflect the new structure. This change adds code to update the flush dependencies, and a test for this. Also fixed a bug where the flush dependency no the object header proxy was not being destroyed when an object header chunk was deleted. Tested: ummon --- src/H5Oalloc.c | 71 +++++++++++++++++++++++++++++++++++++++++++-------- src/H5Ochunk.c | 11 ++++++-- test/swmr_generator.c | 32 +++++++++++++++-------- test/testswmr.sh | 27 ++++++++++++++++++++ 4 files changed, 117 insertions(+), 24 deletions(-) diff --git a/src/H5Oalloc.c b/src/H5Oalloc.c index 0ff52f1..c889abb 100644 --- a/src/H5Oalloc.c +++ b/src/H5Oalloc.c @@ -920,7 +920,7 @@ H5O_alloc_new_chunk(H5F_t *f, hid_t dxpl_id, H5O_t *oh, size_t size, size_t *new oh->chunk = x; } /* end if */ - chunkno = oh->nchunks++; + chunkno = (unsigned)oh->nchunks++; oh->chunk[chunkno].addr = new_chunk_addr; oh->chunk[chunkno].size = size; oh->chunk[chunkno].gap = 0; @@ -965,6 +965,8 @@ H5O_alloc_new_chunk(H5F_t *f, hid_t dxpl_id, H5O_t *oh, size_t size, size_t *new oh->nmesgs--; } /* end if */ else { + HDassert(curr_msg->type->id != H5O_CONT_ID); + /* Copy the raw data */ HDmemcpy(p, curr_msg->raw - (size_t)H5O_SIZEOF_MSGHDR_OH(oh), curr_msg->raw_size + (size_t)H5O_SIZEOF_MSGHDR_OH(oh)); @@ -1454,6 +1456,7 @@ H5O_move_msgs_forward(H5F_t *f, hid_t dxpl_id, H5O_t *oh) { H5O_chunk_proxy_t *null_chk_proxy = NULL; /* Chunk that null message is in */ H5O_chunk_proxy_t *curr_chk_proxy = NULL; /* Chunk that message is in */ + H5O_chunk_proxy_t *cont_targ_chk_proxy = NULL; /* Chunk that continuation message points to */ hbool_t null_chk_dirtied = FALSE; /* Flags for unprotecting null chunk */ hbool_t curr_chk_dirtied = FALSE; /* Flags for unprotecting curr chunk */ hbool_t packed_msg; /* Flag to indicate that messages were packed */ @@ -1543,13 +1546,13 @@ H5O_move_msgs_forward(H5F_t *f, hid_t dxpl_id, H5O_t *oh) if(H5O_CONT_ID == curr_msg->type->id) { htri_t status; /* Status from moving messages */ - if((status = H5O_move_cont(f, dxpl_id, oh, u)) < 0) - HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, FAIL, "Error in moving messages into cont message") - else if(status > 0) { /* Message(s) got moved into "continuation" message */ + if((status = H5O_move_cont(f, dxpl_id, oh, u)) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTDELETE, FAIL, "Error in moving messages into cont message") + else if(status > 0) { /* Message(s) got moved into "continuation" message */ packed_msg = TRUE; - break; - } /* end else-if */ - } /* end if */ + break; + } /* end else-if */ + } /* end if */ /* Don't let locked messages be moved into earlier chunk */ if(!curr_msg->locked) { @@ -1570,6 +1573,46 @@ H5O_move_msgs_forward(H5F_t *f, hid_t dxpl_id, H5O_t *oh) if(NULL == (curr_chk_proxy = H5O_chunk_protect(f, dxpl_id, oh, curr_msg->chunkno))) HGOTO_ERROR(H5E_OHDR, H5E_CANTPROTECT, FAIL, "unable to load object header chunk") + /* If the message being moved is a continuation + * message and we are doing SWMR writes, we must + * update the flush dependencies */ + if((H5F_INTENT(f) & H5F_ACC_SWMR_WRITE) + && (H5O_CONT_ID == curr_msg->type->id)) { + void *null_chk_mdc_obj = NULL; /* The metadata cache object for the null_msg chunk */ + + /* Point to the metadata cache object for the + * null message chunk, oh if in chunk 0 or the + * proxy otherwise */ + null_chk_mdc_obj = (null_msg->chunkno == 0 + ? (void *)oh + : (void *)null_chk_proxy); + + /* The other chunks involved should never be + * chunk 0 */ + HDassert(curr_msg->chunkno > 0); + HDassert(((H5O_cont_t *)(curr_msg->native))->chunkno > 0); + + /* Protect continuation message target chunk */ + if(NULL == (cont_targ_chk_proxy = H5O_chunk_protect(f, dxpl_id, oh, ((H5O_cont_t *)(curr_msg->native))->chunkno))) + HGOTO_ERROR(H5E_OHDR, H5E_CANTPROTECT, FAIL, "unable to load object header chunk") + + /* Remove flush dependency on old continuation + * message chunk */ + if(H5AC_destroy_flush_dependency(curr_chk_proxy, cont_targ_chk_proxy) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTUNDEPEND, FAIL, "unable to destroy flush dependency") + + /* Create flush dependency on new continuation + * message chunk */ + if(H5AC_create_flush_dependency(null_chk_mdc_obj, cont_targ_chk_proxy) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTDEPEND, FAIL, "unable to create flush dependency") + + /* Unprotect continuation message target chunk + */ + if(H5O_chunk_unprotect(f, dxpl_id, cont_targ_chk_proxy, FALSE) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect object header chunk") + cont_targ_chk_proxy = NULL; + } /* end if */ + /* Copy raw data for non-null message to new chunk */ HDmemcpy(null_msg->raw - H5O_SIZEOF_MSGHDR_OH(oh), curr_msg->raw - H5O_SIZEOF_MSGHDR_OH(oh), curr_msg->raw_size + (size_t)H5O_SIZEOF_MSGHDR_OH(oh)); @@ -1714,10 +1757,16 @@ H5O_move_msgs_forward(H5F_t *f, hid_t dxpl_id, H5O_t *oh) ret_value = (htri_t)did_packing; done: - if(null_chk_proxy && H5O_chunk_unprotect(f, dxpl_id, null_chk_proxy, null_chk_dirtied) < 0) - HDONE_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect null object header chunk") - if(curr_chk_proxy && H5O_chunk_unprotect(f, dxpl_id, curr_chk_proxy, curr_chk_dirtied) < 0) - HDONE_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect current object header chunk") + if(ret_value < 0) { + if(null_chk_proxy && H5O_chunk_unprotect(f, dxpl_id, null_chk_proxy, null_chk_dirtied) < 0) + HDONE_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect null object header chunk") + if(curr_chk_proxy && H5O_chunk_unprotect(f, dxpl_id, curr_chk_proxy, curr_chk_dirtied) < 0) + HDONE_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect current object header chunk") + if(cont_targ_chk_proxy && H5O_chunk_unprotect(f, dxpl_id, cont_targ_chk_proxy, FALSE) < 0) + HDONE_ERROR(H5E_OHDR, H5E_CANTUNPROTECT, FAIL, "unable to unprotect continuation message target object header chunk") + } /* end if */ + else + HDassert(!null_chk_proxy && !curr_chk_proxy && !cont_targ_chk_proxy); FUNC_LEAVE_NOAPI(ret_value) } /* H5O_move_msgs_forward() */ diff --git a/src/H5Ochunk.c b/src/H5Ochunk.c index af4233e..8b380b2 100644 --- a/src/H5Ochunk.c +++ b/src/H5Ochunk.c @@ -402,8 +402,15 @@ H5O_chunk_delete(H5F_t *f, hid_t dxpl_id, H5O_t *oh, unsigned idx) HDassert(chk_proxy->oh == oh); HDassert(chk_proxy->chunkno == idx); - /* Only free file space if not doing SWMR writes */ - if(!(H5F_INTENT(f) & H5F_ACC_SWMR_WRITE)) + /* Update flush dependencies if doing SWMR writes */ + if(H5F_INTENT(f) & H5F_ACC_SWMR_WRITE) { + /* Remove flush dependency on object header proxy, if proxy exists */ + if(oh->proxy_present) + if(H5O_proxy_undepend(f, dxpl_id, oh, chk_proxy) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTUNDEPEND, FAIL, "can't destroy flush dependency on object header proxy") + } /* end if */ + else + /* Only free file space if not doing SWMR writes */ cache_flags |= H5AC__DIRTIED_FLAG | H5AC__FREE_FILE_SPACE_FLAG; /* Release the chunk proxy from the cache, marking it deleted */ diff --git a/test/swmr_generator.c b/test/swmr_generator.c index f904378..f0fcee6 100644 --- a/test/swmr_generator.c +++ b/test/swmr_generator.c @@ -42,8 +42,9 @@ /* Local Prototypes */ /********************/ -static int gen_skeleton(const char *filename, unsigned verbose, int comp_level, - const char *index_type, unsigned random_seed); +static int gen_skeleton(const char *filename, unsigned verbose, + unsigned swmr_write, int comp_level, const char *index_type, + unsigned random_seed); static void usage(void); @@ -76,8 +77,8 @@ static void usage(void); *------------------------------------------------------------------------- */ static int -gen_skeleton(const char *filename, unsigned verbose, int comp_level, - const char *index_type, unsigned random_seed) +gen_skeleton(const char *filename, unsigned verbose, unsigned swmr_write, + int comp_level, const char *index_type, unsigned random_seed) { hid_t fid; /* File ID for new HDF5 file */ hid_t fcpl; /* File creation property list */ @@ -145,7 +146,7 @@ gen_skeleton(const char *filename, unsigned verbose, int comp_level, fprintf(stderr, "Creating file\n"); /* Create the file */ - if((fid = H5Fcreate(filename, H5F_ACC_TRUNC, fcpl, fapl)) < 0) + if((fid = H5Fcreate(filename, H5F_ACC_TRUNC | (swmr_write ? H5F_ACC_SWMR_WRITE : 0), fcpl, fapl)) < 0) return -1; /* Close file creation property list */ @@ -234,8 +235,8 @@ usage(void) printf("\n"); printf("Usage error!\n"); printf("\n"); - printf("Usage: swmr_generator [-q] [-c ] [-i ]\n"); - printf(" [-r ]\n"); + printf("Usage: swmr_generator [-q] [-s] [-c ]\n"); + printf(" [-i ] [-r ]\n"); printf("\n"); printf("NOTE: The random seed option is only used by the sparse test. Other\n"); printf(" tests specify the random seed as a reader/writer option.\n"); @@ -244,8 +245,9 @@ usage(void) printf("\n"); printf(" should be b1, b2, fa, or ea (fa not yet implemented)\n"); printf("\n"); - printf("Defaults to verbose (no '-q' given), no compression ('-c -1'), v1 b-tree\n"); - printf("indexing (-i b1), and will generate a random seed (no -r given).\n"); + printf("Defaults to verbose (no '-q' given), no SWMR_WRITE mode (no '-s' given) no\n"); + printf("compression ('-c -1'), v1 b-tree indexing (-i b1), and will generate a random\n"); + printf("seed (no -r given).\n"); printf("\n"); exit(1); } /* end usage() */ @@ -254,6 +256,7 @@ int main(int argc, const char *argv[]) { int comp_level = -1; /* Compression level (-1 is no compression) */ unsigned verbose = 1; /* Whether to emit some informational messages */ + unsigned swmr_write = 0; /* Whether to create file with SWMR_WRITE access */ const char *index_type = "b1"; /* Chunk index type */ unsigned use_seed = 0; /* Set to 1 if a seed was set on the command line */ unsigned random_seed = 0; /* Random # seed */ @@ -301,6 +304,12 @@ int main(int argc, const char *argv[]) u++; break; + /* Run with SWMR_WRITE */ + case 's': + swmr_write = 1; + u++; + break; + default: usage(); break; @@ -312,8 +321,9 @@ int main(int argc, const char *argv[]) /* Emit informational message */ if(verbose) { fprintf(stderr, "Parameters:\n"); + fprintf(stderr, "\tswmr writes %s\n", swmr_write ? "on" : "off"); fprintf(stderr, "\tcompression level = %d\n", comp_level); - fprintf(stderr, "\tindex_type = %s\n", index_type); + fprintf(stderr, "\tindex type = %s\n", index_type); } /* end if */ /* Set the random seed */ @@ -331,7 +341,7 @@ int main(int argc, const char *argv[]) fprintf(stderr, "Generating skeleton file: %s\n", FILENAME); /* Generate file skeleton */ - if(gen_skeleton(FILENAME, verbose, comp_level, index_type, random_seed) < 0) { + if(gen_skeleton(FILENAME, verbose, swmr_write, comp_level, index_type, random_seed) < 0) { fprintf(stderr, "Error generating skeleton file!\n"); exit(1); } /* end if */ diff --git a/test/testswmr.sh b/test/testswmr.sh index 43f9f39..b178af0 100755 --- a/test/testswmr.sh +++ b/test/testswmr.sh @@ -76,6 +76,33 @@ do do echo echo "###############################################################################" + echo "## Generator test" + echo "###############################################################################" + # Launch the Generator without SWMR_WRITE + echo launch the swmr_generator + ./swmr_generator $compress $index_type + if test $? -ne 0; then + echo generator had error + nerrors=`expr $nerrors + 1` + fi + + # Launch the Generator with SWMR_WRITE + echo launch the swmr_generator with SWMR_WRITE + ./swmr_generator -s $compress $index_type + if test $? -ne 0; then + echo generator had error + nerrors=`expr $nerrors + 1` + fi + + # Check for error and exit if one occured + $DPRINT nerrors=$nerrors + if test $nerrors -ne 0 ; then + echo "SWMR tests failed with $nerrors errors." + exit 1 + fi + + echo + echo "###############################################################################" echo "## Writer test - test expanding the dataset" echo "###############################################################################" -- cgit v0.12