objectcacher: fix infinite loop in flusher_entry

The addition of accounting for simultaneous writers in
f3043fee3e22600cb4349072287842db129588eb could lead to an infinite
loop. This could happen because starting a flush doesn't decrease
dirty_waiters, only the number of dirty bytes (they change to tx). If
dirty_waiters is more than target_dirty, the loop would continue
forever while holding the object cacher lock, since the object cacher
lock is required by the write callbacks.

The extra while (!flusher_stop) loop is unnecessary, so remove it.
This means the flusher thread always releases the lock after starting
flushing, so progress can be made.

Signed-off-by: Sage Weil <sage@inktank.com>
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>

diff --git a/src/osdc/ObjectCacher.cc b/src/osdc/ObjectCacher.cc
index c9825f92205072323c2022a3653afe0de27e6ffe..ade79fb382f43d74fba91f4a2ea5af5ebe510d59 100644 (file)
--- a/src/osdc/ObjectCacher.cc
+++ b/src/osdc/ObjectCacher.cc
@@ -1181,40 +1181,38 @@ void ObjectCacher::flusher_entry()
   ldout(cct, 10) << "flusher start" << dendl;
   lock.Lock();
   while (!flusher_stop) {
-    while (!flusher_stop) {
-      loff_t all = get_stat_tx() + get_stat_rx() + get_stat_clean() + get_stat_dirty();
-      ldout(cct, 11) << "flusher "
-                    << all << " / " << max_size << ":  "
-                    << get_stat_tx() << " tx, "
-                    << get_stat_rx() << " rx, "
-                    << get_stat_clean() << " clean, "
-                    << get_stat_dirty() << " dirty ("
-                    << target_dirty << " target, "
-                    << max_dirty << " max)"
-                    << dendl;
-      if (get_stat_dirty() + get_stat_dirty_waiting() > target_dirty) {
-        // flush some dirty pages
-        ldout(cct, 10) << "flusher " 
-                      << get_stat_dirty() << " dirty + " << get_stat_dirty_waiting()
-                      << " dirty_waiting > target "
-                      << target_dirty
-                      << ", flushing some dirty bhs" << dendl;
-        flush(get_stat_dirty() + get_stat_dirty_waiting() - target_dirty);
-      }
-      else {
-        // check tail of lru for old dirty items
-        utime_t cutoff = ceph_clock_now(cct);
-       cutoff -= max_dirty_age;
-        BufferHead *bh = 0;
-        while ((bh = (BufferHead*)lru_dirty.lru_get_next_expire()) != 0 &&
-               bh->last_write < cutoff) {
-          ldout(cct, 10) << "flusher flushing aged dirty bh " << *bh << dendl;
-          bh_write(bh);
-        }
-        break;
+    loff_t all = get_stat_tx() + get_stat_rx() + get_stat_clean() + get_stat_dirty();
+    ldout(cct, 11) << "flusher "
+                  << all << " / " << max_size << ":  "
+                  << get_stat_tx() << " tx, "
+                  << get_stat_rx() << " rx, "
+                  << get_stat_clean() << " clean, "
+                  << get_stat_dirty() << " dirty ("
+                  << target_dirty << " target, "
+                  << max_dirty << " max)"
+                  << dendl;
+    loff_t actual = get_stat_dirty() + get_stat_dirty_waiting();
+    if (actual > target_dirty) {
+      // flush some dirty pages
+      ldout(cct, 10) << "flusher " 
+                    << get_stat_dirty() << " dirty + " << get_stat_dirty_waiting()
+                    << " dirty_waiting > target "
+                    << target_dirty
+                    << ", flushing some dirty bhs" << dendl;
+      flush(actual - target_dirty);
+    } else {
+      // check tail of lru for old dirty items
+      utime_t cutoff = ceph_clock_now(cct);
+      cutoff -= max_dirty_age;
+      BufferHead *bh = 0;
+      while ((bh = (BufferHead*)lru_dirty.lru_get_next_expire()) != 0 &&
+            bh->last_write < cutoff) {
+       ldout(cct, 10) << "flusher flushing aged dirty bh " << *bh << dendl;
+       bh_write(bh);
       }
     }
-    if (flusher_stop) break;
+    if (flusher_stop)
+      break;
     flusher_cond.WaitInterval(cct, lock, utime_t(1,0));
   }
   lock.Unlock();