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

Timestamp:

May 8, 2009 6:09:57 PM (16 years ago)

Author:

vboxsync

Message:

TMR3TimerDestroy: rewrite to ring-3 only.

Location:

trunk/src/VBox/VMM

Files:

• TM.cpp (modified) (1 diff)
• TMInternal.h (modified) (1 diff)
• VMMAll/TMAll.cpp (modified) (12 diffs)
• testcase/tstVMM.cpp (modified) (1 diff)

trunk/src/VBox/VMM/TM.cpp

@@ -1957 +1957 @@
 
         case TMTIMERSTATE_EXPIRED:
-        case TMTIMERSTATE_PENDING_DESTROY:
-        case TMTIMERSTATE_PENDING_STOP_DESTROY:
+        case TMTIMERSTATE_DESTROY:
         case TMTIMERSTATE_FREE:
             AssertMsgFailed(("Invalid timer state %d %s (%s)\n", pTimer->enmState, tmTimerState(pTimer->enmState), pTimer->pszDesc));

trunk/src/VBox/VMM/TMInternal.h

@@ -91 +91 @@
      * Currently in the ScheduleTimers list and the active list. */
     TMTIMERSTATE_PENDING_RESCHEDULE,
-    /** Timer is destroyed but needs to be replaced from the
-     * active to the free list.
-     * Currently in the ScheduleTimers list and the active list. */
-    TMTIMERSTATE_PENDING_STOP_DESTROY,
-    /** Timer is destroyed but needs moving to the free list.
-     * Currently in the ScheduleTimers list. */
-    TMTIMERSTATE_PENDING_DESTROY,
+    /** Timer is being destroyed. */
+    TMTIMERSTATE_DESTROY,
     /** Timer is free. */
     TMTIMERSTATE_FREE

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

@@ -544 +544 @@
  * @param   pTimer          Timer handle as returned by one of the create functions.
  */
