From: "Ruifeng Wang (Arm Technology China)" <Ruifeng.Wang@arm.com>
To: "Medvedkin, Vladimir" <vladimir.medvedkin@intel.com>
Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
Stephen Hemminger <stephen@networkplumber.org>,
"bruce.richardson@intel.com" <bruce.richardson@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"Gavin Hu (Arm Technology China)" <Gavin.Hu@arm.com>,
nd <nd@arm.com>, nd <nd@arm.com>
Subject: Re: [dpdk-dev] [PATCH v3 1/3] lib/lpm: not inline unnecessary functions
Date: Fri, 5 Jul 2019 10:58:23 +0000 [thread overview]
Message-ID: <AM0PR08MB4418AC65DDDDE279F870341E9EF50@AM0PR08MB4418.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <d79d603c-0463-9f8b-70cf-76aa35d406f4@intel.com>
> -----Original Message-----
> From: Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
> Sent: Friday, July 5, 2019 18:41
> To: Ruifeng Wang (Arm Technology China) <Ruifeng.Wang@arm.com>
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Stephen
> Hemminger <stephen@networkplumber.org>; bruce.richardson@intel.com;
> dev@dpdk.org; Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>; nd
> <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v3 1/3] lib/lpm: not inline unnecessary
> functions
>
>
> On 01/07/2019 07:44, Ruifeng Wang (Arm Technology China) wrote:
> > Hi Medvedkin,
> >
> >> -----Original Message-----
> >> From: Stephen Hemminger <stephen@networkplumber.org>
> >> Sent: Friday, June 28, 2019 23:35
> >> To: Medvedkin, Vladimir <vladimir.medvedkin@intel.com>
> >> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Ruifeng
> Wang
> >> (Arm Technology China) <Ruifeng.Wang@arm.com>;
> >> bruce.richardson@intel.com; dev@dpdk.org; Gavin Hu (Arm Technology
> >> China) <Gavin.Hu@arm.com>; nd <nd@arm.com>
> >> Subject: Re: [dpdk-dev] [PATCH v3 1/3] lib/lpm: not inline
> >> unnecessary functions
> >>
> >> On Fri, 28 Jun 2019 15:16:30 +0100
> >> "Medvedkin, Vladimir" <vladimir.medvedkin@intel.com> wrote:
> >>
> >>> Hi Honnappa,
> >>>
> >>> On 28/06/2019 14:57, Honnappa Nagarahalli wrote:
> >>>>> Hi all,
> >>>>>
> >>>>> On 28/06/2019 05:34, Stephen Hemminger wrote:
> >>>>>> On Fri, 28 Jun 2019 02:44:54 +0000 "Ruifeng Wang (Arm Technology
> >>>>>> China)"<Ruifeng.Wang@arm.com> wrote:
> >>>>>>
> >>>>>>>>> Tests showed that the function inlining caused performance
> >>>>>>>>> drop on some x86 platforms with the memory ordering patches
> applied.
> >>>>>>>>> By force no-inline functions, the performance was better than
> >>>>>>>>> before on x86 and no impact to arm64 platforms.
> >>>>>>>>>
> >>>>>>>>> Suggested-by: Medvedkin
> >> Vladimir<vladimir.medvedkin@intel.com>
> >>>>>>>>> Signed-off-by: Ruifeng Wang<ruifeng.wang@arm.com>
> >>>>>>>>> Reviewed-by: Gavin Hu<gavin.hu@arm.com>
> >>>>>>>> {
> >>>>>>>>
> >>>>>>>> Do you actually need to force noinline or is just taking of
> >>>>>>>> inline
> >> enough?
> >>>>>>>> In general, letting compiler decide is often best practice.
> >>>>>>> The force noinline is an optimization for x86 platforms to keep
> >>>>>>> rte_lpm_add() API performance with memory ordering applied.
> >>>>>> I don't think you answered my question. What does a recent
> >>>>>> version of GCC do if you drop the inline.
> >>>>>>
> >>>>>> Actually all the functions in rte_lpm should drop inline.
> >>>>> I'm agree with Stephen. If it is not a fastpath and size of
> >>>>> function is not minimal it is good to remove inline qualifier for
> >>>>> other control plane functions such as rule_add/delete/find/etc and
> >>>>> let the compiler decide to inline it (unless it affects performance).
> >>>> IMO, the rule needs to be simple. If it is control plane function,
> >>>> we should
> >> leave it to the compiler to decide. I do not think we need to worry
> >> too much about performance for control plane functions.
> >>> Control plane is not as important as data plane speed but it is
> >>> still important. For lpm we are talking not about initialization,
> >>> but runtime routes add/del related functions. If it is very slow the
> >>> library will be totally unusable because after it receives a route
> >>> update it will be blocked for a long time and route update queue
> >>> would
> >> overflow.
> >>
> >> Control plane performance is more impacted by algorithmic choice.
> >> The original LPM had terrible (n^2?) control path. Current code is better.
> >> I had a patch using RB tree, but it was rejected because it used the
> >> /usr/include/bsd/sys/tree.h which added a dependency.
> > Based on current discussion, I'd like to drop this single patch from the patch
> set.
> > Since it is not directly related to memory ordering changes in this library.
> > We can remove inlines in a follow up patch.
> I think this patch is indirectly related to changes. I can't accept a memory
> ordering patch series _before_ this patch because a repository state will
> appear in which the performance of LPM add/delete has dropped. So if it
> could be avoided it have to be avoided.
>
In patch set v4, I dropped this patch and added atomic relaxed store for other
table entries. Test result showed no performance drop with the whole patch set.
Test data was updated in commit message of the first patch.
Please have a check. Thanks.
next prev parent reply other threads:[~2019-07-05 10:58 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-27 9:37 Ruifeng Wang
2019-06-27 9:37 ` [dpdk-dev] [PATCH v3 2/3] lib/lpm: memory orderings to avoid race conditions for v1604 Ruifeng Wang
2019-06-27 9:37 ` [dpdk-dev] [PATCH v3 3/3] lib/lpm: memory orderings to avoid race conditions for v20 Ruifeng Wang
2019-06-28 13:33 ` Medvedkin, Vladimir
2019-06-29 17:35 ` Honnappa Nagarahalli
2019-07-05 13:45 ` Alex Kiselev
2019-07-05 16:56 ` Medvedkin, Vladimir
2019-07-01 7:08 ` Ruifeng Wang (Arm Technology China)
2019-06-27 15:24 ` [dpdk-dev] [PATCH v3 1/3] lib/lpm: not inline unnecessary functions Stephen Hemminger
2019-06-28 2:44 ` Ruifeng Wang (Arm Technology China)
2019-06-28 4:34 ` Stephen Hemminger
2019-06-28 5:48 ` Ruifeng Wang (Arm Technology China)
2019-06-28 13:47 ` Medvedkin, Vladimir
2019-06-28 13:57 ` Honnappa Nagarahalli
2019-06-28 14:16 ` Medvedkin, Vladimir
2019-06-28 15:35 ` Stephen Hemminger
2019-07-01 6:44 ` Ruifeng Wang (Arm Technology China)
2019-07-05 10:40 ` Medvedkin, Vladimir
2019-07-05 10:58 ` Ruifeng Wang (Arm Technology China) [this message]
2019-07-05 10:31 ` Medvedkin, Vladimir
2019-07-05 13:37 ` Alex Kiselev
2019-07-05 16:53 ` Medvedkin, Vladimir
2019-06-28 13:38 ` Medvedkin, Vladimir
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=AM0PR08MB4418AC65DDDDE279F870341E9EF50@AM0PR08MB4418.eurprd08.prod.outlook.com \
--to=ruifeng.wang@arm.com \
--cc=Gavin.Hu@arm.com \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=nd@arm.com \
--cc=stephen@networkplumber.org \
--cc=vladimir.medvedkin@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).