[ovs-dev] [PATCH v7 11/16] ovs-thread: Do not quiesce in ovs_mutex_cond_wait().

Daniele Di Proietto diproiettod at vmware.com
Fri Apr 15 23:59:02 UTC 2016


Hi Mark,

On 14/04/2016 05:36, "Kavanagh, Mark B" <mark.b.kavanagh at intel.com> wrote:

>Hi Daniele,
>
>One comment inline.
>
>Cheers,
>Mark
>
>>
>>ovs_mutex_cond_wait() is used in many functions in dpif-netdev to
>>synchronize with pmd threads, but we can't guarantee that the callers do
>>not hold RCU references, so it's better to avoid quiescing.
>
>You'll need to update the following comment in lib/rcu.h accordingly:
>
><snip>
> For example, poll_block() includes a quiescent state, as does
>ovs_mutex_cond_wait().
></snip>

You're right, I removed the reference to ovs_mutex_cond_wait() there.

Thanks for noticing this!

>
>>
>>In system_stats_thread_func() the code relied on ovs_mutex_cond_wait()
>>to introduce a quiescent state, so explicit calls to
>>ovsrcu_quiesce_start() and ovsrcu_quiesce_end() are added there.
>>
>>Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
>>---
>> lib/ovs-thread.c        | 2 --
>> vswitchd/system-stats.c | 6 ++++++
>> 2 files changed, 6 insertions(+), 2 deletions(-)
>>
>>diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
>>index 3c065cf..26dd928 100644
>>--- a/lib/ovs-thread.c
>>+++ b/lib/ovs-thread.c
>>@@ -253,9 +253,7 @@ ovs_mutex_cond_wait(pthread_cond_t *cond, const
>>struct ovs_mutex *mutex_)
>>     struct ovs_mutex *mutex = CONST_CAST(struct ovs_mutex *, mutex_);
>>     int error;
>>
>>-    ovsrcu_quiesce_start();
>>     error = pthread_cond_wait(cond, &mutex->lock);
>>-    ovsrcu_quiesce_end();
>>
>>     if (OVS_UNLIKELY(error)) {
>>         ovs_abort(error, "pthread_cond_wait failed");
>>diff --git a/vswitchd/system-stats.c b/vswitchd/system-stats.c
>>index df4971e..129f0cf 100644
>>--- a/vswitchd/system-stats.c
>>+++ b/vswitchd/system-stats.c
>>@@ -37,6 +37,7 @@
>> #include "json.h"
>> #include "latch.h"
>> #include "openvswitch/ofpbuf.h"
>>+#include "ovs-rcu.h"
>> #include "ovs-thread.h"
>> #include "poll-loop.h"
>> #include "shash.h"
>>@@ -615,7 +616,12 @@ system_stats_thread_func(void *arg OVS_UNUSED)
>>
>>         ovs_mutex_lock(&mutex);
>>         while (!enabled) {
>>+            /* The thread is sleeping, potentially for a long time, and
>>it's
>>+             * not holding RCU protected references, so it makes sense
>>to
>>+             * quiesce */
>>+            ovsrcu_quiesce_start();
>>             ovs_mutex_cond_wait(&cond, &mutex);
>>+            ovsrcu_quiesce_end();
>>         }
>>         ovs_mutex_unlock(&mutex);
>>
>>--
>>2.1.4
>




More information about the dev mailing list