Changeset 90531 in vbox for trunk/src/VBox/VMM

Timestamp:

Aug 5, 2021 8:54:46 PM (4 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

146137

Message:

VMM/PDMCritSect: When VMCPU_FF_PDM_CRITSECT is set we must not wait on any lock if we can help it as we might be sitting on a lock order violation. Corrected incorrect NIL_RTNATIVETHREAD test in the leave function. Logging. bugref:6695

File:

• trunk/src/VBox/VMM/VMMAll/PDMAllCritSect.cpp (modified) (11 diffs)

trunk/src/VBox/VMM/VMMAll/PDMAllCritSect.cpp

@@ -114 +114 @@
 DECL_FORCE_INLINE(int) pdmCritSectEnterFirst(PPDMCRITSECT pCritSect, RTNATIVETHREAD hNativeSelf, PCRTLOCKVALSRCPOS pSrcPos)
 {
+    Assert(hNativeSelf != NIL_RTNATIVETHREAD);
     AssertMsg(pCritSect->s.Core.NativeThreadOwner == NIL_RTNATIVETHREAD, ("NativeThreadOwner=%p\n", pCritSect->s.Core.NativeThreadOwner));
     Assert(!(pCritSect->s.Core.fFlags & PDMCRITSECT_FLAGS_PENDING_UNLOCK));
@@ -130 +131 @@
     NOREF(pSrcPos);
 # endif
+    if (pSrcPos)
+        Log12Func(("%p: uId=%p ln=%u fn=%s\n", pCritSect, pSrcPos->uId, pSrcPos->uLine, pSrcPos->pszFunction));
+    else
+        Log12Func(("%p\n", pCritSect));
 
     STAM_PROFILE_ADV_START(&pCritSect->s.StatLocked, l);
@@ -154 +159 @@
                                          PCRTLOCKVALSRCPOS pSrcPos, int rcBusy)
 {
+# ifdef IN_RING0
+    /*
+     * If we've got queued critical section leave operations and rcBusy isn't
+     * VINF_SUCCESS, return to ring-3 immediately to avoid deadlocks.
+     */
+    if (   !pVCpu
+        || !VMCPU_FF_IS_SET(pVCpu, VMCPU_FF_PDM_CRITSECT)
+        || rcBusy == VINF_SUCCESS )
+    { /* likely */ }
+    else
+    {
+        /** @todo statistics. */
+        STAM_REL_COUNTER_INC(&pCritSect->s.StatContentionRZLock);
+        return rcBusy;
+    }
+# endif
+
     /*
      * Start waiting.
@@ -221 +243 @@
                      ? SUPSemEventWaitNoResume(pSession, hEvent, cMsMaxOne)
                      : SUPSemEventWait(pSession, hEvent, cMsMaxOne);
+        Log11Func(("%p: rc=%Rrc %'RU64 ns (cMsMaxOne=%RU64 hOwner=%p)\n",
+                   pCritSect, rc, RTTimeNanoTS() - tsStart, cMsMaxOne, pCritSect->s.Core.NativeThreadOwner));
 # endif /* IN_RING0 */
 
@@ -319 +343 @@
                         }
                         cCmpXchgs++;
+                        if ((cCmpXchgs & 0xffff) == 0)
+                            Log11Func(("%p: cLockers=%d cCmpXchgs=%u (hOwner=%p)\n",
+                                       pCritSect, cLockers, cCmpXchgs, pCritSect->s.Core.NativeThreadOwner));
                         ASMNopPause();
                         continue;
@@ -446 +473 @@
 # endif
         ASMAtomicIncS32(&pCritSect->s.Core.cLockers);
+        Log12Func(("%p: cNestings=%d cLockers=%d\n", pCritSect, pCritSect->s.Core.cNestings, pCritSect->s.Core.cLockers));
         return VINF_SUCCESS;
     }
@@ -654 +682 @@
 # endif
         ASMAtomicIncS32(&pCritSect->s.Core.cLockers);
+        Log12Func(("%p: cNestings=%d cLockers=%d\n", pCritSect, pCritSect->s.Core.cNestings, pCritSect->s.Core.cLockers));
         return VINF_SUCCESS;
     }
@@ -781 +810 @@
      */
     RTNATIVETHREAD const hNativeSelf = pdmCritSectGetNativeSelf(pVM, pCritSect);
-    VMM_ASSERT_RELEASE_MSG_RETURN(pVM, pCritSect->s.Core.NativeThreadOwner == hNativeSelf || hNativeSelf == NIL_RTNATIVETHREAD,
+    VMM_ASSERT_RELEASE_MSG_RETURN(pVM, pCritSect->s.Core.NativeThreadOwner == hNativeSelf && hNativeSelf != NIL_RTNATIVETHREAD,
                                   ("%p %s: %p != %p; cLockers=%d cNestings=%d\n", pCritSect, R3STRING(pCritSect->s.pszName),
                                    pCritSect->s.Core.NativeThreadOwner, hNativeSelf,
@@ -799 +828 @@
         ASMAtomicWriteS32(&pCritSect->s.Core.cNestings, cNestings - 1);
 #endif
-        ASMAtomicDecS32(&pCritSect->s.Core.cLockers);
-        Assert(pCritSect->s.Core.cLockers >= 0);
+        int32_t const cLockers = ASMAtomicDecS32(&pCritSect->s.Core.cLockers);
+        Assert(cLockers >= 0); RT_NOREF(cLockers);
+        Log12Func(("%p: cNestings=%d cLockers=%d\n", pCritSect, cNestings - 1, cLockers));
         return VINF_SEM_NESTED;
     }
 
+    Log12Func(("%p: cNestings=%d cLockers=%d hOwner=%p - leave for real\n",
+               pCritSect, cNestings, pCritSect->s.Core.cLockers, pCritSect->s.Core.NativeThreadOwner));
 
 #ifdef IN_RING3
@@ -839 +871 @@
         /* Someone is waiting, wake up one of them. */
         Assert(cLockers < _8K);
+        Log8(("PDMCritSectLeave: Waking up %p (cLockers=%u)\n", pCritSect, cLockers));
         SUPSEMEVENT hEvent = (SUPSEMEVENT)pCritSect->s.Core.EventSem;
         int rc = SUPSemEventSignal(pVM->pSession, hEvent);
@@ -849 +882 @@
     else
     {
-        Log8(("Signalling %#p\n", hEventToSignal));
+        Log8(("PDMCritSectLeave: Signalling %#p (%p)\n", hEventToSignal, pCritSect));
         int rc = SUPSemEventSignal(pVM->pSession, hEventToSignal);
         AssertRC(rc);