Version 6.2.4.14

- Fixed a bug which causes the UDS index to consume an excessive
  number of CPU cycles when the VDO device is idle.

kvdo.spec

@@ -1,6 +1,6 @@
 %define spec_release 1
 %define kmod_name		kvdo
-%define kmod_driver_version	6.2.3.114
+%define kmod_driver_version	6.2.4.14
 %define kmod_rpm_release	%{spec_release}
 %define kmod_kernel_version	3.10.0-693.el7
 
@@ -85,5 +85,5 @@ rm -rf $RPM_BUILD_ROOT
 %{_usr}/src/%{kmod_name}-%{version}-%{kmod_driver_version}/*
 
 %changelog
-* Thu Jul 30 2020 - J. corwin Coburn <corwin@redhat.com> - 6.2.3.114-1
-HASH(0x14d09e8)
+* Thu Oct 01 2020 - Red Hat VDO Group <vdo-devel@redhat.com> - 6.2.4.14-1
+HASH(0x55c9459fe3b8)

uds/Makefile

@@ -1,4 +1,4 @@
-UDS_VERSION = 8.0.1.6
+UDS_VERSION = 8.0.2.2
 
 SOURCES =  $(notdir $(wildcard $(src)/*.c)) murmur/MurmurHash3.c
 SOURCES += $(addprefix util/,$(notdir $(wildcard $(src)/util/*.c)))

uds/requestQueueKernel.c

@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
  * 02110-1301, USA. 
  *
- * $Id: //eng/uds-releases/jasper/kernelLinux/uds/requestQueueKernel.c#2 $
+ * $Id: //eng/uds-releases/jasper/kernelLinux/uds/requestQueueKernel.c#3 $
  */
 
 #include "requestQueue.h"
@@ -126,6 +126,22 @@ static INLINE Request *pollQueues(RequestQueue *queue)
   return NULL;
 }
 
+/*****************************************************************************/
+/**
+ * Check if the underlying lock-free queues appear not just not to have any
+ * requests available right now, but also not to be in the intermediate state
+ * of getting requests added. Must only be called by the worker thread.
+ *
+ * @param queue  the RequestQueue being serviced
+ *
+ * @return true iff both funnel queues are idle
+ **/
+static INLINE bool areQueuesIdle(RequestQueue *queue)
+{
+  return (isFunnelQueueIdle(queue->retryQueue) &&
+          isFunnelQueueIdle(queue->mainQueue));
+}
+
 /*****************************************************************************/
 /**
  * Remove the next request to be processed from the queue.  Must only be called
@@ -175,29 +191,48 @@ static void requestQueueWorker(void *arg)
     Request *request;
     bool waited = false;
     if (dormant) {
+      /*
+       * Sleep/wakeup protocol:
+       *
+       * The enqueue operation updates "newest" in the
+       * funnel queue via xchg which is a memory barrier,
+       * and later checks "dormant" to decide whether to do
+       * a wakeup of the worker thread.
+       *
+       * The worker thread, when deciding to go to sleep,
+       * sets "dormant" and then examines "newest" to decide
+       * if the funnel queue is idle. In dormant mode, the
+       * last examination of "newest" before going to sleep
+       * is done inside the wait_event_interruptible macro,
+       * after a point where (one or more) memory barriers
+       * have been issued. (Preparing to sleep uses spin
+       * locks.) Even if the "next" field update isn't
+       * visible yet to make the entry accessible, its
+       * existence will kick the worker thread out of
+       * dormant mode and back into timer-based mode.
+       *
+       * So the two threads should agree on the ordering of
+       * the updating of the two fields.
+       */
       wait_event_interruptible(queue->wqhead,
-                               dequeueRequest(queue, &request, &waited));
+                               dequeueRequest(queue, &request, &waited) ||
+                               !areQueuesIdle(queue));
     } else {
       wait_event_interruptible_hrtimeout(queue->wqhead,
                                          dequeueRequest(queue, &request,
                                                         &waited),
                                          ns_to_ktime(timeBatch));
     }
 
