[ovs-dev] [RFC 04/14] revalidator: Protect ukeys with a mutex.

Joe Stringer joestringer at nicira.com
Thu Aug 21 05:41:59 UTC 2014


Currently, ukeys are protected during revalidator_sweep__() as only one
thread accesses the ukey at a time. This is ensured using barriers:
all revalidators will be in the GC phase, so they will only access their
own ukey collection.

A future patch will change the access patterns to allow these ukey
collections to be read or modified while a revalidator is garbage
collecting it. To protect the ukeys, this patch adds locking on the ukey
collection.

Signed-off-by: Joe Stringer <joestringer at nicira.com>
---
 ofproto/ofproto-dpif-upcall.c |   29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index bccd7e8..db11964 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -63,6 +63,9 @@ struct revalidator {
     struct udpif *udpif;               /* Parent udpif. */
     pthread_t thread;                  /* Thread ID. */
     unsigned int id;                   /* ovsthread_id_self(). */
+    struct ovs_mutex *mutex;           /* Points into udpif->ukeys for this
+                                          revalidator. Required for writing
+                                          to 'ukeys'. */
     struct cmap *ukeys;                /* Points into udpif->ukeys for this
                                           revalidator. Used for GC phase. */
 };
@@ -409,6 +412,7 @@ udpif_start_threads(struct udpif *udpif, size_t n_handlers,
             revalidator->udpif = udpif;
             cmap_init(&udpif->ukeys[i].cmap);
             ovs_mutex_init(&udpif->ukeys[i].mutex);
+            revalidator->mutex = &udpif->ukeys[i].mutex;
             revalidator->ukeys = &udpif->ukeys[i].cmap;
             revalidator->thread = ovs_thread_create(
                 "revalidator", udpif_revalidator, revalidator);
@@ -1494,9 +1498,11 @@ push_dump_ops(struct revalidator *revalidator,
     int i;
 
     push_dump_ops__(revalidator->udpif, ops, n_ops);
+    ovs_mutex_lock(revalidator->mutex);
     for (i = 0; i < n_ops; i++) {
         ukey_delete(revalidator, ops[i].ukey);
     }
+    ovs_mutex_unlock(revalidator->mutex);
 }
 
 static void
@@ -1597,7 +1603,6 @@ revalidate(struct revalidator *revalidator)
 static bool
 handle_missed_revalidation(struct revalidator *revalidator,
                            struct udpif_key *ukey)
-    OVS_NO_THREAD_SAFETY_ANALYSIS
 {
     struct udpif *udpif = revalidator->udpif;
     struct dpif_flow flow;
@@ -1609,7 +1614,9 @@ handle_missed_revalidation(struct revalidator *revalidator,
 
     ofpbuf_use_stub(&buf, &stub, sizeof stub);
     if (!dpif_flow_get(udpif->dpif, ukey->key, ukey->key_len, &buf, &flow)) {
+        ovs_mutex_lock(&ukey->mutex);
         keep = revalidate_ukey(udpif, ukey, &flow);
+        ovs_mutex_unlock(&ukey->mutex);
     }
     ofpbuf_uninit(&buf);
 
@@ -1618,7 +1625,6 @@ handle_missed_revalidation(struct revalidator *revalidator,
 
 static void
 revalidator_sweep__(struct revalidator *revalidator, bool purge)
-    OVS_NO_THREAD_SAFETY_ANALYSIS
 {
     struct dump_op ops[REVALIDATE_MAX_BATCH];
     struct udpif_key *ukey;
@@ -1628,13 +1634,18 @@ revalidator_sweep__(struct revalidator *revalidator, bool purge)
     n_ops = 0;
     dump_seq = seq_read(revalidator->udpif->dump_seq);
 
-    /* During garbage collection, this revalidator completely owns its ukeys
-     * map, and therefore doesn't need to do any locking. */
     CMAP_FOR_EACH(ukey, cmap_node, revalidator->ukeys) {
-        if (ukey->flow_exists
+        bool flow_exists, seq_mismatch;
+
+        ovs_mutex_lock(&ukey->mutex);
+        flow_exists = ukey->flow_exists;
+        seq_mismatch = (ukey->dump_seq != dump_seq
+                        && revalidator->udpif->need_revalidate);
+        ovs_mutex_unlock(&ukey->mutex);
+
+        if (flow_exists
             && (purge
-                || (ukey->dump_seq != dump_seq
-                    && revalidator->udpif->need_revalidate
+                || (seq_mismatch
                     && !handle_missed_revalidation(revalidator, ukey)))) {
             struct dump_op *op = &ops[n_ops++];
 
@@ -1643,8 +1654,10 @@ revalidator_sweep__(struct revalidator *revalidator, bool purge)
                 push_dump_ops(revalidator, ops, n_ops);
                 n_ops = 0;
             }
-        } else if (!ukey->flow_exists) {
+        } else if (!flow_exists) {
+            ovs_mutex_lock(revalidator->mutex);
             ukey_delete(revalidator, ukey);
+            ovs_mutex_unlock(revalidator->mutex);
         }
     }
 
-- 
1.7.10.4




More information about the dev mailing list