From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id ACB81A0613 for ; Thu, 26 Sep 2019 22:11:14 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5F31B11A2; Thu, 26 Sep 2019 22:11:13 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by dpdk.org (Postfix) with ESMTP id D8B2191 for ; Thu, 26 Sep 2019 22:11:11 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 82FB54000C for ; Thu, 26 Sep 2019 22:11:10 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 7321940008; Thu, 26 Sep 2019 22:11:10 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on bernadotte.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-0.9 required=5.0 tests=ALL_TRUSTED,AWL autolearn=disabled version=3.4.2 X-Spam-Score: -0.9 Received: from [192.168.1.59] (host-90-232-207-236.mobileonline.telia.com [90.232.207.236]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id BA22440006; Thu, 26 Sep 2019 22:11:07 +0200 (CEST) To: Bruce Richardson Cc: =?UTF-8?Q?Morten_Br=c3=b8rup?= , olivier.matz@6wind.com, stephen@networkplumber.org, harry.van.haaren@intel.com, konstantin.ananyev@intel.com, dev@dpdk.org References: <20190925120355.44821-1-mb@smartsharesystems.com> <20190925120355.44821-3-mb@smartsharesystems.com> <20190926083024.GA1821@bricha3-MOBL.ger.corp.intel.com> From: =?UTF-8?Q?Mattias_R=c3=b6nnblom?= Message-ID: <699b4e3e-e253-5a6a-6488-3f9b38fd8f4c@ericsson.com> Date: Thu, 26 Sep 2019 22:11:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190926083024.GA1821@bricha3-MOBL.ger.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP Subject: Re: [dpdk-dev] [PATCH v2 2/2] mbuf: add bulk free function X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 2019-09-26 10:30, Bruce Richardson wrote: > On Wed, Sep 25, 2019 at 09:02:28PM +0200, Mattias Rönnblom wrote: >> On 2019-09-25 14:03, Morten Brørup wrote: >>> Add function for freeing a bulk of mbufs. >>> >>> Signed-off-by: Morten Brørup >>> --- >>> lib/librte_mbuf/rte_mbuf.c | 35 +++++++++++++++++++++++++++++++++++ >>> lib/librte_mbuf/rte_mbuf.h | 16 +++++----------- >>> 2 files changed, 40 insertions(+), 11 deletions(-) >>> >>> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c >>> index 37718d49c..b63a0eced 100644 >>> --- a/lib/librte_mbuf/rte_mbuf.c >>> +++ b/lib/librte_mbuf/rte_mbuf.c >>> @@ -245,6 +245,41 @@ int rte_mbuf_check(const struct rte_mbuf *m, int is_header, >>> return 0; >>> } >>> +/** >>> + * Maximum bulk of mbufs rte_pktmbuf_free_bulk() returns to mempool. >>> + */ >>> +#define RTE_PKTMBUF_FREE_BULK_SZ 64 >>> + >>> +/* Free a bulk of mbufs back into their original mempools. */ >>> +void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int count) >>> +{ >>> + struct rte_mbuf *m, *free[RTE_PKTMBUF_FREE_BULK_SZ]; >>> + unsigned int idx, nb_free = 0; >>> + >>> + for (idx = 0; idx < count; idx++) { >>> + m = mbufs[idx]; >>> + if (unlikely(m == NULL)) >>> + continue; >>> + >>> + __rte_mbuf_sanity_check(m, 1); >>> + m = rte_pktmbuf_prefree_seg(m); >>> + if (unlikely(m == NULL)) >>> + continue; >>> + >>> + if (nb_free >= RTE_PKTMBUF_FREE_BULK_SZ || >>> + (nb_free > 0 && m->pool != free[0]->pool)) { >> >> Maybe an unlikely() would be in order here? >> > I'd caution against it, since it can penalize the cold branch a lot. If a > branch really is predictable the HW branch predictors generally are good > enough to handle it at runtime. So long as a path is a valid path for a > runtime app, i.e. not something like a fatal error only ever hit once in an > entire run, I'd tend to omit likely()/unlikely() calls unless profiling > shows a real performance difference. > Let's see if I understand you: your worry is that wrapping that expression in an unlikely() will lead to code that is slower (than w/o the hint), if during runtime the probability turns out to be 50/50? Wouldn't leaving out unlikely() just lead to the compiler using its fancy heuristics in an attempt to come to a conclusion, what path is the more likely? About HW branch prediction - I'm sure it's good, but still the compiler needs to decided which code code path requires a branch, and which need not. Even if HW branch prediction successfully predicted a branch being taken, actually branching is going to be somewhat more expensive than to not branch?