Changeset 90608 in vbox

Timestamp:

Aug 10, 2021 9:33:32 PM (3 years ago)

Author:

vboxsync

Message:

VMM/PDMCritSecRw: Some code refactoring. bugref:6695

Location:

trunk/src/VBox/VMM/VMMAll

Files:

• PDMAllCritSect.cpp (modified) (1 diff)
• PDMAllCritSectRw.cpp (modified) (5 diffs)

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

@@ -508 +508 @@
      * account when waiting on contended locks.
      *
-     * While we usually (it can be VINF_SUCCESS) have to option via the rcBusy
-     * parameter of going to back to ring-3 and to re-start the work there, it's
-     * almost always more efficient to try wait for the lock here.  The rcBusy
-     * will be used if we encounter an VERR_INTERRUPTED situation though.
-     *
-     * We must never block if VMMRZCallRing3Disable is active.
+     * While we usually (it can be VINF_SUCCESS) have the option of returning
+     * rcBusy and force the caller to go back to ring-3 and to re-start the work
+     * there, it's almost always more efficient to try wait for the lock here.
+     * The rcBusy will be used if we encounter an VERR_INTERRUPTED situation
+     * though.
      */
     PVMCPUCC pVCpu = VMMGetCpu(pVM);

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

@@ -734 +734 @@
 
 /**
+ * Worker for pdmCritSectRwEnterExcl that bails out on wait failure.
+ *
+ * @returns @a rc unless corrupted.
+ * @param   pVM         The cross context VM structure.
+ * @param   rc          The status to return.
+ */
+DECL_NO_INLINE(static, int) pdmCritSectRwEnterExclBailOut(PPDMCRITSECTRW pThis, int rc)
+{
+    /*
+     * Decrement the counts and return the error.
+     */
+    for (;;)
+    {
+        uint64_t       u64State    = ASMAtomicReadU64(&pThis->s.Core.u64State);
+        uint64_t const u64OldState = u64State;
+        uint64_t c                 = (u64State & RTCSRW_CNT_WR_MASK) >> RTCSRW_CNT_WR_SHIFT;
+        AssertReturn(c > 0, pdmCritSectRwCorrupted(pThis, "Invalid write count on bailout"));
+        c--;
+        u64State &= ~RTCSRW_CNT_WR_MASK;
+        u64State |= c << RTCSRW_CNT_WR_SHIFT;
+        if (ASMAtomicCmpXchgU64(&pThis->s.Core.u64State, u64State, u64OldState))
+            return rc;
+
+        ASMNopPause();
+        AssertReturn(pThis->s.Core.u32Magic == RTCRITSECTRW_MAGIC, VERR_SEM_DESTROYED);
+        ASMNopPause();
+    }
+}
+
+
+/**
  * Worker that enters a read/write critical section with exclusive access.
  *
@@ -879 +910 @@
            )
         {
-
             /*
              * Wait for our turn.
@@ -918 +948 @@
                 }
                 if (RT_FAILURE(rc))
-                {
-                    /* Decrement the counts and return the error. */
-                    for (;;)
-                    {
-                        u64OldState = u64State = ASMAtomicReadU64(&pThis->s.Core.u64State);
-                        uint64_t c = (u64State & RTCSRW_CNT_WR_MASK) >> RTCSRW_CNT_WR_SHIFT;
-                        AssertReturn(c > 0, pdmCritSectRwCorrupted(pThis, "Invalid write count on bailout"));
-                        c--;
-                        u64State &= ~RTCSRW_CNT_WR_MASK;
-                        u64State |= c << RTCSRW_CNT_WR_SHIFT;
-                        if (ASMAtomicCmpXchgU64(&pThis->s.Core.u64State, u64State, u64OldState))
-                            break;
-                    }
-                    return rc;
-                }
-
+                    return pdmCritSectRwEnterExclBailOut(pThis, rc);
+
+                /* Try take exclusive write ownership. */
                 u64State = ASMAtomicReadU64(&pThis->s.Core.u64State);
                 if ((u64State & RTCSRW_DIR_MASK) == (RTCSRW_DIR_WRITE << RTCSRW_DIR_SHIFT))
@@ -950 +967 @@
 #ifdef IN_RING3
             /* TryEnter call - decrement the number of (waiting) writers.  */
+            return pdmCritSectRwEnterExclBailOut(pThis, VERR_SEM_BUSY);
 #else
             /* We cannot call SUPSemEventWaitNoResume in this context. Go back to
                ring-3 and do it there or return rcBusy. */
-#endif
-
-            for (;;)
-            {
-                u64OldState = u64State = ASMAtomicReadU64(&pThis->s.Core.u64State);
-                uint64_t c = (u64State & RTCSRW_CNT_WR_MASK) >> RTCSRW_CNT_WR_SHIFT;
-                AssertReturn(c > 0, pdmCritSectRwCorrupted(pThis, "Invalid write count on bailout"));
-                c--;
-                u64State &= ~RTCSRW_CNT_WR_MASK;
-                u64State |= c << RTCSRW_CNT_WR_SHIFT;
-                if (ASMAtomicCmpXchgU64(&pThis->s.Core.u64State, u64State, u64OldState))
-                    break;
-            }
-
-#ifdef IN_RING3
-            return VERR_SEM_BUSY;
-#else
+            rcBusy = pdmCritSectRwEnterExclBailOut(pThis, rcBusy);
             if (rcBusy == VINF_SUCCESS)
             {
@@ -987 +989 @@
      */
     Assert((ASMAtomicReadU64(&pThis->s.Core.u64State) & RTCSRW_DIR_MASK) == (RTCSRW_DIR_WRITE << RTCSRW_DIR_SHIFT));
+#if 1 /** @todo consider generating less noise... */
     ASMAtomicWriteU32(&pThis->s.Core.cWriteRecursions, 1);
+#else
+    pThis->s.Core.cWriteRecursions = 1;
+#endif
     Assert(pThis->s.Core.cWriterReads == 0);
 #if defined(PDMCRITSECTRW_STRICT) && defined(IN_RING3)