Hi Chengwen,
Thanks for the review and feedback. I will send v2 with suggested changes.
Thanks,
Amit Shukla
> -----Original Message-----
> From: fengchengwen <fengcheng...@huawei.com>
> Sent: Thursday, November 2, 2023 7:37 AM
> To: Amit Prakash Shukla <amitpraka...@marvell.com>; Kevin Laatz
> <kevin.la...@intel.com>; Bruce Richardson <bruce.richard...@intel.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jer...@marvell.com>;
> conor.wa...@intel.com; Vamsi Krishna Attunuru <vattun...@marvell.com>;
> g.si...@nxp.com; sachin.sax...@oss.nxp.com; hemant.agra...@nxp.com;
> cheng1.ji...@intel.com; Nithin Kumar Dabilpuram
> <ndabilpu...@marvell.com>; Anoob Joseph <ano...@marvell.com>;
> m...@smartsharesystems.com
> Subject: [EXT] Re: [PATCH] test/dma: fix for buffer auto free
>
> External Email
>
> ----------------------------------------------------------------------
> Hi Amit,
>
> I prefer not use static variable to control it because it introduce many
> coupling.
>
> Suggest add one function which prepare the test_m2d_auto_free, like
> prepare_m2d_auto_free
>
> if ((info.dev_capa & RTE_DMA_CAPA_M2D_AUTO_FREE) &&
> dma_add_test[TEST_M2D_AUTO_FREE].enabled == true) {
> if (prepare_m2d_auto_free(dev_id) != 0)
> goto err;
> if (runtest("m2d_auto_free", test_m2d_auto_free, 128,
> dev_id, vchan,
> CHECK_ERRS) < 0)
> goto err;
> }
>
> In the new function, could do reinit vchan just like the beginging
> test_m2d_auto_free.
> static int prepare_m2d_auto_free(int dev_id) {
> const struct rte_dma_vchan_conf qconf = {
> .direction = RTE_DMA_DIR_MEM_TO_DEV,
> .nb_desc = TEST_RINGSIZE,
> .auto_free.m2d.pool = pool,
> .dst_port.port_type = RTE_DMA_PORT_PCIE,
> .dst_port.pcie.coreid = 0,
> };
> /* Stop the device to reconfigure vchan because should use Mem-to-
> Dev mode. */
> if (rte_dma_stop(dev_id) < 0)
> ERR_RETURN("Error stopping device %u\n", dev_id);
> if (rte_dma_vchan_setup(dev_id, vchan, &qconf) < 0)
> ERR_RETURN("Error with queue configuration\n");
> if (rte_dma_start(dev_id) != 0)
> ERR_RETURN("Error with rte_dma_start()\n");
> return 0;
> }
>
>
>
> Also I found a bug in test_m2d_auto_free function, if above path taken:
> if (rte_pktmbuf_alloc_bulk(pool, src, NR_MBUF) != 0) {
> printf("alloc src mbufs failed.\n");
> ret = -1;
> goto done;
> }
>
> done:
> rte_pktmbuf_free_bulk(dst, NR_MBUF);
> /* If the test passes source buffer will be freed in hardware. */
> if (ret < 0)
> rte_pktmbuf_free_bulk(&src[nb_done], (NR_MBUF -
> nb_done));
> ----- then it will free invalid mbuf to pool because src was not
> success init
>
>
<snip>
