From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id AD1C4A0093; Tue, 8 Nov 2022 16:51:18 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 61B8E40151; Tue, 8 Nov 2022 16:51:17 +0100 (CET) Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by mails.dpdk.org (Postfix) with ESMTP id AEFDE400D4 for ; Tue, 8 Nov 2022 16:51:15 +0100 (CET) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 5F89A5C00B1; Tue, 8 Nov 2022 10:51:15 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Tue, 08 Nov 2022 10:51:15 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm1; t=1667922675; x= 1668009075; bh=Wn+6MhNk/70onelZjdQaDt2oqIaLzEgFvfuEsgTL49I=; b=r PWPlcFzjoTY+wavwMgeQeqO1w2yM0s/deOHE/YlF0E8Xedi4o/lDhAB93Bt7pHtw 1xXxSwwutshAUpZf/JMO2+OwLGhXrxhIm4vGDpyGoM1Quk5kYyQmWwbU72RO4KBo lnZQgYkn+pFSaFLzOX3lNWXmx6/FGDyEsuWUvzJzd71yVh48TVJbdEbNYTK6AWEc /57XADA8F4FoGzU2ykcEF2qKwJwoJqzapp9aoQ3sVv6JF2rIL+B2kwx6a7Fvjr2K 3eKtALAdjch+LqYUkQIRp3ZvfBZ2dDARsGnHFwCnGMh/W8D4KatxnXXTkJy1oPYI 2+beAKH/jkBgWEdl1+gEQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1667922675; x= 1668009075; bh=Wn+6MhNk/70onelZjdQaDt2oqIaLzEgFvfuEsgTL49I=; b=L BEggcb2ETrUhErcCraIxMbzUh+01ml+DcE/hX6MlltVwZkx7H0UWGD/IZqRQ45jW SlnNtfPnamFLFZ5Scs6jHoMqxmv8c57zYbTcTTL+QSzMnB06bLULi1gHtmnoi2M6 OrLko74qYAEVC3Gd+YrLI1L7iO6PILCprZZR9gQDQfI1JPfcvmZy2p3Ep52r3V3i AFzqF51TGKD+XSn8QE6AfipfbpX2iJNPNhCiu7yWzzbWsL4C6KLbMjZTIa0voE2d BNno0InV5Mb4aXR5AtGYaMrTBrTbD0ulNEp7zzxPyIritVzHlbqdGCrEIGtDx90k kIoTyrsckXwjDgiza25oQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvgedrfedtgdekfecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttddunecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepfefhjeeluedvvedtuddtuedtvefhieejtefhffeujefhteduudev tdektdeikeffnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 8 Nov 2022 10:51:13 -0500 (EST) From: Thomas Monjalon To: Morten =?ISO-8859-1?Q?Br=F8rup?= Cc: andrew.rybchenko@oktetlabs.ru, olivier.matz@6wind.com, david.marchand@redhat.com, dev@dpdk.org, hofors@lysator.liu.se, dev@dpdk.org, Konstantin Ananyev , mattias.ronnblom@ericsson.com, stephen@networkplumber.org, jerinj@marvell.com, bruce.richardson@intel.com Subject: Re: FW: [PATCH v4 3/3] mempool: use cache for frequently updated stats Date: Tue, 08 Nov 2022 16:51:11 +0100 Message-ID: <2028060.trqCLbgVIZ@thomas> In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D874A3@smartserver.smartshare.dk> References: <98CBD80474FA8B44BF855DF32C47DC35D874A1@smartserver.smartshare.dk> <6110999.17fYzF0512@thomas> <98CBD80474FA8B44BF855DF32C47DC35D874A3@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 08/11/2022 15:30, Morten Br=F8rup: > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > 08/11/2022 12:25, Morten Br=F8rup: > > > From: Morten Br=F8rup > > > > From: Konstantin Ananyev [mailto:konstantin.ananyev@huawei.com] > > > > Sent: Tuesday, 8 November 2022 10.20 > > > > > +#ifdef RTE_LIBRTE_MEMPOOL_STATS > > > > > +#define RTE_MEMPOOL_CACHE_STAT_ADD(cache, name, n) (cache)- > > >stats.name +=3D n > > > > > > > > As Andrew already pointed, it needs to be: ((cache)->stats.name +=3D > > (n)) > > > > Apart from that, LGTM. > > > > Series-Acked-by: Konstantin Ananyev > > > > > > @Thomas, this series should be ready to apply... it now has been: > > > Reviewed-by: (mempool maintainer) Andrew Rybchenko > > > > > Reviewed-By: Mattias R=F6nnblom > > > Acked-by: Konstantin Ananyev > >=20 > > Being acked does not mean it is good to apply in -rc3. >=20 > I understand that the RFC/v1 of this series was formally too late to make= it in 22.11, so I will not complain loudly if you choose to omit it for 22= =2E11. >=20 > With two independent reviews, including from a mempool maintainer, I stil= l have some hope. Also considering the risk assessment below. ;-) >=20 > > Please tell what is the benefit for 22.11 (before/after and condition). >=20 > Short version: With this series, mempool statistics can be used in produc= tion. Without it, the performance cost (mempool_perf_autotest: -74 %) is pr= ohibitive! >=20 > Long version: >=20 > The patch series provides significantly higher performance for mempool st= atistics, which are readable through rte_mempool_dump(FILE *f, struct rte_m= empool *mp). >=20 > Without this series, you have to set RTE_LIBRTE_MEMPOOL_DEBUG at build ti= me to get mempool statistics. RTE_LIBRTE_MEMPOOL_DEBUG also enables protect= ive cookies before and after each mempool object, which are all verified on= get/put from the mempool. According to mempool_perf_autotest, the performa= nce cost of mempool statistics (by setting RTE_LIBRTE_MEMPOOL_DEBUG) is a 7= 4 % decrease in rate_persec for mempools with cache (i.e. mbuf pools). Proh= ibitive for use in production! >=20 > With this series, the performance cost of mempool statistics (by setting = RTE_LIBRTE_MEMPOOL_STATS) in mempool_perf_autotest is only 6.7 %, so mempoo= l statistics can be used in production. >=20 > > Note there is a real risk doing such change that late. >=20 > Risk assessment: >=20 > The patch series has zero effect unless either RTE_LIBRTE_MEMPOOL_DEBUG o= r RTE_LIBRTE_MEMPOOL_STATS are set when building. They are not set in the d= efault build. If theses build flags are not set, there is no risk and no benefit. But if they are set, there is a risk of regression, for the benefit of an increased performance of a debug feature. I would say it is better to avoid any functional regression in a debug feat= ure at this stage. Any other opinion?