DPDK patches and discussions
 help / color / mirror / Atom feed
From: Anoob Joseph <anoobj@marvell.com>
To: Cheng Jiang <cheng1.jiang@intel.com>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	"bruce.richardson@intel.com" <bruce.richardson@intel.com>,
	"mb@smartsharesystems.com" <mb@smartsharesystems.com>,
	"chenbo.xia@intel.com" <chenbo.xia@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"jiayu.hu@intel.com" <jiayu.hu@intel.com>,
	 "xuan.ding@intel.com" <xuan.ding@intel.com>,
	"wenwux.ma@intel.com" <wenwux.ma@intel.com>,
	"yuanx.wang@intel.com" <yuanx.wang@intel.com>,
	"xingguang.he@intel.com" <xingguang.he@intel.com>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	Vamsi Krishna Attunuru <vattunuru@marvell.com>,
	Amit Prakash Shukla <amitprakashs@marvell.com>,
	Satha Koteswara Rao Kottidi <skoteshwar@marvell.com>,
	Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>,
	Vidya Sagar Velumuri <vvelumuri@marvell.com>
Subject: RE: [EXT] [PATCH v5] app/dma-perf: introduce dma-perf application
Date: Fri, 9 Jun 2023 11:44:13 +0000	[thread overview]
Message-ID: <PH0PR18MB46725C4C78AF8575A75633E1DF51A@PH0PR18MB4672.namprd18.prod.outlook.com> (raw)
In-Reply-To: <20230608084355.16209-1-cheng1.jiang@intel.com>

Hi,

Thanks for adding the app. Few comments inline. Please check.

Thanks,
Anoob

> -----Original Message-----
> From: Cheng Jiang <cheng1.jiang@intel.com>
> Sent: Thursday, June 8, 2023 2:14 PM
> To: thomas@monjalon.net; bruce.richardson@intel.com;
> mb@smartsharesystems.com; chenbo.xia@intel.com
> Cc: dev@dpdk.org; jiayu.hu@intel.com; xuan.ding@intel.com;
> wenwux.ma@intel.com; yuanx.wang@intel.com; xingguang.he@intel.com;
> Cheng Jiang <cheng1.jiang@intel.com>
> Subject: [EXT] [PATCH v5] app/dma-perf: introduce dma-perf application
> 
> External Email
> 
> ----------------------------------------------------------------------
> There are many high-performance DMA devices supported in DPDK now,
> and these DMA devices can also be integrated into other modules of DPDK as
> accelerators, such as Vhost. Before integrating DMA into applications,
> developers need to know the performance of these DMA devices in various
> scenarios and the performance of CPUs in the same scenario, such as
> different buffer lengths. Only in this way can we know the target
> performance of the application accelerated by using them. This patch
> introduces a high-performance testing tool, which supports comparing the
> performance of CPU and DMA in different scenarios automatically with a pre-
> set config file. Memory Copy performance test are supported for now.
> 
> Signed-off-by: Cheng Jiang <cheng1.jiang@intel.com>
> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Chenbo Xia <chenbo.xia@intel.com>
> ---
> v5:
>   fixed some LONG_LINE warnings;
> v4:
>   fixed inaccuracy of the memory footprint display;
> v3:
>   fixed some typos;
> v2:
>   added lcore/dmadev designation;
>   added error case process;
>   removed worker_threads parameter from config.ini;
>   improved the logs;
>   improved config file;
> 
>  app/meson.build               |   1 +
>  app/test-dma-perf/benchmark.c | 472 ++++++++++++++++++++++++++++
> app/test-dma-perf/config.ini  |  59 ++++
>  app/test-dma-perf/main.c      | 569
> ++++++++++++++++++++++++++++++++++
>  app/test-dma-perf/main.h      |  69 +++++
>  app/test-dma-perf/meson.build |  17 +
>  6 files changed, 1187 insertions(+)
>  create mode 100644 app/test-dma-perf/benchmark.c  create mode 100644
> app/test-dma-perf/config.ini  create mode 100644 app/test-dma-
> perf/main.c  create mode 100644 app/test-dma-perf/main.h  create mode
> 100644 app/test-dma-perf/meson.build
> 

<snip>

