A new thread must be started in a non quiescent state. There is a call
to ovsrcu_quiesce_end() in ovsthread_wrapper(), to enforce this.
ovs_thread_create(), instead, is executed in the parent thread. It must
call ovsrcu_quiesce_end() on its first invocation, to put the main
thread in a non quiescent state. On every other invocation, it doesn't
make sense to alter the calling thread state, so this commits wraps the
call to ovsrcu_quiesce_end() in an ovsthread_once construct.
This fixes a bug in ovs-rcu where the first call in the process to
ovsrcu_quiesce_start() will not be honored, because the calling thread
will need to create the 'urcu' thread (and creating a thread will
wrongly end its quiescent state).
ovsrcu_quiesce_start()
ovs_rcu_quiesced()
if (ovsthread_once_start(&once)) {
ovs_thread_create("urcu") /*This will end the quiescent state*/
}
This bug affects in particular ovs-vswitchd with DPDK.
In the DPDK case the first threads created are "vhost_thread" and
"dpdk_watchdog". If dpdk_watchdog is the first to call
ovsrcu_quiesce_start() (via xsleep()), the call is not honored and
the RCU grace period lasts at least for DPDK_PORT_WATCHDOG_INTERVAL
(5s on current master). If vhost_thread, on the other hand, is the
first to call ovsrcu_quiesce_start(), the call is not honored and the
RCU grace period lasts undefinitely, because no more calls to
ovsrcu_quiesce_start() are issued from vhost_thread.
For some reason (it's a race condition after all), on current master,
dpdk_watchdog will always be the first to call ovsrcu_quiesce_start(),
but with the upcoming DPDK database configuration changes, sometimes
vhost_thread will issue the first call to ovsrcu_quiesce_start().
Sample ovs-vswitchd.log:
2016-03-23T22:34:28.532Z|00004|ovs_rcu(urcu3)|WARN|blocked 8000 ms
waiting for vhost_thread2 to quiesce
2016-03-23T22:34:30.501Z|00118|ovs_rcu|WARN|blocked 8000 ms waiting for
vhost_thread2 to quiesce
2016-03-23T22:34:36.532Z|00005|ovs_rcu(urcu3)|WARN|blocked 16000 ms
waiting for vhost_thread2 to quiesce
2016-03-23T22:34:38.501Z|00119|ovs_rcu|WARN|blocked 16000 ms waiting for
vhost_thread2 to quiesce
The commit also adds a test for the ovs-rcu module to make sure that:
* A new thread is started in a non quiescent state.
* The first call to ovsrcu_quiesce_start() is honored.
* When a process becomes multithreaded the main thread is put in an
active state
Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
---
v2:
The first call to ovs_thread_create must put the main thread
in an active state, so:
* Document this in ovs-rcu.h.
* Keep the call to ovsrcu_quiesce_end() in ovs_thread_create() for
the _first_ invocation, with a comment.
* Test this in test-rcu-quiesce.
---
lib/ovs-rcu.h | 3 ++-
lib/ovs-thread.c | 17 ++++++++++++++++-
tests/automake.mk | 1 +
tests/library.at | 4 ++++
tests/test-rcu.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 75 insertions(+), 2 deletions(-)
create mode 100644 tests/test-rcu.c
diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
index 7fe7d47..600be0b 100644
--- a/lib/ovs-rcu.h
+++ b/lib/ovs-rcu.h
@@ -56,7 +56,8 @@
*
* Brackets a time period during which the current thread is quiescent.
*
- * A newly created thread is initially active, not quiescent.
+ * A newly created thread is initially active, not quiescent. When a process
+ * becomes multithreaded, the main thread becomes active, not quiescent.
*
* When a quiescient state has occurred in every thread, we say that a "grace
* period" has occurred. Following a grace period, all of the callbacks
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index 7777298..1a463a7 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -364,13 +364,28 @@ set_min_stack_size(pthread_attr_t *attr, size_t
min_stacksize)
pthread_t
ovs_thread_create(const char *name, void *(*start)(void *), void *arg)
{
+ static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
struct ovsthread_aux *aux;
pthread_t thread;
int error;
forbid_forking("multiple threads exist");
multithreaded = true;
- ovsrcu_quiesce_end();
+
+ if (ovsthread_once_start(&once)) {
+ /* The first call to this function has to happen in the main thread.
+ * Before the process becomes multithreaded we make sure that the
+ * main thread is considered non quiescent.
+ *
+ * For other threads this is done in ovs_thread_wrapper(), but the
+ * main thread has no such wrapper.
+ *
+ * There's no reason to call ovsrcu_quiesce_end() in subsequent
+ * invocations of this function and it might introduce problems
+ * for other threads. */
+ ovsrcu_quiesce_end();
+ ovsthread_once_done(&once);
+ }
aux = xmalloc(sizeof *aux);
aux->start = start;
diff --git a/tests/automake.mk b/tests/automake.mk
index 98c4df0..ca65ed5 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -327,6 +327,7 @@ tests_ovstest_SOURCES = \
tests/test-ovn.c \
tests/test-packets.c \
tests/test-random.c \
+ tests/test-rcu.c \
tests/test-reconnect.c \
tests/test-rstp.c \
tests/test-sflow.c \
diff --git a/tests/library.at b/tests/library.at
index 4094348..e1bac92 100644
--- a/tests/library.at
+++ b/tests/library.at
@@ -230,3 +230,7 @@ AT_CLEANUP
AT_SETUP([test ofpbuf module])
AT_CHECK([ovstest test-ofpbuf], [0], [])
AT_CLEANUP
+
+AT_SETUP([test rcu])
+AT_CHECK([ovstest test-rcu-quiesce], [0], [])
+AT_CLEANUP
diff --git a/tests/test-rcu.c b/tests/test-rcu.c
new file mode 100644
index 0000000..e66367e
--- /dev/null
+++ b/tests/test-rcu.c
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2016 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <config.h>
+#undef NDEBUG
+#include "fatal-signal.h"
+#include "ovs-rcu.h"
+#include "ovs-thread.h"
+#include "ovstest.h"
+#include "util.h"
+
+static void *
+quiescer_main(void *aux OVS_UNUSED)
+{
+ /* A new thread must be not be quiescent */
+ ovs_assert(!ovsrcu_is_quiescent());
+ ovsrcu_quiesce_start();
+ /* After the above call it must be quiescent */
+ ovs_assert(ovsrcu_is_quiescent());
+
+ return NULL;
+}
+
+static void
+test_rcu_quiesce(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
+{
+ pthread_t quiescer;
+
+ fatal_signal_init();
+ quiescer = ovs_thread_create("quiescer", quiescer_main, NULL);
+
+ /* This is the main thread of the process. After spawning its first
+ * thread it must not be quiescent. */
+ ovs_assert(!ovsrcu_is_quiescent());
+
+ xpthread_join(quiescer, NULL);
+}
+
+OVSTEST_REGISTER("test-rcu-quiesce", test_rcu_quiesce);
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
