When handler threads are installing ukeys, they hold the ukey mutex for
the duration of flow setup. If a revalidator thread dumps this flow
while the handler holds the lock, it will attempt to lock it using
ovs_mutex_trylock(), then if it cannot grab the lock, skip the flow.
Attempting to lock on a ukey that is currently locked is a common
occurrence when:
- There are many handler threads running, and
- Large numbers of small flows are passing through OVS.
In these cases, the act of attempting to lock the ukey is quite
expensive. This patch adds a flag 'installed' to the ukey, which is
raised when flow setup is complete. By checking this flag before
attempting to perform a trylock(), we can reduce lock contention between
handler and revalidator threads.
Signed-off-by: Joe Stringer <joestrin...@nicira.com>
---
v3: No change.
v2: No change.
---
ofproto/ofproto-dpif-upcall.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 81ea1c0..93c13ab 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -208,14 +208,17 @@ struct udpif_key {
struct ofpbuf *actions; /* Datapath flow actions as nlattrs. */
ovs_u128 uid; /* Unique flow identifier. */
uint32_t hash; /* Pre-computed hash for 'key'. */
+ atomic_bool installed; /* True if the datapath flow has been
+ installed and handlers are finished with
+ this ukey. Used to reduce contention. */
struct ovs_mutex mutex; /* Guards the following. */
struct dpif_flow_stats stats OVS_GUARDED; /* Last known stats.*/
long long int created OVS_GUARDED; /* Estimate of creation time. */
uint64_t dump_seq OVS_GUARDED; /* Tracks udpif->dump_seq. */
uint64_t reval_seq OVS_GUARDED; /* Tracks udpif->reval_seq. */
- bool flow_exists OVS_GUARDED; /* Ensures flows are only deleted
- once. */
+ bool flow_exists OVS_GUARDED; /* The flow is currently in the
+ datapath. */
struct xlate_cache *xcache OVS_GUARDED; /* Cache for xlate entries that
* are affected by this ukey.
@@ -1312,6 +1315,7 @@ ukey_new(struct udpif *udpif, struct upcall *upcall)
ukey->hash = get_uid_hash(&ukey->uid);
ukey->actions = ofpbuf_clone(&upcall->put_actions);
+ atomic_init(&ukey->installed, false);
ovs_mutex_init(&ukey->mutex);
ukey->dump_seq = upcall->dump_seq;
ukey->reval_seq = upcall->reval_seq;
@@ -1355,6 +1359,7 @@ ukey_install_finish(struct udpif_key *ukey, int error)
{
if (!error) {
ukey->flow_exists = true;
+ atomic_store(&ukey->installed, true);
}
ovs_mutex_unlock(&ukey->mutex);
}
@@ -1370,6 +1375,22 @@ ukey_install(struct udpif *udpif, struct udpif_key *ukey)
return true;
}
+/* Wrapper for ovs_mutex_trylock() which reduces contention between handler
+ * threads and revalidator threads by not bothering to try locking if the
+ * flow is still being processed. */
+static int
+ukey_trylock(struct udpif_key *ukey)
+ OVS_TRY_LOCK(0, ukey->mutex)
+{
+ bool installed;
+
+ atomic_read(&ukey->installed, &installed);
+ if (!installed) {
+ return EBUSY;
+ }
+ return ovs_mutex_trylock(&ukey->mutex);
+}
+
/* Searches for a ukey in 'udpif->ukeys' that matches 'flow' and attempts to
* lock the ukey.
*
@@ -1391,7 +1412,7 @@ ukey_acquire(struct udpif *udpif, const struct dpif_flow
*flow,
ukey = ukey_lookup(udpif, flow->uid, flow->uid_len,
flow->key, flow->key_len);
if (ukey) {
- retval = ovs_mutex_trylock(&ukey->mutex);
+ retval = ukey_trylock(ukey);
} else {
struct ds ds = DS_EMPTY_INITIALIZER;
ovs_u128 uid;
@@ -1848,7 +1869,7 @@ revalidator_sweep__(struct revalidator *revalidator, bool
purge)
/* Handler threads could be holding a ukey lock while it installs a
* new flow, so don't hang around waiting for access to it. */
- if (ovs_mutex_trylock(&ukey->mutex)) {
+ if (ukey_trylock(ukey)) {
continue;
}
flow_exists = ukey->flow_exists;
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