-    if (unlikely(request == NULL)) {
-      if (READ_ONCE(queue->alive)) {
-        // Must have taken an interrupt; keep polling.
-        continue;
-      } else {
-        // We got no request and we know we are shutting down.
-        break;
-      }
+    if (likely(request != NULL)) {
+      // We got a request.
+      currentBatch++;
+      queue->processOne(request);
+    } else if (!READ_ONCE(queue->alive)) {
+      // We got no request and we know we are shutting down.
+      break;
     }
 
-    // We got a request.
-    currentBatch++;
-    queue->processOne(request);
-
     if (dormant) {
       // We've been roused from dormancy. Clear the flag so enqueuers can stop
       // broadcasting (no fence needed for this transition).

uds/util/funnelQueue.c

@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
  * 02110-1301, USA. 
  *
- * $Id: //eng/uds-releases/jasper/src/uds/util/funnelQueue.c#1 $
+ * $Id: //eng/uds-releases/jasper/src/uds/util/funnelQueue.c#2 $
  */
 
 #include "funnelQueue.h"
@@ -140,3 +140,33 @@ bool isFunnelQueueEmpty(FunnelQueue *queue)
 {
   return getOldest(queue) == NULL;
 }
+
+/**********************************************************************/
+bool isFunnelQueueIdle(FunnelQueue *queue)
+{
+  /*
+   * Oldest is not the stub, so there's another entry, though if next is
+   * NULL we can't retrieve it yet.
+   */
+  if (queue->oldest != &queue->stub) {
+    return false;
+  }
+
+  /*
+   * Oldest is the stub, but newest has been updated by _put(); either
+   * there's another, retrievable entry in the list, or the list is
+   * officially empty but in the intermediate state of having an entry
+   * added.
+   *
+   * Whether anything is retrievable depends on whether stub.next has
+   * been updated and become visible to us, but for idleness we don't
+   * care. And due to memory ordering in _put(), the update to newest
+   * would be visible to us at the same time or sooner.
+   */
+  if (queue->newest != &queue->stub) {
+    return false;
+  }
+
+  // Otherwise, we're idle.
+  return true;
+}

uds/util/funnelQueue.h

@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
  * 02110-1301, USA. 
  *
- * $Id: //eng/uds-releases/jasper/src/uds/util/funnelQueue.h#1 $
+ * $Id: //eng/uds-releases/jasper/src/uds/util/funnelQueue.h#2 $
  */
 
 #ifndef FUNNEL_QUEUE_H
@@ -171,9 +171,27 @@ FunnelQueueEntry *funnelQueuePoll(FunnelQueue *queue)
  *
  * @param queue  the queue which to check for entries.
  *
- * @return true iff queue contains an entry which can be retrieved
+ * @return true iff queue contains no entry which can be retrieved
  **/
 bool isFunnelQueueEmpty(FunnelQueue *queue)
   __attribute__((warn_unused_result));
 
+/**
+ * Check whether the funnel queue is idle or not. This function must only be
+ * called from a single consumer thread, as with funnel_queue_poll.
+ *
+ * If the queue has entries available to be retrieved, it is not idle. If the
+ * queue is in a transition state with one or more entries being added such
+ * that the list view is incomplete, it may not be possible to retrieve an
+ * entry with the funnel_queue_poll() function, but the queue will not be
+ * considered idle.
+ *
+ * @param queue  the queue which to check for entries.
+ *
+ * @return true iff queue contains no entry which can be retrieved nor is
+ *              known to be having an entry added
+ **/
+bool isFunnelQueueIdle(FunnelQueue *queue)
+  __attribute__((warn_unused_result));
+
 #endif /* FUNNEL_QUEUE_H */

vdo/Makefile

@@ -1,4 +1,4 @@
-VDO_VERSION = 6.2.3.114
+VDO_VERSION = 6.2.4.14
 
 VDO_VERSION_MAJOR = $(word 1,$(subst ., ,$(VDO_VERSION)))
 VDO_VERSION_MINOR = $(word 2,$(subst ., ,$(VDO_VERSION)))