summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorQuincey Koziol <koziol@hdfgroup.org>2009-10-09 04:09:34 (GMT)
committerQuincey Koziol <koziol@hdfgroup.org>2009-10-09 04:09:34 (GMT)
commitdcbf871fed9a07211458188fb52174c72d93f759 (patch)
treede1400b1b610853017fc0548c1a95a5ad2c0a836
parent13e5632d32650ff53190bdc37777277d0ae2913d (diff)
downloadhdf5-dcbf871fed9a07211458188fb52174c72d93f759.zip
hdf5-dcbf871fed9a07211458188fb52174c72d93f759.tar.gz
hdf5-dcbf871fed9a07211458188fb52174c72d93f759.tar.bz2
[svn-r17624] Description:
Don't allow reads to change or add to the metadata accumulator, since they might be speculative and could bring raw data into the metadata accumulator. Tested on: FreeBSD/32 6.3 (duty) in debug mode FreeBSD/64 6.3 (liberty) w/C++ & FORTRAN, in debug mode Linux/32 2.6 (jam) w/PGI compilers, w/default API=1.8.x, w/C++ & FORTRAN, w/threadsafe, in debug mode Linux/64-amd64 2.6 (smirom) w/Intel compilers, w/default API=1.6.x, w/C++ & FORTRAN, in production mode Solaris/32 2.10 (linew) w/deprecated symbols disabled, w/C++ & FORTRAN, w/szip filter, in production mode Linux/64-ia64 2.6 (cobalt) w/Intel compilers, w/C++ & FORTRAN, in production mode Linux/64-ia64 2.4 (tg-login3) w/parallel, w/FORTRAN, in debug mode Linux/64-amd64 2.6 (abe) w/parallel, w/FORTRAN, in production mode Mac OS X/32 10.6.1 (amazon) in debug mode Mac OS X/32 10.6.1 (amazon) w/C++ & FORTRAN, w/threadsafe, in production mode
-rw-r--r--MANIFEST2
-rw-r--r--src/H5Faccum.c70
-rw-r--r--test/Makefile.am2
-rw-r--r--test/Makefile.in17
-rw-r--r--test/gen_specmetaread.c96
-rw-r--r--test/specmetaread.h5bin0 -> 1752 bytes
-rw-r--r--test/tmisc.c67
7 files changed, 187 insertions, 67 deletions
diff --git a/MANIFEST b/MANIFEST
index 16714aa..1c69313 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -851,6 +851,7 @@
./test/gen_old_group.c _DO_NOT_DISTRIBUTE_
./test/gen_old_layout.c _DO_NOT_DISTRIBUTE_
./test/gen_old_mtime.c _DO_NOT_DISTRIBUTE_
+./test/gen_specmetaread.c _DO_NOT_DISTRIBUTE_
./test/gen_udlinks.c _DO_NOT_DISTRIBUTE_
./test/getname.c
./test/gheap.c
@@ -876,6 +877,7 @@
./test/pool.c
./test/set_extent.c
./test/space_overflow.c _DO_NOT_DISTRIBUTE_
+./test/specmetaread.h5
./test/stab.c
./test/tarray.c
./test/tarrold.h5
diff --git a/src/H5Faccum.c b/src/H5Faccum.c
index d8b9820..e525a08 100644
--- a/src/H5Faccum.c
+++ b/src/H5Faccum.c
@@ -99,6 +99,10 @@ H5FL_BLK_DEFINE_STATIC(meta_accum);
* Purpose: Attempts to read some data from the metadata accumulator for
* a file into a buffer.
*
+ * Note: We can't change (or add to) the metadata accumulator, because
+ * this might be a speculative read and could possibly read raw
+ * data into the metadata accumulator.
+ *
* Return: Non-negative on success/Negative on failure
*
* Programmer: Quincey Koziol
@@ -185,69 +189,11 @@ H5F_accum_read(const H5F_t *f, hid_t dxpl_id, H5FD_mem_t type, haddr_t addr,
/* Make certain we've read it all */
HDassert(size == 0);
} /* end if */
- /* Current read doesn't overlap with metadata accumulator, read it into accumulator */
+ /* Current read doesn't overlap with metadata accumulator, read it from file */
else {
- /* Only update the metadata accumulator if it is not dirty or if
- * we are allowed to write the accumulator out during reads (when
- * it is dirty)
- */
- if(f->shared->feature_flags & H5FD_FEAT_ACCUMULATE_METADATA_READ || !f->shared->accum.dirty) {
- /* Flush current contents, if dirty */
- if(f->shared->accum.dirty) {
- if(H5FD_write(f->shared->lf, dxpl_id, H5FD_MEM_DEFAULT, f->shared->accum.loc, f->shared->accum.size, f->shared->accum.buf) < 0)
- HGOTO_ERROR(H5E_IO, H5E_WRITEERROR, FAIL, "driver write request failed")
-
- /* Reset accumulator dirty flag */
- f->shared->accum.dirty = FALSE;
- } /* end if */
-
- /* Cache the new piece of metadata */
- /* Check if we need to resize the buffer */
- if(size > f->shared->accum.alloc_size) {
- size_t new_size; /* New size of accumulator */
-
- /* Adjust the buffer size to be a power of 2 that is large enough to hold data */
- new_size = (size_t)1 << (1 + H5V_log2_gen((uint64_t)(size - 1)));
-
- /* Grow the metadata accumulator buffer */
- if(NULL == (f->shared->accum.buf = H5FL_BLK_REALLOC(meta_accum, f->shared->accum.buf, new_size)))
- HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to allocate metadata accumulator buffer")
-
- /* Note the new buffer size */
- f->shared->accum.alloc_size = new_size;
- } /* end if */
- else {
- /* Check if we should shrink the accumulator buffer */
- if(size < (f->shared->accum.alloc_size / H5F_ACCUM_THROTTLE) &&
- f->shared->accum.alloc_size > H5F_ACCUM_THRESHOLD) {
- size_t new_size = (f->shared->accum.alloc_size / H5F_ACCUM_THROTTLE); /* New size of accumulator buffer */
-
- /* Shrink the accumulator buffer */
- if(NULL == (f->shared->accum.buf = H5FL_BLK_REALLOC(meta_accum, f->shared->accum.buf, new_size)))
- HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to allocate metadata accumulator buffer")
-
- /* Note the new buffer size */
- f->shared->accum.alloc_size = new_size;
- } /* end if */
- } /* end else */
-
- /* Update accumulator information */
- f->shared->accum.loc = addr;
- f->shared->accum.size = size;
- f->shared->accum.dirty = FALSE;
-
- /* Read into accumulator */
- if(H5FD_read(f->shared->lf, dxpl_id, H5FD_MEM_DEFAULT, f->shared->accum.loc, f->shared->accum.size, f->shared->accum.buf) < 0)
- HGOTO_ERROR(H5E_IO, H5E_READERROR, FAIL, "driver read request failed")
-
- /* Copy into buffer */
- HDmemcpy(buf, f->shared->accum.buf, size);
- } /* end if */
- else {
- /* Dispatch to driver */
- if(H5FD_read(f->shared->lf, dxpl_id, type, addr, size, buf) < 0)
- HGOTO_ERROR(H5E_IO, H5E_READERROR, FAIL, "driver read request failed")
- } /* end else */
+ /* Dispatch to driver */
+ if(H5FD_read(f->shared->lf, dxpl_id, type, addr, size, buf) < 0)
+ HGOTO_ERROR(H5E_IO, H5E_READERROR, FAIL, "driver read request failed")
} /* end else */
/* Indicate success */
diff --git a/test/Makefile.am b/test/Makefile.am
index d233f96..ec1eed7 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -62,7 +62,7 @@ check_PROGRAMS=$(TEST_PROG) error_test err_compat tcheck_version testmeta
# so do not appear in this list.
BUILD_ALL_PROGS=gen_bad_ohdr gen_bogus gen_cross gen_deflate gen_filters gen_new_array \
gen_new_fill gen_new_group gen_new_mtime gen_new_super gen_noencoder \
- gen_nullspace gen_udlinks space_overflow gen_filespace
+ gen_nullspace gen_udlinks space_overflow gen_filespace gen_specmetaread
if BUILD_ALL_CONDITIONAL
noinst_PROGRAMS=$(BUILD_ALL_PROGS)
diff --git a/test/Makefile.in b/test/Makefile.in
index e1ed182..3f063e9 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -92,7 +92,8 @@ am__EXEEXT_2 = gen_bad_ohdr$(EXEEXT) gen_bogus$(EXEEXT) \
gen_new_group$(EXEEXT) gen_new_mtime$(EXEEXT) \
gen_new_super$(EXEEXT) gen_noencoder$(EXEEXT) \
gen_nullspace$(EXEEXT) gen_udlinks$(EXEEXT) \
- space_overflow$(EXEEXT) gen_filespace$(EXEEXT)
+ space_overflow$(EXEEXT) gen_filespace$(EXEEXT) \
+ gen_specmetaread$(EXEEXT)
PROGRAMS = $(noinst_PROGRAMS)
app_ref_SOURCES = app_ref.c
app_ref_OBJECTS = app_ref.$(OBJEXT)
@@ -246,6 +247,10 @@ gen_nullspace_SOURCES = gen_nullspace.c
gen_nullspace_OBJECTS = gen_nullspace.$(OBJEXT)
gen_nullspace_LDADD = $(LDADD)
gen_nullspace_DEPENDENCIES = libh5test.la $(LIBHDF5)
+gen_specmetaread = gen_specmetaread.c
+gen_specmetaread_OBJECTS = gen_specmetaread.$(OBJEXT)
+gen_specmetaread_LDADD = $(LDADD)
+gen_specmetaread_DEPENDENCIES = libh5test.la $(LIBHDF5)
gen_udlinks_SOURCES = gen_udlinks.c
gen_udlinks_OBJECTS = gen_udlinks.$(OBJEXT)
gen_udlinks_LDADD = $(LDADD)
@@ -377,7 +382,7 @@ SOURCES = $(libh5test_la_SOURCES) app_ref.c big.c bittests.c btree2.c \
mount.c mtime.c ntypes.c objcopy.c ohdr.c pool.c reserved.c \
set_extent.c space_overflow.c stab.c tcheck_version.c \
$(testhdf5_SOURCES) testmeta.c $(ttsafe_SOURCES) unlink.c \
- vfd.c
+ vfd.c gen_specmetaread.c
DIST_SOURCES = $(libh5test_la_SOURCES) app_ref.c big.c bittests.c \
btree2.c cache.c cache_api.c cmpd_dset.c cross_read.c dangle.c \
dsets.c dt_arith.c dtransform.c dtypes.c earray.c enum.c \
@@ -390,7 +395,7 @@ DIST_SOURCES = $(libh5test_la_SOURCES) app_ref.c big.c bittests.c \
istore.c lheap.c links.c mf.c mount.c mtime.c ntypes.c \
objcopy.c ohdr.c pool.c reserved.c set_extent.c \
space_overflow.c stab.c tcheck_version.c $(testhdf5_SOURCES) \
- testmeta.c $(ttsafe_SOURCES) unlink.c vfd.c
+ testmeta.c $(ttsafe_SOURCES) unlink.c vfd.c gen_specmetaread.c
ETAGS = etags
CTAGS = ctags
am__tty_colors = \
@@ -712,7 +717,7 @@ TEST_PROG = testhdf5 lheap ohdr stab gheap cache cache_api \
# so do not appear in this list.
BUILD_ALL_PROGS = gen_bad_ohdr gen_bogus gen_cross gen_deflate gen_filters gen_new_array \
gen_new_fill gen_new_group gen_new_mtime gen_new_super gen_noencoder \
- gen_nullspace gen_udlinks space_overflow gen_filespace
+ gen_nullspace gen_udlinks space_overflow gen_filespace gen_specmetaread
# The libh5test library provides common support code for the tests.
@@ -937,6 +942,9 @@ gen_noencoder$(EXEEXT): $(gen_noencoder_OBJECTS) $(gen_noencoder_DEPENDENCIES)
gen_nullspace$(EXEEXT): $(gen_nullspace_OBJECTS) $(gen_nullspace_DEPENDENCIES)
@rm -f gen_nullspace$(EXEEXT)
$(LINK) $(gen_nullspace_OBJECTS) $(gen_nullspace_LDADD) $(LIBS)
+gen_specmetaread$(EXEEXT): $(gen_specmetaread_OBJECTS) $(gen_specmetaread_DEPENDENCIES)
+ @rm -f gen_specmetaread$(EXEEXT)
+ $(LINK) $(gen_specmetaread_OBJECTS) $(gen_specmetaread_LDADD) $(LIBS)
gen_udlinks$(EXEEXT): $(gen_udlinks_OBJECTS) $(gen_udlinks_DEPENDENCIES)
@rm -f gen_udlinks$(EXEEXT)
$(LINK) $(gen_udlinks_OBJECTS) $(gen_udlinks_LDADD) $(LIBS)
@@ -1055,6 +1063,7 @@ distclean-compile:
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_new_super.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_noencoder.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_nullspace.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_specmetaread.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gen_udlinks.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/getname.Po@am__quote@
@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/gheap.Po@am__quote@
diff --git a/test/gen_specmetaread.c b/test/gen_specmetaread.c
new file mode 100644
index 0000000..f489119
--- /dev/null
+++ b/test/gen_specmetaread.c
@@ -0,0 +1,96 @@
+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
+ * Copyright by The HDF Group. *
+ * Copyright by the Board of Trustees of the University of Illinois. *
+ * All rights reserved. *
+ * *
+ * This file is part of HDF5. The full HDF5 copyright notice, including *
+ * terms governing use, modification, and redistribution, is contained in *
+ * the files COPYING and Copyright.html. COPYING can be found at the root *
+ * of the source code distribution tree; Copyright.html can be found at the *
+ * root level of an installed copy of the electronic HDF5 document set and *
+ * is linked from the top-level documents page. It can also be found at *
+ * http://hdfgroup.org/HDF5/doc/Copyright.html. If you do not have *
+ * access to either file, you may request a copy from help@hdfgroup.org. *
+ * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
+
+/*
+ * Programmer: Quincey Koziol <koziol@hdfgroup.org>
+ * Thursday, October 8, 2009
+ *
+ * Purpose: Create a file with a dataset who's raw data immediately follows
+ * its object header, so that when the dataset is unlinked from its parent
+ * group, a speculative read of the object header would get the raw data
+ * into the metadata accumulator, "polluting" it.
+ * To build the test file, this program MUST be compiled and linked with
+ * the library on the trunk as of when this file is checked in.
+ */
+
+#include "hdf5.h"
+#include <assert.h>
+
+#define FILENAME "specmetaread.h5"
+#define DIM 10
+
+int
+main(void)
+{
+ hid_t fid;
+ hid_t fapl;
+ hid_t did;
+ hid_t space;
+ hsize_t dim[1] = {DIM};
+ unsigned data[DIM];
+ unsigned u;
+ herr_t ret; /* Generic return value */
+
+ /* Initialize the data */
+ for(u = 0; u < DIM; u++)
+ data[u] = u;
+
+ /* Create a FAPL with the metadata and small data aggregators turned off */
+ fapl = H5Pcreate(H5P_FILE_ACCESS);
+ assert(fapl > 0);
+ ret = H5Pset_meta_block_size(fapl, (hsize_t)0);
+ assert(ret >= 0);
+ ret = H5Pset_small_data_block_size(fapl, (hsize_t)0);
+ assert(ret >= 0);
+
+ /* Create file */
+ fid = H5Fcreate(FILENAME, H5F_ACC_TRUNC, H5P_DEFAULT, fapl);
+ assert(fid > 0);
+
+ /* Close FAPL */
+ ret = H5Pclose(fapl);
+ assert(ret >= 0);
+
+ /* Create dataspace */
+ space = H5Screate_simple(1, dim, NULL);
+ assert(space > 0);
+
+ /* Create dataset #1 */
+ did = H5Dcreate2(fid, "dset1", H5T_NATIVE_UINT, space, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+ assert(did > 0);
+ ret = H5Dwrite(did, H5T_NATIVE_UINT, H5S_ALL, H5S_ALL, H5P_DEFAULT, data);
+ assert(ret >= 0);
+ ret = H5Dclose(did);
+ assert(ret >= 0);
+
+ /* Create dataset #2 */
+ did = H5Dcreate2(fid, "dset2", H5T_NATIVE_UINT, space, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+ assert(did > 0);
+ ret = H5Dwrite(did, H5T_NATIVE_UINT, H5S_ALL, H5S_ALL, H5P_DEFAULT, data);
+ assert(ret >= 0);
+ ret = H5Dclose(did);
+ assert(ret >= 0);
+
+ /* Close dataspace */
+ ret = H5Sclose(space);
+ assert(ret >= 0);
+
+ /* Close file */
+ ret = H5Fclose(fid);
+ assert(ret >= 0);
+
+ return 0;
+}
+
diff --git a/test/specmetaread.h5 b/test/specmetaread.h5
new file mode 100644
index 0000000..50b0e69
--- /dev/null
+++ b/test/specmetaread.h5
Binary files differ
diff --git a/test/tmisc.c b/test/tmisc.c
index 41128a2..21715b4 100644
--- a/test/tmisc.c
+++ b/test/tmisc.c
@@ -32,6 +32,9 @@
#include "testhdf5.h"
#include "H5Dpkg.h" /* Datasets */
+#define NAME_BUF_SIZE 1024
+#define READ_OLD_BUFSIZE 1024
+
/* Definitions for misc. test #1 */
#define MISC1_FILE "tmisc1.h5"
#define MISC1_VAL (13417386) /* 0xccbbaa */
@@ -305,6 +308,11 @@ unsigned m13_rdata[MISC13_DIM1][MISC13_DIM2]; /* Data read from dataset
#define MISC28_SIZE 10
#define MISC28_NSLOTS 10000
+/* Definitions for misc. test #29 */
+#define MISC29_ORIG_FILE "specmetaread.h5"
+#define MISC29_COPY_FILE "tmisc29.h5"
+#define MISC29_DSETNAME "dset2"
+
/****************************************************************
**
** test_misc1(): test unlinking a dataset from a group and immediately
@@ -5106,6 +5114,63 @@ test_misc28(void)
CHECK_I(ret, "H5Pclose");
} /* end test_misc28() */
+
+/****************************************************************
+**
+** test_misc29(): Ensure that speculative metadata reads don't
+** get raw data into the metadata accumulator.
+**
+****************************************************************/
+static void
+test_misc29(void)
+{
+ int fd_old = (-1), fd_new = (-1); /* File descriptors for copying data */
+ char buf[READ_OLD_BUFSIZE]; /* Buffer for copying data */
+ ssize_t nread; /* Number of bytes read in */
+ char *srcdir = HDgetenv("srcdir"); /* where the src code is located */
+ char filename[NAME_BUF_SIZE] = ""; /* old test file name */
+ hid_t fid; /* File ID */
+ herr_t ret; /* Generic return value */
+
+ /* Output message about test being performed */
+ MESSAGE(5, ("Speculative metadata reads\n"));
+
+ /* Generate correct name for test file by prepending the source path */
+ if(srcdir && ((HDstrlen(srcdir) + HDstrlen(MISC29_ORIG_FILE) + 1) < sizeof(filename))) {
+ HDstrcpy(filename, srcdir);
+ HDstrcat(filename, "/");
+ } /* end if */
+ HDstrcat(filename, MISC29_ORIG_FILE);
+
+ /* Copy old file into temporary file */
+ fd_old = HDopen(filename, O_RDONLY, 0666);
+ CHECK(fd_old, -1, "HDopen");
+ fd_new = HDopen(MISC29_COPY_FILE, O_RDWR|O_CREAT|O_TRUNC, 0666);
+ CHECK(fd_new, -1, "HDopen");
+
+ /* Copy data */
+ while((nread = HDread(fd_old, buf, (size_t)READ_OLD_BUFSIZE)) > 0)
+ HDwrite(fd_new, buf, (size_t)nread);
+
+ /* Close files */
+ ret = HDclose(fd_old);
+ CHECK(ret, -1, "HDclose");
+ ret = HDclose(fd_new);
+ CHECK(ret, -1, "HDclose");
+
+ /* Open the copied file */
+ fid = H5Fopen(MISC29_COPY_FILE, H5F_ACC_RDWR, H5P_DEFAULT);
+ CHECK(fid, FAIL, "H5Fopen");
+
+ /* Delete the last dataset */
+ ret = H5Ldelete(fid, MISC29_DSETNAME, H5P_DEFAULT);
+ CHECK(ret, FAIL, "H5Ldelete");
+
+ /* Close the file */
+ ret = H5Fclose(fid);
+ CHECK(ret, FAIL, "H5Fclose");
+} /* end test_misc29() */
+
/****************************************************************
**
** test_misc(): Main misc. test routine.
@@ -5149,6 +5214,7 @@ test_misc(void)
test_misc26(); /* Test closing property lists with long filter pipelines */
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_misc() */
@@ -5203,5 +5269,6 @@ cleanup_misc(void)
HDremove(MISC25C_FILE);
HDremove(MISC26_FILE);
HDremove(MISC28_FILE);
+ HDremove(MISC29_COPY_FILE);
}