From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id DFF7B8E97 for ; Mon, 4 Jan 2016 14:16:01 +0100 (CET) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84) (envelope-from ) id 1aG50O-0000us-On; Mon, 04 Jan 2016 14:17:02 +0100 To: Jerin Jacob , dev@dpdk.org References: <1449765378-29563-1-git-send-email-jerin.jacob@caviumnetworks.com> <1450067576-18803-1-git-send-email-jerin.jacob@caviumnetworks.com> <1450067576-18803-2-git-send-email-jerin.jacob@caviumnetworks.com> From: Olivier MATZ X-Enigmail-Draft-Status: N1110 Message-ID: <568A7089.3070506@6wind.com> Date: Mon, 4 Jan 2016 14:15:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.3.0 MIME-Version: 1.0 In-Reply-To: <1450067576-18803-2-git-send-email-jerin.jacob@caviumnetworks.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 1/4] eal: Introduce new cache macro definitions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Jan 2016 13:16:02 -0000 Hi Jerin, Please see some comments below. On 12/14/2015 05:32 AM, Jerin Jacob wrote: > - RTE_CACHE_MIN_LINE_SIZE(Supported minimum cache line size) > - __rte_cache_min_aligned(Force minimum cache line alignment) > - RTE_CACHE_LINE_SIZE_LOG2(Express cache line size in terms of log2) > > Signed-off-by: Jerin Jacob > Suggested-by: Konstantin Ananyev > --- > lib/librte_eal/common/include/rte_memory.h | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h > index 9c9e40f..b67a76f 100644 > --- a/lib/librte_eal/common/include/rte_memory.h > +++ b/lib/librte_eal/common/include/rte_memory.h > @@ -77,11 +77,27 @@ enum rte_page_sizes { > (RTE_CACHE_LINE_SIZE * ((size + RTE_CACHE_LINE_SIZE - 1) / RTE_CACHE_LINE_SIZE)) > /**< Return the first cache-aligned value greater or equal to size. */ > > +/**< Cache line size in terms of log2 */ > +#if RTE_CACHE_LINE_SIZE == 64 > +#define RTE_CACHE_LINE_SIZE_LOG2 6 > +#elif RTE_CACHE_LINE_SIZE == 128 > +#define RTE_CACHE_LINE_SIZE_LOG2 7 > +#else > +#error "Unsupported cache line size" > +#endif > + > +#define RTE_CACHE_MIN_LINE_SIZE 64 /**< Minimum Cache line size. */ > + I think RTE_CACHE_LINE_MIN_SIZE or RTE_MIN_CACHE_LINE_SIZE would be clearer than RTE_CACHE_MIN_LINE_SIZE. > /** > * Force alignment to cache line. > */ > #define __rte_cache_aligned __rte_aligned(RTE_CACHE_LINE_SIZE) > > +/** > + * Force minimum cache line alignment. > + */ > +#define __rte_cache_min_aligned __rte_aligned(RTE_CACHE_MIN_LINE_SIZE) I'm not really convinced that __rte_cache_min_aligned is straightforward for someone reading the code that it means "aligned to the minimum cache line size supported by the dpdk". In the two cases you are using this macro (mbuf structure and queue info), I'm wondering if using __attribute__((aligned(64))) wouldn't be clearer? - for mbuf, it could be a local define, like MBUF_ALIGN_SIZE - for queue info, using 64 makes sense as it's used to reserve space for future use What do you think? Regards, Olivier