--- a/src/share/vm/gc_implementation/g1/concurrentG1Refine.cpp	Fri Jul 16 21:33:21 2010 -0700
+++ b/src/share/vm/gc_implementation/g1/concurrentG1Refine.cpp	Mon Jul 19 11:06:34 2010 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2009, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2010, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -271,21 +271,16 @@
     if (cas_res == prev_epoch_entry) {
       // We successfully updated the card num value in the epoch entry
       count_ptr->_count = 0; // initialize counter for new card num
+      jbyte* old_card_ptr = card_num_2_ptr(old_card_num);
 
       // Even though the region containg the card at old_card_num was not
       // in the young list when old_card_num was recorded in the epoch
       // cache it could have been added to the free list and subsequently
-      // added to the young list in the intervening time. If the evicted
-      // card is in a young region just return the card_ptr and the evicted
-      // card will not be cleaned. See CR 6817995.
-
-      jbyte* old_card_ptr = card_num_2_ptr(old_card_num);
-      if (is_young_card(old_card_ptr)) {
-        *count = 0;
-        // We can defer the processing of card_ptr
-        *defer = true;
-        return card_ptr;
-      }
+      // added to the young list in the intervening time. See CR 6817995.
+      // We do not deal with this case here - it will be handled in
+      // HeapRegion::oops_on_card_seq_iterate_careful after it has been
+      // determined that the region containing the card has been allocated
+      // to, and it's safe to check the young type of the region.
 
       // We do not want to defer processing of card_ptr in this case
       // (we need to refine old_card_ptr and card_ptr)
@@ -301,22 +296,22 @@
   jbyte* cached_ptr = add_card_count(card_ptr, &count, defer);
   assert(cached_ptr != NULL, "bad cached card ptr");
 
-  if (is_young_card(cached_ptr)) {
-    // The region containing cached_ptr has been freed during a clean up
-    // pause, reallocated, and tagged as young.
-    assert(cached_ptr != card_ptr, "shouldn't be");
+  // We've just inserted a card pointer into the card count cache
+  // and got back the card that we just inserted or (evicted) the
+  // previous contents of that count slot.
 
-    // We've just inserted a new old-gen card pointer into the card count
-    // cache and evicted the previous contents of that count slot.
-    // The evicted card pointer has been determined to be in a young region
-    // and so cannot be the newly inserted card pointer (that will be
-    // in an old region).
-    // The count for newly inserted card will be set to zero during the
-    // insertion, so we don't want to defer the cleaning of the newly
-    // inserted card pointer.
-    assert(*defer == false, "deferring non-hot card");
-    return NULL;
-  }
+  // The card we got back could be in a young region. When the
+  // returned card (if evicted) was originally inserted, we had
+  // determined that its containing region was not young. However
+  // it is possible for the region to be freed during a cleanup
+  // pause, then reallocated and tagged as young which will result
+  // in the returned card residing in a young region.
+  //
+  // We do not deal with this case here - the change from non-young
+  // to young could be observed at any time - it will be handled in
+  // HeapRegion::oops_on_card_seq_iterate_careful after it has been
+  // determined that the region containing the card has been allocated
+  // to.
 
   // The card pointer we obtained from card count cache is not hot
   // so do not store it in the cache; return it for immediate
@@ -325,7 +320,7 @@
     return cached_ptr;
   }
 
-  // Otherwise, the pointer we got from the _card_counts is hot.
+  // Otherwise, the pointer we got from the _card_counts cache is hot.
   jbyte* res = NULL;
   MutexLockerEx x(HotCardCache_lock, Mutex::_no_safepoint_check_flag);
   if (_n_hot == _hot_cache_size) {
@@ -338,17 +333,8 @@
   if (_hot_cache_idx == _hot_cache_size) _hot_cache_idx = 0;
   _n_hot++;
 
-  if (res != NULL) {
-    // Even though the region containg res was not in the young list
-    // when it was recorded in the hot cache it could have been added
-    // to the free list and subsequently added to the young list in
-    // the intervening time. If res is in a young region, return NULL
-    // so that res is not cleaned. See CR 6817995.
-
-    if (is_young_card(res)) {
-      res = NULL;
-    }
-  }
+  // The card obtained from the hot card cache could be in a young
+  // region. See above on how this can happen.
 
   return res;
 }

--- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Fri Jul 16 21:33:21 2010 -0700
+++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Mon Jul 19 11:06:34 2010 -0700
@@ -638,6 +638,11 @@
 
     // Now retry the allocation.
     if (_cur_alloc_region != NULL) {
+      if (allocated_young_region != NULL) {
+        // We need to ensure that the store to top does not
+        // float above the setting of the young type.
+        OrderAccess::storestore();
+      }
       res = _cur_alloc_region->allocate(word_size);
     }
   }

