VirtualBox

儲存庫 vbox 的更動 83587


忽略:
時間撮記:
2020-4-6 下午12:31:32 (5 年 以前)
作者:
vboxsync
訊息:

Virtio_1_0: Reworked the VIRTIO_DESC_CHAIN_T handling to decrease the potential for memory leaks by employing reference counting and not having virtioCoreR3QueuePut be the one to free/release the chain. Combined the 5 allocations into a single one. bugref:9440

位置:
trunk/src/VBox/Devices
檔案:
修改 4 筆資料

圖例:

未更動
新增
刪除

  • trunk/src/VBox/Devices/Network/DevVirtioNet_1_0.cpp

    r83499 r83587  
    14271427    uint16_t cSegsAllocated = VIRTIONET_PREALLOCATE_RX_SEG_COUNT;
    14281428
    1429     PRTSGBUF pVirtSegBufToGuest = (PRTSGBUF)RTMemAllocZ(sizeof(RTSGBUF));
     1429    /**  @todo r=bird: error codepaths below are almost all leaky!  Maybe keep
     1430     *         allocations and cleanup here and put the code doing the complicated
     1431     *         work into a helper that can AssertReturn at will without needing to
     1432     *         care about cleaning stuff up. */
     1433    PRTSGBUF pVirtSegBufToGuest = (PRTSGBUF)RTMemAllocZ(sizeof(RTSGBUF)); /** @todo r=bird: Missing check. */
    14301434    PRTSGSEG paVirtSegsToGuest  = (PRTSGSEG)RTMemAllocZ(sizeof(RTSGSEG) * cSegsAllocated);
    14311435    AssertReturn(paVirtSegsToGuest, VERR_NO_MEMORY);
    … …  
    14381442    for (cDescs = uOffset = 0; uOffset < cb; )
    14391443    {
    1440         PVIRTIO_DESC_CHAIN_T pDescChain;
     1444        PVIRTIO_DESC_CHAIN_T pDescChain = NULL;
    14411445
    14421446        int rc = virtioCoreR3QueueGet(pDevIns, &pThis->Virtio, RXQIDX_QPAIR(idxQueue), &pDescChain, true);
    1443         AssertRC(rc == VINF_SUCCESS || rc == VERR_NOT_AVAILABLE);
     1447        Assert(rc == VINF_SUCCESS || rc == VERR_NOT_AVAILABLE, ("%Rrc\n", rc));
    14441448
    14451449        /** @todo  Find a better way to deal with this */
    1446         AssertMsgReturn(rc == VINF_SUCCESS && pDescChain->cbPhysReturn,
    1447                         ("Not enough Rx buffers in queue to accomodate ethernet packet\n"),
    1448                         VERR_INTERNAL_ERROR);
     1450        AssertMsgReturnStmt(rc == VINF_SUCCESS && pDescChain->cbPhysReturn,
     1451                            ("Not enough Rx buffers in queue to accomodate ethernet packet\n"),
     1452                            virtioCoreR3DescChainRelease(pDescChain),
     1453                            VERR_INTERNAL_ERROR);
    14491454
    14501455        /* Unlikely that len of 1st seg of guest Rx (IN) buf is less than sizeof(virtio_net_pkt_hdr) == 12.
    14511456         * Assert it to reduce complexity. Robust solution would entail finding seg idx and offset of
    14521457         * virtio_net_header.num_buffers (to update field *after* hdr & pkts copied to gcPhys) */
    1453         AssertMsgReturn(pDescChain->pSgPhysReturn->paSegs[0].cbSeg >= sizeof(VIRTIONET_PKT_HDR_T),
    1454                         ("Desc chain's first seg has insufficient space for pkt header!\n"),
    1455                         VERR_INTERNAL_ERROR);
     1458        AssertMsgReturnStmt(pDescChain->pSgPhysReturn->paSegs[0].cbSeg >= sizeof(VIRTIONET_PKT_HDR_T),
     1459                            ("Desc chain's first seg has insufficient space for pkt header!\n"),
     1460                            virtioCoreR3DescChainRelease(pDescChain),
     1461                            VERR_INTERNAL_ERROR);
    14561462
    14571463        uint32_t cbDescChainLeft = pDescChain->cbPhysReturn;
    … …  
    15001506                break;
    15011507        }
     1508
     1509        virtioCoreR3DescChainRelease(pDescChain);
    15021510    }
    15031511
    … …  
    20632071
    20642072    int rc;
    2065     PVIRTIO_DESC_CHAIN_T pDescChain;
     2073    PVIRTIO_DESC_CHAIN_T pDescChain = NULL;
    20662074    while ((rc = virtioCoreR3QueuePeek(pVirtio->pDevIns, pVirtio, idxQueue, &pDescChain)) == VINF_SUCCESS)
    20672075    {
    2068         if (RT_SUCCESS(rc))
     2076        if (RT_SUCCESS(rc)) /** @todo r=bird: pointless, see loop condition. */
    20692077            Log10Func(("%s fetched descriptor chain from %s\n", INSTANCE(pThis), VIRTQNAME(idxQueue)));
    20702078        else
    20712079        {
    20722080            LogFunc(("%s failed to find expected data on %s, rc = %Rrc\n", INSTANCE(pThis), VIRTQNAME(idxQueue), rc));
     2081            virtioCoreR3DescChainRelease(pDescChain);
    20732082            break;
    20742083        }
    … …  
    20962105        if (pThisCC->pDrv)
    20972106        {
    2098             PDMNETWORKGSO Gso, *pGso = virtioNetR3SetupGsoCtx(&Gso, &PktHdr);
     2107            PDMNETWORKGSO  Gso;
     2108            PPDMNETWORKGSO pGso = virtioNetR3SetupGsoCtx(&Gso, &PktHdr);
    20992109
    21002110            /** @todo Optimize away the extra copying! (lazy bird) */
    … …  
    21412151                Log4Func(("Failed to allocate S/G buffer: size=%u rc=%Rrc\n", uSize, rc));
    21422152                /* Stop trying to fetch TX descriptors until we get more bandwidth. */
     2153                virtioCoreR3DescChainRelease(pDescChain);
    21432154                break;
    21442155            }
    … …  
    21522163            virtioCoreQueueSync(pVirtio->pDevIns, pVirtio, idxQueue);
    21532164        }
     2165
     2166        virtioCoreR3DescChainRelease(pDescChain);
     2167        pDescChain = NULL;
    21542168    }
    21552169    virtioNetR3SetWriteLed(pThisCC, false);
    … …  
    22662280             {
    22672281                 Log10Func(("%s fetching next descriptor chain from %s\n", INSTANCE(pThis), VIRTQNAME(idxQueue)));
    2268                  PVIRTIO_DESC_CHAIN_T pDescChain;
     2282                 PVIRTIO_DESC_CHAIN_T pDescChain = NULL;
    22692283                 int rc = virtioCoreR3QueueGet(pDevIns, &pThis->Virtio, idxQueue, &pDescChain, true);
    22702284                 if (rc == VERR_NOT_AVAILABLE)
    … …  
    22742288                 }
    22752289                 virtioNetR3Ctrl(pDevIns, pThis, pThisCC, pDescChain);
     2290                 virtioCoreR3DescChainRelease(pDescChain);
    22762291             }
    22772292             else if (IS_TX_QUEUE(idxQueue))

  • trunk/src/VBox/Devices/Storage/DevVirtioSCSI.cpp

    r83582 r83587  
    717717    }
    718718
    719     PVIRTIO_DESC_CHAIN_T pDescChain;
     719    PVIRTIO_DESC_CHAIN_T pDescChain = NULL;
    720720    int rc = virtioCoreR3QueueGet(pDevIns, &pThis->Virtio, EVENTQ_IDX, &pDescChain, true);
    721721    AssertRCReturn(rc, rc);
    … …  
    742742    virtioCoreR3QueuePut(pDevIns, &pThis->Virtio, EVENTQ_IDX, &ReqSgBuf, pDescChain, true /*fFence*/);
    743743    virtioCoreQueueSync(pDevIns, &pThis->Virtio, EVENTQ_IDX);
     744    virtioCoreR3DescChainRelease(pDescChain);
    744745
    745746    return VINF_SUCCESS;
    … …  
    751752    RTMemFree(pReq->pbSense);
    752753    pReq->pbSense = NULL;
     754    virtioCoreR3DescChainRelease(pReq->pDescChain);
     755    pReq->pDescChain = NULL;
    753756    pTarget->pDrvMediaEx->pfnIoReqFree(pTarget->pDrvMediaEx, pReq->hIoReq);
    754757}
    … …  
    12571260    pReq->cbDataOut   = cbDataOut;
    12581261    pReq->pDescChain  = pDescChain;
     1262    virtioCoreR3DescChainRetain(pDescChain); /* (For pReq->pDescChain. Released by virtioScsiR3FreeReq.) */
    12591263    pReq->uDataInOff  = offDataIn;
    12601264    pReq->uDataOutOff = offDataOut;
    … …  
    15501554                                                    pWorkerR3->auRedoDescs[i], &pDescChain);
    15511555                  if (RT_FAILURE(rc))
    1552                      LogRel(("Error fetching desc chain to redo, %Rrc", rc));
     1556                      LogRel(("Error fetching desc chain to redo, %Rrc", rc));
    15531557
    15541558                  rc = virtioScsiR3ReqSubmit(pDevIns, pThis, pThisCC, qIdx, pDescChain);
    15551559                  if (RT_FAILURE(rc))
    1556                      LogRel(("Error submitting req packet, resetting %Rrc", rc));
     1560                      LogRel(("Error submitting req packet, resetting %Rrc", rc));
     1561
     1562                  virtioCoreR3DescChainRelease(pDescChain);
    15571563             }
    15581564             pWorkerR3->cRedoDescs = 0;
    15591565
    15601566             Log6Func(("fetching next descriptor chain from %s\n", VIRTQNAME(qIdx)));
    1561              PVIRTIO_DESC_CHAIN_T pDescChain;
     1567             PVIRTIO_DESC_CHAIN_T pDescChain = NULL;
    15621568             int rc = virtioCoreR3QueueGet(pDevIns, &pThis->Virtio, qIdx, &pDescChain, true);
    15631569             if (rc == VERR_NOT_AVAILABLE)
    15641570             {
    1565                 Log6Func(("Nothing found in %s\n", VIRTQNAME(qIdx)));
    1566                 continue;
     1571                 Log6Func(("Nothing found in %s\n", VIRTQNAME(qIdx)));
     1572                 continue;
    15671573             }
    15681574
    … …  
    15721578             else /* request queue index */
    15731579             {
    1574                   rc = virtioScsiR3ReqSubmit(pDevIns, pThis, pThisCC, qIdx, pDescChain);
    1575                   if (RT_FAILURE(rc))
    1576                       LogRel(("Error submitting req packet, resetting %Rrc", rc));
     1580                 rc = virtioScsiR3ReqSubmit(pDevIns, pThis, pThisCC, qIdx, pDescChain);
     1581                 if (RT_FAILURE(rc))
     1582                     LogRel(("Error submitting req packet, resetting %Rrc", rc));
    15771583             }
     1584
     1585             virtioCoreR3DescChainRelease(pDescChain);
    15781586        }
    15791587    }

  • trunk/src/VBox/Devices/VirtIO/Virtio_1_0.cpp

    r83575 r83587  
    602602                             uint16_t uHeadIdx, PPVIRTIO_DESC_CHAIN_T ppDescChain)
    603603{
    604     AssertReturn(ppDescChain, VERR_INVALID_PARAMETER);
     604    AssertReturn(ppDescChain, VERR_INVALID_POINTER);
     605    *ppDescChain = NULL;
    605606
    606607    Assert(idxQueue < RT_ELEMENTS(pVirtio->virtqState));
    607608
    608     PVIRTQSTATE pVirtq  = &pVirtio->virtqState[idxQueue];
    609 
    610     PVIRTIOSGSEG paSegsIn = (PVIRTIOSGSEG)RTMemAlloc(VIRTQ_MAX_SIZE * sizeof(VIRTIOSGSEG));
    611     AssertReturn(paSegsIn, VERR_NO_MEMORY);
    612 
    613     PVIRTIOSGSEG paSegsOut = (PVIRTIOSGSEG)RTMemAlloc(VIRTQ_MAX_SIZE * sizeof(VIRTIOSGSEG));
    614     AssertReturn(paSegsOut, VERR_NO_MEMORY);
     609    PVIRTQSTATE pVirtq = &pVirtio->virtqState[idxQueue];
    615610
    616611    AssertMsgReturn(IS_DRIVER_OK(pVirtio) && pVirtio->uQueueEnable[idxQueue],
    … …  
    622617    RT_NOREF(pVirtq);
    623618
     619    /*
     620     * Allocate and initialize the descriptor chain structure.
     621     */
     622    PVIRTIO_DESC_CHAIN_T pDescChain = (PVIRTIO_DESC_CHAIN_T)RTMemAllocZ(sizeof(VIRTIO_DESC_CHAIN_T));
     623    AssertReturn(pDescChain, VERR_NO_MEMORY);
     624    pDescChain->u32Magic = VIRTIO_DESC_CHAIN_MAGIC;
     625    pDescChain->cRefs    = 1;
     626    pDescChain->uHeadIdx = uHeadIdx;
     627    *ppDescChain = pDescChain;
     628
     629    /*
     630     * Gather segments.
     631     */
    624632    VIRTQ_DESC_T desc;
    625633
    626     uint32_t cbIn = 0, cbOut = 0, cSegsIn = 0, cSegsOut = 0;
     634    uint32_t cbIn = 0;
     635    uint32_t cbOut = 0;
     636    uint32_t cSegsIn = 0;
     637    uint32_t cSegsOut = 0;
     638    PVIRTIOSGSEG paSegsIn  = pDescChain->aSegsIn;
     639    PVIRTIOSGSEG paSegsOut = pDescChain->aSegsOut;
    627640
    628641    do
    … …  
    657670            Log6Func(("%s IN  desc_idx=%u seg=%u addr=%RGp cb=%u\n", VIRTQNAME(pVirtio, idxQueue), uDescIdx, cSegsIn, desc.GCPhysBuf, desc.cb));
    658671            cbIn += desc.cb;
    659             pSeg = &(paSegsIn[cSegsIn++]);
     672            pSeg = &paSegsIn[cSegsIn++];
    660673        }
    661674        else
    … …  
    663676            Log6Func(("%s OUT desc_idx=%u seg=%u addr=%RGp cb=%u\n", VIRTQNAME(pVirtio, idxQueue), uDescIdx, cSegsOut, desc.GCPhysBuf, desc.cb));
    664677            cbOut += desc.cb;
    665             pSeg = &(paSegsOut[cSegsOut++]);
     678            pSeg = &paSegsOut[cSegsOut++];
    666679        }
    667680
    … …  
    672685    } while (desc.fFlags & VIRTQ_DESC_F_NEXT);
    673686
    674     PVIRTIO_DESC_CHAIN_T pDescChain = (PVIRTIO_DESC_CHAIN_T)RTMemAllocZ(sizeof(VIRTIO_DESC_CHAIN_T));
    675     AssertReturn(pDescChain, VERR_NO_MEMORY);
    676 
    677     pDescChain->uHeadIdx      = uHeadIdx;
    678     *ppDescChain = pDescChain;
    679 
     687    /*
     688     * Add segments to the descriptor chain structure.
     689     */
    680690    if (cSegsIn)
    681691    {
    682         PVIRTIOSGBUF pSgPhysIn = (PVIRTIOSGBUF)RTMemAllocZ(sizeof(VIRTIOSGBUF));
    683         AssertReturn(pSgPhysIn, VERR_NO_MEMORY);
    684 
    685         virtioCoreSgBufInit(pSgPhysIn, paSegsIn, cSegsIn);
    686         pDescChain->pSgPhysReturn = pSgPhysIn;
     692        virtioCoreSgBufInit(&pDescChain->SgBufIn, paSegsIn, cSegsIn);
     693        pDescChain->pSgPhysReturn = &pDescChain->SgBufIn;
    687694        pDescChain->cbPhysReturn  = cbIn;
    688695    }
    … …  
    690697    if (cSegsOut)
    691698    {
    692         PVIRTIOSGBUF pSgPhysOut = (PVIRTIOSGBUF)RTMemAllocZ(sizeof(VIRTIOSGBUF));
    693         AssertReturn(pSgPhysOut, VERR_NO_MEMORY);
    694 
    695         virtioCoreSgBufInit(pSgPhysOut, paSegsOut, cSegsOut);
    696         pDescChain->pSgPhysSend   = pSgPhysOut;
     699        virtioCoreSgBufInit(&pDescChain->SgBufOut, paSegsOut, cSegsOut);
     700        pDescChain->pSgPhysSend   = &pDescChain->SgBufOut;
    697701        pDescChain->cbPhysSend    = cbOut;
    698702    }
    699703
    700 
    701704    Log6Func(("%s -- segs OUT: %u (%u bytes)   IN: %u (%u bytes) --\n", pVirtq->szVirtqName, cSegsOut, cbOut, cSegsIn, cbIn));
    702705
    703706    return VINF_SUCCESS;
    704707}
     708
     709
     710/**
     711 * Retains a reference to the given descriptor chain.
     712 *
     713 * @returns New reference count.
     714 * @retval  UINT32_MAX on invalid parameter.
     715 * @param   pDescChain      The descriptor chain to reference.
     716 */
     717uint32_t virtioCoreR3DescChainRetain(PVIRTIO_DESC_CHAIN_T pDescChain)
     718{
     719    AssertReturn(pDescChain, UINT32_MAX);
     720    AssertReturn(pDescChain->u32Magic == VIRTIO_DESC_CHAIN_MAGIC, UINT32_MAX);
     721    uint32_t cRefs = ASMAtomicIncU32(&pDescChain->cRefs);
     722    Assert(cRefs > 1);
     723    Assert(cRefs < 16);
     724    return cRefs;
     725}
     726
     727
     728/**
     729 * Releases a reference to the given descriptor chain.
     730 *
     731 * @returns New reference count.
     732 * @retval  0 if freed or invalid parameter.
     733 * @param   pDescChain      The descriptor chain to reference.  NULL is quietly
     734 *                          ignored (returns 0).
     735 */
     736uint32_t virtioCoreR3DescChainRelease(PVIRTIO_DESC_CHAIN_T pDescChain)
     737{
     738    if (!pDescChain)
     739        return 0;
     740    AssertReturn(pDescChain, 0);
     741    AssertReturn(pDescChain->u32Magic == VIRTIO_DESC_CHAIN_MAGIC, 0);
     742    uint32_t cRefs = ASMAtomicDecU32(&pDescChain->cRefs);
     743    Assert(cRefs < 16);
     744    if (cRefs == 0)
     745    {
     746        pDescChain->u32Magic = ~VIRTIO_DESC_CHAIN_MAGIC;
     747        RTMemFree(pDescChain);
     748    }
     749    return cRefs;
     750}
     751
    705752
    706753/*
    … …  
    839886 * @param   ppDescChain Address to store pointer to descriptor chain that contains the
    840887 *                      pre-processed transaction information pulled from the virtq.
     888 *                      Returned reference must be released by calling
     889 *                      virtioCoreR3DescChainRelease().
    841890 * @param   fRemove     flags whether to remove desc chain from queue (false = peek)
    842891 *
    … …  
    893942 * @retval  VERR_INVALID_STATE VirtIO not in ready state
    894943 * @retval  VERR_NOT_AVAILABLE Queue is empty
     944 *
     945 * @note    This function will not release any reference to pDescChain.  The
     946 *          caller must take care of that.
    895947 */
    896948int virtioCoreR3QueuePut(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue, PRTSGBUF pSgVirtReturn,
    … …  
    900952    PVIRTQSTATE pVirtq = &pVirtio->virtqState[idxQueue];
    901953    PVIRTIOSGBUF pSgPhysReturn = pDescChain->pSgPhysReturn;
     954
     955    Assert(pDescChain->u32Magic == VIRTIO_DESC_CHAIN_MAGIC);
     956    Assert(pDescChain->cRefs > 0);
    902957
    903958    AssertMsgReturn(IS_DRIVER_OK(pVirtio) /*&& pVirtio->uQueueEnable[idxQueue]*/,
    … …  
    9511006    Log6Func(("Write ahead used_idx=%u, %s used_idx=%u\n",
    9521007              pVirtq->uUsedIdx, VIRTQNAME(pVirtio, idxQueue), virtioReadUsedRingIdx(pDevIns, pVirtio, idxQueue)));
    953 
    954     if (pDescChain->pSgPhysSend)
    955     {
    956         RTMemFree(pDescChain->pSgPhysSend->paSegs);
    957         RTMemFree(pDescChain->pSgPhysSend);
    958     }
    959     if (pDescChain->pSgPhysReturn)
    960     {
    961         RTMemFree(pSgPhysReturn->paSegs);
    962         RTMemFree(pSgPhysReturn);
    963     }
    964     RTMemFree(pDescChain);
    9651008
    9661009    return VINF_SUCCESS;

  • trunk/src/VBox/Devices/VirtIO/Virtio_1_0.h

    r83576 r83587  
    9393typedef PVIRTIOSGBUF *PPVIRTIOSGBUF;
    9494
     95/**
     96 * Virtio descriptor chain representation.
     97 */
    9598typedef struct VIRTIO_DESC_CHAIN
    9699{
    97     uint32_t     uHeadIdx;                                        /**< Head idx of associated desc chain        */
    98     uint32_t     cbPhysSend;                                      /**< Total size of src buffer                 */
    99     PVIRTIOSGBUF pSgPhysSend;                                     /**< Phys S/G/ buf for data from guest        */
    100     uint32_t     cbPhysReturn;                                    /**< Total size of dst buffer                 */
    101     PVIRTIOSGBUF pSgPhysReturn;                                   /**< Phys S/G buf to store result for guest   */
    102 } VIRTIO_DESC_CHAIN_T, *PVIRTIO_DESC_CHAIN_T, **PPVIRTIO_DESC_CHAIN_T;
     100    uint32_t            u32Magic;                                   /**< Magic value, VIRTIO_DESC_CHAIN_MAGIC.    */
     101    uint32_t volatile   cRefs;                                      /**< Reference counter. */
     102    uint32_t            uHeadIdx;                                   /**< Head idx of associated desc chain        */
     103    uint32_t            cbPhysSend;                                 /**< Total size of src buffer                 */
     104    PVIRTIOSGBUF        pSgPhysSend;                                /**< Phys S/G/ buf for data from guest        */
     105    uint32_t            cbPhysReturn;                               /**< Total size of dst buffer                 */
     106    PVIRTIOSGBUF        pSgPhysReturn;                              /**< Phys S/G buf to store result for guest   */
     107
     108    /** @name Internal (bird combined 5 allocations into a single), fingers off.
     109     * @{ */
     110    VIRTIOSGBUF         SgBufIn;
     111    VIRTIOSGBUF         SgBufOut;
     112    VIRTIOSGSEG         aSegsIn[VIRTQ_MAX_SIZE];
     113    VIRTIOSGSEG         aSegsOut[VIRTQ_MAX_SIZE];
     114    /** @} */
     115} VIRTIO_DESC_CHAIN_T;
     116/** Pointer to a Virtio descriptor chain. */
     117typedef VIRTIO_DESC_CHAIN_T *PVIRTIO_DESC_CHAIN_T;
     118/** Pointer to a Virtio descriptor chain pointer. */
     119typedef VIRTIO_DESC_CHAIN_T **PPVIRTIO_DESC_CHAIN_T;
     120/** Magic value for VIRTIO_DESC_CHAIN_T::u32Magic. */
     121#define VIRTIO_DESC_CHAIN_MAGIC             UINT32_C(0x19600219)
    103122
    104123typedef struct VIRTIOPCIPARAMS
    … …  
    377396int  virtioCoreR3DescChainGet(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue,
    378397                             uint16_t uHeadIdx, PPVIRTIO_DESC_CHAIN_T ppDescChain);
     398uint32_t virtioCoreR3DescChainRetain(PVIRTIO_DESC_CHAIN_T pDescChain);
     399uint32_t virtioCoreR3DescChainRelease(PVIRTIO_DESC_CHAIN_T pDescChain);
    379400
    380401int  virtioCoreR3QueuePeek(PPDMDEVINS pDevIns, PVIRTIOCORE pVirtio, uint16_t idxQueue,
注意: 瀏覽 TracChangeset 來幫助您使用更動檢視器

© 2024 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette