Oops, I forgot my signoff. Here it is:
Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
On 23/03/2016 18:02, "Daniele Di Proietto" <diproiet...@vmware.com> wrote:
>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, and
>calling ovsrcu_quiesce_end() there will cause the parent to end its
>(possible) quiescing state, which it's not required to start a child
>thread.
>
>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 manifest itself especially when staring ovs-vswitchd with DPDK.
>In the DPDK case the first threads create 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.
>---
> lib/ovs-thread.c | 1 -
> tests/automake.mk | 1 +
> tests/library.at | 4 ++++
> tests/test-rcu.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 51 insertions(+), 1 deletion(-)
> create mode 100644 tests/test-rcu.c
>
>diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
>index 7777298..13bf6b0 100644
>--- a/lib/ovs-thread.c
>+++ b/lib/ovs-thread.c
>@@ -370,7 +370,6 @@ ovs_thread_create(const char *name, void
>*(*start)(void *), void *arg)
>
> forbid_forking("multiple threads exist");
> multithreaded = true;
>- ovsrcu_quiesce_end();
>
> 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..7c0ffe4
>--- /dev/null
>+++ b/tests/test-rcu.c
>@@ -0,0 +1,46 @@
>+/*
>+ * 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)
>+{
>+ ovs_assert(!ovsrcu_is_quiescent());
>+ ovsrcu_quiesce_start();
>+ 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);
>+
>+ 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
