From e060b80dfb3f1a52d3c0b7399a2fcdba83b6f876 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Thu, 9 Oct 2014 08:23:37 +0000 Subject: [PATCH] ovs-vswitchd: Fix high cpu utilization when acquire idl lock fails. When ovs-vswitchd fails to acquire the ovsdb idl lock (either due to contention or due to invalid database path), ovs-vswitchd will spin on the global connectivity sequence number and consume 100% cpu. This is in that the local copy is different to the global sequence number and never updated when ovsdb idl is not locked. To fix this issue, this commit makes ovs-vswitchd not checking the global connectivity sequence number in that situation. Reported-by: Ben Pfaff Signed-off-by: Alex Wang Acked-by: Ben Pfaff --- tests/ovs-vswitchd.at | 24 ++++++++++++++++++++++++ vswitchd/bridge.c | 24 +++++++++++++----------- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/tests/ovs-vswitchd.at b/tests/ovs-vswitchd.at index d3120f7c4..e82c868c9 100644 --- a/tests/ovs-vswitchd.at +++ b/tests/ovs-vswitchd.at @@ -129,3 +129,27 @@ AT_CHECK([sed -n " # cleanup. kill `cat ovsdb-server.pid` AT_CLEANUP + +dnl ---------------------------------------------------------------------- +AT_SETUP([ovs-vswitchd -- invalid database path]) + +# start an ovs-vswitchd process with invalid db path. +ovs-vswitchd unix:invalid.db.sock --log-file=fakelog --enable-dummy --unixctl="`pwd`"/unixctl & + +# sleep for a while. +sleep 10 + +# stop the process. +ovs-appctl -t `pwd`/unixctl exit + +# should not see this log (which indicates high cpu utilization). +AT_CHECK([grep "wakeup due to" fakelog], [ignore]) + +# check the fakelog, should not see any WARN/ERR/EMER log. +AT_CHECK([sed -n " +/|WARN|/p +/|ERR|/p +/|EMER|/p" fakelog +]) + +AT_CLEANUP diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 69d439637..6cd30b8e8 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -2311,9 +2311,6 @@ bridge_run(void) * with the current situation of multiple ovs-vswitchd daemons, * disable system stats collection. */ system_stats_enable(false); - /* This prevents the process from constantly waking up on - * connectivity seq. */ - connectivity_seqno = seq_read(connectivity_seq_get()); return; } else if (!ovsdb_idl_has_lock(idl)) { return; @@ -2528,14 +2525,19 @@ bridge_wait(void) poll_timer_wait_until(stats_timer); } - /* If the status database transaction is 'TXN_INCOMPLETE' or is - * unsuccessful, register a timeout in 'STATUS_CHECK_AGAIN_MSEC'. Else, - * wait on the global connectivity sequence number. Note, this also helps - * batch multiple status changes into one transaction. */ - if (force_status_commit) { - poll_timer_wait_until(time_msec() + STATUS_CHECK_AGAIN_MSEC); - } else { - seq_wait(connectivity_seq_get(), connectivity_seqno); + /* This prevents the process from constantly waking up on + * connectivity seq, when there is no connection to ovsdb. */ + if (ovsdb_idl_has_lock(idl)) { + /* If the status database transaction is 'TXN_INCOMPLETE' or is + * unsuccessful, register a timeout in 'STATUS_CHECK_AGAIN_MSEC'. + * Else, wait on the global connectivity sequence number. Note, + * this also helps batch multiple status changes into one + * transaction. */ + if (force_status_commit) { + poll_timer_wait_until(time_msec() + STATUS_CHECK_AGAIN_MSEC); + } else { + seq_wait(connectivity_seq_get(), connectivity_seqno); + } } system_stats_wait(); -- 2.20.1