From: "Jiang, Cheng1" <cheng1.jiang@intel.com>
To: Anoob Joseph <anoobj@marvell.com>,
"thomas@monjalon.net" <thomas@monjalon.net>,
"Richardson, Bruce" <bruce.richardson@intel.com>,
"mb@smartsharesystems.com" <mb@smartsharesystems.com>,
"Xia, Chenbo" <chenbo.xia@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Hu, Jiayu" <jiayu.hu@intel.com>,
"Ding, Xuan" <xuan.ding@intel.com>,
"Ma, WenwuX" <wenwux.ma@intel.com>,
"Wang, YuanX" <yuanx.wang@intel.com>,
"He, Xingguang" <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: Mon, 12 Jun 2023 07:40:17 +0000 [thread overview]
Message-ID: <SN7PR11MB7019D1CBA63497FB9AAC1889DC54A@SN7PR11MB7019.namprd11.prod.outlook.com> (raw)
In-Reply-To: <PH0PR18MB46725C4C78AF8575A75633E1DF51A@PH0PR18MB4672.namprd18.prod.outlook.com>
Hi,
Thanks for your comments, the replies are inline.
Thanks,
Cheng
> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Friday, June 9, 2023 7:44 PM
> To: Jiang, Cheng1 <cheng1.jiang@intel.com>; thomas@monjalon.net;
> Richardson, Bruce <bruce.richardson@intel.com>;
> mb@smartsharesystems.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan
> <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; Wang,
> YuanX <yuanx.wang@intel.com>; He, Xingguang <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
>
> 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.
[Cheng] Really good suggestion. This is feasible, but in the initial stage, we want to keep things simple. Perhaps in the future, we can add a parameter to configure the number of vchans for each device and then launch the corresponding number of threads for each vchan.
>
> > + 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.
[Cheng] Good idea, I'll improve it in the next version.
>
> > + 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 */
> }
> }
>
>
[Cheng] Good idea, we don't have to check the capacity explicitly. I think your implementation is more clear, thanks. I will fix it in the next version.
> > + }
> > + 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.
>
[Cheng] sure, got it. Thanks!
> <snip>
next prev parent reply other threads:[~2023-06-12 7:40 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 ` [EXT] " Anoob Joseph
2023-06-12 7:40 ` Jiang, Cheng1 [this message]
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=SN7PR11MB7019D1CBA63497FB9AAC1889DC54A@SN7PR11MB7019.namprd11.prod.outlook.com \
--to=cheng1.jiang@intel.com \
--cc=amitprakashs@marvell.com \
--cc=anoobj@marvell.com \
--cc=bruce.richardson@intel.com \
--cc=chenbo.xia@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).