[trafficserver] 04/05: TS-4872: Hoist lock-free log buffer manipulation.

[so...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sorber pushed a commit to branch 6.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 549826fb54399181065e0ad95e28e597fdcd62cf
Author: James Peach <jp...@apache.org>
AuthorDate: Thu Sep 15 11:58:02 2016 -0700

    TS-4872: Hoist lock-free log buffer manipulation.
    
    Hoist the lock-free log buffer manipulations into helper functions.
    This make is a bit clearer what they are trying to do, and clarifies
    things so that the clang static analyzer doesn't think there is a
    memory leak here.
    
    (cherry picked from commit cb566cc3cf19892a07d7d274af2b045e957d6ce1)
    
     Conflicts:
    	proxy/logging/LogObject.cc
---
 proxy/logging/LogObject.cc | 53 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 16 deletions(-)

diff --git a/proxy/logging/LogObject.cc b/proxy/logging/LogObject.cc
index c6c94c4..d272b18 100644
--- a/proxy/logging/LogObject.cc
+++ b/proxy/logging/LogObject.cc
@@ -380,6 +380,29 @@ LogObject::displayAsXML(FILE *fd, bool extended)
   fprintf(fd, "</LogObject>\n");
 }
 
+static head_p
+increment_pointer_version(volatile head_p *dst)
+{
+  head_p h;
+  head_p new_h;
+
+  do {
+    INK_QUEUE_LD(h, *dst);
+    SET_FREELIST_POINTER_VERSION(new_h, FREELIST_POINTER(h), FREELIST_VERSION(h) + 1);
+  } while (ink_atomic_cas(&dst->data, h.data, new_h.data) == false);
+
+  return h;
+}
+
+static bool
+write_pointer_version(volatile head_p *dst, head_p old_h, void *ptr, head_p::version_type vers)
+{
+  head_p tmp_h;
+
+  SET_FREELIST_POINTER_VERSION(tmp_h, ptr, vers);
+  return ink_atomic_cas(&dst->data, old_h.data, tmp_h.data);
+}
+
 LogBuffer *
 LogObject::_checkout_write(size_t *write_offset, size_t bytes_needed)
 {
@@ -391,14 +414,10 @@ LogObject::_checkout_write(size_t *write_offset, size_t bytes_needed)
   do {
     // To avoid a race condition, we keep a count of held references in
     // the pointer itself and add this to m_outstanding_references.
-    head_p h;
-    int result = 0;
-    do {
-      INK_QUEUE_LD(h, m_log_buffer);
-      head_p new_h;
-      SET_FREELIST_POINTER_VERSION(new_h, FREELIST_POINTER(h), FREELIST_VERSION(h) + 1);
-      result = ink_atomic_cas(&m_log_buffer.data, h.data, new_h.data);
-    } while (!result);
+
+    // Increment the version of m_log_buffer, returning the previous version.
+    head_p h = increment_pointer_version(&m_log_buffer);
+
     buffer           = (LogBuffer *)FREELIST_POINTER(h);
     result_code      = buffer->checkout_write(write_offset, bytes_needed);
     bool decremented = false;
@@ -418,6 +437,7 @@ LogObject::_checkout_write(size_t *write_offset, size_t bytes_needed)
       // swap the new buffer for the old one
       INK_WRITE_MEMORY_BARRIER;
       head_p old_h;
+
       do {
         INK_QUEUE_LD(old_h, m_log_buffer);
         if (FREELIST_POINTER(old_h) != FREELIST_POINTER(h)) {
@@ -428,10 +448,8 @@ LogObject::_checkout_write(size_t *write_offset, size_t bytes_needed)
           delete new_buffer;
           break;
         }
-        head_p tmp_h;
-        SET_FREELIST_POINTER_VERSION(tmp_h, new_buffer, 0);
-        result = ink_atomic_cas(&m_log_buffer.data, old_h.data, tmp_h.data);
-      } while (!result);
+      } while (!write_pointer_version(&m_log_buffer, old_h, new_buffer, 0));
+
       if (FREELIST_POINTER(old_h) == FREELIST_POINTER(h)) {
         ink_atomic_increment(&buffer->m_references, FREELIST_VERSION(old_h) - 1);
 
@@ -439,7 +457,9 @@ LogObject::_checkout_write(size_t *write_offset, size_t bytes_needed)
         Debug("log-logbuffer", "adding buffer %d to flush list after checkout", buffer->get_id());
         m_buffer_manager[idx].add_to_flush_queue(buffer);
         Log::preproc_notify[idx].signal();
+        buffer = NULL;
       }
+
       decremented = true;
       break;
 
@@ -460,19 +480,20 @@ LogObject::_checkout_write(size_t *write_offset, size_t bytes_needed)
     default:
       ink_assert(false);
     }
+
     if (!decremented) {
       head_p old_h;
+
       do {
         INK_QUEUE_LD(old_h, m_log_buffer);
         if (FREELIST_POINTER(old_h) != FREELIST_POINTER(h))
           break;
-        head_p tmp_h;
-        SET_FREELIST_POINTER_VERSION(tmp_h, FREELIST_POINTER(h), FREELIST_VERSION(old_h) - 1);
-        result = ink_atomic_cas(&m_log_buffer.data, old_h.data, tmp_h.data);
-      } while (!result);
+      } while (!write_pointer_version(&m_log_buffer, old_h, FREELIST_POINTER(h), FREELIST_VERSION(old_h) - 1));
+
       if (FREELIST_POINTER(old_h) != FREELIST_POINTER(h))
         ink_atomic_increment(&buffer->m_references, -1);
     }
+
   } while (retry && write_offset); // if write_offset is null, we do
   // not retry because we really do
   // not want to write to the buffer

-- 
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.