bridge: Fix bug where IDL wakeup causes 100% CPU.
authorJoe Stringer <joestringer@nicira.com>
Mon, 29 Sep 2014 18:09:57 +0000 (18:09 +0000)
committerJoe Stringer <joestringer@nicira.com>
Mon, 29 Sep 2014 18:52:08 +0000 (07:52 +1300)
Commit 9c537baf613a16e (bridge: Refactor the stats and status update.)
inadvertently changed the way that the status transaction is destroyed,
which could cause the main thread to constantly wake up.

The bug occurs when ovsdb_idl_txn_commit() returns TXN_INCOMPLETE and
there are no further changes to connectivity or 'status_txn_try_again'.

- ovsdb_idl_run() receives the transaction reply and updates the
  transaction status to TXN_SUCCESS.
- status_update_wait() detects that the transaction is in progress, and
  the status is TXN_SUCCESS so wakes up the main thread immediately.
- run_status_update() is meant to destroy the transaction now that it is
  finished, however the logic is never run because there were no changes.
- Repeat the wakeup every time the main loop runs.

This patch fixes the behaviour by ensuring that ovsdb_idl_txn_commit()
gets a chance to run whenever there is an ongoing status transaction.

Bug was found by unloading and reloading the kernel module, then
immediately restarting ovs-vswitchd.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
vswitchd/bridge.c

index 5ba8d64..99dd21f 100644 (file)
@@ -2656,16 +2656,13 @@ run_stats_update(void)
 static void
 run_status_update(void)
 {
-    uint64_t seq;
-
-    /* Check the need to update status. */
-    seq = seq_read(connectivity_seq_get());
-    if (seq != connectivity_seqno || status_txn_try_again) {
-        enum ovsdb_idl_txn_status status;
+    if (!status_txn) {
+        uint64_t seq;
 
         /* Rate limit the update.  Do not start a new update if the
          * previous one is not done. */
-        if (!status_txn) {
+        seq = seq_read(connectivity_seq_get());
+        if (seq != connectivity_seqno || status_txn_try_again) {
             struct bridge *br;
 
             connectivity_seqno = seq;
@@ -2687,6 +2684,13 @@ run_status_update(void)
                 }
             }
         }
+    }
+
+    /* Commit the transaction and get the status. If the transaction finishes,
+     * then destroy the transaction. Otherwise, keep it so that we can check
+     * progress the next time that this function is called. */
+    if (status_txn) {
+        enum ovsdb_idl_txn_status status;
 
         status = ovsdb_idl_txn_commit(status_txn);
         if (status != TXN_INCOMPLETE) {