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 D7FEFA2F6B for ; Tue, 8 Oct 2019 19:25:59 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 45AA61C10D; Tue, 8 Oct 2019 19:25:59 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 9C0B61C0C0 for ; Tue, 8 Oct 2019 19:25:57 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us4.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 1341D80065; Tue, 8 Oct 2019 17:25:54 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 8 Oct 2019 18:25:47 +0100 To: =?UTF-8?Q?Morten_Br=c3=b8rup?= , CC: , , , , , References: <20191007101427.16191-1-mb@smartsharesystems.com> <20191007101427.16191-2-mb@smartsharesystems.com> From: Andrew Rybchenko Message-ID: Date: Tue, 8 Oct 2019 20:25:43 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20191007101427.16191-2-mb@smartsharesystems.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24962.003 X-TM-AS-Result: No-11.302800-8.000000-10 X-TMASE-MatchedRID: vbSD0OnL8/LmLzc6AOD8DfHkpkyUphL9NV9S7O+u3Kbg91xayX4L8wfU ttYsKa6IlfQVJ0SKvCOltz4XrP4tnOQ/4WYdQ4Q1lGudLLtRO1t8s0cy6t/KSBQUOSCpbPwOv6I sUj4vY8WAYPCOtmtoVcvag90krfyaiVr6OqJ75nSBFNZJ/RfzGT+k5IvvZ1N/Av57j5eT9BaNhV Uy0CfBHfdbjBBjMRvUP60TTvjTfIFiWV0DQ85LUkhwlOfYeSqxMHi1Ydy2WEhHZg0gWH5yUVjiI KUaEypfefwhycM0BbP22dbE4mBG4EJcg9sKmyUQ5CghTisABMxQdXu/xf76ua1stzIom4KHbgWU zd5K/4bgROVLFVAP+HVPadPYHhMLT12Hc9FhVjUIvxEURH+P02f6wD367Vgtn0rL9iE2gT99Z+T 97iimsufOVcxjDhcwAYt5KiTiutkLbigRnpKlKSPzRlrdFGDwpEZA6mAyo6SkoMHm4vwBs5MP+h 3wUOnfgYPO4boRXVes/agQAQFd+A== X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--11.302800-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24962.003 X-MDID: 1570555556-utjDH3D7BMKR Subject: Re: [dpdk-dev] [PATCH v4 1/1] 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 10/7/19 1:14 PM, Morten Brørup wrote: > Add function for freeing a bulk of mbufs. > > v4: > * Marked as experimental by adding __rte_experimental. > * Add function to experimental section of map file. > * Fix source code formatting regarding pointer to pointer. > * Squashed multiple modifications into one. > v3: > * Bugfix: Handle pakets with multiple segments. > * Added inline helper function, mainly for readability. > * Fix source code formatting regarding indentation. > v2: > * Function is not inline. > * Optimized to free multible mbufs belonging to the same mempool in > bulk. Inspired by ixgbe_tx_free_bufs(), but allowing NULL pointers > in the array, just like rte_pktmbuf_free() can take a NULL pointer. > * Use unsigned int instead of unsigned. Passes checkpatch, but > mismatches the original coding style of the modified files. > * Fixed a typo in the description headline: mempools is plural. > > Signed-off-by: Morten Brørup > --- > lib/librte_mbuf/rte_mbuf.c | 50 ++++++++++++++++++++++++++++ > lib/librte_mbuf/rte_mbuf.h | 12 +++++++ > lib/librte_mbuf/rte_mbuf_version.map | 1 + > 3 files changed, 63 insertions(+) > > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c > index 37718d49c..0088117a3 100644 > --- a/lib/librte_mbuf/rte_mbuf.c > +++ b/lib/librte_mbuf/rte_mbuf.c > @@ -245,6 +245,56 @@ int rte_mbuf_check(const struct rte_mbuf *m, int is_header, > return 0; > } > > +/** > + * Size of the array holding mbufs from the same membool to be freed in bulk. > + */ > +#define RTE_PKTMBUF_FREE_BULK_SZ 64 How is the constant chosen? When we benchmarked similar thing in net/sfc, we finally stick to 32 as the best option. > + > +/** > + * @internal helper function for freeing a bulk of mbufs via an array holding > + * mbufs from the same mempool. > + */ > +static __rte_always_inline void > +rte_pktmbuf_free_seg_via_array(struct rte_mbuf *m, Other internal functions in the file have __ (two underscore) prefix. > + struct rte_mbuf ** const free, unsigned int * const nb_free) free is bad variable name because of libc free() function. It is a bit confusing. Same below. > +{ > + m = rte_pktmbuf_prefree_seg(m); > + if (likely(m != NULL)) { I'm not sure about likely() here, but hope that compiler is wise enough to inline rte_pktmbuf_prefree_seg() here and avoid the branch completely. > + if (*nb_free >= RTE_PKTMBUF_FREE_BULK_SZ || > + (*nb_free > 0 && m->pool != free[0]->pool)) { > + rte_mempool_put_bulk(free[0]->pool, (void **)free, > + *nb_free); > + *nb_free = 0; > + } > + > + free[(*nb_free)++] = m; > + } > +} > + > +/* 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, *m_next, *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); > + > + do { > + m_next = m->next; > + rte_pktmbuf_free_seg_via_array(m, free, &nb_free); > + m = m_next; > + } while (m != NULL); > + } > + > + if (nb_free > 0) > + rte_mempool_put_bulk(free[0]->pool, (void **)free, nb_free); > +} > + > /* dump a mbuf on console */ > void > rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len) > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index 98225ec80..871145796 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -1907,6 +1907,18 @@ static inline void rte_pktmbuf_free(struct rte_mbuf *m) > } > } > > +/** > + * Free a bulk of mbufs back into their original mempools. > + * > + * @param mbufs > + * Array of pointers to mbufs. > + * The array may contain NULL pointers. > + * @param count > + * Array size. > + */ > +__rte_experimental > +void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int count); > + > /** > * Creates a "clone" of the given packet mbuf. > * > diff --git a/lib/librte_mbuf/rte_mbuf_version.map b/lib/librte_mbuf/rte_mbuf_version.map > index 2662a37bf..cd6154ef2 100644 > --- a/lib/librte_mbuf/rte_mbuf_version.map > +++ b/lib/librte_mbuf/rte_mbuf_version.map > @@ -50,4 +50,5 @@ EXPERIMENTAL { > global: > > rte_mbuf_check; > + rte_pktmbuf_free_bulk; > } DPDK_18.08;