# HG changeset patch
# User johnc
# Date 1259104770 28800
# Node ID fa357420e7d2572dad78240c71fdadc287cdb89e
# Parent  db0d5eba9d20c5191cc8ed07fd31f1766b7c3a5c
6899058: G1: Internal error in ptrQueue.cpp:201 in nightly tests
Summary: Fixes a race on the dirty card queue completed buffer list between worker thread(s) performing a flush of a deferred store barrier (enqueueing a newly completed buffer) and worker thread(s) in the RSet updating code claiming completed buffers. Removed the routine that removes elements from the completed update buffer queue using a CAS.
Reviewed-by: ysr, tonyp

diff -r db0d5eba9d20 -r fa357420e7d2 src/share/vm/gc_implementation/g1/dirtyCardQueue.cpp
--- a/src/share/vm/gc_implementation/g1/dirtyCardQueue.cpp	Fri Nov 20 14:47:01 2009 -0500
+++ b/src/share/vm/gc_implementation/g1/dirtyCardQueue.cpp	Tue Nov 24 15:19:30 2009 -0800
@@ -155,7 +155,7 @@
 }
 
 DirtyCardQueueSet::CompletedBufferNode*
-DirtyCardQueueSet::get_completed_buffer_lock(int stop_at) {
+DirtyCardQueueSet::get_completed_buffer(int stop_at) {
   CompletedBufferNode* nd = NULL;
   MutexLockerEx x(_cbl_mon, Mutex::_no_safepoint_check_flag);
 
@@ -175,28 +175,6 @@
   return nd;
 }
 
-// We only do this in contexts where there is no concurrent enqueueing.
-DirtyCardQueueSet::CompletedBufferNode*
-DirtyCardQueueSet::get_completed_buffer_CAS() {
-  CompletedBufferNode* nd = _completed_buffers_head;
-
-  while (nd != NULL) {
-    CompletedBufferNode* next = nd->next;
-    CompletedBufferNode* result =
-      (CompletedBufferNode*)Atomic::cmpxchg_ptr(next,
-                                                &_completed_buffers_head,
-                                                nd);
-    if (result == nd) {
-      return result;
-    } else {
-      nd = _completed_buffers_head;
-    }
-  }
-  assert(_completed_buffers_head == NULL, "Loop post");
-  _completed_buffers_tail = NULL;
-  return NULL;
-}
-
 bool DirtyCardQueueSet::
 apply_closure_to_completed_buffer_helper(int worker_i,
                                          CompletedBufferNode* nd) {
@@ -222,15 +200,10 @@
 
 bool DirtyCardQueueSet::apply_closure_to_completed_buffer(int worker_i,
                                                           int stop_at,
-                                                          bool with_CAS)
+                                                          bool during_pause)
 {
-  CompletedBufferNode* nd = NULL;
-  if (with_CAS) {
-    guarantee(stop_at == 0, "Precondition");
-    nd = get_completed_buffer_CAS();
-  } else {
-    nd = get_completed_buffer_lock(stop_at);
-  }
+  assert(!during_pause || stop_at == 0, "Should not leave any completed buffers during a pause");
+  CompletedBufferNode* nd = get_completed_buffer(stop_at);
   bool res = apply_closure_to_completed_buffer_helper(worker_i, nd);
   if (res) Atomic::inc(&_processed_buffers_rs_thread);
   return res;
diff -r db0d5eba9d20 -r fa357420e7d2 src/share/vm/gc_implementation/g1/dirtyCardQueue.hpp
--- a/src/share/vm/gc_implementation/g1/dirtyCardQueue.hpp	Fri Nov 20 14:47:01 2009 -0500
+++ b/src/share/vm/gc_implementation/g1/dirtyCardQueue.hpp	Tue Nov 24 15:19:30 2009 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright 2001-2007 Sun Microsystems, Inc.  All Rights Reserved.
+ * Copyright 2001-2009 Sun Microsystems, Inc.  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
@@ -120,12 +120,13 @@
   // is returned to the completed buffer set, and this call returns false.
   bool apply_closure_to_completed_buffer(int worker_i = 0,
                                          int stop_at = 0,
-                                         bool with_CAS = false);
+                                         bool during_pause = false);
+
   bool apply_closure_to_completed_buffer_helper(int worker_i,
                                                 CompletedBufferNode* nd);
 
-  CompletedBufferNode* get_completed_buffer_CAS();
-  CompletedBufferNode* get_completed_buffer_lock(int stop_at);
+  CompletedBufferNode* get_completed_buffer(int stop_at);
+
   // Applies the current closure to all completed buffers,
   // non-consumptively.
   void apply_closure_to_all_completed_buffers();