Changeset 92873 in vbox for trunk/src/VBox/Runtime/r0drv

Timestamp:

Dec 11, 2021 1:38:33 AM (3 years ago)

Author:

vboxsync

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

148821

Message:

IPRT/timer-r0drv-nt.cpp,mp-r0drv-nt.cpp: Work around rtMpNtSetTargetProcessorDpc failure on systems with sparse CPU ID assignments (like on testboxmem1). For timers, if the function fails for an offline CPU, just defer the DPC setup.

Location:

trunk/src/VBox/Runtime/r0drv/nt

Files:

• mp-r0drv-nt.cpp (modified) (3 diffs)
• timer-r0drv-nt.cpp (modified) (9 diffs)

trunk/src/VBox/Runtime/r0drv/nt/mp-r0drv-nt.cpp

@@ -1350 +1350 @@
  *
  * @returns IPRT status code (errors are asserted).
+ * @retval  VERR_CPU_NOT_FOUND if impossible CPU. Not asserted.
  * @param   pDpc                The DPC.
  * @param   idCpu               The ID of the new target CPU.
+ * @note    Callable at any IRQL.
  */
 DECLHIDDEN(int) rtMpNtSetTargetProcessorDpc(KDPC *pDpc, RTCPUID idCpu)
@@ -1361 +1363 @@
         PROCESSOR_NUMBER ProcNum;
         NTSTATUS rcNt = g_pfnrtKeGetProcessorNumberFromIndex(RTMpCpuIdToSetIndex(idCpu), &ProcNum);
-        AssertLogRelMsgReturn(NT_SUCCESS(rcNt), ("KeGetProcessorNumberFromIndex(%u) -> %#x\n", idCpu, rcNt),
-                              RTErrConvertFromNtStatus(rcNt));
-
-        rcNt = g_pfnrtKeSetTargetProcessorDpcEx(pDpc, &ProcNum);
-        AssertLogRelMsgReturn(NT_SUCCESS(rcNt),
-                              ("KeSetTargetProcessorDpcEx(,%u(%u/%u)) -> %#x\n", idCpu, ProcNum.Group, ProcNum.Number, rcNt),
-                              RTErrConvertFromNtStatus(rcNt));
+        if (NT_SUCCESS(rcNt))
+        {
+            rcNt = g_pfnrtKeSetTargetProcessorDpcEx(pDpc, &ProcNum);
+            AssertLogRelMsgReturn(NT_SUCCESS(rcNt),
+                                  ("KeSetTargetProcessorDpcEx(,%u(%u/%u)) -> %#x\n", idCpu, ProcNum.Group, ProcNum.Number, rcNt),
+                                  RTErrConvertFromNtStatus(rcNt));
+        }
+        else if (rcNt == STATUS_INVALID_PARAMETER)
+            return VERR_CPU_NOT_FOUND;
+        else
+            AssertLogRelMsgReturn(NT_SUCCESS(rcNt), ("KeGetProcessorNumberFromIndex(%u) -> %#x\n", idCpu, rcNt),
+                                  RTErrConvertFromNtStatus(rcNt));
+
     }
     else if (g_pfnrtKeSetTargetProcessorDpc)
@@ -1936 +1944 @@
                 g_pfnrtKeSetImportanceDpc(&s_aPokeDpcs[i], HighImportance);
             int rc = rtMpNtSetTargetProcessorDpc(&s_aPokeDpcs[i], idCpu);
-            if (RT_FAILURE(rc))
+            if (RT_FAILURE(rc) && rc != VERR_CPU_NOT_FOUND)
                 return rc;
         }

trunk/src/VBox/Runtime/r0drv/nt/timer-r0drv-nt.cpp

@@ -74 +74 @@
     /** The NT DPC object. */
     KDPC                    NtDpc;
