Index: /trunk/src/VBox/VMM/VM.cpp =================================================================== --- /trunk/src/VBox/VMM/VM.cpp (revision 24737) +++ /trunk/src/VBox/VMM/VM.cpp (revision 24738) @@ -811,4 +811,7 @@ STAM_REG(pVM, &pUVM->vm.s.StatReqFree, STAMTYPE_COUNTER, "/VM/Req/Free", STAMUNIT_OCCURENCES, "Number of VMR3ReqFree calls."); STAM_REG(pVM, &pUVM->vm.s.StatReqFreeOverflow, STAMTYPE_COUNTER, "/VM/Req/FreeOverflow", STAMUNIT_OCCURENCES, "Number of times the request was actually freed."); + STAM_REG(pVM, &pUVM->vm.s.StatReqProcessed, STAMTYPE_COUNTER, "/VM/Req/Processed", STAMUNIT_OCCURENCES, "Number of processed requests (any queue)."); + STAM_REG(pVM, &pUVM->vm.s.StatReqMoreThan1, STAMTYPE_COUNTER, "/VM/Req/MoreThan1", STAMUNIT_OCCURENCES, "Number of times there are more than one request on the queue when processing it."); + STAM_REG(pVM, &pUVM->vm.s.StatReqPushBackRaces, STAMTYPE_COUNTER, "/VM/Req/PushBackRaces", STAMUNIT_OCCURENCES, "Number of push back races."); rc = CPUMR3Init(pVM); Index: /trunk/src/VBox/VMM/VMInternal.h =================================================================== --- /trunk/src/VBox/VMM/VMInternal.h (revision 24737) +++ /trunk/src/VBox/VMM/VMInternal.h (revision 24738) @@ -198,4 +198,12 @@ /** Number of times the request was actually freed. */ STAMCOUNTER StatReqFreeOverflow; + /** Number of requests served. */ + STAMCOUNTER StatReqProcessed; + /** Number of times there are more than one request and the others needed to be + * pushed back onto the list. */ + STAMCOUNTER StatReqMoreThan1; + /** Number of times we've raced someone when pushing the other requests back + * onto the list. */ + STAMCOUNTER StatReqPushBackRaces; #endif Index: /trunk/src/VBox/VMM/VMReq.cpp =================================================================== --- /trunk/src/VBox/VMM/VMReq.cpp (revision 24737) +++ /trunk/src/VBox/VMM/VMReq.cpp (revision 24738) @@ -983,4 +983,59 @@ /** + * VMR3ReqProcessU helper that handles cases where there are more than one + * pending request. + * + * @returns The oldest request. + * @param pUVM Pointer to the user mode VM structure + * @param idDstCpu VMCPUID_ANY or virtual CPU ID. + * @param pReqList The list of requests. + * @param ppvReqs Pointer to the list head. + */ +static PVMREQ vmR3ReqProcessUTooManyHelper(PUVM pUVM, VMCPUID idDstCpu, PVMREQ pReqList, void * volatile *ppvReqs) +{ + STAM_COUNTER_INC(&pUVM->vm.s.StatReqMoreThan1); + /* Chop off the last one (pReq). */ + PVMREQ pPrev; + PVMREQ pReqRet = pReqList; + do + { + pPrev = pReqRet; + pReqRet = pReqRet->pNext; + } while (pReqRet->pNext); + ASMAtomicWritePtr((void * volatile *)&pPrev->pNext, NULL); + + /* Push the others back onto the list (end of it). */ + Log2(("VMR3ReqProcess: Pushing back %p %p...\n", pReqList, pReqList->pNext)); + if (RT_UNLIKELY(!ASMAtomicCmpXchgPtr(ppvReqs, pReqList, NULL))) + { + STAM_COUNTER_INC(&pUVM->vm.s.StatReqPushBackRaces); + do + { + ASMNopPause(); + PVMREQ pReqList2 = (PVMREQ)ASMAtomicXchgPtr(ppvReqs, NULL); + if (pReqList2) + { + PVMREQ pLast = pReqList2; + while (pLast->pNext) + pLast = pLast->pNext; + ASMAtomicWritePtr((void * volatile *)&pLast->pNext, pReqList); + pReqList = pReqList2; + } + } while (!ASMAtomicCmpXchgPtr(ppvReqs, pReqList, NULL)); + } + + if (RT_LIKELY(pUVM->pVM)) + { + if (idDstCpu == VMCPUID_ANY) + VM_FF_SET(pUVM->pVM, VM_FF_REQUEST); + else + VMCPU_FF_SET(&pUVM->pVM->aCpus[idDstCpu], VMCPU_FF_REQUEST); + } + + return pReqRet; +} + + +/** * Process pending request(s). * @@ -996,4 +1051,6 @@ * * @note SMP safe (multiple EMTs trying to satisfy VM_FF_REQUESTs). + * + * @remarks This was made reentrant for */ VMMR3DECL(int) VMR3ReqProcessU(PUVM pUVM, VMCPUID idDstCpu) @@ -1011,10 +1068,12 @@ { /* - * Get pending requests. + * Get the pending requests. + * If there are more than one request, unlink the oldest and put the + * rest back so that we're reentrant. */ - void * volatile *ppReqs; + void * volatile *ppvReqs; if (idDstCpu == VMCPUID_ANY) { - ppReqs = (void * volatile *)&pUVM->vm.s.pReqs; + ppvReqs = (void * volatile *)&pUVM->vm.s.pReqs; if (RT_LIKELY(pUVM->pVM)) VM_FF_CLEAR(pUVM->pVM, VM_FF_REQUEST); @@ -1024,59 +1083,30 @@ Assert(idDstCpu < pUVM->cCpus); Assert(pUVM->aCpus[idDstCpu].vm.s.NativeThreadEMT == RTThreadNativeSelf()); - ppReqs = (void * volatile *)&pUVM->aCpus[idDstCpu].vm.s.pReqs; + ppvReqs = (void * volatile *)&pUVM->aCpus[idDstCpu].vm.s.pReqs; if (RT_LIKELY(pUVM->pVM)) VMCPU_FF_CLEAR(&pUVM->pVM->aCpus[idDstCpu], VMCPU_FF_REQUEST); } - PVMREQ pReqs = (PVMREQ)ASMAtomicXchgPtr(ppReqs, NULL); - if (!pReqs) + + PVMREQ pReq = (PVMREQ)ASMAtomicXchgPtr(ppvReqs, NULL); + if (!pReq) break; + if (RT_UNLIKELY(pReq->pNext)) + pReq = vmR3ReqProcessUTooManyHelper(pUVM, idDstCpu, pReq, ppvReqs); /* - * Reverse the list to process it in FIFO order. + * Process the request. + * Note! The status code handling here extremely important and yet very + * fragile. */ - PVMREQ pReq = pReqs; - if (pReq->pNext) - Log2(("VMR3ReqProcess: 2+ requests: %p %p %p\n", pReq, pReq->pNext, pReq->pNext->pNext)); - pReqs = NULL; - while (pReq) - { - Assert(pReq->enmState == VMREQSTATE_QUEUED); - Assert(pReq->pUVM == pUVM); - PVMREQ pCur = pReq; - pReq = pReq->pNext; - pCur->pNext = pReqs; - pReqs = pCur; - } - - - /* - * Process the requests. - * - * Since this is a FF worker certain rules applies to the - * status codes. See the EM section in VBox/err.h and EM.cpp for details. - */ - while (pReqs) - { - /* Unchain the first request and advance the list. */ - pReq = pReqs; - pReqs = pReqs->pNext; - pReq->pNext = NULL; - - /* Process the request */ - int rc2 = vmR3ReqProcessOneU(pUVM, pReq); - - /* - * The status code handling extremely important yet very fragile. Should probably - * look for a better way of communicating status changes to EM... - */ - if ( rc2 >= VINF_EM_FIRST - && rc2 <= VINF_EM_LAST - && ( rc == VINF_SUCCESS - || rc2 < rc) ) - rc = rc2; - /** @todo may have to abort processing to propagate EM scheduling status codes - * up to the caller... See the ugly hacks after VMMR3EmtRendezvousFF - * and VMR3ReqProcessU in EM.cpp. */ - } + STAM_COUNTER_INC(&pUVM->vm.s.StatReqProcessed); + int rc2 = vmR3ReqProcessOneU(pUVM, pReq); + if ( rc2 >= VINF_EM_FIRST + && rc2 <= VINF_EM_LAST + && ( rc == VINF_SUCCESS + || rc2 < rc) ) + rc = rc2; + /** @todo may have to abort processing to propagate EM scheduling status codes + * up to the caller... See the ugly hacks after VMMR3EmtRendezvousFF + * and VMR3ReqProcessU in EM.cpp. */ } @@ -1096,5 +1126,5 @@ static int vmR3ReqProcessOneU(PUVM pUVM, PVMREQ pReq) { - LogFlow(("vmR3ReqProcessOne: pReq=%p type=%d fFlags=%#x\n", pReq, pReq->enmType, pReq->fFlags)); + LogFlow(("vmR3ReqProcessOneU: pReq=%p type=%d fFlags=%#x\n", pReq, pReq->enmType, pReq->fFlags)); /* @@ -1210,5 +1240,5 @@ { /* Free the packet, nobody is waiting. */ - LogFlow(("vmR3ReqProcessOne: Completed request %p: rcReq=%Rrc rcRet=%Rrc - freeing it\n", + LogFlow(("vmR3ReqProcessOneU: Completed request %p: rcReq=%Rrc rcRet=%Rrc - freeing it\n", pReq, rcReq, rcRet)); VMR3ReqFree(pReq); @@ -1217,5 +1247,5 @@ { /* Notify the waiter and him free up the packet. */ - LogFlow(("vmR3ReqProcessOne: Completed request %p: rcReq=%Rrc rcRet=%Rrc - notifying waiting thread\n", + LogFlow(("vmR3ReqProcessOneU: Completed request %p: rcReq=%Rrc rcRet=%Rrc - notifying waiting thread\n", pReq, rcReq, rcRet)); ASMAtomicXchgSize(&pReq->fEventSemClear, false);