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 77852A0542; Sat, 8 Oct 2022 22:56:12 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 20285400D5; Sat, 8 Oct 2022 22:56:12 +0200 (CEST) Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by mails.dpdk.org (Postfix) with ESMTP id AC7FA40042 for ; Sat, 8 Oct 2022 22:56:10 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id 63FEF320082A; Sat, 8 Oct 2022 16:56:09 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Sat, 08 Oct 2022 16:56:09 -0400 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=fm3; t=1665262568; x= 1665348968; bh=kvi9K35rwlfw2g8g/QtuqC9jLOvIVHGWM5sVllOr/BY=; b=r IUOuaEa3n/jporUJqXoV4j+to8gqaItBXFiCAr2lmlhOW/yXJ/aAEMSNUJ5dNqDA C6N7rbYbbXxjq4DWJ2s8slHQHqW3AKD9deqLODnctFISYLz1nPGwRayVttTZRTjG YwJ6QSoXi9ktjMgpitqxhUkRUyQjWyoAAEHNFopcwQX1XHuGAUd/bco8rSQ6py+d x8OxsQphT6KO56tLBayOkgbq8umTwJwpeDViqXSIUAuCyk7XVsqugUAH0zY+l7Kw I+jbQRgM54BLEhcEBYlupcK3VObAPY0Bs1KEHTpXiehBDYSVBQa3DnflOY8BJL0R 9TbvMSZzUOet2wY9iC4PA== 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=fm3; t=1665262568; x= 1665348968; bh=kvi9K35rwlfw2g8g/QtuqC9jLOvIVHGWM5sVllOr/BY=; b=o ZkXH5j23h/WuoprIZNRTIYFCFkuDkZpFUMT6EHtF+ESzOuR1rLT6F53FVA71JarA FkyofwDYsmqRMS40UkIY4vu1PH0dHN1f1FWQxid01N2WKx0/oy+o5GpojmYQA6vj GnhW7cXqFCL9EXF/W/h87er0hluMW5bO/cw7KjbFdmynFo/1EcMxdBFseSrA8H+L ffwrYenOHQwEqJm2aIKNPcOpD+lbKSoBgSQZQ31f4hN4oP3GiVJlJG76tdSR67Nr BOTREClRHnuOXdJSnX1v/NSs0p7M/0k9p1LKWXuX5C64TWt67fc+P7F8a/nD44+5 2nWCHrNo1g1k5ztnQhWaw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeeiledgudehfecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttddunecuhfhrohhmpefvhhho mhgrshcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqne cuggftrfgrthhtvghrnhepfefhjeeluedvvedtuddtuedtvefhieejtefhffeujefhtedu udevtdektdeikeffnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilh hfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sat, 8 Oct 2022 16:56:08 -0400 (EDT) From: Thomas Monjalon To: Morten =?ISO-8859-1?Q?Br=F8rup?= , Andrew Rybchenko Cc: Olivier Matz , dev@dpdk.org Subject: Re: [PATCH v4] mempool: fix get objects from mempool with cache Date: Sat, 08 Oct 2022 22:56:06 +0200 Message-ID: <4406925.8F6SAcFxjW@thomas> In-Reply-To: <20221007104450.2567961-1-andrew.rybchenko@oktetlabs.ru> References: <98CBD80474FA8B44BF855DF32C47DC35D86DB2@smartserver.smartshare.dk> <20221007104450.2567961-1-andrew.rybchenko@oktetlabs.ru> 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 07/10/2022 12:44, Andrew Rybchenko: > From: Morten Br=F8rup >=20 > A flush threshold for the mempool cache was introduced in DPDK version > 1.3, but rte_mempool_do_generic_get() was not completely updated back > then, and some inefficiencies were introduced. >=20 > Fix the following in rte_mempool_do_generic_get(): >=20 > 1. The code that initially screens the cache request was not updated > with the change in DPDK version 1.3. > The initial screening compared the request length to the cache size, > which was correct before, but became irrelevant with the introduction of > the flush threshold. E.g. the cache can hold up to flushthresh objects, > which is more than its size, so some requests were not served from the > cache, even though they could be. > The initial screening has now been corrected to match the initial > screening in rte_mempool_do_generic_put(), which verifies that a cache > is present, and that the length of the request does not overflow the > memory allocated for the cache. >=20 > This bug caused a major performance degradation in scenarios where the > application burst length is the same as the cache size. In such cases, > the objects were not ever fetched from the mempool cache, regardless if > they could have been. > This scenario occurs e.g. if an application has configured a mempool > with a size matching the application's burst size. >=20 > 2. The function is a helper for rte_mempool_generic_get(), so it must > behave according to the description of that function. > Specifically, objects must first be returned from the cache, > subsequently from the backend. > After the change in DPDK version 1.3, this was not the behavior when > the request was partially satisfied from the cache; instead, the objects > from the backend were returned ahead of the objects from the cache. > This bug degraded application performance on CPUs with a small L1 cache, > which benefit from having the hot objects first in the returned array. > (This is probably also the reason why the function returns the objects > in reverse order, which it still does.) > Now, all code paths first return objects from the cache, subsequently > from the backend. >=20 > The function was not behaving as described (by the function using it) > and expected by applications using it. This in itself is also a bug. >=20 > 3. If the cache could not be backfilled, the function would attempt > to get all the requested objects from the backend (instead of only the > number of requested objects minus the objects available in the backend), > and the function would fail if that failed. > Now, the first part of the request is always satisfied from the cache, > and if the subsequent backfilling of the cache from the backend fails, > only the remaining requested objects are retrieved from the backend. >=20 > The function would fail despite there are enough objects in the cache > plus the common pool. >=20 > 4. The code flow for satisfying the request from the cache was slightly > inefficient: > The likely code path where the objects are simply served from the cache > was treated as unlikely. Now it is treated as likely. >=20 > Signed-off-by: Morten Br=F8rup > Signed-off-by: Andrew Rybchenko > Reviewed-by: Morten Br=F8rup Applied, thanks.