.

wcfs/client/wcfs.cpp

@@ -380,6 +380,31 @@ error _Conn::__pin1(PinReq *req) {
     FileH f;
     bool  ok;
 
+    // lock virtmem first.
+    //
+    // The reason we do it here instead of closely around call to
+    // mmap->_remmapblk() is to avoid deadlocks: virtmem calls FileH.mmap,
+    // Mapping.remmap_blk and Mapping.unmap under virt_lock locked. In those
+    // functions the order of locks is
+    //
+    //      virt_lock, wconn.atMu.R, fileh.mu
+    //
+    // So if we take virt_lock right around mmap._remmapblk(), the order of
+    // locks in pinner would be
+    //
+    //      wconn.atMu.R, wconn.filehMu.R, f.mu, virt_lock
+    //
+    // which means there is AB-BA deadlock possibility.
+    //
+    // TODO try to take virt_lock only around virtmem-associated VMAs and with
+    // better granularity. NOTE it is possible to teach virtmem to call
+    // FileH.mmap and Mapping.unmap without virtmem locked. However reworking
+    // virtmem to call Mapping.remmap_blk without virt_lock is not so easy.
+    virt_lock();
+    defer([&]() {
+        virt_unlock();
+    });
+
     wconn._atMu.RLock();
     defer([&]() {
         wconn._atMu.RUnlock();
@@ -426,18 +451,10 @@ error _Conn::__pin1(PinReq *req) {
         error err;
         if (mmap->vma != nil) {
             mmap->_assertVMAOk();
-            // XXX recheck wrt Mapping.unmap: there it locks:
-            // 1. virt_lock
-            // 2. wconn.atMu.R
-            // 3. f.mu
-            //
-            // -> if here we do
-            // 1. wconn.atMu.R
-            // 2. f.mu
-            // 3. virt_lock
-            //
-            // -> deadlock
-            virt_lock();
+
+            // see ^^^ about deadlock
+            //virt_lock();
+
             BigFileH *virt_fileh = mmap->vma->fileh;
             TODO (mmap->fileh->blksize != virt_fileh->ramh->ram->pagesize);
             do_pin = !__fileh_page_isdirty(virt_fileh, req->blk);
@@ -446,8 +463,9 @@ error _Conn::__pin1(PinReq *req) {
         if (do_pin)
             err = mmap->_remmapblk(req->blk, req->at);
 
-        if (mmap->vma != nil)
-            virt_unlock();
+        // see ^^^ about deadlock
+        //if (mmap->vma != nil)
+        //    virt_unlock();
 
         // on error don't need to continue with other mappings - all fileh and
         // all mappings become marked invalid on pinner failure.