From: Anoob Joseph <anoobj@marvell.com>
To: "Jiang, Cheng1" <cheng1.jiang@intel.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>,
Amit Prakash Shukla <amitprakashs@marvell.com>,
"huangdengdui@huawei.com" <huangdengdui@huawei.com>,
"Laatz, Kevin" <kevin.laatz@intel.com>,
"fengchengwen@huawei.com" <fengchengwen@huawei.com>,
Jerin Jacob Kollanukkaran <jerinj@marvell.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>,
"Ling, WeiX" <weix.ling@intel.com>
Subject: RE: [EXT] [PATCH v8] app/dma-perf: introduce dma-perf application
Date: Mon, 26 Jun 2023 05:41:38 +0000 [thread overview]
Message-ID: <PH0PR18MB467235AF5600B687BA91729ADF26A@PH0PR18MB4672.namprd18.prod.outlook.com> (raw)
In-Reply-To: <SN7PR11MB70191146947F0CCA3DD3075BDC20A@SN7PR11MB7019.namprd11.prod.outlook.com>
Hi Cheng,
Please see inline.
Thanks,
Anoob
> -----Original Message-----
> From: Jiang, Cheng1 <cheng1.jiang@intel.com>
> Sent: Saturday, June 24, 2023 5:23 PM
> To: Anoob Joseph <anoobj@marvell.com>; thomas@monjalon.net;
> Richardson, Bruce <bruce.richardson@intel.com>;
> mb@smartsharesystems.com; Xia, Chenbo <chenbo.xia@intel.com>; Amit
> Prakash Shukla <amitprakashs@marvell.com>; huangdengdui@huawei.com;
> Laatz, Kevin <kevin.laatz@intel.com>; fengchengwen@huawei.com; Jerin
> Jacob Kollanukkaran <jerinj@marvell.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>;
> Ling, WeiX <weix.ling@intel.com>
> Subject: RE: [EXT] [PATCH v8] app/dma-perf: introduce dma-perf application
>
> Hi Anoob,
>
> Replies are inline.
>
> Thanks,
> Cheng
>
> > -----Original Message-----
> > From: Anoob Joseph <anoobj@marvell.com>
> > Sent: Friday, June 23, 2023 2:53 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>; Amit
> > Prakash Shukla <amitprakashs@marvell.com>;
> huangdengdui@huawei.com;
> > Laatz, Kevin <kevin.laatz@intel.com>; fengchengwen@huawei.com; Jerin
> > Jacob Kollanukkaran <jerinj@marvell.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>;
> Ling,
> > WeiX <weix.ling@intel.com>
> > Subject: RE: [EXT] [PATCH v8] app/dma-perf: introduce dma-perf
> > application
> >
> > Hi Cheng,
> >
> > Thanks for the new version. Please see inline.
> >
> > Thanks,
> > Anoob
> >
> > > -----Original Message-----
> > > From: Cheng Jiang <cheng1.jiang@intel.com>
> > > Sent: Tuesday, June 20, 2023 12:24 PM
> > > To: thomas@monjalon.net; bruce.richardson@intel.com;
> > > mb@smartsharesystems.com; chenbo.xia@intel.com; Amit Prakash
> Shukla
> > > <amitprakashs@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> > > huangdengdui@huawei.com; kevin.laatz@intel.com;
> > > fengchengwen@huawei.com; Jerin Jacob Kollanukkaran
> > > <jerinj@marvell.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;
> > > weix.ling@intel.com; Cheng Jiang <cheng1.jiang@intel.com>
> > > Subject: [EXT] [PATCH v8] 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>
> > > ---
> > > v8:
> > > fixed string copy issue in parse_lcore();
> > > improved some data display format;
> > > added doc in doc/guides/tools;
> > > updated release notes;
> > >
> > > v7:
> > > fixed some strcpy issues;
> > > removed cache setup in calling rte_pktmbuf_pool_create();
> > > fixed some typos;
> > > added some memory free and null set operations;
> > > improved result calculation;
> > > v6:
> > > improved code based on Anoob's comments;
> > > fixed some code structure issues;
> > > 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 | 498 +++++++++++++++++++++
> > > app/test-dma-perf/config.ini | 61 +++
> > > app/test-dma-perf/main.c | 594 +++++++++++++++++++++++++
> > > app/test-dma-perf/main.h | 69 +++
> > > app/test-dma-perf/meson.build | 17 +
> > > doc/guides/rel_notes/release_23_07.rst | 6 +
> > > doc/guides/tools/dmaperf.rst | 103 +++++
> > > doc/guides/tools/index.rst | 1 +
> > > 9 files changed, 1350 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 create mode 100644
> > > doc/guides/tools/dmaperf.rst
> > >
> >
<snip>
> >
> > > + fprintf(stderr, "Error: Fail to find DMA %s.\n",
> > > dma_name);
> > > + goto end;
> > > + }
> > > +
> > > + ldm->dma_ids[i] = dev_id;
> > > + configure_dmadev_queue(dev_id, ring_size);
> > > + ++nb_dmadevs;
> > > + }
> > > +
> > > +end:
> > > + if (nb_dmadevs < nb_workers) {
> > > + printf("Not enough dmadevs (%u) for all workers (%u).\n",
> > > nb_dmadevs, nb_workers);
> > > + return -1;
> > > + }
> > > +
> > > + printf("Number of used dmadevs: %u.\n", nb_dmadevs);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static inline void
> > > +do_dma_submit_and_poll(uint16_t dev_id, uint64_t *async_cnt,
> > > + volatile struct worker_info *worker_info) {
> > > + int ret;
> > > + uint16_t nr_cpl;
> > > +
> > > + ret = rte_dma_submit(dev_id, 0);
> > > + if (ret < 0) {
> > > + rte_dma_stop(dev_id);
> > > + rte_dma_close(dev_id);
> > > + rte_exit(EXIT_FAILURE, "Error with dma submit.\n");
> > > + }
> > > +
> > > + nr_cpl = rte_dma_completed(dev_id, 0, MAX_DMA_CPL_NB, NULL,
> > > NULL);
> > > + *async_cnt -= nr_cpl;
> > > + worker_info->total_cpl += nr_cpl;
> > > +}
> > > +
> > > +static inline int
> > > +do_dma_mem_copy(void *p)
> >
> > [Anoob] Just curious, why not pass struct lcore_params *para itself?
> > Is it because the pointer is volatile? If yes, then we can take an AI
> > to split the struct into volatile and non-volatile parts.
>
> [Cheng] The reason I did it this way is because I want to launch this function
> on another core by spawning a new thread, and rte_eal_remote_launch()
> takes a void * as the parameter. That's why I passed void *p. Your
> suggestion to split the struct into volatile and non-volatile parts is quite
> reasonable. I am thinking about the best way to implement it. Thanks.
[Anoob] Instead of passing the address of index variable as void *, you can easily send lcore_params pointer, right?
>
> >
> > > +{
> > > + const uint16_t *para_idx = (uint16_t *)p;
> > > + volatile struct lcore_params *para = lcores_p[*para_idx].v_ptr;
> > > + volatile struct worker_info *worker_info = &(para->worker_info);
> > > + const uint16_t dev_id = para->dev_id;
> > > + const uint32_t nr_buf = para->nr_buf;
> > > + const uint16_t kick_batch = para->kick_batch;
> > > + const uint32_t buf_size = para->buf_size;
> > > + struct rte_mbuf **srcs = para->srcs;
> > > + struct rte_mbuf **dsts = para->dsts;
> > > + uint16_t nr_cpl;
> > > + uint64_t async_cnt = 0;
> > > + uint32_t i;
> > > + uint32_t poll_cnt = 0;
> > > + int ret;
> > > +
> > > + worker_info->stop_flag = false;
> > > + worker_info->ready_flag = true;
> > > +
> > > + while (!worker_info->start_flag)
> > > + ;
> > > +
> > > + while (1) {
> > > + for (i = 0; i < nr_buf; i++) {
> > > +dma_copy:
> > > + ret = rte_dma_copy(dev_id, 0,
> > > rte_pktmbuf_iova(srcs[i]),
> > > + rte_pktmbuf_iova(dsts[i]), buf_size, 0);
> >
> > [Anoob] Do we need to use ' rte_mbuf_data_iova' here instead of
> > 'rte_pktmbuf_iova'?
>
> [Cheng] yes rte_mbuf_data_iova is more appropriate, I'll fix it in the next
> version. Thanks.
>
> >
> > > + if (unlikely(ret < 0)) {
> > > + if (ret == -ENOSPC) {
> > > + do_dma_submit_and_poll(dev_id,
> > > &async_cnt, worker_info);
> > > + goto dma_copy;
> > > + } else {
> > > + /* Error exit */
> > > + rte_dma_stop(dev_id);
> >
> > [Anoob] Missing rte_dma_close() here. Also, may be introduce a static
> > void function so that rte_exit call etc won't be part of the fastpath loop.
> >
> > May be something like below and you can call it from here and
> > "do_dma_submit_and_poll".
> >
> > static void
> > error_exit(int dev_id)
> > {
> > /* Error exit */
> > rte_dma_stop(dev_id);
> > rte_dma_close(dev_id);
> > rte_exit(EXIT_FAILURE, "DMA enqueue failed\n"); }
> >
>
> [Cheng] I'm not so sure here. rte_dma_close() is called in the rte_exit(). Do
> we still call it explicitly before rte_exit()?
[Anoob] In ' do_dma_submit_and_poll', there is rte_dma_close() before rte_exit(). I'm fine either way as long is it is consistent. Said that, I think it is better to call close() from app, rather than relying on rte_exit.
<snip>
next prev parent reply other threads:[~2023-06-26 5:41 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
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 [this message]
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=PH0PR18MB467235AF5600B687BA91729ADF26A@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=fengchengwen@huawei.com \
--cc=huangdengdui@huawei.com \
--cc=jerinj@marvell.com \
--cc=jiayu.hu@intel.com \
--cc=kevin.laatz@intel.com \
--cc=mb@smartsharesystems.com \
--cc=thomas@monjalon.net \
--cc=weix.ling@intel.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).