--- a/src/share/vm/gc_implementation/g1/g1RemSet.cpp	Fri Jul 16 21:33:21 2010 -0700
+++ b/src/share/vm/gc_implementation/g1/g1RemSet.cpp	Mon Jul 19 11:06:34 2010 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2009, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2010, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -676,9 +676,27 @@
   // We must complete this write before we do any of the reads below.
   OrderAccess::storeload();
   // And process it, being careful of unallocated portions of TLAB's.
+
+  // The region for the current card may be a young region. The
+  // current card may have been a card that was evicted from the
+  // card cache. When the card was inserted into the cache, we had
+  // determined that its region was non-young. While in the cache,
+  // the region may have been freed during a cleanup pause, reallocated
+  // and tagged as young.
+  //
+  // We wish to filter out cards for such a region but the current
+  // thread, if we're running conucrrently, may "see" the young type
+  // change at any time (so an earlier "is_young" check may pass or
+  // fail arbitrarily). We tell the iteration code to perform this
+  // filtering when it has been determined that there has been an actual
+  // allocation in this region and making it safe to check the young type.
+  bool filter_young = true;
+
   HeapWord* stop_point =
     r->oops_on_card_seq_iterate_careful(dirtyRegion,
-                                        &filter_then_update_rs_oop_cl);
+                                        &filter_then_update_rs_oop_cl,
+                                        filter_young);
+
   // If stop_point is non-null, then we encountered an unallocated region
   // (perhaps the unfilled portion of a TLAB.)  For now, we'll dirty the
   // card and re-enqueue: if we put off the card until a GC pause, then the
@@ -789,8 +807,14 @@
       if (r == NULL) {
         assert(_g1->is_in_permanent(start), "Or else where?");
       } else {
-        guarantee(!r->is_young(), "It was evicted in the current minor cycle.");
-        // Process card pointer we get back from the hot card cache
+        // Checking whether the region we got back from the cache
+        // is young here is inappropriate. The region could have been
+        // freed, reallocated and tagged as young while in the cache.
+        // Hence we could see its young type change at any time.
+        //
+        // Process card pointer we get back from the hot card cache. This
+        // will check whether the region containing the card is young
+        // _after_ checking that the region has been allocated from.
         concurrentRefineOneCard_impl(res, worker_i);
       }
     }

--- a/src/share/vm/gc_implementation/g1/heapRegion.cpp	Fri Jul 16 21:33:21 2010 -0700
+++ b/src/share/vm/gc_implementation/g1/heapRegion.cpp	Mon Jul 19 11:06:34 2010 -0700
@@ -658,7 +658,8 @@
 HeapWord*
 HeapRegion::
 oops_on_card_seq_iterate_careful(MemRegion mr,
-                                     FilterOutOfRegionClosure* cl) {
+                                 FilterOutOfRegionClosure* cl,
+                                 bool filter_young) {
   G1CollectedHeap* g1h = G1CollectedHeap::heap();
 
   // If we're within a stop-world GC, then we might look at a card in a
@@ -672,6 +673,16 @@
   if (mr.is_empty()) return NULL;
   // Otherwise, find the obj that extends onto mr.start().
 
+  // The intersection of the incoming mr (for the card) and the
+  // allocated part of the region is non-empty. This implies that
+  // we have actually allocated into this region. The code in
+  // G1CollectedHeap.cpp that allocates a new region sets the
+  // is_young tag on the region before allocating. Thus we
+  // safely know if this region is young.
+  if (is_young() && filter_young) {
+    return NULL;
+  }
+
   // We used to use "block_start_careful" here.  But we're actually happy
   // to update the BOT while we do this...
   HeapWord* cur = block_start(mr.start());

--- a/src/share/vm/gc_implementation/g1/heapRegion.hpp	Fri Jul 16 21:33:21 2010 -0700
+++ b/src/share/vm/gc_implementation/g1/heapRegion.hpp	Mon Jul 19 11:06:34 2010 -0700
@@ -252,7 +252,7 @@
                                 // survivor
   };
 
-  YoungType _young_type;
+  volatile YoungType _young_type;
   int  _young_index_in_cset;
   SurvRateGroup* _surv_rate_group;
   int  _age_index;
@@ -726,9 +726,12 @@
   HeapWord*
   object_iterate_mem_careful(MemRegion mr, ObjectClosure* cl);
 
+  // In this version - if filter_young is true and the region
+  // is a young region then we skip the iteration.
   HeapWord*
   oops_on_card_seq_iterate_careful(MemRegion mr,
-                                   FilterOutOfRegionClosure* cl);
+                                   FilterOutOfRegionClosure* cl,
+                                   bool filter_young);
 
   // The region "mr" is entirely in "this", and starts and ends at block
   // boundaries. The caller declares that all the contained blocks are