-VMMR3DECL(int) TMR3TimerDestroy(PTMTIMER pTimer)
-{
-    int cRetries = 1000;
-    do
+VMMR3DECL(int) TMR3TimerDestroy(TMTIMER *pTimer)
+{
+    /*
+     * Be extra careful here.
+     */
+    if (!pTimer)
+        return VINF_SUCCESS;
+    AssertPtr(pTimer);
+    Assert((unsigned)pTimer->enmClock < (unsigned)TMCLOCK_MAX);
+
+    PVM             pVM      = pTimer->CTX_SUFF(pVM);
+    PTMTIMERQUEUE   pQueue   = &pVM->tm.s.CTX_SUFF(paTimerQueues)[pTimer->enmClock];
+    bool            fActive  = false;
+    bool            fPending = false;
+
+    /*
+     * The rest of the game happens behind the lock, just
+     * like create does. All the work is done here.
+     */
+    tmLock(pVM);
+    for (int cRetries = 1000;; cRetries--)
     {
         /*
-         * Change to any of the DESTROY states if valid.
+         * Change to the DESTROY state.
          */
-        TMTIMERSTATE enmState = pTimer->enmState;
+        TMTIMERSTATE enmState    = pTimer->enmState;
+        TMTIMERSTATE enmNewState = enmState;
         Log2(("TMTimerDestroy: %p:{.enmState=%s, .pszDesc='%s'} cRetries=%d\n",
               pTimer, tmTimerState(enmState), R3STRING(pTimer->pszDesc), cRetries));
         switch (enmState)
         {
+            case TMTIMERSTATE_STOPPED:
             case TMTIMERSTATE_EXPIRED:
-                if (!VM_IS_EMT(pTimer->CTX_SUFF(pVM)))
-                {
-                    AssertMsgFailed(("Attempted timer destruction from other thread while expire pending! (%s)\n", R3STRING(pTimer->pszDesc)));
-                    return VERR_INVALID_PARAMETER;
-                }
-                /* fall thru */
-            case TMTIMERSTATE_STOPPED:
-                if (tmTimerTryWithLink(pTimer, TMTIMERSTATE_PENDING_DESTROY, enmState))
-                {
-                    tmSchedule(pTimer);
-                    return VINF_SUCCESS;
-                }
                 break;
 
             case TMTIMERSTATE_ACTIVE:
-                if (tmTimerTryWithLink(pTimer, TMTIMERSTATE_PENDING_STOP_DESTROY, enmState))
-                {
-                    tmSchedule(pTimer);
-                    return VINF_SUCCESS;
-                }
+                fActive     = true;
                 break;
 
             case TMTIMERSTATE_PENDING_STOP:
             case TMTIMERSTATE_PENDING_STOP_SCHEDULE:
-                if (tmTimerTry(pTimer, TMTIMERSTATE_PENDING_STOP_DESTROY, enmState))
-                {
-                    tmSchedule(pTimer);
-                    return VINF_SUCCESS;
-                }
+            case TMTIMERSTATE_PENDING_RESCHEDULE:
+                fActive     = true;
+                fPending    = true;
                 break;
 
-            case TMTIMERSTATE_PENDING_DESTROY:
-            case TMTIMERSTATE_PENDING_STOP_DESTROY:
-                AssertMsgFailed(("How many times do you think you can destroy the same timer... (%s)\n", R3STRING(pTimer->pszDesc)));
-                return VERR_INVALID_PARAMETER;
-
-            case TMTIMERSTATE_PENDING_RESCHEDULE:
-                if (tmTimerTry(pTimer, TMTIMERSTATE_PENDING_STOP_DESTROY, enmState))
-                {
-                    tmSchedule(pTimer);
-                    return VINF_SUCCESS;
-                }
+            case TMTIMERSTATE_PENDING_SCHEDULE:
+                fPending    = true;
                 break;
 
-            case TMTIMERSTATE_PENDING_SCHEDULE:
-                if (tmTimerTry(pTimer, TMTIMERSTATE_PENDING_DESTROY, enmState))
-                {
-                    tmSchedule(pTimer);
-                    return VINF_SUCCESS;
-                }
-                break;
-
+            /*
+             * This shouldn't happen as the caller should make sure there are no races.
+             */
             case TMTIMERSTATE_PENDING_SCHEDULE_SET_EXPIRE:
             case TMTIMERSTATE_PENDING_RESCHEDULE_SET_EXPIRE:
-#ifdef IN_RING3
+                AssertMsgFailed(("%p:.enmState=%s %s\n", pTimer, tmTimerState(enmState), pTimer->pszDesc));
+                tmUnlock(pVM);
                 if (!RTThreadYield())
                     RTThreadSleep(1);
-#endif
-                break;
+                AssertMsgReturn(cRetries > 0, ("Failed waiting for stable state. state=%d (%s)\n", pTimer->enmState, pTimer->pszDesc),
+                                VERR_TM_UNSTABLE_STATE);
+                tmLock(pVM);
+                continue;
 
             /*
@@ -622 +612 @@
              */
             case TMTIMERSTATE_FREE:
-                AssertMsgFailed(("Invalid timer state %d (%s)\n", enmState, R3STRING(pTimer->pszDesc)));
-                return VERR_TM_INVALID_STATE;
+            case TMTIMERSTATE_DESTROY:
+                tmUnlock(pVM);
+                AssertLogRelMsgFailedReturn(("pTimer=%p %s\n", pTimer, tmTimerState(enmState)), VERR_TM_INVALID_STATE);
+
             default:
                 AssertMsgFailed(("Unknown timer state %d (%s)\n", enmState, R3STRING(pTimer->pszDesc)));
+                tmUnlock(pVM);
                 return VERR_TM_UNKNOWN_STATE;
         }
-    } while (cRetries-- > 0);
-
-    AssertMsgFailed(("Failed waiting for stable state. state=%d (%s)\n", pTimer->enmState, R3STRING(pTimer->pszDesc)));
-    return VERR_INTERNAL_ERROR;
+
+        /*
+         * Try switch to the destroy state.
+         * This should always succeed as the caller should make sure there are no race.
+         */
+        if (tmTimerTry(pTimer, TMTIMERSTATE_DESTROY, enmState))
+            break;
+        AssertMsgFailed(("%p:.enmState=%s %s\n", pTimer, tmTimerState(enmState), pTimer->pszDesc));
+        tmUnlock(pVM);
+        AssertMsgReturn(cRetries > 0, ("Failed waiting for stable state. state=%d (%s)\n", pTimer->enmState, pTimer->pszDesc),
+                        VERR_TM_UNSTABLE_STATE);
+        tmLock(pVM);
+    }
+
+    /*
+     * Unlink from the active list.
+     */
+    if (fActive)
+    {
+        const PTMTIMER pPrev = TMTIMER_GET_PREV(pTimer);
+        const PTMTIMER pNext = TMTIMER_GET_NEXT(pTimer);
+        if (pPrev)
+            TMTIMER_SET_NEXT(pPrev, pNext);
+        else
+        {
+            TMTIMER_SET_HEAD(pQueue, pNext);
+            pQueue->u64Expire = pNext ? pNext->u64Expire : INT64_MAX;
+        }
+        if (pNext)
+            TMTIMER_SET_PREV(pNext, pPrev);
+        pTimer->offNext = 0;
+        pTimer->offPrev = 0;
+    }
+
+    /*
+     * Unlink from the schedule list by running it.
+     */
+    if (fPending)
+    {
+        Log3(("TMR3TimerDestroy: tmTimerQueueSchedule\n"));
+        STAM_PROFILE_START(&pVM->tm.s.CTXALLSUFF(StatScheduleOne), a);
+        Assert(pQueue->offSchedule);
+        tmTimerQueueSchedule(pVM, pQueue);
+    }
+
+    /*
+     * Read to move the timer from the created list and onto the free list.
+     */
+    Assert(!pTimer->offNext); Assert(!pTimer->offPrev); Assert(!pTimer->offScheduleNext);
+
+    /* unlink from created list */
+    if (pTimer->pBigPrev)
+        pTimer->pBigPrev->pBigNext = pTimer->pBigNext;
+    else
+        pVM->tm.s.pCreated         = pTimer->pBigNext;
+    if (pTimer->pBigNext)
+        pTimer->pBigNext->pBigPrev = pTimer->pBigPrev;
+    pTimer->pBigNext = 0;
+    pTimer->pBigPrev = 0;
+
+    /* free */
+    Log2(("TM: Inserting %p into the free list ahead of %p!\n", pTimer, pVM->tm.s.pFree));
+    TM_SET_STATE(pTimer, TMTIMERSTATE_FREE);
+    pTimer->pBigNext = pVM->tm.s.pFree;
+    pVM->tm.s.pFree = pTimer;
+
+#ifdef VBOX_STRICT
+    tmTimerQueuesSanityChecks(pVM, "TMR3TimerDestroy");
+#endif
+    tmUnlock(pVM);
+    return VINF_SUCCESS;
 }
 #endif /* IN_RING3 */
