commit 69c18c8530ab7cfb81732a5106b704170d51fde7
author Suraj Malhotra <surajmalhotra@google.com> Fri Feb 15 13:48:06 2019 -0800
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Fri Feb 15 22:42:52 2019 +0000
tree ca223ed32aab59e64130f6336d43db20eca370ab
parent 95077ae625eaec966693a3921f3b2dff87041597

[nand] Fix race where we miss wakeups.

Tested: runtests -t nand-unittest
Change-Id: Ibd986138813d07d8001151ea1b0bb0ccdec8f2b0

zircon/system/dev/nand/nand/nand.cpp

diff --git a/zircon/system/dev/nand/nand/nand.cpp b/zircon/system/dev/nand/nand/nand.cpp
index b692d903..5a5d150 100644
--- a/zircon/system/dev/nand/nand/nand.cpp
+++ b/zircon/system/dev/nand/nand/nand.cpp
@@ -190,23 +190,16 @@
 // Initialization is complete by the time the thread starts.
 zx_status_t NandDevice::WorkerThread() {
     for (;;) {
-        // Don't loop until txn_queue_ is empty to check for shutdown.
-        // between each io.
-        for (auto txn = txn_queue_.pop(); txn; txn = txn_queue_.pop()) {
-            {
-                fbl::AutoLock al(&lock_);
-                if (shutdown_) {
-                    break;
-                }
-            }
-            DoIo(*std::move(txn));
-        }
-
         fbl::AutoLock al(&lock_);
+        while (txn_queue_.is_empty() && !shutdown_) {
+            worker_event_.Wait(&lock_);
+        }
         if (shutdown_) {
             break;
         }
-        worker_event_.Wait(&lock_);
+        auto txn = txn_queue_.pop();
+        al.release();
+        DoIo(*std::move(txn));
     }
 
     zxlogf(TRACE, "nand: worker thread terminated\n");
@@ -257,6 +250,7 @@
     }
 
     // TODO: UPDATE STATS HERE.
+    fbl::AutoLock al(&lock_);
     txn_queue_.push(std::move(txn));
     worker_event_.Signal();
 }
@@ -276,11 +270,12 @@
     {
         fbl::AutoLock al(&lock_);
         shutdown_ = true;
+        worker_event_.Signal();
     }
-    worker_event_.Signal();
     thrd_join(worker_thread_, nullptr);
 
     // Error out all pending requests.
+    fbl::AutoLock al(&lock_);
     txn_queue_.Release();
 }
 

zircon/system/dev/nand/nand/nand.h

diff --git a/zircon/system/dev/nand/nand/nand.h b/zircon/system/dev/nand/nand/nand.h
index 4bba55f..b286c2e 100644
--- a/zircon/system/dev/nand/nand/nand.h
+++ b/zircon/system/dev/nand/nand/nand.h
@@ -79,10 +79,10 @@
     uint32_t num_nand_pages_;
 
     thrd_t worker_thread_;
-    nand::UnownedOperationQueue<> txn_queue_;
 
     fbl::Mutex lock_;
-    fbl::ConditionVariable worker_event_;
+    nand::UnownedOperationQueue<> txn_queue_ TA_GUARDED(lock_);
+    fbl::ConditionVariable worker_event_ TA_GUARDED(lock_);
     bool shutdown_ TA_GUARDED(lock_) = false;
 };