[ovs-dev] [PATCHv5 10/12] revalidator: Reduce ukey contention.
Joe Stringer
joestringer at nicira.com
Mon Sep 15 02:25:16 UTC 2014
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 <joestringer at nicira.com>
---
v5: No change.
v4: Use atomic_read_relaxed().
v2-v3: 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 947a225..1333d41 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -206,14 +206,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.
@@ -1267,6 +1270,7 @@ ukey_new(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;
@@ -1310,6 +1314,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);
}
@@ -1325,6 +1330,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_relaxed(&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.
*
@@ -1345,7 +1366,7 @@ ukey_acquire(struct udpif *udpif, const struct dpif_flow *flow,
ukey = ukey_lookup(udpif, &flow->uid);
if (ukey) {
- retval = ovs_mutex_trylock(&ukey->mutex);
+ retval = ukey_trylock(ukey);
} else {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 60);
struct ds ds = DS_EMPTY_INITIALIZER;
@@ -1817,7 +1838,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
More information about the dev
mailing list