+    /** Whether we failed to set the target CPU for the DPC and that this needs
+     * to be done at RTTimerStart (simple timers) or during timer callback (omni). */
+    bool                    fDpcNeedTargetCpuSet;
 } RTTIMERNTSUBTIMER;
 /** Pointer to a NT sub-timer structure. */
@@ -426 +429 @@
 
 /**
+ * Called when we have an impcomplete DPC object.
+ *
+ * @returns KeInsertQueueDpc return value.
+ * @param   pSubTimer       The sub-timer to queue an DPC for.
+ * @param   iCpu            The CPU set index corresponding to that sub-timer.
+ */
+DECL_NO_INLINE(static, BOOLEAN) rtTimerNtOmniQueueDpcSlow(PRTTIMERNTSUBTIMER pSubTimer, int iCpu)
+{
+    int rc = rtMpNtSetTargetProcessorDpc(&pSubTimer->NtDpc, RTMpCpuIdFromSetIndex(iCpu));
+    if (RT_SUCCESS(rc))
+    {
+        pSubTimer->fDpcNeedTargetCpuSet = false;
+        return KeInsertQueueDpc(&pSubTimer->NtDpc, 0, 0);
+    }
+    return FALSE;
+}
+
+
+/**
+ * Wrapper around KeInsertQueueDpc that makes sure the target CPU has been set.
+ *
+ * This is for handling deferred rtMpNtSetTargetProcessorDpc failures during
+ * creation.  These errors happens for offline CPUs which probably never every
+ * will come online, as very few systems do CPU hotplugging.
+ *
+ * @returns KeInsertQueueDpc return value.
+ * @param   pSubTimer       The sub-timer to queue an DPC for.
+ * @param   iCpu            The CPU set index corresponding to that sub-timer.
+ */
+DECLINLINE(BOOLEAN) rtTimerNtOmniQueueDpc(PRTTIMERNTSUBTIMER pSubTimer, int iCpu)
+{
+    if (RT_LIKELY(!pSubTimer->fDpcNeedTargetCpuSet))
+        return KeInsertQueueDpc(&pSubTimer->NtDpc, 0, 0);
+    return rtTimerNtOmniQueueDpcSlow(pSubTimer, iCpu);
+}
+
+
+/**
  * Common timer callback worker for omni-timers.
  *
@@ -457 +498 @@
                 if (   RTCpuSetIsMemberByIndex(&OnlineSet, iCpu)
                     && iCpuSelf != iCpu)
-                    KeInsertQueueDpc(&pTimer->aSubTimers[iCpu].NtDpc, 0, 0);
+                    rtTimerNtOmniQueueDpc(&pTimer->aSubTimers[iCpu], iCpu);
 
             pTimer->pfnTimer(pTimer, pTimer->pvUser, ++pSubTimer->iTick);
@@ -475 +516 @@
                 if (RTCpuSetIsMemberByIndex(&OnlineSet, iCpu))
                     cCpus++;
-            ASMAtomicAddS32(&pTimer->cOmniSuspendCountDown, cCpus);
+            ASMAtomicAddS32(&pTimer->cOmniSuspendCountDown, cCpus); /** @todo this is bogus bogus bogus. The counter is only used here. */
 
             for (int iCpu = 0; iCpu < RTCPUSET_MAX_CPUS; iCpu++)
                 if (   RTCpuSetIsMemberByIndex(&OnlineSet, iCpu)
                     && iCpuSelf != iCpu)
-                    if (!KeInsertQueueDpc(&pTimer->aSubTimers[iCpu].NtDpc, 0, 0))
+                    if (!rtTimerNtOmniQueueDpc(&pTimer->aSubTimers[iCpu], iCpu))
                         ASMAtomicDecS32(&pTimer->cOmniSuspendCountDown); /* already queued and counted. */
 
@@ -584 +625 @@
         KeReleaseSpinLock(&pTimer->Spinlock, bSavedIrql);
         return VERR_CPU_OFFLINE;
