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 <joestrin...@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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
