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 C11C642338; Mon, 9 Oct 2023 10:59:10 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 482C440282; Mon, 9 Oct 2023 10:59:10 +0200 (CEST) Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by mails.dpdk.org (Postfix) with ESMTP id BDE424026B for ; Mon, 9 Oct 2023 10:59:07 +0200 (CEST) Received: from dggpeml100024.china.huawei.com (unknown [172.30.72.53]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4S3tDx5pphz1M9KC; Mon, 9 Oct 2023 16:56:33 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml100024.china.huawei.com (7.185.36.115) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Mon, 9 Oct 2023 16:59:04 +0800 Subject: Re: [EXT] Re: [PATCH v3 1/2] dmadev: offload to free source buffer To: Amit Prakash Shukla , Kevin Laatz , Bruce Richardson CC: "dev@dpdk.org" , Jerin Jacob Kollanukkaran , "mb@smartsharesystems.com" , "conor.walsh@intel.com" , Vamsi Krishna Attunuru , "g.singh@nxp.com" , "sachin.saxena@oss.nxp.com" , "hemant.agrawal@nxp.com" , "cheng1.jiang@intel.com" , Nithin Kumar Dabilpuram , Anoob Joseph References: <20230926121747.4037154-1-amitprakashs@marvell.com> <20230928115031.259592-1-amitprakashs@marvell.com> <20230928115031.259592-2-amitprakashs@marvell.com> <655af04b-4410-79f4-8ea7-bfd8c56ef0fc@huawei.com> From: fengchengwen Message-ID: <45664b7f-4f85-8c5b-6e24-b4d0d1fcc2a7@huawei.com> Date: Mon, 9 Oct 2023 16:59:04 +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: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To dggpeml100024.china.huawei.com (7.185.36.115) X-CFilter-Loop: Reflected 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 Hi Amit, Rethink again, I suggest use auto-free to denote this feature. So we could re-define as: RTE_DMA_CAPA_M2D_AUTO_FREE RTE_DMA_OP_FLAG_AUTO_FREE struct rte_dma_auto_free_param { union { struct { struct rte_mempool *pool; } m2d; }; uint64_t reserved[2]; /**< Reserved for future fields. */ }; }; note: because one vchan only support one DMA direction, so we could use union. struct rte_dma_vchan_conf { enum rte_dma_direction direction; struct rte_dma_port_param src_port; struct rte_dma_port_param dst_port; struct rte_dma_auto_free_param auto_free; }; As for later extension, if support other type auto-free, so could: 1. add one capa: RTE_DMA_CAPA_XXX_AUTO_FREE 2. modify rte_dma_auto_free_param: struct rte_dma_auto_free_param { union { struct { struct rte_mempool *pool; } m2d; struct { // params; } xxx; }; uint64_t reserved[2]; /**< Reserved for future fields. */ }; }; Thanks. On 2023/10/9 15:00, Amit Prakash Shukla wrote: > Hi Chengwen, > > Thanks for the review and feedback. Please find my reply in-line. > > Thanks, > Amit Shukla > >> -----Original Message----- >> From: fengchengwen >> Sent: Saturday, October 7, 2023 2:31 PM >> To: Amit Prakash Shukla ; Kevin Laatz >> ; Bruce Richardson >> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran ; >> mb@smartsharesystems.com; conor.walsh@intel.com; Vamsi Krishna >> Attunuru ; g.singh@nxp.com; >> sachin.saxena@oss.nxp.com; hemant.agrawal@nxp.com; >> cheng1.jiang@intel.com; Nithin Kumar Dabilpuram >> ; Anoob Joseph >> Subject: [EXT] Re: [PATCH v3 1/2] dmadev: offload to free source buffer >> >> External Email >> >> ---------------------------------------------------------------------- >> Hi Amit, >> >> On 2023/9/28 19:50, Amit Prakash Shukla wrote: >>> This changeset adds support in DMA library to free source DMA buffer >>> by hardware. On a supported hardware, application can pass on the >>> mempool information as part of vchan config when the DMA transfer >>> direction is configured as RTE_DMA_DIR_MEM_TO_DEV. >>> >>> Signed-off-by: Amit Prakash Shukla >>> Acked-by: Morten Brørup >>> Acked-by: Anoob Joseph >>> --- >>> lib/dmadev/rte_dmadev.h | 27 +++++++++++++++++++++++++++ >>> 1 file changed, 27 insertions(+) >>> >>> diff --git a/lib/dmadev/rte_dmadev.h b/lib/dmadev/rte_dmadev.h index >>> b157ab7600..f7a6af2528 100644 >>> --- a/lib/dmadev/rte_dmadev.h >>> +++ b/lib/dmadev/rte_dmadev.h >>> @@ -278,6 +278,13 @@ int16_t rte_dma_next_dev(int16_t start_dev_id); >>> #define RTE_DMA_CAPA_OPS_COPY_SG RTE_BIT64(33) >>> /** Support fill operation. */ >>> #define RTE_DMA_CAPA_OPS_FILL RTE_BIT64(34) >>> +/** Support for source buffer free for mem to dev transfer. >> >> Support auto free source buffer once the M2D (memory-to-device) transfer >> completed. > > Ack > >> >>> + * >>> + * @note Even though the DMA driver has this capability, it may not >>> + support all >>> + * mempool drivers. If the mempool is not supported by the DMA >>> + driver, >>> + * rte_dma_vchan_setup() will fail. >> >> In addition to hardware support, there are requirements for buffer attribute >> (e.g. only specific mempool support it). >> >>> + **/ >>> +#define RTE_DMA_CAPA_MEM_TO_DEV_SOURCE_BUFFER_FREE >> RTE_BIT64(35) >> >> 1) this should follow RTE_DMA_CAPA_HANDLES_ERRORS, because it not >> new OPS. >> 2) the name is too long. how abort >> RTE_DMA_CAPA_AUTO_FREE_M2D_SBUF? I am not sure, suggest get better >> name. > > Ack. > >> >>> /**@}*/ >>> >>> /** >>> @@ -581,6 +588,19 @@ struct rte_dma_vchan_conf { >>> * @see struct rte_dma_port_param >>> */ >>> struct rte_dma_port_param dst_port; >>> + /** Mempool from which source buffer is allocated. Mempool info is >> used >>> + * for freeing source buffer by hardware when configured direction is >>> + * RTE_DMA_DIR_MEM_TO_DEV. To free the source buffer by >> hardware, >>> + * RTE_DMA_OP_FLAG_FREE_SBUF must be set while calling >> rte_dma_copy and >>> + * rte_dma_copy_sg(). >>> + * >>> + * @note If the mempool is not supported by the DMA device, >>> + * rte_dma_vchan_setup() will fail. >>> + * >>> + * @see RTE_DMA_OP_FLAG_FREE_SBUF >>> + */ >>> + struct rte_mempool *mem_to_dev_src_buf_pool; >>> + >>> }; >> >> Suggest add one extra struct e.g. >> struct rte_dma_auto_free_param { >> union { >> struct rte_mempool *pool; >> } >> uint64_t reserved[2]; /**< Reserved for future fields. */ }; >> >> In the above conf, we could add a new field: struct >> rte_dma_auto_free_param m2d_buf > > Ack, will add new struct. > struct rte_dma_auto_free_buf_param { > struct rte_mempool *pool; > uint64_t reserved[2]; /**< Reserved for future fields. */ }; > }; > > struct rte_dma_auto_free_buf_param m2d_sbuf; > >> >>> >>> /** >>> @@ -818,6 +838,13 @@ struct rte_dma_sge { >>> * capability bit for this, driver should not return error if this flag was set. >>> */ >>> #define RTE_DMA_OP_FLAG_LLC RTE_BIT64(2) >>> +/** Mem to dev source buffer free flag. >>> + * Used for freeing source DMA buffer by hardware when the transfer >>> +direction is >>> + * configured as RTE_DMA_DIR_MEM_TO_DEV. >>> + * >>> + * @see struct rte_dma_vchan_conf::mem_to_dev_src_buf_pool >>> + */ >>> +#define RTE_DMA_OP_FLAG_FREE_SBUF RTE_BIT64(3) >> >> Suggest RTE_DMA_OP_FLAG_AUTO_FREE_SBUF > > Ack > >> >>> /**@}*/ >>> >>> /** >>> >> >> The S in SBUF seem useless, because it should not auto free dstbuf in >> logically. >> >> Maybe we should direct use auto-free (just like silent)