+    }
+
+    /*
+     * Lazy set the DPC target CPU if needed.
+     */
+    if (   !pTimer->fSpecificCpu
+        || !pTimer->aSubTimers[0].fDpcNeedTargetCpuSet)
+    { /* likely */ }
+    else
+    {
+        int rc = rtMpNtSetTargetProcessorDpc(&pTimer->aSubTimers[0].NtDpc, pTimer->idCpu);
+        if (RT_FAILURE(rc))
+        {
+            KeReleaseSpinLock(&pTimer->Spinlock, bSavedIrql);
+            return rc;
+        }
     }
 
@@ -902 +959 @@
     if (RT_SUCCESS(rc))
     {
+        RTCPUSET OnlineSet;
+        RTMpGetOnlineSet(&OnlineSet);
+
         if (pTimer->fOmniTimer)
         {
@@ -915 +975 @@
             pTimer->iMasterTick = 0;
             pTimer->idCpu       = NIL_RTCPUID;
-            for (unsigned iCpu = 0; iCpu < cSubTimers && RT_SUCCESS(rc); iCpu++)
+            for (unsigned iCpu = 0; iCpu < cSubTimers; iCpu++)
             {
                 pTimer->aSubTimers[iCpu].iTick   = 0;
                 pTimer->aSubTimers[iCpu].pParent = pTimer;
 
-                if (    pTimer->idCpu == NIL_RTCPUID
-                    &&  RTMpIsCpuOnline(RTMpCpuIdFromSetIndex(iCpu)))
+                if (   pTimer->idCpu == NIL_RTCPUID
+                    && RTCpuSetIsMemberByIndex(&OnlineSet, iCpu))
                 {
                     pTimer->idCpu = RTMpCpuIdFromSetIndex(iCpu);
@@ -935 +995 @@
                 if (g_pfnrtKeSetImportanceDpc)
                     g_pfnrtKeSetImportanceDpc(&pTimer->aSubTimers[iCpu].NtDpc, HighImportance);
-                rc = rtMpNtSetTargetProcessorDpc(&pTimer->aSubTimers[iCpu].NtDpc, iCpu);
+
+                /* This does not necessarily work for offline CPUs that could potentially be onlined
+                   at runtime, so postpone it. (See troubles on testboxmem1 after r148799.) */
+                int rc2 = rtMpNtSetTargetProcessorDpc(&pTimer->aSubTimers[iCpu].NtDpc, iCpu);
+                if (RT_SUCCESS(rc2))
+                    pTimer->aSubTimers[0].fDpcNeedTargetCpuSet = false;
+                else if (!RTCpuSetIsMemberByIndex(&OnlineSet, iCpu))
+                    pTimer->aSubTimers[0].fDpcNeedTargetCpuSet = true;
+                else
+                {
+                    rc = rc2;
+                    break;
+                }
             }
             Assert(pTimer->idCpu != NIL_RTCPUID);
@@ -953 +1025 @@
                 g_pfnrtKeSetImportanceDpc(&pTimer->aSubTimers[0].NtDpc, HighImportance);
             if (pTimer->fSpecificCpu)
-                rc = rtMpNtSetTargetProcessorDpc(&pTimer->aSubTimers[0].NtDpc, fFlags & RTTIMER_FLAGS_CPU_MASK);
+            {
+                /* This does not necessarily work for offline CPUs that could potentially be onlined
+                   at runtime, so postpone it. (See troubles on testboxmem1 after r148799.) */
+                int rc2 = rtMpNtSetTargetProcessorDpc(&pTimer->aSubTimers[0].NtDpc, pTimer->idCpu);
+                if (RT_SUCCESS(rc2))
+                    pTimer->aSubTimers[0].fDpcNeedTargetCpuSet = false;
+                else if (!RTCpuSetIsMember(&OnlineSet, pTimer->idCpu))
+                    pTimer->aSubTimers[0].fDpcNeedTargetCpuSet = true;
+                else
+                    rc = rc2;
+            }
         }
         if (RT_SUCCESS(rc))