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 60771A00C2; Fri, 14 Oct 2022 16:01:32 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4BCD542D89; Fri, 14 Oct 2022 16:01:31 +0200 (CEST) Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) by mails.dpdk.org (Postfix) with ESMTP id F1CCD42C6E for ; Fri, 14 Oct 2022 16:01:29 +0200 (CEST) Received: by mail-wm1-f52.google.com with SMTP id iv17so3122215wmb.4 for ; Fri, 14 Oct 2022 07:01:29 -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=XXQItj6DqpqvWwgERp7CCx1e+jk8Bo1stT/Nj5syx5M=; b=bhikRtvc+JjPzRxgVu5SGQeZ8nwQoE1WEFrCsQ+AkaUCai8CdPVOOln3EwYIm4963v qTwjYccK27bI9RnRyi4btUyk5FMTAFq8Wecsq1VbA9ABL6ajL58NWowr1ZExnhYqzQiI x+Af+s3heEYRVyPhUGXGrgSAhZkcJQAX6xk7VgkdQljkKDip0eDuvlYRQhWogR8p1bMa 2bD1tMUChi7WZ/J5q/4DF7uB22fAPcAIlsygvaeJfTCjSfuutKlvDngnP8L5FDzEpVw5 Jx1QaIyYzZDKV6elfMylHRK/s5r69peTJKt4kYjRqPUEd4bKYmyCZCavQnY5B2qGLq+J e0hg== 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=XXQItj6DqpqvWwgERp7CCx1e+jk8Bo1stT/Nj5syx5M=; b=JZS2Vw1cLsm5qcMPxW3QJybUooRUm1HXjAcEZTnK/9A/zWUTSSackboThQky4jKM9d UlDJFbR/KissetNrSpBhjf3pGa5WiRm+f7e6Ivjcu12rgSYWuX9AWjR4TEeoHrMb403/ fZRnf1H9Ae6dBgZkf01FzLUITD8U2fRqQR7Gf7CyHj5tB3ihA0EI25xNhSZtpaDAQAOA /wJEn1dr5qEClY4dNMLvt27UgODc/gvIFmPaUAlHpzh9DaOecFiMl9zGljX+PtpAlgDH tHm25dt2xx2oqlJYFpeR/FwzbkyB/5ecm8uepZE26nLoZXRDmFz/r4irGe5nr0ubnFEN KudQ== X-Gm-Message-State: ACrzQf0O3/NXeE7wS0QW7urYzjz7hxbtUObg7WIMfNRZWBqCK5EasLMu ocHIUP7ZmHa8OLoJEefVDZbsudf3aQXvTg== X-Google-Smtp-Source: AMsMyM7FG99C5zhoSz86rAj9dSD29CGmsKQjA1CDh///hXaJ+8ClwA0HuLJlCE5EZ/vNjtHzUSKkyg== X-Received: by 2002:a05:600c:695:b0:3c6:b7f1:6f39 with SMTP id a21-20020a05600c069500b003c6b7f16f39mr3821649wmn.5.1665756089627; Fri, 14 Oct 2022 07:01:29 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id dn7-20020a05600c654700b003b47ff307e1sm2038387wmb.31.2022.10.14.07.01.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 14 Oct 2022 07:01:27 -0700 (PDT) Date: Fri, 14 Oct 2022 16:01:21 +0200 From: Olivier Matz To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: Andrew Rybchenko , dev@dpdk.org, Bruce Richardson Subject: Re: [PATCH v6 3/4] mempool: fix cache flushing algorithm Message-ID: References: <98CBD80474FA8B44BF855DF32C47DC35D86DB2@smartserver.smartshare.dk> <20221009133737.795377-1-andrew.rybchenko@oktetlabs.ru> <20221009133737.795377-4-andrew.rybchenko@oktetlabs.ru> <98CBD80474FA8B44BF855DF32C47DC35D873B6@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35D873B8@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D873B8@smartserver.smartshare.dk> 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 Hi Morten, Andrew, On Sun, Oct 09, 2022 at 05:08:39PM +0200, Morten Brørup wrote: > > From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru] > > Sent: Sunday, 9 October 2022 16.52 > > > > On 10/9/22 17:31, Morten Brørup wrote: > > >> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru] > > >> Sent: Sunday, 9 October 2022 15.38 > > >> > > >> From: Morten Brørup > > >> > > [...] I finally took a couple of hours to carefully review the mempool-related series (including the ones that have already been pushed). The new behavior looks better to me in all situations I can think about. > > > >> --- a/lib/mempool/rte_mempool.h > > >> +++ b/lib/mempool/rte_mempool.h > > >> @@ -90,7 +90,7 @@ struct rte_mempool_cache { > > >> * Cache is allocated to this size to allow it to overflow in > > >> certain > > >> * cases to avoid needless emptying of cache. > > >> */ > > >> - void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 3]; /**< Cache objects */ > > >> + void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 2]; /**< Cache objects */ > > >> } __rte_cache_aligned; > > > > > > How much are we allowed to break the ABI here? > > > > > > This patch reduces the size of the structure by removing a now unused > > part at the end, which should be harmless. It is an ABI breakage: an existing application will use the new 22.11 function to create the mempool (with a smaller cache), but will use the old inlined get/put that can exceed MAX_SIZE x 2 will remain. But this is a nice memory consumption improvement, in my opinion we should accept it for 22.11 with an entry in the release note. > > > > > > If we may also move the position of the objs array, I would add > > __rte_cache_aligned to the objs array. It makes no difference in the > > general case, but if get/put operations are always 32 objects, it will > > reduce the number of memory (or last level cache) accesses from five to > > four 64 B cache lines for every get/put operation. Will it really be the case? Since cache->len has to be accessed too, I don't think it would make a difference. > > > > > > uint32_t len; /**< Current cache count */ > > > - /* > > > - * Cache is allocated to this size to allow it to overflow in > > certain > > > - * cases to avoid needless emptying of cache. > > > - */ > > > - void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 3]; /**< Cache objects */ > > > + /** > > > + * Cache objects > > > + * > > > + * Cache is allocated to this size to allow it to overflow in > > certain > > > + * cases to avoid needless emptying of cache. > > > + */ > > > + void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 2] __rte_cache_aligned; > > > } __rte_cache_aligned; > > > > I think aligning objs on cacheline should be a separate patch. > > Good point. I'll let you do it. :-) > > PS: Thank you for following up on this patch series, Andrew! Many thanks for this rework. Acked-by: Olivier Matz