From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D98A5A0C45; Sat, 4 Sep 2021 09:17:08 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5EFE940E3C; Sat, 4 Sep 2021 09:17:08 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id 5A62640DDD for ; Sat, 4 Sep 2021 09:17:06 +0200 (CEST) Received: from dggemv704-chm.china.huawei.com (unknown [172.30.72.55]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4H1m8d6l8YzbdTw; Sat, 4 Sep 2021 15:13:05 +0800 (CST) Received: from dggpeml500024.china.huawei.com (7.185.36.10) by dggemv704-chm.china.huawei.com (10.3.19.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Sat, 4 Sep 2021 15:17:03 +0800 Received: from [10.40.190.165] (10.40.190.165) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Sat, 4 Sep 2021 15:17:03 +0800 To: Kevin Laatz , , , , , , CC: , , , , , , , , , , References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com> <1630588395-2804-1-git-send-email-fengchengwen@huawei.com> <1630588395-2804-7-git-send-email-fengchengwen@huawei.com> <91f6b538-88a8-ce80-36ff-161ff75e2fea@intel.com> From: fengchengwen Message-ID: Date: Sat, 4 Sep 2021 15:17:03 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <91f6b538-88a8-ce80-36ff-161ff75e2fea@intel.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.40.190.165] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To dggpeml500024.china.huawei.com (7.185.36.10) X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH v19 6/7] dma/skeleton: introduce skeleton dmadev driver X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 2021/9/3 23:14, Kevin Laatz wrote: > On 02/09/2021 14:13, Chengwen Feng wrote: >> Skeleton dmadevice driver, on the lines of rawdev skeleton, is for >> showcasing of the dmadev library. >> >> Design of skeleton involves a virtual device which is plugged into VDEV >> bus on initialization. >> >> Also, enable compilation of dmadev skeleton drivers. >> >> Signed-off-by: Chengwen Feng >> --- >>   MAINTAINERS                            |   1 + >>   drivers/dma/meson.build                |  11 + >>   drivers/dma/skeleton/meson.build       |   7 + >>   drivers/dma/skeleton/skeleton_dmadev.c | 601 +++++++++++++++++++++++++++++++++ >>   drivers/dma/skeleton/skeleton_dmadev.h |  59 ++++ >>   drivers/dma/skeleton/version.map       |   3 + >>   drivers/meson.build                    |   1 + >>   7 files changed, 683 insertions(+) >>   create mode 100644 drivers/dma/meson.build >>   create mode 100644 drivers/dma/skeleton/meson.build >>   create mode 100644 drivers/dma/skeleton/skeleton_dmadev.c >>   create mode 100644 drivers/dma/skeleton/skeleton_dmadev.h >>   create mode 100644 drivers/dma/skeleton/version.map >> snip >> >> + >> +static int >> +vchan_setup(struct skeldma_hw *hw, uint16_t nb_desc) >> +{ >> +    struct skeldma_desc *desc; >> +    struct rte_ring *empty; >> +    struct rte_ring *pending; >> +    struct rte_ring *running; >> +    struct rte_ring *completed; >> +    uint16_t i; >> + >> +    desc = rte_zmalloc_socket("dma_skelteon_desc", >> +                  nb_desc * sizeof(struct skeldma_desc), >> +                  RTE_CACHE_LINE_SIZE, hw->socket_id); >> +    if (desc == NULL) { >> +        SKELDMA_LOG(ERR, "Malloc dma skeleton desc fail!"); >> +        return -ENOMEM; >> +    } >> + >> +    empty = rte_ring_create("dma_skeleton_desc_empty", nb_desc, >> +                hw->socket_id, RING_F_SP_ENQ | RING_F_SC_DEQ); >> +    pending = rte_ring_create("dma_skeleton_desc_pending", nb_desc, >> +                  hw->socket_id, RING_F_SP_ENQ | RING_F_SC_DEQ); >> +    running = rte_ring_create("dma_skeleton_desc_running", nb_desc, >> +                  hw->socket_id, RING_F_SP_ENQ | RING_F_SC_DEQ); >> +    completed = rte_ring_create("dma_skeleton_desc_completed", nb_desc, >> +                  hw->socket_id, RING_F_SP_ENQ | RING_F_SC_DEQ); >> +    if (empty == NULL || pending == NULL || running == NULL || >> +        completed == NULL) { >> +        SKELDMA_LOG(ERR, "Create dma skeleton desc ring fail!"); >> +        rte_ring_free(empty); >> +        rte_ring_free(pending); >> +        rte_ring_free(running); >> +        rte_ring_free(completed); >> +        rte_free(desc); > > These pointers should be set to NULL after free'ing, similar to what you have in "vchan_release()". > These pointers are local variables, Need to clean them up? The set to NULL operation in 'vhcan_release', is because those pointers are held by dmadev with a longer life cycle. Thanks > >> +        return -ENOMEM; >> +    } >> + >> +    /* The real usable ring size is *count-1* instead of *count* to >> +     * differentiate a free ring from an empty ring. >> +     * @see rte_ring_create >> +     */ >> +    for (i = 0; i < nb_desc - 1; i++) >> +        (void)rte_ring_enqueue(empty, (void *)(desc + i)); >> + >> +    hw->desc_mem = desc; >> +    hw->desc_empty = empty; >> +    hw->desc_pending = pending; >> +    hw->desc_running = running; >> +    hw->desc_completed = completed; >> + >> +    return 0; >> +} >> + >> +static void >> +vchan_release(struct skeldma_hw *hw) >> +{ >> +    if (hw->desc_mem == NULL) >> +        return; >> + >> +    rte_free(hw->desc_mem); >> +    hw->desc_mem = NULL; >> +    rte_ring_free(hw->desc_empty); >> +    hw->desc_empty = NULL; >> +    rte_ring_free(hw->desc_pending); >> +    hw->desc_pending = NULL; >> +    rte_ring_free(hw->desc_running); >> +    hw->desc_running = NULL; >> +    rte_ring_free(hw->desc_completed); >> +    hw->desc_completed = NULL; >> +} >> + >> > > > With the minor comments above addressed, > > Reviewed-by: Kevin Laatz > > > > .