> +
> +/* Configuration of device. */
> +static void
> +configure_dmadev_queue(uint32_t dev_id, uint32_t ring_size) {
> +	uint16_t vchan = 0;
> +	struct rte_dma_info info;
> +	struct rte_dma_conf dev_config = { .nb_vchans = 1 };

[Anoob] Is it possible to use more vchans? The code launches as many threads as the number of dma devices. Instead it should be total number of vchans.

> +	struct rte_dma_vchan_conf qconf = {
> +		.direction = RTE_DMA_DIR_MEM_TO_MEM,
> +		.nb_desc = ring_size
> +	};
> +
> +	if (rte_dma_configure(dev_id, &dev_config) != 0)
> +		rte_exit(EXIT_FAILURE, "Error with dma configure.\n");
> +
> +	if (rte_dma_vchan_setup(dev_id, vchan, &qconf) != 0)
> +		rte_exit(EXIT_FAILURE, "Error with queue configuration.\n");
> +
> +	rte_dma_info_get(dev_id, &info);
> +	if (info.nb_vchans != 1)
> +		rte_exit(EXIT_FAILURE, "Error, no configured queues
> reported on device id. %u\n",
> +				dev_id);
> +
> +	if (rte_dma_start(dev_id) != 0)
> +		rte_exit(EXIT_FAILURE, "Error with dma start.\n"); }
> +
> +

<snip>