@@ -727 +787 @@
              * Invalid states.
              */
-            case TMTIMERSTATE_PENDING_DESTROY:
-            case TMTIMERSTATE_PENDING_STOP_DESTROY:
+            case TMTIMERSTATE_DESTROY:
             case TMTIMERSTATE_FREE:
                 AssertMsgFailed(("Invalid timer state %d (%s)\n", enmState, R3STRING(pTimer->pszDesc)));
@@ -918 +977 @@
              * Invalid states.
              */
-            case TMTIMERSTATE_PENDING_DESTROY:
-            case TMTIMERSTATE_PENDING_STOP_DESTROY:
+            case TMTIMERSTATE_DESTROY:
             case TMTIMERSTATE_FREE:
                 AssertMsgFailed(("Invalid timer state %d (%s)\n", enmState, R3STRING(pTimer->pszDesc)));
@@ -1273 +1331 @@
              * Invalid states.
              */
-            case TMTIMERSTATE_PENDING_DESTROY:
-            case TMTIMERSTATE_PENDING_STOP_DESTROY:
+            case TMTIMERSTATE_DESTROY:
             case TMTIMERSTATE_FREE:
                 AssertMsgFailed(("Invalid timer state %d (%s)\n", enmState, R3STRING(pTimer->pszDesc)));
@@ -1325 +1382 @@
          * Invalid states.
          */
-        case TMTIMERSTATE_PENDING_DESTROY:
-        case TMTIMERSTATE_PENDING_STOP_DESTROY:
+        case TMTIMERSTATE_DESTROY:
         case TMTIMERSTATE_FREE:
             AssertMsgFailed(("Invalid timer state %s (%s)\n", tmTimerState(enmState), R3STRING(pTimer->pszDesc)));
@@ -1362 +1418 @@
         CASE( 8,PENDING_RESCHEDULE_SET_EXPIRE);
         CASE( 9,PENDING_RESCHEDULE);
-        CASE(10,PENDING_STOP_DESTROY);
-        CASE(11,PENDING_DESTROY);
-        CASE(12,FREE);
+        CASE(10,DESTROY);
+        CASE(11,FREE);
         default:
             AssertMsgFailed(("Invalid state enmState=%d\n", enmState));
