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 93E92A00C2; Fri, 14 Oct 2022 16:01:52 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8880042DF6; Fri, 14 Oct 2022 16:01:52 +0200 (CEST) Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by mails.dpdk.org (Postfix) with ESMTP id EE8C742DF5 for ; Fri, 14 Oct 2022 16:01:51 +0200 (CEST) Received: by mail-wm1-f49.google.com with SMTP id o20-20020a05600c4fd400b003b4a516c479so3590359wmq.1 for ; Fri, 14 Oct 2022 07:01:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=LXx2FLltwKb0a9QeAIBuNIS5/GiC+MunLeCLuw58xMM=; b=Wkz5NhUSkhryuhwobVccJSUtEcE/86jP3SaXibj6t7o/aSfcC5cVYErhMt7UFMLlud Rg6hu4HboETTvQPqgOCbj5vD/7i7i6yV4sgiv41jM8MC+clJQy+J/ouaflJGIBwrpGL1 hHuhNU2Hko4d3liTqkf7x9FUqMMhCJDGGNvY44u5SBSvJ2be3+6G0ePXpoKS++uF/Ojj FSm2yU93BfnHRtf9QZnC5F3xJEjUIJfusGkLGWFepO5jzmqs9qbgXyQZW/3pPmPvH5c5 4qWXY1AZ45QB/7URtksqcWS4jkhheNCqWcIQbETMrkjkaMduxfhABhEavA5iQddg7mPY a0AA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=LXx2FLltwKb0a9QeAIBuNIS5/GiC+MunLeCLuw58xMM=; b=ew1mC6n+O2yCZaZE7D2yhFBpIORPE8gY/uq01g04KWfqLWNBs8sfevmpJdOXHXxosi rvDd0Kh4UJflkpmmnBba4kV0oP8fvo31ym3s5cq5JMEKw/O0odWyd7Q6WhDFyM2m1ZKY rqgu6E0a192ExJ5w7NrzdejxrXgJ5tXvzCRMtlbgRu93wN28UFaYL828KALN96yIaxE/ pSW1RUoCOhSBy2end6pLPr7YBcgeTFvRho7a8oe5t1eKhUHfw8Sdz+KEGhVKUHs0JM6T 5OpALdJNSmEJUqQHzCXa/rLpgd9b+hUKd4DsUNTD0OsM5MAAZy7eqPNkpi6nLRhCF7dV q6Pw== X-Gm-Message-State: ACrzQf3zPPvubrS32k37fvVxQFE9XSnmA5oa/IoWz4+ZkGEL0+oNhGYs GBdzQ4LvT5zV0sHISVXd5sc9ew== X-Google-Smtp-Source: AMsMyM55ApJyLQ2gNZeR1s9KsU9cywl3ZakEJMpNZRSB+2A5GHxcCjl2+9MqdXCcUnBEBkRscXjrdw== X-Received: by 2002:a05:600c:358f:b0:3c6:da94:66f9 with SMTP id p15-20020a05600c358f00b003c6da9466f9mr3625388wmq.142.1665756111609; Fri, 14 Oct 2022 07:01:51 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id v5-20020a5d6105000000b0022afbd02c69sm1964304wrt.56.2022.10.14.07.01.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 14 Oct 2022 07:01:49 -0700 (PDT) Date: Fri, 14 Oct 2022 16:01:48 +0200 From: Olivier Matz To: Thomas Monjalon Cc: Morten =?iso-8859-1?Q?Br=F8rup?= , Andrew Rybchenko , dev@dpdk.org Subject: Re: [PATCH v4] mempool: fix get objects from mempool with cache Message-ID: References: <98CBD80474FA8B44BF855DF32C47DC35D86DB2@smartserver.smartshare.dk> <20221007104450.2567961-1-andrew.rybchenko@oktetlabs.ru> <4406925.8F6SAcFxjW@thomas> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <4406925.8F6SAcFxjW@thomas> 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 On Sat, Oct 08, 2022 at 10:56:06PM +0200, Thomas Monjalon wrote: > 07/10/2022 12:44, Andrew Rybchenko: > > From: Morten Brørup > > > > 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. > > > > Fix the following in rte_mempool_do_generic_get(): > > > > 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. > > > > 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. > > > > 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. > > > > 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. > > > > 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. > > > > The function would fail despite there are enough objects in the cache > > plus the common pool. > > > > 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. > > > > Signed-off-by: Morten Brørup > > Signed-off-by: Andrew Rybchenko > > Reviewed-by: Morten Brørup > > Applied, thanks. Better late than never: I reviewed this patch after it has been pushed, and it looks good to me. Thanks, Olivier