diff options
author | David Young <dyoung@hdfgroup.org> | 2020-06-12 16:24:27 (GMT) |
---|---|---|
committer | David Young <dyoung@hdfgroup.org> | 2020-06-12 16:24:27 (GMT) |
commit | 625ef85fe56d8265989e8c1ed32331064012a068 (patch) | |
tree | 814195dc9c8e3620e9a85adc6e0e4e7569563485 | |
parent | ff43cd3631eaa13886933db3030728fc374c7123 (diff) | |
download | hdf5-625ef85fe56d8265989e8c1ed32331064012a068.zip hdf5-625ef85fe56d8265989e8c1ed32331064012a068.tar.gz hdf5-625ef85fe56d8265989e8c1ed32331064012a068.tar.bz2 |
Avoid leaving a v1 B-tree used as a chunk index in a bad state
that makes assertions fail.
Add an optional `close` method to the `H5D_chunk_ops_t`, and use that to
release "holds" on metadata cache (MDC) entries.
For extensible arrays and v2 B-trees, use the existing `dest`(roy)
method to implement `close`. For v1 B-trees and other chunk indices,
don't provide `close`: we cannot safely close the v1 B-tree index, and
the other indices don't have a meaningful presence in the MDC.
Revert my first attempt at making v1 B-tree chunk indices closeable
with `dest`.
Put my comment about the stopgap fix for VFD SWMR at the right place
in src/H5Dchunk.c.
-rw-r--r-- | src/H5Dbtree.c | 16 | ||||
-rw-r--r-- | src/H5Dbtree2.c | 3 | ||||
-rw-r--r-- | src/H5Dchunk.c | 42 | ||||
-rw-r--r-- | src/H5Dearray.c | 3 | ||||
-rw-r--r-- | src/H5Dfarray.c | 3 | ||||
-rw-r--r-- | src/H5Dnone.c | 3 | ||||
-rw-r--r-- | src/H5Dpkg.h | 5 | ||||
-rw-r--r-- | src/H5Dsingle.c | 3 |
8 files changed, 45 insertions, 33 deletions
diff --git a/src/H5Dbtree.c b/src/H5Dbtree.c index 996c89e..7741e99 100644 --- a/src/H5Dbtree.c +++ b/src/H5Dbtree.c @@ -166,7 +166,8 @@ const H5D_chunk_ops_t H5D_COPS_BTREE[1] = {{ H5D__btree_idx_size, /* size */ H5D__btree_idx_reset, /* reset */ H5D__btree_idx_dump, /* dump */ - H5D__btree_idx_dest /* destroy */ + H5D__btree_idx_dest, /* destroy */ + NULL /* close */ }}; @@ -1234,8 +1235,8 @@ H5D__btree_idx_delete(const H5D_chk_idx_info_t *idx_info) /* Release the shared B-tree page */ if(NULL == tmp_storage.u.btree.shared) - ; /* do nothing */ - else if(H5UC_DEC(tmp_storage.u.btree.shared) < 0) + HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "ref-counted page nil") + if(H5UC_DEC(tmp_storage.u.btree.shared) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "unable to decrement ref-counted page") } /* end if */ @@ -1454,12 +1455,10 @@ H5D__btree_idx_dest(const H5D_chk_idx_info_t *idx_info) /* Free the raw B-tree node buffer */ if(NULL == idx_info->storage->u.btree.shared) - HGOTO_DONE(SUCCEED); + HGOTO_ERROR(H5E_IO, H5E_CANTFREE, FAIL, "ref-counted page nil") if(H5UC_DEC(idx_info->storage->u.btree.shared) < 0) HGOTO_ERROR(H5E_IO, H5E_CANTFREE, FAIL, "unable to decrement ref-counted page") - idx_info->storage->u.btree.shared = NULL; - done: FUNC_LEAVE_NOAPI(ret_value) } /* end H5D__btree_idx_dest() */ @@ -1519,8 +1518,9 @@ done: /* Free the raw B-tree node buffer */ if(NULL == storage.u.btree.shared) HDONE_ERROR(H5E_IO, H5E_CANTFREE, FAIL, "ref-counted shared info nil") - else if(H5UC_DEC(storage.u.btree.shared) < 0) - HDONE_ERROR(H5E_IO, H5E_CANTFREE, FAIL, "unable to decrement ref-counted shared info") + else + if(H5UC_DEC(storage.u.btree.shared) < 0) + HDONE_ERROR(H5E_IO, H5E_CANTFREE, FAIL, "unable to decrement ref-counted shared info") } /* end if */ FUNC_LEAVE_NOAPI(ret_value) diff --git a/src/H5Dbtree2.c b/src/H5Dbtree2.c index ae98654..509a67b 100644 --- a/src/H5Dbtree2.c +++ b/src/H5Dbtree2.c @@ -161,7 +161,8 @@ const H5D_chunk_ops_t H5D_COPS_BT2[1] = {{ H5D__bt2_idx_size, /* size */ H5D__bt2_idx_reset, /* reset */ H5D__bt2_idx_dump, /* dump */ - H5D__bt2_idx_dest /* destroy */ + H5D__bt2_idx_dest, /* destroy */ + H5D__bt2_idx_dest /* close (same as destroy) */ }}; diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c index 556f61b..a1bbb24 100644 --- a/src/H5Dchunk.c +++ b/src/H5Dchunk.c @@ -266,7 +266,7 @@ static herr_t H5D__chunk_flush(H5D_t *dset); static herr_t H5D__chunk_io_term(const H5D_chunk_map_t *fm); static herr_t H5D__chunk_dest(H5D_t *dset); -static herr_t H5D__chunk_index_close(H5D_t *); +static herr_t H5D__chunk_index_close(const H5D_t *, bool); /* Chunk query operation callbacks */ static int H5D__get_num_chunks_cb(const H5D_chunk_rec_t *chunk_rec, void *_udata); @@ -2627,7 +2627,16 @@ H5D__chunk_read(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, chunk_node = H5D_CHUNK_GET_NEXT_NODE(fm, chunk_node); } /* end while */ - if(H5D__chunk_index_close(io_info->dset) < 0) + /* Stopgap fix for VFD SWMR: close the chunk index so that + * pinned/tagged entries in the metadata cache (MDC) are released. + * + * Extensible chunked datasets use extensible arrays or btrees as + * chunk indices. Open chunk indices leave pinned/tagged entries + * in the MDC, and VFD SWMR cannot (yet) evict or refresh those + * entries. After we write refresh routines for those entries, this + * stopgap fix can go away. + */ + if(H5D__chunk_index_close(io_info->dset, false) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "unable to close chunk index") @@ -2888,31 +2897,37 @@ done: FUNC_LEAVE_NOAPI(ret_value) } /* end H5D__chunk_io_term() */ -/* Close the given dataset's chunk index. +/* Close the given dataset's chunk index, or destroy it if `destroy` + * is true. A closed index merely releases holds on metadata cache + * entries; the index can be reopened. Once a dataset's index is + * destroyed, however, the dataset must not try to use the index, again. * * A useful side-effect of closing the chunk index is the release * pinned/tagged metadata cache entries connected with the index. */ static herr_t -H5D__chunk_index_close(H5D_t *dset) +H5D__chunk_index_close(const H5D_t *dset, bool destroy) { H5D_chk_idx_info_t idx_info; H5O_storage_chunk_t *sc = &(dset->shared->layout.storage.u.chunk); herr_t ret_value = SUCCEED; /* Return value */ + H5D_chunk_close_func_t fn; FUNC_ENTER_STATIC H5D_CHUNK_STORAGE_INDEX_CHK(sc); - /* Compose chunked index info struct */ idx_info.f = dset->oloc.file; idx_info.pline = &dset->shared->dcpl_cache.pline; idx_info.layout = &dset->shared->layout.u.chunk; idx_info.storage = sc; - /* Free any index structures */ - if(sc->ops->dest && (sc->ops->dest)(&idx_info) < 0) - HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "unable to release chunk index info") + fn = destroy ? sc->ops->dest : sc->ops->close; + + if (fn != NULL && (*fn)(&idx_info) < 0) { + HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, + "unable to release chunk index info") + } done: FUNC_LEAVE_NOAPI(ret_value) @@ -2960,16 +2975,7 @@ H5D__chunk_dest(H5D_t *dset) rdcc->slot = H5FL_SEQ_FREE(H5D_rdcc_ent_ptr_t, rdcc->slot); HDmemset(rdcc, 0, sizeof(H5D_rdcc_t)); - /* Stopgap fix for VFD SWMR: close the chunk index so that - * pinned/tagged entries in the metadata cache (MDC) are released. - * - * Extensible chunked datasets use extensible arrays or btrees as - * chunk indices. Open chunk indices leave pinned/tagged entries - * in the MDC, and VFD SWMR cannot (yet) evict or refresh those - * entries. After we write refresh routines for those entries, this - * stopgap fix can go away. - */ - if (H5D__chunk_index_close(dset) < 0) { + if (H5D__chunk_index_close(dset, true) < 0) { HGOTO_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "unable to close chunk index") } diff --git a/src/H5Dearray.c b/src/H5Dearray.c index eaa8c46..3a9dcc7 100644 --- a/src/H5Dearray.c +++ b/src/H5Dearray.c @@ -165,7 +165,8 @@ const H5D_chunk_ops_t H5D_COPS_EARRAY[1] = {{ H5D__earray_idx_size, /* size */ H5D__earray_idx_reset, /* reset */ H5D__earray_idx_dump, /* dump */ - H5D__earray_idx_dest /* destroy */ + H5D__earray_idx_dest, /* destroy */ + H5D__earray_idx_dest /* close (same as destroy) */ }}; diff --git a/src/H5Dfarray.c b/src/H5Dfarray.c index cbaa52d..bcdbc92 100644 --- a/src/H5Dfarray.c +++ b/src/H5Dfarray.c @@ -161,7 +161,8 @@ const H5D_chunk_ops_t H5D_COPS_FARRAY[1] = {{ H5D__farray_idx_size, /* size */ H5D__farray_idx_reset, /* reset */ H5D__farray_idx_dump, /* dump */ - H5D__farray_idx_dest /* destroy */ + H5D__farray_idx_dest, /* destroy */ + NULL /* close */ }}; diff --git a/src/H5Dnone.c b/src/H5Dnone.c index 5ba3f5b..654775e 100644 --- a/src/H5Dnone.c +++ b/src/H5Dnone.c @@ -95,7 +95,8 @@ const H5D_chunk_ops_t H5D_COPS_NONE[1] = {{ H5D__none_idx_size, /* size */ H5D__none_idx_reset, /* reset */ H5D__none_idx_dump, /* dump */ - NULL /* dest */ + NULL, /* dest */ + NULL /* close */ }}; diff --git a/src/H5Dpkg.h b/src/H5Dpkg.h index c46e38b6..e74501d 100644 --- a/src/H5Dpkg.h +++ b/src/H5Dpkg.h @@ -308,7 +308,7 @@ typedef herr_t (*H5D_chunk_size_func_t)(const H5D_chk_idx_info_t *idx_info, typedef herr_t (*H5D_chunk_reset_func_t)(H5O_storage_chunk_t *storage, hbool_t reset_addr); typedef herr_t (*H5D_chunk_dump_func_t)(const H5O_storage_chunk_t *storage, FILE *stream); -typedef herr_t (*H5D_chunk_dest_func_t)(const H5D_chk_idx_info_t *idx_info); +typedef herr_t (*H5D_chunk_close_func_t)(const H5D_chk_idx_info_t *idx_info); /* Typedef for grouping chunk I/O routines */ typedef struct H5D_chunk_ops_t { @@ -327,7 +327,8 @@ typedef struct H5D_chunk_ops_t { H5D_chunk_size_func_t size; /* Routine to get size of indexing information */ H5D_chunk_reset_func_t reset; /* Routine to reset indexing information */ H5D_chunk_dump_func_t dump; /* Routine to dump indexing information */ - H5D_chunk_dest_func_t dest; /* Routine to destroy indexing information in memory */ + H5D_chunk_close_func_t dest; /* Routine to destroy indexing information in memory */ + H5D_chunk_close_func_t close; /* Routine to destroy indexing information in memory */ } H5D_chunk_ops_t; /* Structure holding information about a chunk's selection for mapping */ diff --git a/src/H5Dsingle.c b/src/H5Dsingle.c index 33274bb..3fa9bc2 100644 --- a/src/H5Dsingle.c +++ b/src/H5Dsingle.c @@ -97,7 +97,8 @@ const H5D_chunk_ops_t H5D_COPS_SINGLE[1] = {{ H5D__single_idx_size, /* size */ H5D__single_idx_reset, /* reset */ H5D__single_idx_dump, /* dump */ - NULL /* destroy */ + NULL, /* destroy */ + NULL /* close */ }}; |