@@ -1496 +1551 @@
 
             /*
-             * Stop & destroy the timer.
+             * The timer is pending destruction by TMR3TimerDestroy, our caller.
+             * Nothing to do here.
              */
-            case TMTIMERSTATE_PENDING_STOP_DESTROY:
-            {
-                const PTMTIMER pPrev = TMTIMER_GET_PREV(pTimer);
-                const PTMTIMER pNext = TMTIMER_GET_NEXT(pTimer);
-                if (pPrev)
-                    TMTIMER_SET_NEXT(pPrev, pNext);
-                else
-                {
-                    TMTIMER_SET_HEAD(pQueue, pNext);
-                    pQueue->u64Expire = pNext ? pNext->u64Expire : INT64_MAX;
-                }
-                if (pNext)
-                    TMTIMER_SET_PREV(pNext, pPrev);
-                pTimer->offNext = 0;
-                pTimer->offPrev = 0;
-                /* fall thru */
-            }
-
-            /*
-             * Destroy the timer.
-             */
-            case TMTIMERSTATE_PENDING_DESTROY:
-            {
-                Assert(!pTimer->offNext); Assert(!pTimer->offPrev);
-                PVM pVM = pTimer->CTX_SUFF(pVM);
-                const PTMTIMER pBigPrev = (PTMTIMER)(pTimer->pBigPrev ? MMHyperR3ToCC(pVM, pTimer->pBigPrev) : NULL);
-                const PTMTIMER pBigNext = (PTMTIMER)(pTimer->pBigNext ? MMHyperR3ToCC(pVM, pTimer->pBigNext) : NULL);
-
-                /* unlink from created list */
-                if (pBigPrev)
-                    pBigPrev->pBigNext = pTimer->pBigNext;
-                else
-                    pVM->tm.s.pCreated = pTimer->pBigNext;
-                if (pBigNext)
-                    pBigNext->pBigPrev = pTimer->pBigPrev;
-                pTimer->pBigNext = 0;
-                pTimer->pBigPrev = 0;
-
-                /* free */
-                Log2(("TM: Inserting %p into the free list ahead of %p!\n", pTimer, pVM->tm.s.pFree));
-                pTimer->pBigNext = pVM->tm.s.pFree;
-                pVM->tm.s.pFree = (PTMTIMERR3)MMHyperCCToR3(pVM, pTimer);
-                TM_SET_STATE(pTimer, TMTIMERSTATE_FREE);
-                return;
-            }
+            case TMTIMERSTATE_DESTROY:
+                break;
 
             /*
@@ -1642 +1655 @@
                     break;
                 case TMTIMERSTATE_PENDING_STOP:
-                case TMTIMERSTATE_PENDING_STOP_DESTROY:
                 case TMTIMERSTATE_PENDING_RESCHEDULE:
                 case TMTIMERSTATE_PENDING_RESCHEDULE_SET_EXPIRE:
@@ -1669 +1681 @@
             case TMTIMERSTATE_ACTIVE:
             case TMTIMERSTATE_PENDING_STOP:
-            case TMTIMERSTATE_PENDING_STOP_DESTROY:
             case TMTIMERSTATE_PENDING_RESCHEDULE:
             case TMTIMERSTATE_PENDING_RESCHEDULE_SET_EXPIRE:
@@ -1681 +1692 @@
             }
 
-            case TMTIMERSTATE_PENDING_DESTROY:
             case TMTIMERSTATE_PENDING_SCHEDULE:
             case TMTIMERSTATE_PENDING_STOP_SCHEDULE:
@@ -1705 +1715 @@
 
             /* shouldn't get here! */
+            case TMTIMERSTATE_DESTROY:
             default:
                 AssertMsgFailed(("Invalid state enmState=%d %s\n", enmState, tmTimerState(enmState)));

trunk/src/VBox/VMM/testcase/tstVMM.cpp

@@ -108 +108 @@
                 &&  TMTimerIsActive(pTimer))
             {
-//                rc = TMTimerStop(pTimer);
-//                RTTEST_CHECK_MSG(hTest, RT_SUCCESS(rc), (hTest, "TMTimerStop: %Rrc\n",  rc));
+                rc = TMTimerStop(pTimer);
+                RTTEST_CHECK_MSG(hTest, RT_SUCCESS(rc), (hTest, "TMTimerStop: %Rrc\n",  rc));
             }
             else