From 57c50fd416d8f854a43288706f47b2e54b1afadb Mon Sep 17 00:00:00 2001 From: corwin Date: Thu, 1 Oct 2020 16:23:41 -0400 Subject: [PATCH] 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 | 6 ++-- uds/Makefile | 2 +- uds/requestQueueKernel.c | 63 +++++++++++++++++++++++++++++++--------- uds/util/funnelQueue.c | 32 +++++++++++++++++++- uds/util/funnelQueue.h | 22 ++++++++++++-- vdo/Makefile | 2 +- 6 files changed, 105 insertions(+), 22 deletions(-) diff --git a/kvdo.spec b/kvdo.spec index e2dc2a91..0b84f719 100644 --- a/kvdo.spec +++ b/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 - 6.2.3.114-1 -HASH(0x14d09e8) \ No newline at end of file +* Thu Oct 01 2020 - Red Hat VDO Group - 6.2.4.14-1 +HASH(0x55c9459fe3b8) \ No newline at end of file diff --git a/uds/Makefile b/uds/Makefile index 80086304..6e0dc60a 100644 --- a/uds/Makefile +++ b/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))) diff --git a/uds/requestQueueKernel.c b/uds/requestQueueKernel.c index 50fff229..a53ff126 100644 --- a/uds/requestQueueKernel.c +++ b/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,8 +191,32 @@ 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, @@ -184,20 +224,15 @@ static void requestQueueWorker(void *arg) 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). diff --git a/uds/util/funnelQueue.c b/uds/util/funnelQueue.c index 922c74e1..017e405c 100644 --- a/uds/util/funnelQueue.c +++ b/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; +} diff --git a/uds/util/funnelQueue.h b/uds/util/funnelQueue.h index f701f2bd..083d00b8 100644 --- a/uds/util/funnelQueue.h +++ b/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 */ diff --git a/vdo/Makefile b/vdo/Makefile index 7c7b06d6..53dff5c2 100644 --- a/vdo/Makefile +++ b/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)))