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 48CAEA04F0 for ; Fri, 27 Dec 2019 16:54:23 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0B29A1BF6F; Fri, 27 Dec 2019 16:54:23 +0100 (CET) Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by dpdk.org (Postfix) with ESMTP id 8A3661BF6F for ; Fri, 27 Dec 2019 16:54:22 +0100 (CET) Received: by mail-wr1-f68.google.com with SMTP id c14so26398068wrn.7 for ; Fri, 27 Dec 2019 07:54:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=TVsN4ta37kYiPwfi+4QMcKNnHrFUAsJQNJm/SD6/A88=; b=Pi5z+2x/OZqy+5eZ8elk5/bes1aCo/pEDh3oM+jXMaxQLadYHCu4mBIoa4Vbh8lLKd MNCR1plQS7JpIgCQVDF+0xyVBRpYvS61A7up18KB/jpG3gZTrV22lckU4+Mc35jLhM+T EJrrGceDn607e37JrPHZIdjDZfUFUgPca9BuIH0KDGcNDCAmOLl6nhWqbOxVkvr5EpSC 8bBWbcS/23R2uDBvxQcTxjxYXQkS80BHE34ru7jpgWh2h/h0RqtxYikS19Oq38n2RzrA npmUCplM0zHkYkuogQ2Og/gH2lCusUWZxVY82zAjbnToqgorVt3SE7X/ds2s7Ba+ZREI 3plQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=TVsN4ta37kYiPwfi+4QMcKNnHrFUAsJQNJm/SD6/A88=; b=o7CE9TOm4oWL6R/rgExgYdNfvdb6Adn1zcl9NPcF+C2nZ3DKweGYJz+1proSdsKBqg I/qIDrEu7RsYQlTc3Ldv+k8lJ9GxbPqgOzRNNb4OY/pmPrTCeFWIIdXKP3sJOgyBtLEb UQ+zD1kj8OK8h2WpsAJ+Do202UvgkDZmre67vCKgYa8/xMuSBx+JC4FLUxyYlPBTpETr zwUzg+yK7Q0/l7CLEsbvIpvCHwUDbdEx2bw560dCywTXpylFBxfwjVNDNoVYBfRfDDeU qwyNZTRxMzDNuIflzAqSpfWbY5f76+9CoZbHUK3h7RdErW7mPphNhvXaXZMnaC7WpHIZ Sv/Q== X-Gm-Message-State: APjAAAWFT9oDfHh5iSNSYDfI4jHP4mTCuzK/nI7gJucxh0bP3XnCY2ay rW0kFteAPo5ka/xMwmev7TPBGg== X-Google-Smtp-Source: APXvYqzUAmz+CzFi8/KLJ74Trz4J9tvXpXk8LJTWtH3OKZlVrQXn94Bitzc28BjdQzXVWAj0rHCSpg== X-Received: by 2002:a5d:608a:: with SMTP id w10mr48321863wrt.136.1577462062101; Fri, 27 Dec 2019 07:54:22 -0800 (PST) Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0]) by smtp.gmail.com with ESMTPSA id e18sm34792773wrr.95.2019.12.27.07.54.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Dec 2019 07:54:21 -0800 (PST) Date: Fri, 27 Dec 2019 16:54:20 +0100 From: Olivier Matz To: Jerin Jacob Cc: Honnappa Nagarahalli , "jerinj@marvell.com" , "dev@dpdk.org" , "thomas@monjalon.net" , "arybchenko@solarflare.com" , "bruce.richardson@intel.com" , "konstantin.ananyev@intel.com" , "hemant.agrawal@nxp.com" , "shahafs@mellanox.com" , Gavin Hu , "viktorin@rehivetech.com" , "drc@linux.vnet.ibm.com" , "anatoly.burakov@intel.com" , "stable@dpdk.org" , nd Message-ID: <20191227155420.GR22738@platinum> References: <20191219134227.3841799-1-jerinj@marvell.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] mempool: fix mempool obj alignment for non x86 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi, On Sat, Dec 21, 2019 at 10:36:15AM +0530, Jerin Jacob wrote: > On Sat, Dec 21, 2019 at 2:37 AM Honnappa Nagarahalli > wrote: > > > > > > > > > From: Jerin Jacob > > > > > > > > > > The exiting optimize_object_size() function address the memory > > > > > object alignment constraint on x86 for better performance. > > > > > > > > > > Different (Mirco) architecture may have different memory alignment > > > > > constraint for better performance and it not same as the existing > > > > > optimize_object_size() function. Some use, XOR(kind of CRC) scheme > > > > > to enable DRAM channel distribution based on the address and some > > > > > may have a different formula. typo: Mirco -> Micro Maybe the whole sentence can be reworded a bit (I think a word is missing). > > > > If I understand correctly, address interleaving is the characteristic of the > > > memory controller and not the CPU. > > > > For ex: different SoCs using the same Arm architecture might have different > > > memory controllers. So, the solution should not be architecture specific, but > > > SoC specific. > > > > > > Yes. See below. > > > > > > > > -static unsigned optimize_object_size(unsigned obj_size) > > > > > +static unsigned > > > > > +arch_mem_object_align(unsigned obj_size) > > > > > { > > > > > unsigned nrank, nchan; > > > > > unsigned new_obj_size; > > > > > @@ -99,6 +101,13 @@ static unsigned optimize_object_size(unsigned > > > > > obj_size) > > > > > new_obj_size++; > > > > > return new_obj_size * RTE_MEMPOOL_ALIGN; } > > > > > +#else > > > > This applies to add Arm (PPC as well) SoCs which might have different > > > schemes depending on the memory controller. IMO, this should not be > > > architecture specific. > > > > > > I agree in principle. > > > I will summarize the > > > https://www.mail-archive.com/dev@dpdk.org/msg149157.html feedback: > > > > > > 1) For x86 arch, it is architecture-specific > > > 2) For power PC arch, It is architecture-specific > > > 3) For the ARM case, it will be the memory controller specific. > > > 4) For the ARM case, The memory controller is not using the existing > > > x86 arch formula. > > > 5) If it is memory/arch-specific, Can userspace code find the optimal > > > alignment? In the case of octeontx2/arm64, the memory controller does XOR > > > on PA address which userspace code doesn't have much control. > > > > > > This patch address the known case of (1), (2), (4) and (5). (2) can be added to > > > this framework when POWER9 folks want it. > > > > > > We can extend this patch to address (3) if there is a case. Without the actual > > > requirement(If some can share the formula of alignment which is the > > > memory controller specific and it does not come under (4))) then we can > > > create extra layer for the memory controller and abstraction to probe it. > > > Again there is no standard way of probing the memory controller in > > > userspace and we need platform #define, which won't work for distribution > > > build. > > > So solution needs to be arch-specific and then fine-tune to memory controller > > > if possible. > > > > > > I can work on creating an extra layer of code if some can provide the details > > > of the memory controller and probing mechanism or this patch be extended > > Inputs for BlueField, DPAAx, ThunderX2 would be helpful. > > Yes. Probably memory controller used in n1sdp SoC also. > > > > > > to support such case if it arises in future. > > > > > > Thoughts? > > How much memory will this save for your platform? Is it affecting performance? Currently, I think Arm-based architectures use the default (nchan=4, nrank=1). The worst case is for object whose size (including mempool header) is 2 cache lines, where it is optimized to 3 cache lines (+50%). Examples for cache lines size = 64: orig optimized 64 -> 64 +0% 128 -> 192 +50% 192 -> 192 +0% 256 -> 320 +25% 320 -> 320 +0% 384 -> 448 +16% ... 2304 -> 2368 +2.7% (~mbuf size) > No performance difference. > > The existing code adding the tailer for each objs. > Additional space/Trailer space will be function of number of objects > in mempool and its obj_size, its alignment and selected > rte_memory_get_nchannel() and rte_memory_get_nrank() > > I will wait for inputs from Bluefield, DPAAx, ThunderX2 and n1sdp(if > any) for any rework on the patch. If there is no performance impact on other supporter Arm-based architectures, I think it is a step in a right direction. > > > > > +static unsigned > > > > > +arch_mem_object_align(unsigned obj_size) { > > > > > + return obj_size; > > > > > +} > > > > > +#endif I'd prefer "unsigned int" for new code. Also, the opening brace should be on a separate line. The documentation of the MEMPOOL_F_NO_SPREAD flag in the .h could be slightly modified, as you did for the comment above arch_mem_object_align(). > > > > > > > > > > struct pagesz_walk_arg { > > > > > int socket_id; > > > > > @@ -234,8 +243,8 @@ rte_mempool_calc_obj_size(uint32_t elt_size, > > > > > uint32_t flags, > > > > > */ > > > > > if ((flags & MEMPOOL_F_NO_SPREAD) == 0) { > > > > > unsigned new_size; > > > > > - new_size = optimize_object_size(sz->header_size + sz- > > > > > >elt_size + > > > > > - sz->trailer_size); > > > > > + new_size = arch_mem_object_align > > > > > + (sz->header_size + sz->elt_size + > > > > > + sz->trailer_size); > > > > > sz->trailer_size = new_size - sz->header_size - sz->elt_size; > > > > > } > > > > > > > > > > -- > > > > > 2.24.1 > > > >