Omnibus checkin for several relatively minor modifications:

1) Added code test/page_buffer.c to verify that page buffering is
   disabled in parallel builds.

2) Added code to test/cache_image.c to verify correct interaction
   between evict on close and cache image -- in particular management
   of a file containing a cache image containing dirty metadata that
   has been opened R/O.  Also fix for the bug exposed.

3) Added code to testpar/t_cache_image.c to verify expected procedure
   for reading cache images, and also supporting stats collection code
   needed for the test.

4) Repair of an overactive sanity check in H5C__reconstruct_cache_contents().

5) Other minor tidies in passing.

Tested serial and parallel, debug and production on Jelly.

4 files changed, 116 insertions, 29 deletions

diff --git a/src/H5Cdbg.c b/src/H5Cdbg.c
index a955eaf..0a98406 100644
--- a/src/H5Cdbg.c
+++ b/src/H5Cdbg.c
@@ -738,9 +738,10 @@ H5C_stats(H5C_t * cache_ptr,
               (long long)(cache_ptr->index_scan_restarts));
 
     HDfprintf(stdout,
-	      "%s  cache image creations/loads/size   = %d / %d / %Hu\n",
+	    "%s  cache image creations/reads/loads/size = %d / %d /%d / %Hu\n",
               cache_ptr->prefix,
               cache_ptr->images_created,
+              cache_ptr->images_read,
               cache_ptr->images_loaded,
               cache_ptr->last_image_size);
 
@@ -993,6 +994,7 @@ H5C_stats__reset(H5C_t H5_ATTR_UNUSED * cache_ptr)
     cache_ptr->index_scan_restarts              = 0;
 
     cache_ptr->images_created           = 0;
+    cache_ptr->images_read              = 0;
     cache_ptr->images_loaded            = 0;
     cache_ptr->last_image_size          = (hsize_t)0;
 
diff --git a/src/H5Cimage.c b/src/H5Cimage.c
index fc58dac..d60ee05 100644
--- a/src/H5Cimage.c
+++ b/src/H5Cimage.c
@@ -122,7 +122,7 @@ static H5C_cache_entry_t *H5C__reconstruct_cache_entry(const H5F_t *f,
     H5C_t *cache_ptr, const uint8_t **buf);
 static herr_t H5C__write_cache_image_superblock_msg(H5F_t *f, hid_t dxpl_id, 
     hbool_t create);
-static herr_t H5C__read_cache_image(H5F_t * f, hid_t dxpl_id, const H5C_t *cache_ptr);
+static herr_t H5C__read_cache_image(H5F_t * f, hid_t dxpl_id, H5C_t *cache_ptr);
 static herr_t H5C__write_cache_image(H5F_t *f, hid_t dxpl_id, const H5C_t *cache_ptr);
 static herr_t H5C__construct_cache_image_buffer(H5F_t *f, H5C_t *cache_ptr);
 static herr_t H5C__free_image_entries_array(H5C_t *cache_ptr);
@@ -1035,7 +1035,7 @@ done:
  *-------------------------------------------------------------------------
  */
 herr_t
