[ovs-dev] [threaded-learning v2 18/25] ofproto: Remove ->timeout_mutex from struct rule (just use ->mutex).
Ben Pfaff
blp at nicira.com
Thu Sep 12 07:39:30 UTC 2013
I think that ->timeout_mutex and ->mutex were separate because the latter
(which was actually a rwlock named ->rwlock until recently) was held for
long periods of time, which meant that having a separate ->timeout_mutex
reduced lock contention. But ->mutex is now only held briefly, so it seems
reasonable to just use it.
Signed-off-by: Ben Pfaff <blp at nicira.com>
Acked-by: Ethan Jackson <ethan at nicira.com>
---
ofproto/connmgr.c | 4 ++--
ofproto/ofproto-dpif.c | 4 ++--
ofproto/ofproto-provider.h | 3 +--
ofproto/ofproto.c | 50 +++++++++++++++++++++-----------------------
4 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 2172789..0b6d7e8 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1897,10 +1897,10 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
fu.match = &match;
fu.priority = rule->cr.priority;
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_lock(&rule->mutex);
fu.idle_timeout = rule->idle_timeout;
fu.hard_timeout = rule->hard_timeout;
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
if (flags & NXFMF_ACTIONS) {
fu.ofpacts = rule->actions->ofpacts;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f5f91dd..81adb0a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3914,10 +3914,10 @@ rule_expire(struct rule_dpif *rule)
ovs_assert(!rule->up.pending);
- ovs_mutex_lock(&rule->up.timeout_mutex);
+ ovs_mutex_lock(&rule->up.mutex);
hard_timeout = rule->up.hard_timeout;
idle_timeout = rule->up.idle_timeout;
- ovs_mutex_unlock(&rule->up.timeout_mutex);
+ ovs_mutex_unlock(&rule->up.mutex);
/* Has 'rule' expired? */
now = time_msec();
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 4e5b0bf..f62b57a 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -243,7 +243,6 @@ struct rule {
uint8_t table_id; /* Index in ofproto's 'tables' array. */
enum ofputil_flow_mod_flags flags;
- struct ovs_mutex timeout_mutex;
uint16_t hard_timeout OVS_GUARDED; /* In seconds from ->modified. */
uint16_t idle_timeout OVS_GUARDED; /* In seconds from ->used. */
@@ -342,7 +341,7 @@ void ofproto_rule_delete(struct ofproto *, struct classifier *cls,
struct rule *) OVS_REQ_WRLOCK(cls->rwlock);
void ofproto_rule_reduce_timeouts(struct rule *rule, uint16_t idle_timeout,
uint16_t hard_timeout)
- OVS_EXCLUDED(ofproto_mutex, rule->timeout_mutex);
+ OVS_EXCLUDED(ofproto_mutex, rule->mutex);
bool ofproto_rule_has_out_port(const struct rule *, ofp_port_t out_port);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 8473559..6222899 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2379,7 +2379,6 @@ ofproto_rule_destroy__(struct rule *rule)
{
cls_rule_destroy(&rule->cr);
rule_actions_unref(rule->actions);
- ovs_mutex_destroy(&rule->timeout_mutex);
ovs_mutex_destroy(&rule->mutex);
rule->ofproto->ofproto_class->rule_dealloc(rule);
}
@@ -3340,10 +3339,10 @@ handle_flow_stats_request(struct ofconn *ofconn,
fs.ofpacts = rule->actions->ofpacts;
fs.ofpacts_len = rule->actions->ofpacts_len;
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_lock(&rule->mutex);
fs.idle_timeout = rule->idle_timeout;
fs.hard_timeout = rule->hard_timeout;
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
fs.flags = rule->flags;
@@ -3767,11 +3766,11 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
rule->flow_cookie = fm->new_cookie;
rule->created = rule->modified = rule->used = time_msec();
- ovs_mutex_init(&rule->timeout_mutex);
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_init(&rule->mutex);
+ ovs_mutex_lock(&rule->mutex);
rule->idle_timeout = fm->idle_timeout;
rule->hard_timeout = fm->hard_timeout;
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
rule->table_id = table - ofproto->tables;
rule->flags = fm->flags & OFPUTIL_FF_STATE;
@@ -3782,7 +3781,6 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
rule->monitor_flags = 0;
rule->add_seqno = 0;
rule->modify_seqno = 0;
- ovs_mutex_init(&rule->mutex);
/* Construct rule, initializing derived state. */
error = ofproto->ofproto_class->rule_construct(rule);
@@ -3856,10 +3854,10 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
ofproto_rule_change_cookie(ofproto, rule, fm->new_cookie);
}
if (type == OFOPERATION_REPLACE) {
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_lock(&rule->mutex);
rule->idle_timeout = fm->idle_timeout;
rule->hard_timeout = fm->hard_timeout;
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
rule->flags = fm->flags & OFPUTIL_FF_STATE;
if (fm->idle_timeout || fm->hard_timeout) {
@@ -4068,10 +4066,10 @@ ofproto_rule_send_removed(struct rule *rule, uint8_t reason)
fr.table_id = rule->table_id;
calc_duration(rule->created, time_msec(),
&fr.duration_sec, &fr.duration_nsec);
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_lock(&rule->mutex);
fr.idle_timeout = rule->idle_timeout;
fr.hard_timeout = rule->hard_timeout;
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
rule->ofproto->ofproto_class->rule_get_stats(rule, &fr.packet_count,
&fr.byte_count);
@@ -4120,7 +4118,7 @@ reduce_timeout(uint16_t max, uint16_t *timeout)
void
ofproto_rule_reduce_timeouts(struct rule *rule,
uint16_t idle_timeout, uint16_t hard_timeout)
- OVS_EXCLUDED(ofproto_mutex, rule->timeout_mutex)
+ OVS_EXCLUDED(ofproto_mutex, rule->mutex)
{
if (!idle_timeout && !hard_timeout) {
return;
@@ -4132,10 +4130,10 @@ ofproto_rule_reduce_timeouts(struct rule *rule,
}
ovs_mutex_unlock(&ofproto_mutex);
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_lock(&rule->mutex);
reduce_timeout(idle_timeout, &rule->idle_timeout);
reduce_timeout(hard_timeout, &rule->hard_timeout);
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
}
static enum ofperr
@@ -4434,10 +4432,10 @@ ofproto_compose_flow_refresh_update(const struct rule *rule,
fu.event = (flags & (NXFMF_INITIAL | NXFMF_ADD)
? NXFME_ADDED : NXFME_MODIFIED);
fu.reason = 0;
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_lock(&rule->mutex);
fu.idle_timeout = rule->idle_timeout;
fu.hard_timeout = rule->hard_timeout;
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
fu.table_id = rule->table_id;
fu.cookie = rule->flow_cookie;
minimatch_expand(&rule->cr.match, &match);
@@ -5787,10 +5785,10 @@ ofopgroup_complete(struct ofopgroup *group)
}
} else {
ofproto_rule_change_cookie(ofproto, rule, op->flow_cookie);
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_lock(&rule->mutex);
rule->idle_timeout = op->idle_timeout;
rule->hard_timeout = op->hard_timeout;
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
if (op->actions) {
struct rule_actions *old_actions;
@@ -5856,10 +5854,10 @@ ofoperation_create(struct ofopgroup *group, struct rule *rule,
op->type = type;
op->reason = reason;
op->flow_cookie = rule->flow_cookie;
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_lock(&rule->mutex);
op->idle_timeout = rule->idle_timeout;
op->hard_timeout = rule->hard_timeout;
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
op->flags = rule->flags;
group->n_running++;
@@ -6171,7 +6169,7 @@ rule_eviction_priority(struct rule *rule)
uint32_t expiration_offset;
/* Calculate time of expiration. */
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_lock(&rule->mutex);
hard_expiration = (rule->hard_timeout
? rule->modified + rule->hard_timeout * 1000
: LLONG_MAX);
@@ -6179,7 +6177,7 @@ rule_eviction_priority(struct rule *rule)
? rule->used + rule->idle_timeout * 1000
: LLONG_MAX);
expiration = MIN(hard_expiration, idle_expiration);
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
if (expiration == LLONG_MAX) {
return 0;
}
@@ -6208,9 +6206,9 @@ eviction_group_add_rule(struct rule *rule)
struct oftable *table = &ofproto->tables[rule->table_id];
bool has_timeout;
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_lock(&rule->mutex);
has_timeout = rule->hard_timeout || rule->idle_timeout;
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
if (table->eviction_fields && has_timeout) {
struct eviction_group *evg;
@@ -6376,9 +6374,9 @@ oftable_insert_rule(struct rule *rule)
struct oftable *table = &ofproto->tables[rule->table_id];
bool may_expire;
- ovs_mutex_lock(&rule->timeout_mutex);
+ ovs_mutex_lock(&rule->mutex);
may_expire = rule->hard_timeout || rule->idle_timeout;
- ovs_mutex_unlock(&rule->timeout_mutex);
+ ovs_mutex_unlock(&rule->mutex);
if (may_expire) {
ovs_mutex_lock(&ofproto_mutex);
--
1.7.10.4
More information about the dev
mailing list