From: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
To: "Chalupnik, KamilX" <kamilx.chalupnik@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Cc: "Mokhtar, Amr" <amr.mokhtar@intel.com>
Subject: Re: [dpdk-dev] [PATCH 09/13] bbdev: measure offload cost
Date: Tue, 8 May 2018 10:16:21 +0000 [thread overview]
Message-ID: <E115CCD9D858EF4F90C690B0DCB4D8976CD03C4A@IRSMSX108.ger.corp.intel.com> (raw)
In-Reply-To: <EEA9FF629BF25B47BD67ADE995041EE23D035350@IRSMSX103.ger.corp.intel.com>
> -----Original Message-----
> From: Chalupnik, KamilX
> Sent: Tuesday, May 8, 2018 10:48 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; dev@dpdk.org
> Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> Subject: RE: [PATCH 09/13] bbdev: measure offload cost
>
> Hi Pablo,
>
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Tuesday, May 8, 2018 11:08 AM
> > To: Chalupnik, KamilX <kamilx.chalupnik@intel.com>; dev@dpdk.org
> > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> > Subject: RE: [PATCH 09/13] bbdev: measure offload cost
> >
> > Hi Kamil,
> >
> > > -----Original Message-----
> > > From: Chalupnik, KamilX
> > > Sent: Tuesday, May 8, 2018 8:56 AM
> > > To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> > > dev@dpdk.org
> > > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> > > Subject: RE: [PATCH 09/13] bbdev: measure offload cost
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: De Lara Guarch, Pablo
> > > > Sent: Monday, May 7, 2018 3:29 PM
> > > > To: Chalupnik, KamilX <kamilx.chalupnik@intel.com>; dev@dpdk.org
> > > > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>
> > > > Subject: RE: [PATCH 09/13] bbdev: measure offload cost
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Chalupnik, KamilX
> > > > > Sent: Thursday, April 26, 2018 2:30 PM
> > > > > To: dev@dpdk.org
> > > > > Cc: Mokhtar, Amr <amr.mokhtar@intel.com>; De Lara Guarch, Pablo
> > > > > <pablo.de.lara.guarch@intel.com>; Chalupnik, KamilX
> > > > > <kamilx.chalupnik@intel.com>
> > > > > Subject: [PATCH 09/13] bbdev: measure offload cost
> > > > >
> > > >
> > > > ...
> > > >
> > > > > --- a/lib/librte_bbdev/rte_bbdev.h
> > > > > +++ b/lib/librte_bbdev/rte_bbdev.h
> > > > > @@ -239,6 +239,10 @@ struct rte_bbdev_stats {
> > > > > uint64_t enqueue_err_count;
> > > > > /** Total error count on operations dequeued */
> > > > > uint64_t dequeue_err_count;
> > > > > +#ifdef RTE_TEST_BBDEV
> > > > > + /** It stores offload time. */
> > > >
> > > > Just "offload time" is fine.
> > > >
> > > > > + uint64_t offload_time;
> > > > > +#endif
> > > >
> > > > Again, I don't think it is a good idea to have this compilation check.
> > > > RTE_TEST_BBDEV is used to enable the compilation of the test app,
> > > > so it shouldn't be used for anything else.
> > > > Also, in DPDK, we are avoiding the usage of this conditionals to
> > > > enable/disable pieces of code.
> > >
> > > > If you want to avoid the computation of this time, add a
> > > > configuration option in bbdev configuration structure
> > > > (rte_bbdev_queue_conf?), so the decision is made at runtime.
> > >
> > > Ok, but this 'offload_time' variable is used only for testing
> > > purposes so we decided that it should exist only when test application is
> being built.
> > > In case where we move the decision to runtime phase it may have bad
> > > impact on driver performance because then each time we have to check
> > > if
> > 'offload_time'
> > > has to be calculated or not.
> >
> > I understand the performance penalty. Anyway, if you go for build time
> > check, you should have another option name. The test app option should
> > not affect the code in a library/PMD.
> > Also, this option should probably be disabled, to avoid the extra
> > calculations unnecessarily.
> > Lastly, I think it would be better to always have "offload_time" field
> > in the structure, so remove the check there.
> >
>
> So should I create and use in driver new guard for 'offload_time' and add it to
> config/common_base (or set it in Makefile when RTE_TEST_BBDEV is set). Are
> you ok for such exception to the DPDK code guidelines in this case?
I would leave "offload_time" always available in the structure, so its size doesn't change
depending on this option.
It is better to avoid adding these options, but if we do it for perf reasons (if it affects data path code),
then at least it should be well designed, so there should be a separate option (either under TURBO_SW or BBDEV),
and I think it should be disabled by default.
The test app is enabled by default, so users most likely would have this enabled without needing it,
impacting performance. Therefore, I'd say it is better to avoid relating this to the test app.
You can document that this option should be enabled if offload times are wanted
(and maybe check in the test app if the compilation flag is set and show a message to enable it if user is interested).
>
> Best regards,
> Kamil
next prev parent reply other threads:[~2018-05-08 10:16 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-26 13:29 [dpdk-dev] [PATCH 01/13] baseband/turbo_sw: update DPDK to work with FlexRAN 1.4.0 Kamil Chalupnik
2018-04-26 13:29 ` [dpdk-dev] [PATCH 02/13] doc/turbo_sw: update Wireless Baseband Device documentation Kamil Chalupnik
2018-05-07 13:37 ` De Lara Guarch, Pablo
2018-05-09 14:46 ` [dpdk-dev] [PATCH v2 12/14] " Kamil Chalupnik
2018-04-26 13:29 ` [dpdk-dev] [PATCH 03/13] doc/bbdev: dynamic lib support Kamil Chalupnik
2018-05-09 14:51 ` [dpdk-dev] [PATCH v2 13/14] " Kamil Chalupnik
2018-04-26 13:29 ` [dpdk-dev] [PATCH 04/13] baseband/turbo_sw: memcpy changed or removed from driver Kamil Chalupnik
2018-05-07 12:26 ` De Lara Guarch, Pablo
2018-05-09 14:17 ` [dpdk-dev] [PATCH v2 02/14] baseband/turbo_sw: memory copying optimized or removed Kamil Chalupnik
2018-04-26 13:29 ` [dpdk-dev] [PATCH 05/13] baseband/turbo_sw: scalling input LLR to range [-16 16] Kamil Chalupnik
2018-05-07 12:50 ` De Lara Guarch, Pablo
2018-05-09 14:23 ` [dpdk-dev] [PATCH v2 04/14] baseband/turbo_sw: scalling likelihood ratio (LLR) input Kamil Chalupnik
2018-04-26 13:30 ` [dpdk-dev] [PATCH 06/13] baseband/turbo_sw: increase internal buffers Kamil Chalupnik
2018-05-09 14:25 ` [dpdk-dev] [PATCH v2 05/14] " Kamil Chalupnik
2018-04-26 13:30 ` [dpdk-dev] [PATCH 07/13] baseband/turbo_sw: support for optional CRC overlap Kamil Chalupnik
2018-05-09 14:28 ` [dpdk-dev] [PATCH v2 06/14] " Kamil Chalupnik
2018-04-26 13:30 ` [dpdk-dev] [PATCH 08/13] app/bbdev: update test vectors names Kamil Chalupnik
2018-05-07 13:15 ` De Lara Guarch, Pablo
2018-05-09 14:37 ` [dpdk-dev] [PATCH v2 09/14] " Kamil Chalupnik
2018-04-26 13:30 ` [dpdk-dev] [PATCH 09/13] bbdev: measure offload cost Kamil Chalupnik
2018-05-07 13:29 ` De Lara Guarch, Pablo
[not found] ` <EEA9FF629BF25B47BD67ADE995041EE23D0352A7@IRSMSX103.ger.corp.intel.com>
2018-05-08 9:08 ` De Lara Guarch, Pablo
[not found] ` <EEA9FF629BF25B47BD67ADE995041EE23D035350@IRSMSX103.ger.corp.intel.com>
2018-05-08 10:16 ` De Lara Guarch, Pablo [this message]
2018-05-09 14:30 ` [dpdk-dev] [PATCH v2 07/14] " Kamil Chalupnik
2018-04-26 13:30 ` [dpdk-dev] [PATCH 10/13] doc: update tests and usage of test app description Kamil Chalupnik
2018-05-09 14:55 ` [dpdk-dev] [PATCH v2 14/14] " Kamil Chalupnik
2018-04-26 13:30 ` [dpdk-dev] [PATCH 11/13] app/bbdev: added new test vectors Kamil Chalupnik
2018-05-09 14:39 ` [dpdk-dev] [PATCH v2 10/14] " Kamil Chalupnik
2018-04-26 13:30 ` [dpdk-dev] [PATCH 12/13] bbdev: split queue groups Kamil Chalupnik
2018-05-09 14:35 ` [dpdk-dev] [PATCH v2 08/14] " Kamil Chalupnik
2018-04-26 13:30 ` [dpdk-dev] [PATCH 13/13] app/bbdev: improve readability of test application Kamil Chalupnik
2018-05-09 14:42 ` [dpdk-dev] [PATCH v2 11/14] " Kamil Chalupnik
2018-05-09 18:55 ` De Lara Guarch, Pablo
2018-04-26 13:30 ` [dpdk-dev] [PATCH 00/13] Documentation and Turbo Software Baseband Device Update Kamil Chalupnik
2018-05-09 14:14 ` [dpdk-dev] [PATCH v2 01/14] baseband/turbo_sw: update DPDK to work with FlexRAN 1.4.0 Kamil Chalupnik
2018-05-09 14:14 ` [dpdk-dev] [PATCH v2 00/14] Documentation and Turbo Software Baseband Device Update Kamil Chalupnik
2018-05-09 19:21 ` De Lara Guarch, Pablo
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=E115CCD9D858EF4F90C690B0DCB4D8976CD03C4A@IRSMSX108.ger.corp.intel.com \
--to=pablo.de.lara.guarch@intel.com \
--cc=amr.mokhtar@intel.com \
--cc=dev@dpdk.org \
--cc=kamilx.chalupnik@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).