From 9fa5dca9c2ebc2c3cb25ded77d37b9c666e11fbd Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Fri, 14 Oct 2011 08:28:23 -0500 Subject: [svn-r21561] Description: Correct error in loading local heap prefix & data block from the file. Sometimes the local heap's prefix could be loaded before the data block (e.g. using H5Oget_info), but then when the data block was loaded later, the free list information would get lost, causing the heap's size to grow larger than necessary. This is Jira bug #HDFFV-7767 Tested on: Mac OS X/32 10.7.2 (amazon) w/debug (h5committest coming up) --- src/H5HL.c | 17 +++-------- src/H5HLcache.c | 28 +++++++++-------- src/H5HLpkg.h | 6 ++-- test/h5test.c | 2 +- test/h5test.h | 2 +- test/tmisc.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 6 files changed, 115 insertions(+), 34 deletions(-) diff --git a/src/H5HL.c b/src/H5HL.c index d806308..e37eb9f 100644 --- a/src/H5HL.c +++ b/src/H5HL.c @@ -153,9 +153,12 @@ H5HL_create(H5F_t *f, hid_t dxpl_id, size_t size_hint, haddr_t *addr_p/*out*/) heap->freelist->offset = 0; heap->freelist->size = size_hint; heap->freelist->prev = heap->freelist->next = NULL; + heap->free_block = 0; } /* end if */ - else + else { heap->freelist = NULL; + heap->free_block = H5HL_FREE_NULL; + } /* end else */ /* Allocate the heap prefix */ if(NULL == (prfx = H5HL_prfx_new(heap))) @@ -459,8 +462,6 @@ H5HL_protect(H5F_t *f, hid_t dxpl_id, haddr_t addr, H5AC_protect_t rw) prfx_udata.sizeof_addr = H5F_SIZEOF_ADDR(f); prfx_udata.prfx_addr = addr; prfx_udata.sizeof_prfx = H5HL_SIZEOF_HDR(f); - prfx_udata.loaded = FALSE; - prfx_udata.free_block = H5HL_FREE_NULL; /* Protect the local heap prefix */ if(NULL == (prfx = (H5HL_prfx_t *)H5AC_protect(f, dxpl_id, H5AC_LHEAP_PRFX, addr, &prfx_udata, rw))) @@ -482,8 +483,6 @@ H5HL_protect(H5F_t *f, hid_t dxpl_id, haddr_t addr, H5AC_protect_t rw) /* Construct the user data for protect callback */ dblk_udata.heap = heap; - dblk_udata.free_block = prfx_udata.loaded ? prfx_udata.free_block : - (heap->freelist ? heap->freelist->offset : H5HL_FREE_NULL); dblk_udata.loaded = FALSE; /* Protect the local heap data block */ @@ -1076,8 +1075,6 @@ H5HL_delete(H5F_t *f, hid_t dxpl_id, haddr_t addr) prfx_udata.sizeof_addr = H5F_SIZEOF_ADDR(f); prfx_udata.prfx_addr = addr; prfx_udata.sizeof_prfx = H5HL_SIZEOF_HDR(f); - prfx_udata.loaded = FALSE; - prfx_udata.free_block = H5HL_FREE_NULL; /* Protect the local heap prefix */ if(NULL == (prfx = (H5HL_prfx_t *)H5AC_protect(f, dxpl_id, H5AC_LHEAP_PRFX, addr, &prfx_udata, H5AC_WRITE))) @@ -1092,8 +1089,6 @@ H5HL_delete(H5F_t *f, hid_t dxpl_id, haddr_t addr) /* Construct the user data for protect callback */ dblk_udata.heap = heap; - dblk_udata.free_block = prfx_udata.loaded ? prfx_udata.free_block : - (heap->freelist ? heap->freelist->offset : H5HL_FREE_NULL); dblk_udata.loaded = FALSE; /* Protect the local heap data block */ @@ -1156,8 +1151,6 @@ H5HL_get_size(H5F_t *f, hid_t dxpl_id, haddr_t addr, size_t *size) prfx_udata.sizeof_addr = H5F_SIZEOF_ADDR(f); prfx_udata.prfx_addr = addr; prfx_udata.sizeof_prfx = H5HL_SIZEOF_HDR(f); - prfx_udata.loaded = FALSE; - prfx_udata.free_block = H5HL_FREE_NULL; /* Protect the local heap prefix */ if(NULL == (prfx = (H5HL_prfx_t *)H5AC_protect(f, dxpl_id, H5AC_LHEAP_PRFX, addr, &prfx_udata, H5AC_READ))) @@ -1210,8 +1203,6 @@ H5HL_heapsize(H5F_t *f, hid_t dxpl_id, haddr_t addr, hsize_t *heap_size) prfx_udata.sizeof_addr = H5F_SIZEOF_ADDR(f); prfx_udata.prfx_addr = addr; prfx_udata.sizeof_prfx = H5HL_SIZEOF_HDR(f); - prfx_udata.loaded = FALSE; - prfx_udata.free_block = H5HL_FREE_NULL; /* Protect the local heap prefix */ if(NULL == (prfx = (H5HL_prfx_t *)H5AC_protect(f, dxpl_id, H5AC_LHEAP_PRFX, addr, &prfx_udata, H5AC_READ))) diff --git a/src/H5HLcache.c b/src/H5HLcache.c index 56d9919..daf1d87 100644 --- a/src/H5HLcache.c +++ b/src/H5HLcache.c @@ -134,9 +134,10 @@ const H5AC_class_t H5AC_LHEAP_DBLK[1] = {{ *------------------------------------------------------------------------- */ static herr_t -H5HL_fl_deserialize(H5HL_t *heap, hsize_t free_block) +H5HL_fl_deserialize(H5HL_t *heap) { H5HL_free_t *fl = NULL, *tail = NULL; /* Heap free block nodes */ + hsize_t free_block; /* Offset of free block */ herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5HL_fl_deserialize) @@ -146,6 +147,7 @@ H5HL_fl_deserialize(H5HL_t *heap, hsize_t free_block) HDassert(!heap->freelist); /* Build free list */ + free_block = heap->free_block; while(H5HL_FREE_NULL != free_block) { const uint8_t *p; /* Pointer into image buffer */ @@ -310,8 +312,8 @@ H5HL_prefix_load(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *_udata) H5F_DECODE_LENGTH_LEN(p, heap->dblk_size, udata->sizeof_size); /* Free list head */ - H5F_DECODE_LENGTH_LEN(p, udata->free_block, udata->sizeof_size); - if(udata->free_block != H5HL_FREE_NULL && udata->free_block >= heap->dblk_size) + H5F_DECODE_LENGTH_LEN(p, heap->free_block, udata->sizeof_size); + if(heap->free_block != H5HL_FREE_NULL && heap->free_block >= heap->dblk_size) HGOTO_ERROR(H5E_HEAP, H5E_BADVALUE, NULL, "bad heap free list") /* Heap data address */ @@ -345,7 +347,7 @@ H5HL_prefix_load(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *_udata) } /* end else */ /* Build free list */ - if(H5HL_fl_deserialize(heap, udata->free_block) < 0) + if(H5HL_fl_deserialize(heap) < 0) HGOTO_ERROR(H5E_HEAP, H5E_CANTINIT, NULL, "can't initialize free list") } /* end if */ else @@ -353,9 +355,6 @@ H5HL_prefix_load(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *_udata) heap->single_cache_obj = FALSE; } /* end if */ - /* Set flag to indicate prefix from loaded from file */ - udata->loaded = TRUE; - /* Set return value */ ret_value = prfx; @@ -408,7 +407,6 @@ H5HL_prefix_flush(H5F_t *f, hid_t dxpl_id, hbool_t destroy, haddr_t addr, if(prfx->cache_info.is_dirty) { H5HL_t *heap = prfx->heap; /* Pointer to the local heap */ - H5HL_free_t *fl = heap->freelist; /* Pointer to heap's free list */ uint8_t *buf; /* Pointer to heap buffer */ size_t buf_size; /* Size of buffer for encoding & writing heap info */ uint8_t *p; /* Pointer into raw data buffer */ @@ -426,6 +424,9 @@ H5HL_prefix_flush(H5F_t *f, hid_t dxpl_id, hbool_t destroy, haddr_t addr, if(NULL == (buf = (uint8_t *)H5WB_actual(wb, buf_size))) HGOTO_ERROR(H5E_HEAP, H5E_NOSPACE, FAIL, "can't get actual buffer") + /* Update the free block value from the free list */ + heap->free_block = heap->freelist ? heap->freelist->offset : H5HL_FREE_NULL; + /* Serialize the heap prefix */ p = buf; HDmemcpy(p, H5HL_MAGIC, (size_t)H5_SIZEOF_MAGIC); @@ -435,18 +436,18 @@ H5HL_prefix_flush(H5F_t *f, hid_t dxpl_id, hbool_t destroy, haddr_t addr, *p++ = 0; /*reserved*/ *p++ = 0; /*reserved*/ H5F_ENCODE_LENGTH_LEN(p, heap->dblk_size, heap->sizeof_size); - H5F_ENCODE_LENGTH_LEN(p, fl ? fl->offset : H5HL_FREE_NULL, heap->sizeof_size); + H5F_ENCODE_LENGTH_LEN(p, heap->free_block, heap->sizeof_size); H5F_addr_encode_len(heap->sizeof_addr, &p, heap->dblk_addr); /* Check if the local heap is a single object in cache */ if(heap->single_cache_obj) { - if((p - buf) < heap->prfx_size) { + if((size_t)(p - buf) < heap->prfx_size) { size_t gap; /* Size of gap between prefix and data block */ /* Set p to the start of the data block. This is necessary because * there may be a gap between the used portion of the prefix and the * data block due to alignment constraints. */ - gap = heap->prfx_size - (p - buf); + gap = heap->prfx_size - (size_t)(p - buf); HDmemset(p, 0, gap); p += gap; } /* end if */ @@ -654,7 +655,7 @@ H5HL_datablock_load(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *_udata) HGOTO_ERROR(H5E_HEAP, H5E_READERROR, NULL, "unable to read local heap data block") /* Build free list */ - if(H5HL_fl_deserialize(udata->heap, udata->free_block) < 0) + if(H5HL_fl_deserialize(udata->heap) < 0) HGOTO_ERROR(H5E_HEAP, H5E_CANTINIT, NULL, "can't initialize free list") } /* end if */ @@ -707,6 +708,9 @@ H5HL_datablock_flush(H5F_t *f, hid_t dxpl_id, hbool_t destroy, haddr_t addr, if(dblk->cache_info.is_dirty) { H5HL_t *heap = dblk->heap; /* Pointer to the local heap */ + /* Update the free block value from the free list */ + heap->free_block = heap->freelist ? heap->freelist->offset : H5HL_FREE_NULL; + /* Serialize the free list into the heap data's image */ H5HL_fl_serialize(heap); diff --git a/src/H5HLpkg.h b/src/H5HLpkg.h index b7e0ece..bf9be2c 100644 --- a/src/H5HLpkg.h +++ b/src/H5HLpkg.h @@ -90,18 +90,19 @@ struct H5HL_t { size_t sizeof_size; /* Size of file sizes */ size_t sizeof_addr; /* Size of file addresses */ hbool_t single_cache_obj; /* Indicate if the heap is a single object in the cache */ + H5HL_free_t *freelist; /*the free list */ /* Prefix-specific fields */ H5HL_prfx_t *prfx; /* The prefix object for the heap */ haddr_t prfx_addr; /* address of heap prefix */ size_t prfx_size; /* size of heap prefix */ + hsize_t free_block; /* Address of first free block */ /* Data block-specific fields */ H5HL_dblk_t *dblk; /* The data block object for the heap */ haddr_t dblk_addr; /* address of data block */ size_t dblk_size; /* size of heap data block on disk and in mem */ uint8_t *dblk_image; /* The data block image */ - H5HL_free_t *freelist; /*the free list */ }; /* Struct for heap data block */ @@ -127,15 +128,12 @@ typedef struct H5HL_cache_prfx_ud_t { size_t sizeof_prfx; /* Size of heap prefix */ /* Upwards */ - hbool_t loaded; /* Whether prefix was loaded from file */ - hsize_t free_block; /* First free block in heap */ } H5HL_cache_prfx_ud_t; /* Callback information for loading local heap data block from disk */ typedef struct H5HL_cache_dblk_ud_t { /* Downwards */ H5HL_t *heap; /* Local heap */ - hsize_t free_block; /* First free block in heap */ /* Upwards */ hbool_t loaded; /* Whether data block was loaded from file */ diff --git a/test/h5test.c b/test/h5test.c index f8c3d9c..e5e798e 100644 --- a/test/h5test.c +++ b/test/h5test.c @@ -1116,7 +1116,7 @@ getenv_all(MPI_Comm comm, int root, const char* name) *------------------------------------------------------------------------- */ hid_t -h5_make_local_copy(char *origfilename, char *local_copy_name) +h5_make_local_copy(const char *origfilename, const char *local_copy_name) { int fd_old = (-1), fd_new = (-1); /* File descriptors for copying data */ ssize_t nread; /* Number of bytes read in */ diff --git a/test/h5test.h b/test/h5test.h index 5d3fb17..f36240e 100644 --- a/test/h5test.h +++ b/test/h5test.h @@ -150,7 +150,7 @@ H5TEST_DLL void h5_reset(void); H5TEST_DLL void h5_show_hostname(void); H5TEST_DLL h5_stat_size_t h5_get_file_size(const char *filename, hid_t fapl); H5TEST_DLL int print_func(const char *format, ...); -H5TEST_DLL int h5_make_local_copy(char *origfilename, char *local_copy_name); +H5TEST_DLL int h5_make_local_copy(const char *origfilename, const char *local_copy_name); H5TEST_DLL herr_t h5_verify_cached_stabs(const char *base_name[], hid_t fapl); /* Routines for operating on the list of tests (for the "all in one" tests) */ diff --git a/test/tmisc.c b/test/tmisc.c index b7ed202..cb61eaa 100644 --- a/test/tmisc.c +++ b/test/tmisc.c @@ -239,6 +239,7 @@ unsigned m13_rdata[MISC13_DIM1][MISC13_DIM2]; /* Data read from dataset #define MISC20_SPACE2_DIM0 8 #define MISC20_SPACE2_DIM1 4 +#ifdef H5_HAVE_FILTER_SZIP /* Definitions for misc. test #21 */ #define MISC21_FILE "tmisc21.h5" #define MISC21_DSET_NAME "Dataset" @@ -256,6 +257,7 @@ unsigned m13_rdata[MISC13_DIM1][MISC13_DIM2]; /* Data read from dataset #define MISC22_CHUNK_DIM1 512 #define MISC22_SPACE_DIM0 639 #define MISC22_SPACE_DIM1 1308 +#endif /* H5_HAVE_FILTER_SZIP */ /* Definitions for misc. test #23 */ #define MISC23_FILE "tmisc23.h5" @@ -311,6 +313,9 @@ unsigned m13_rdata[MISC13_DIM1][MISC13_DIM2]; /* Data read from dataset #define MISC29_COPY_FILE "tmisc29.h5" #define MISC29_DSETNAME "dset2" +/* Definitions for misc. test #30 */ +#define MISC30_FILE "tmisc30.h5" + /**************************************************************** ** ** test_misc1(): test unlinking a dataset from a group and immediately @@ -3529,7 +3534,7 @@ test_misc20(void) and encoder is available. EIP 2004/8/04 */ -#if defined H5_HAVE_FILTER_SZIP +#ifdef H5_HAVE_FILTER_SZIP /**************************************************************** ** @@ -5113,6 +5118,87 @@ test_misc29(void) CHECK(ret, FAIL, "H5Fclose"); } /* end test_misc29() */ + +static int +test_misc30_get_info_cb(hid_t loc_id, const char *name, const H5L_info_t UNUSED *info, + void UNUSED *op_data) +{ + H5O_info_t object_info; + + return H5Oget_info_by_name(loc_id, name, &object_info, H5P_DEFAULT); +} + +static int +test_misc30_get_info(hid_t loc_id) +{ + return H5Literate(loc_id, H5_INDEX_NAME, H5_ITER_INC, NULL, test_misc30_get_info_cb, NULL); +} + + +/**************************************************************** +** +** test_misc30(): Exercise local heap code that loads prefix +** separately from data block, causing the free +** block information to get lost. +** +****************************************************************/ +static void +test_misc30(void) +{ + hsize_t file_size[] = {0, 0}; /* Sizes of file created */ + hbool_t get_info; /* Whether to perform the get info call */ + + /* Output message about test being performed */ + MESSAGE(5, ("Local heap dropping free block info\n")); + + for(get_info = FALSE; get_info <= TRUE; get_info++) { + hid_t fid; /* File ID */ + hid_t gid; /* Group ID */ + int i; /* Local index counter */ + herr_t ret; /* Generic return value */ + + fid = H5Fcreate(MISC30_FILE, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + CHECK(fid, FAIL, "H5Fcreate"); + gid = H5Gcreate2(fid, "/g0", H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); + CHECK(gid, FAIL, "H5Gcreate2"); + + ret = H5Gclose(gid); + CHECK(ret, FAIL, "H5Gclose"); + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); + + for(i = 0; i < 20; i++) { + char gname[32]; + + fid = H5Fopen(MISC30_FILE, H5F_ACC_RDWR, H5P_DEFAULT); + CHECK(fid, FAIL, "H5Fopen"); + + if(get_info) { + ret = test_misc30_get_info(fid); + CHECK(ret, FAIL, "test_misc30_get_info"); + } + + sprintf(gname, "/g0/group%d", i); + gid = H5Gcreate2(fid, gname, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); + CHECK(gid, FAIL, "H5Gcreate2"); + + ret = H5Gclose(gid); + CHECK(ret, FAIL, "H5Gclose"); + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); + } + + fid = H5Fopen(MISC30_FILE, H5F_ACC_RDONLY, H5P_DEFAULT); + CHECK(fid, FAIL, "H5Fopen"); + ret = H5Fget_filesize(fid, &file_size[get_info]); + CHECK(fid, FAIL, "H5Fget_filesize"); + ret = H5Fclose(fid); + CHECK(ret, FAIL, "H5Fclose"); + } + + VERIFY(file_size[0], file_size[1], "test_misc30"); +} /* end test_misc30() */ + /**************************************************************** ** ** test_misc(): Main misc. test routine. @@ -5144,7 +5230,7 @@ test_misc(void) test_misc18(); /* Test new object header information in H5O_info_t struct */ test_misc19(); /* Test incrementing & decrementing ref count on IDs */ test_misc20(); /* Test problems with truncated dimensions in version 2 of storage layout message */ -#if defined H5_HAVE_FILTER_SZIP +#ifdef H5_HAVE_FILTER_SZIP test_misc21(); /* Test that "late" allocation time is treated the same as "incremental", for chunked datasets w/a filters */ test_misc22(); /* check szip bits per pixel */ #endif /* H5_HAVE_FILTER_SZIP */ @@ -5157,6 +5243,7 @@ test_misc(void) test_misc27(); /* Test opening file with object that has bad # of object header messages */ test_misc28(); /* Test that chunks are cached appropriately */ test_misc29(); /* Test that speculative metadata reads are handled correctly */ + test_misc30(); /* Exercise local heap loading bug where free lists were getting dropped */ } /* test_misc() */ @@ -5201,7 +5288,7 @@ cleanup_misc(void) HDremove(MISC18_FILE); HDremove(MISC19_FILE); HDremove(MISC20_FILE); -#if defined H5_HAVE_FILTER_SZIP +#ifdef H5_HAVE_FILTER_SZIP HDremove(MISC21_FILE); HDremove(MISC22_FILE); #endif /* H5_HAVE_FILTER_SZIP */ @@ -5212,5 +5299,6 @@ cleanup_misc(void) HDremove(MISC26_FILE); HDremove(MISC28_FILE); HDremove(MISC29_COPY_FILE); + HDremove(MISC30_FILE); } -- cgit v0.12