-H5C__read_cache_image(H5F_t *f, hid_t dxpl_id, const H5C_t *cache_ptr)
+H5C__read_cache_image(H5F_t *f, hid_t dxpl_id, H5C_t *cache_ptr)
 {
     herr_t              ret_value = SUCCEED;    /* Return value */
 
@@ -1053,31 +1053,53 @@ H5C__read_cache_image(H5F_t *f, hid_t dxpl_id, const H5C_t *cache_ptr)
     H5AC_aux_t *aux_ptr = (H5AC_aux_t *)cache_ptr->aux_ptr;
     int mpi_result;
 
-    if((NULL == aux_ptr) || (aux_ptr->mpi_rank == 0)) {
-	HDassert((NULL == aux_ptr) || (aux_ptr->magic == H5AC__H5AC_AUX_T_MAGIC));
+    if ( ( NULL == aux_ptr ) || ( aux_ptr->mpi_rank == 0 ) ) {
+
+	HDassert((NULL == aux_ptr) || 
+                 (aux_ptr->magic == H5AC__H5AC_AUX_T_MAGIC));
 #endif /* H5_HAVE_PARALLEL */
 
 	/* Read the buffer (if serial access, or rank 0 of parallel access) */
-        if(H5F_block_read(f, H5FD_MEM_SUPER, cache_ptr->image_addr, cache_ptr->image_len, dxpl_id, cache_ptr->image_buffer) < 0)
-            HGOTO_ERROR(H5E_CACHE, H5E_READERROR, FAIL, "Can't read metadata cache image block")
+        if ( H5F_block_read(f, H5FD_MEM_SUPER, cache_ptr->image_addr, 
+                            cache_ptr->image_len, dxpl_id, 
+                            cache_ptr->image_buffer) < 0)
+
+            HGOTO_ERROR(H5E_CACHE, H5E_READERROR, FAIL, \
+                        "Can't read metadata cache image block")
+
+        H5C__UPDATE_STATS_FOR_CACHE_IMAGE_READ(cache_ptr)
 
 #ifdef H5_HAVE_PARALLEL
-	if(aux_ptr) {
+	if ( aux_ptr ) {
+
 	    /* Broadcast cache image */
-            if(MPI_SUCCESS != (mpi_result = MPI_Bcast(cache_ptr->image_buffer, (int)cache_ptr->image_len, MPI_BYTE, 0, aux_ptr->mpi_comm)))
+            if ( MPI_SUCCESS != 
+                 (mpi_result = MPI_Bcast(cache_ptr->image_buffer, 
+                                         (int)cache_ptr->image_len, MPI_BYTE, 
+                                         0, aux_ptr->mpi_comm)) )
+
                 HMPI_GOTO_ERROR(FAIL, "MPI_Bcast failed", mpi_result)
+
         } /* end if */
     } /* end if */
-    else if(aux_ptr) {
+    else if ( aux_ptr ) {
+
         /* Retrieve the contents of the metadata cache image from process 0 */
-        if(MPI_SUCCESS != (mpi_result = MPI_Bcast(cache_ptr->image_buffer, (int)cache_ptr->image_len, MPI_BYTE, 0, aux_ptr->mpi_comm)))
-            HMPI_GOTO_ERROR(FAIL, "can't receive cache image MPI_Bcast", mpi_result)
+        if ( MPI_SUCCESS != 
+             (mpi_result = MPI_Bcast(cache_ptr->image_buffer, 
+                                     (int)cache_ptr->image_len, MPI_BYTE, 
+                                     0, aux_ptr->mpi_comm)) )
+
+            HMPI_GOTO_ERROR(FAIL, "can't receive cache image MPI_Bcast", \
+                            mpi_result)
     } /* end else-if */
 } /* end block */
 #endif /* H5_HAVE_PARALLEL */
 
 done:
+
     FUNC_LEAVE_NOAPI(ret_value)
+
 } /* H5C__read_cache_image() */
 
 
@@ -3203,26 +3225,31 @@ H5C__reconstruct_cache_contents(H5F_t *f, hid_t dxpl_id, H5C_t *cache_ptr)
 
         i = -1;
         entry_ptr = cache_ptr->LRU_head_ptr;
+
         while(entry_ptr != NULL) {
-	    HDassert(entry_ptr->magic == H5C__H5C_CACHE_ENTRY_T_MAGIC);
+
+            HDassert(entry_ptr->magic == H5C__H5C_CACHE_ENTRY_T_MAGIC);
             HDassert(entry_ptr->type != NULL);
 
-	    if(entry_ptr->prefetched) {
-	        HDassert(i <= entry_ptr->lru_rank);
-	        HDassert((entry_ptr->lru_rank <= 2) || 
-                         (entry_ptr->lru_rank == i + 1) ||
-                         (entry_ptr->lru_rank == i + 2));
+            if ( entry_ptr->prefetched ) {
 
-	        if((entry_ptr->lru_rank <= 2) && (entry_ptr->lru_rank == i + 2))
-		    lru_rank_holes++;
+                HDassert(entry_ptr->lru_rank != 0);
+                HDassert((entry_ptr->lru_rank == -1) ||
+                         (entry_ptr->lru_rank > i));
 
-	        i = entry_ptr->lru_rank;
-	    } /* end if */
+                if ( ( entry_ptr->lru_rank > 1 ) && 
+                     ( entry_ptr->lru_rank > i + 1 ) )
+
+                    lru_rank_holes += entry_ptr->lru_rank - (i + 1);
+
+                i = entry_ptr->lru_rank;
+
+            } /* end if */
 
-	    entry_ptr = entry_ptr->next;
+            entry_ptr = entry_ptr->next;
         } /* end while */
 
-	/* Holes of size 1 appear in the LRU ranking due to epoch 
+	/* Holes in the sequences of LRU ranks can appear due to epoch 
          * markers.  They are left in to allow re-insertion of the
          * epoch markers on reconstruction of the cache -- thus 
          * the following sanity check will have to be revised when
diff --git a/src/H5Cpkg.h b/src/H5Cpkg.h
index 69e8145..90a84c4 100644
--- a/src/H5Cpkg.h
+++ b/src/H5Cpkg.h
@@ -686,6 +686,13 @@ if ( ( ( ( (head_ptr) == NULL ) || ( (tail_ptr) == NULL ) ) &&             \
     (cache_ptr)->images_created++;                          \
 }
 
+#define H5C__UPDATE_STATS_FOR_CACHE_IMAGE_READ(cache_ptr)  \
+{                                                          \
+    /* make sure image len is still good */                \
+    HDassert((cache_ptr)->image_len > 0);                  \
+    (cache_ptr)->images_read++;                            \
+}
+
 #define H5C__UPDATE_STATS_FOR_CACHE_IMAGE_LOAD(cache_ptr)  \
 {                                                          \
     /* make sure image len is still good */                \
@@ -931,6 +938,7 @@ if ( ( ( ( (head_ptr) == NULL ) || ( (tail_ptr) == NULL ) ) &&             \
 #define H5C__UPDATE_STATS_FOR_LRU_SCAN_RESTART(cache_ptr)
 #define H5C__UPDATE_STATS_FOR_INDEX_SCAN_RESTART(cache_ptr)
 #define H5C__UPDATE_STATS_FOR_CACHE_IMAGE_CREATE(cache_ptr)
+#define H5C__UPDATE_STATS_FOR_CACHE_IMAGE_READ(cache_ptr)
 #define H5C__UPDATE_STATS_FOR_CACHE_IMAGE_LOAD(cache_ptr)
 #define H5C__UPDATE_STATS_FOR_PREFETCH(cache_ptr, dirty)
 #define H5C__UPDATE_STATS_FOR_PREFETCH_HIT(cache_ptr)
@@ -4549,6 +4557,18 @@ typedef struct H5C_tag_info_t {
  *		Further, since cache images are only created at file 
  *		close, this field should only be set at that time.
  *
+ * images_read: Integer field containing the number of cache images 
+ *              read from file.  Note that reading an image is different
+ *              from loading it -- reading the image means just that,
+ *              while loading the image refers to decoding it and loading
+ *              it into the metadata cache.
+ *
+ *              In the serial case, image_read should always equal 
+ *              images_loaded.  However, in the parallel case, the 
+ *              image should only be read by process 0.  All other 
+ *              processes should receive the cache image via a broadcast
+ *              from process 0.
+ *
  * images_loaded:  Integer field containing the number of cache images
  *		loaded since the last time statistics were reset.
  *
@@ -4864,6 +4884,7 @@ struct H5C_t {
 
     /* Fields for tracking cache image operations */
     int32_t			images_created;
+    int32_t			images_read;
     int32_t			images_loaded;
     hsize_t			last_image_size;
 
diff --git a/src/H5Ctag.c b/src/H5Ctag.c
index a9bcca1..0170ce9 100644
--- a/src/H5Ctag.c
+++ b/src/H5Ctag.c
@@ -58,8 +58,18 @@
 typedef struct {
     H5F_t * f;                          /* File pointer for evicting entry */
     hid_t dxpl_id;                      /* DXPL for evicting entry */
-    hbool_t evicted_entries_last_pass;  /* Flag to indicate that an entry was evicted when iterating over cache */
-    hbool_t pinned_entries_need_evicted;        /* Flag to indicate that a pinned entry was attempted to be evicted */
+    hbool_t evicted_entries_last_pass;  /* Flag to indicate that an entry 
+                                         * was evicted when iterating over 
+                                         * cache 
+                                         */
+    hbool_t pinned_entries_need_evicted;/* Flag to indicate that a pinned 
+                                         * entry was attempted to be evicted 
+                                         */
+    hbool_t skipped_pf_dirty_entries;   /* Flag indicating that one or more 
+                                         * entries marked prefetched_dirty
+                                         * were encountered and not 
+                                         * evicted.
+                                         */
 } H5C_tag_iter_evict_ctx_t;
 
 /* Typedef for tagged entry iterator callback context - expunge tag type metadata */
@@ -470,7 +480,9 @@ H5C__evict_tagged_entries_cb(H5C_cache_entry_t *entry, void *_ctx)
         if(H5C__flush_single_entry(ctx->f, ctx->dxpl_id, entry, H5C__FLUSH_INVALIDATE_FLAG | H5C__FLUSH_CLEAR_ONLY_FLAG | H5C__DEL_FROM_SLIST_ON_DESTROY_FLAG) < 0)
             HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, H5_ITER_ERROR, "Entry eviction failed.")
         ctx->evicted_entries_last_pass = TRUE;
-    } /* end else */
+    } else {
+        ctx->skipped_pf_dirty_entries = TRUE;
+    }
 
 done:
     FUNC_LEAVE_NOAPI(ret_value)
@@ -516,6 +528,7 @@ H5C_evict_tagged_entries(H5F_t * f, hid_t dxpl_id, haddr_t tag, hbool_t match_gl
         /* Reset pinned/evicted tracking flags */
         ctx.pinned_entries_need_evicted = FALSE;
         ctx.evicted_entries_last_pass = FALSE;
+        ctx.skipped_pf_dirty_entries = FALSE;
 
         /* Iterate through entries in the cache */
         if(H5C__iter_tagged_entries(cache, tag, match_global, H5C__evict_tagged_entries_cb, &ctx) < 0)
@@ -524,8 +537,32 @@ H5C_evict_tagged_entries(H5F_t * f, hid_t dxpl_id, haddr_t tag, hbool_t match_gl
         /* Keep doing this until we have stopped evicted entries */
     } while(TRUE == ctx.evicted_entries_last_pass);
 
-    /* Fail if we have finished evicting entries and pinned entries still need evicted */
-    if(ctx.pinned_entries_need_evicted)
+    /* In most cases, fail if we have finished evicting entries and pinned 
+     * entries still need evicted 
+     *
+     * However, things can get strange if the file was opened R/O and 
+     * the file contains a cache image and the cache image contains dirty 
+     * entries.  
+     *
+     * Since the file was opened read only, dirty entries in the cache 
+     * image were marked as clean when they were inserted into the metadata
+     * cache.  This is necessary, as if they are marked dirty, the metadata
+     * cache will attempt to write them on file close, which is frowned 
+     * upon when the file is opened R/O.
+     *
+     * On the other hand, such entries (marked prefetched_dirty) must not 
+     * be evicted, as should the cache be asked to re-load them, the cache
+     * will attempt to read them from the file, and at best load an outdated
+     * version.
+     * 
+     * To avoid this, H5C__evict_tagged_entries_cb has been modified to 
+     * skip such entries.  However, by doing so, it may prevent pinned 
+     * entries from becoming unpinned.
+     *
+     * Thus we must ignore ctx.pinned_entries_need_evicted if 
+     * ctx.skipped_pf_dirty_entries is TRUE.
+     */
+    if((!ctx.skipped_pf_dirty_entries) && (ctx.pinned_entries_need_evicted))
         HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "Pinned entries still need evicted?!")
 
 done: