src/hotspot/share/gc/g1/g1RemSet.cpp

@@ -1259,11 +1259,12 @@
          p2i(ct->addr_for(card_ptr)),
          g1h->addr_to_region(ct->addr_for(card_ptr)));
 #endif
 }
 
-bool G1RemSet::clean_card_before_refine(CardValue*& card_ptr) {
+bool G1RemSet::clean_card_before_refine(CardValue*& card_ptr
+                                        DEBUG_ONLY(COMMA KVHashtable<CardTable::CardValue* COMMA HeapWord* COMMA mtGC>& top_map)) {
   assert(!_g1h->is_gc_active(), "Only call concurrently");
 
   // Find the start address represented by the card.
   HeapWord* start = _ct->addr_for(card_ptr);
   // And find the region containing it.

@@ -1275,10 +1276,12 @@
   }
 
   check_card_ptr(card_ptr, _ct);
 
   // If the card is no longer dirty, nothing to do.
+  // We cannot load the card value before the "r == NULL" check, because G1
+  // could uncommit parts of the card table covering uncommitted regions.
   if (*card_ptr != G1CardTable::dirty_card_val()) {
     return false;
   }
 
   // This check is needed for some uncommon cases where we should

@@ -1357,25 +1360,37 @@
   // Okay to clean and process the card now.  There are still some
   // stale card cases that may be detected by iteration and dealt with
   // as iteration failure.
   *const_cast<volatile CardValue*>(card_ptr) = G1CardTable::clean_card_val();
 
+#ifdef ASSERT
+  HeapWord** existing_top = top_map.lookup(card_ptr);
+  if (existing_top != NULL) {
+    assert(scan_limit == *existing_top, "top must be stable");
+  } else {
+    top_map.add(card_ptr, scan_limit);
+  }
+#endif
+
   return true;
 }
 
 void G1RemSet::refine_card_concurrently(CardValue* const card_ptr,
-                                        const uint worker_id) {
+                                        const uint worker_id
+                                        DEBUG_ONLY(COMMA KVHashtable<CardTable::CardValue* COMMA HeapWord* COMMA mtGC>& top_map)) {
   assert(!_g1h->is_gc_active(), "Only call concurrently");
   check_card_ptr(card_ptr, _ct);
 
   // Construct the MemRegion representing the card.
   HeapWord* start = _ct->addr_for(card_ptr);
   // And find the region containing it.
   HeapRegion* r = _g1h->heap_region_containing(start);
+  // This reload of the top is safe even though it happens after the full
+  // fence, because top is stable for unfiltered humongous regions, so it must
+  // return the same value as the previous load when cleaning the card.
   HeapWord* scan_limit = r->top();
-  // top() can only increase since last read of top() before cleaning the card.
-  assert(scan_limit > start, "sanity");
+  assert(scan_limit == *top_map.lookup(card_ptr), "top must be stable");
 
   // Don't use addr_for(card_ptr + 1) which can ask for
   // a card beyond the heap.
   HeapWord* end = start + G1CardTable::card_size_in_words;
   MemRegion dirty_region(start, MIN2(scan_limit, end));