> +static inline int
> +do_dma_mem_copy(void *p)
> +{
> +	uint16_t *para_idx = (uint16_t *)p;
> +	volatile struct lcore_params *para = worker_params[*para_idx];
> +	volatile struct worker_info *worker_info = &(para->worker_info);
> +	uint16_t dev_id = para->dev_id;
> +	uint32_t nr_buf = para->nr_buf;
> +	uint16_t kick_batch = para->kick_batch;

[Anoob] Some of these variables can be made const. Since this is fast path, might be beneficial doing that way.

> +	uint32_t buf_size = para->buf_size;
> +	struct rte_mbuf **srcs = para->srcs;
> +	struct rte_mbuf **dsts = para->dsts;
> +	int64_t async_cnt = 0;
> +	int nr_cpl = 0;
> +	uint32_t i;
> +	uint32_t poll_cnt = 0;
> +
> +	worker_info->stop_flag = false;
> +	worker_info->ready_flag = true;
> +
> +	while (!worker_info->start_flag)
> +		;
> +
> +	while (1) {
> +		for (i = 0; i < nr_buf; i++) {
> +			if (unlikely(rte_dma_copy(dev_id,
> +						0,
> +						rte_pktmbuf_iova(srcs[i]),
> +						rte_pktmbuf_iova(dsts[i]),
> +						buf_size,
> +						0) < 0)) {
> +				rte_dma_submit(dev_id, 0);
> +				while (rte_dma_burst_capacity(dev_id, 0) ==
> 0) {
> +					nr_cpl = rte_dma_completed(dev_id,
> 0, MAX_DMA_CPL_NB,
> +								NULL, NULL);
> +					async_cnt -= nr_cpl;
> +					worker_info->total_cpl += nr_cpl;
> +				}
> +				if (rte_dma_copy(dev_id,
> +						0,
> +						rte_pktmbuf_iova(srcs[i]),
> +						rte_pktmbuf_iova(dsts[i]),
> +						buf_size,
> +						0) < 0) {
> +					printf("enqueue fail again at %u\n",
> i);
> +					printf("space:%d\n",
> rte_dma_burst_capacity(dev_id, 0));
> +					rte_exit(EXIT_FAILURE, "DMA
> enqueue failed\n");
> +				}

[Anoob] Only if the API returns -ENOSPC we should retry submitting, right? Other errors should be treated as fatal errors.

Do we need to use rte_dma_burst_capacity() API?

Can't we try something like,

dma_copy:
		ret =  rte_dma_copy(dev_id, 0, rte_pktmbuf_iova(srcs[i]), rte_pktmbuf_iova(dsts[i]), buf_size, 0);
		if (unlikely (ret < 0) {
			if (ret == -ENOSPC) {
				rte_dma_submit(dev_id, 0);
				/* DMA completed & other handling */
				goto dma_copy;
			} else {
				/* Error exit */
			}
		}

			
> +			}
> +			async_cnt++;
> +
> +			if ((async_cnt % kick_batch) == 0) {
> +				rte_dma_submit(dev_id, 0);
> +				/* add a poll to avoid ring full */
> +				nr_cpl = rte_dma_completed(dev_id, 0,
> MAX_DMA_CPL_NB, NULL, NULL);
> +				async_cnt -= nr_cpl;
> +				worker_info->total_cpl += nr_cpl;

[Anoob] Above code can be made as a static inline function so that in cases rte_dma_copy returns -ENOSPC, same static inline can be called.

<snip>

  reply	other threads:[~2023-06-09 11:44 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-20  7:22 [PATCH] " Cheng Jiang
2023-05-17  6:16 ` [PATCH v2] " Cheng Jiang
2023-05-17  7:31 ` [PATCH v3] " Cheng Jiang
2023-06-08  5:03 ` [PATCH v4] " Cheng Jiang
2023-06-08  8:27   ` Xia, Chenbo
2023-06-08  8:38     ` Jiang, Cheng1
2023-06-08  8:43 ` [PATCH v5] " Cheng Jiang
2023-06-09 11:44   ` Anoob Joseph [this message]
2023-06-12  7:40     ` [EXT] " Jiang, Cheng1
2023-06-09 14:03   ` Amit Prakash Shukla
2023-06-12  8:26     ` Jiang, Cheng1
2023-06-13  4:51       ` Jiang, Cheng1
2023-06-13  7:34         ` Amit Prakash Shukla
2023-06-13  4:31 ` [PATCH v6] " Cheng Jiang
2023-06-13 12:55   ` huangdengdui
2023-06-14  6:40     ` Jiang, Cheng1
2023-06-15  5:21   ` [EXT] " Anoob Joseph
2023-06-15  8:01     ` Jiang, Cheng1
2023-06-15  8:44       ` Anoob Joseph
2023-06-15 14:05         ` Jiang, Cheng1
2023-06-15 15:47           ` Anoob Joseph
2023-06-16  2:56             ` Jiang, Cheng1
2023-06-16  6:32               ` Anoob Joseph
2023-06-16  8:43                 ` Jiang, Cheng1
2023-06-16  9:48                   ` Anoob Joseph
2023-06-16 10:52                     ` Anoob Joseph
2023-06-16 15:15                       ` Jiang, Cheng1
2023-06-17  4:35                         ` Jiang, Cheng1
2023-06-19  5:48                           ` Anoob Joseph
2023-06-19  6:21                             ` Jiang, Cheng1
2023-06-18  5:34                         ` Jiang, Cheng1
2023-06-19  5:25                           ` Anoob Joseph
2023-06-19  6:17                             ` Jiang, Cheng1
2023-06-18 12:26 ` [PATCH v7] " Cheng Jiang
2023-06-20  6:53 ` [PATCH v8] " Cheng Jiang
2023-06-23  6:52   ` [EXT] " Anoob Joseph
2023-06-24 11:52     ` Jiang, Cheng1
2023-06-26  5:41       ` Anoob Joseph
2023-06-26 10:02         ` Jiang, Cheng1
2023-06-26  9:41 ` [PATCH v9] " Cheng Jiang
2023-06-28  1:20 ` [PATCH v10] " Cheng Jiang
2023-06-28  4:42   ` [EXT] " Anoob Joseph
2023-06-28  6:06   ` Ling, WeiX
2023-06-29  9:08   ` Thomas Monjalon
2023-06-29 12:50     ` Jiang, Cheng1
2023-06-29 13:19       ` Thomas Monjalon
2023-06-29 13:24         ` Jiang, Cheng1
2023-06-29  9:38   ` Thomas Monjalon
2023-06-29 12:51     ` Jiang, Cheng1
2023-06-29 13:14 ` [PATCH v11] " Cheng Jiang
2023-07-03  8:20   ` fengchengwen
2023-07-07  9:56     ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PH0PR18MB46725C4C78AF8575A75633E1DF51A@PH0PR18MB4672.namprd18.prod.outlook.com \
    --to=anoobj@marvell.com \
    --cc=amitprakashs@marvell.com \
    --cc=bruce.richardson@intel.com \
    --cc=chenbo.xia@intel.com \
    --cc=cheng1.jiang@intel.com \
    --cc=dev@dpdk.org \
    --cc=gmuthukrishn@marvell.com \
    --cc=jerinj@marvell.com \
    --cc=jiayu.hu@intel.com \
    --cc=mb@smartsharesystems.com \
    --cc=skoteshwar@marvell.com \
    --cc=thomas@monjalon.net \
    --cc=vattunuru@marvell.com \
    --cc=vvelumuri@marvell.com \
    --cc=wenwux.ma@intel.com \
    --cc=xingguang.he@intel.com \
    --cc=xuan.ding@intel.com \
    --cc=yuanx.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).