Thx a lot for the fix, that was a bad move~ Acked-by: Alex Wang <al...@nicira.com>
On Mon, Sep 29, 2014 at 3:09 AM, Joe Stringer <joestrin...@nicira.com> wrote: > 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 a transaction returns TXN_INCOMPLETE, then there are > no subsequent changes to connectivity or 'status_txn_try_again'. > ovsdb_idl_run() processes the transaction's reply and updates the status > to TXN_SUCCESS. The logic in status_update_wait() wakes up the main > thread instantly so that the transaction can be cleaned up, however the > logic to clean it up in run_status_update() is inaccessible until the > next connectivity change. This happens every time through the main loop, > causing the main thread to spin at 100%. > > This patch fixes the behaviour by ensuring that ovsdb_idl_txn_commit() > gets a chance to run whenever there is an ongoing status transaction. > > Steps to reproduce: Unload/Reload kernel module && restart ovs-vswitchd. > > Signed-off-by: Joe Stringer <joestrin...@nicira.com> > --- > I considered implementing this fix using ovsdb_idl_get_seqno() for the > entire logic block, but concluded that it doesn't make sense to update > the status whenever there is any kind of status change to any ongoing > transaction. Rather, it makes sense that if there is a status > transaction outstanding, to finish that transaction and clean it up when > it is done. > --- > vswitchd/bridge.c | 18 +++++++++++------- > 1 file changed, 11 insertions(+), 7 deletions(-) > > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index 5ba8d64..61896d3 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -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) > } > } > } > + } > + > + /* Make progress on the transaction. If it finishes, then destroy the > + * transaction. Otherwise, keep it so that we can check progress > during > + * the next run through the main loop. */ > + if (status_txn) { > + enum ovsdb_idl_txn_status status; > > status = ovsdb_idl_txn_commit(status_txn); > if (status != TXN_INCOMPLETE) { > -- > 1.7.10.4 > > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev