[PATCH] config: cleanup existing RTE_CACHE_LINE_SIZE selection scheme

[PATCH] config: cleanup existing RTE_CACHE_LINE_SIZE selection scheme

[Jerin Jacob]

by default, all the targets will be configured with the 64-byte cache line
size, targets which have different cache line size can be overridden
through target specific config file.

Selected ThunderX and power8 as CONFIG_RTE_CACHE_LINE_SIZE=128 targets
based on existing configuration.

Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 config/common_bsdapp                                          | 5 +++++
 config/common_linuxapp                                        | 6 +++++-
 config/defconfig_arm64-armv8a-linuxapp-gcc                    | 2 --
 config/defconfig_ppc_64-power8-linuxapp-gcc                   | 2 ++
 lib/librte_eal/common/include/rte_memory.h                    | 3 ---
 lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h | 4 ----
 mk/arch/arm/rte.vars.mk                                       | 2 +-
 mk/arch/ppc_64/rte.vars.mk                                    | 2 +-
 mk/machine/armv8a/rte.vars.mk                                 | 2 +-
 mk/machine/thunderx/rte.vars.mk                               | 2 +-
 mk/machine/xgene1/rte.vars.mk                                 | 2 +-
 11 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/config/common_bsdapp b/config/common_bsdapp
index 3286481..5dbcc33 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -94,6 +94,11 @@ CONFIG_RTE_BUILD_COMBINE_LIBS=n
 CONFIG_RTE_NEXT_ABI=y
 
 #
+# Machine's cache line size
+#
+CONFIG_RTE_CACHE_LINE_SIZE=64
+
+#
 # Compile Environment Abstraction Layer
 #
 CONFIG_RTE_LIBRTE_EAL=y
diff --git a/config/common_linuxapp b/config/common_linuxapp
index 2866986..54168be 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -94,6 +94,11 @@ CONFIG_RTE_BUILD_COMBINE_LIBS=n
 CONFIG_RTE_NEXT_ABI=y
 
 #
+# Machine's cache line size
+#
+CONFIG_RTE_CACHE_LINE_SIZE=64
+
+#
 # Compile Environment Abstraction Layer
 #
 CONFIG_RTE_LIBRTE_EAL=y
@@ -110,7 +115,6 @@ CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n
 CONFIG_RTE_EAL_IGB_UIO=y
 CONFIG_RTE_EAL_VFIO=y
 CONFIG_RTE_MALLOC_DEBUG=n
-
 # Default driver path (or "" to disable)
 CONFIG_RTE_EAL_PMD_PATH=""
 
diff --git a/config/defconfig_arm64-armv8a-linuxapp-gcc b/config/defconfig_arm64-armv8a-linuxapp-gcc
index 57f7941..52e0c97 100644
--- a/config/defconfig_arm64-armv8a-linuxapp-gcc
+++ b/config/defconfig_arm64-armv8a-linuxapp-gcc
@@ -43,8 +43,6 @@ CONFIG_RTE_FORCE_INTRINSICS=y
 CONFIG_RTE_TOOLCHAIN="gcc"
 CONFIG_RTE_TOOLCHAIN_GCC=y
 
-CONFIG_RTE_CACHE_LINE_SIZE=64
-
 CONFIG_RTE_IXGBE_INC_VECTOR=n
 CONFIG_RTE_LIBRTE_VIRTIO_PMD=n
 CONFIG_RTE_LIBRTE_IVSHMEM=n
diff --git a/config/defconfig_ppc_64-power8-linuxapp-gcc b/config/defconfig_ppc_64-power8-linuxapp-gcc
index 03760c4..a80a19e 100644
--- a/config/defconfig_ppc_64-power8-linuxapp-gcc
+++ b/config/defconfig_ppc_64-power8-linuxapp-gcc
@@ -36,6 +36,8 @@ CONFIG_RTE_ARCH="ppc_64"
 CONFIG_RTE_ARCH_PPC_64=y
 CONFIG_RTE_ARCH_64=y
 
+CONFIG_RTE_CACHE_LINE_SIZE=128
+
 CONFIG_RTE_TOOLCHAIN="gcc"
 CONFIG_RTE_TOOLCHAIN_GCC=y
 
diff --git a/lib/librte_eal/common/include/rte_memory.h b/lib/librte_eal/common/include/rte_memory.h
index 33f5a77..b697c90 100644
--- a/lib/librte_eal/common/include/rte_memory.h
+++ b/lib/librte_eal/common/include/rte_memory.h
@@ -68,9 +68,6 @@ enum rte_page_sizes {
 };
 
 #define SOCKET_ID_ANY -1                    /**< Any NUMA socket. */
-#ifndef RTE_CACHE_LINE_SIZE
-#define RTE_CACHE_LINE_SIZE 64                  /**< Cache line size. */
-#endif
 #define RTE_CACHE_LINE_MASK (RTE_CACHE_LINE_SIZE-1) /**< Cache line mask. */
 
 #define RTE_CACHE_LINE_ROUNDUP(size) \
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index e724af7..cfd142a 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -68,10 +68,6 @@
  */
 #define RTE_KNI_NAMESIZE 32
 
-#ifndef RTE_CACHE_LINE_SIZE
-#define RTE_CACHE_LINE_SIZE 64       /**< Cache line size. */
-#endif
-
 /*
  * Request id.
  */
diff --git a/mk/arch/arm/rte.vars.mk b/mk/arch/arm/rte.vars.mk
index df0c043..bd85140 100644
--- a/mk/arch/arm/rte.vars.mk
+++ b/mk/arch/arm/rte.vars.mk
@@ -32,7 +32,7 @@
 ARCH  ?= arm
 CROSS ?=
 
-CPU_CFLAGS  ?= -marm -DRTE_CACHE_LINE_SIZE=64 -munaligned-access
+CPU_CFLAGS  ?= -marm -munaligned-access
 CPU_LDFLAGS ?=
 CPU_ASFLAGS ?= -felf
 
diff --git a/mk/arch/ppc_64/rte.vars.mk b/mk/arch/ppc_64/rte.vars.mk
index fdba019..363fcd1 100644
--- a/mk/arch/ppc_64/rte.vars.mk
+++ b/mk/arch/ppc_64/rte.vars.mk
@@ -32,7 +32,7 @@
 ARCH  ?= powerpc
 CROSS ?=
 
-CPU_CFLAGS  ?= -m64 -DRTE_CACHE_LINE_SIZE=128
+CPU_CFLAGS  ?= -m64
 CPU_LDFLAGS ?=
 CPU_ASFLAGS ?= -felf64
 
diff --git a/mk/machine/armv8a/rte.vars.mk b/mk/machine/armv8a/rte.vars.mk
index 8c018a4..d5049e1 100644
--- a/mk/machine/armv8a/rte.vars.mk
+++ b/mk/machine/armv8a/rte.vars.mk
@@ -55,4 +55,4 @@
 # CPU_LDFLAGS =
 # CPU_ASFLAGS =
 
-MACHINE_CFLAGS += -march=armv8-a+crc -DRTE_CACHE_LINE_SIZE=64
+MACHINE_CFLAGS += -march=armv8-a+crc
diff --git a/mk/machine/thunderx/rte.vars.mk b/mk/machine/thunderx/rte.vars.mk
index 0bb6b3d..81da1b4 100644
--- a/mk/machine/thunderx/rte.vars.mk
+++ b/mk/machine/thunderx/rte.vars.mk
@@ -55,4 +55,4 @@ CROSS ?= aarch64-thunderx-linux-gnu-
 # CPU_LDFLAGS =
 # CPU_ASFLAGS =
 
-MACHINE_CFLAGS += -march=armv8-a+crc -mcpu=thunderx -DRTE_CACHE_LINE_SIZE=128
+MACHINE_CFLAGS += -march=armv8-a+crc -mcpu=thunderx
diff --git a/mk/machine/xgene1/rte.vars.mk b/mk/machine/xgene1/rte.vars.mk
index bdf8c6b..419c234 100644
--- a/mk/machine/xgene1/rte.vars.mk
+++ b/mk/machine/xgene1/rte.vars.mk
@@ -55,4 +55,4 @@
 # CPU_LDFLAGS =
 # CPU_ASFLAGS =
 
-MACHINE_CFLAGS += -march=armv8-a -DRTE_CACHE_LINE_SIZE=64
+MACHINE_CFLAGS += -march=armv8-a
-- 
2.1.0

Re: [PATCH] config: cleanup existing RTE_CACHE_LINE_SIZE selection scheme

[Thomas Monjalon]

2015-12-07 19:52, Jerin Jacob:
> by default, all the targets will be configured with the 64-byte cache line
> size, targets which have different cache line size can be overridden
> through target specific config file.
> 
> Selected ThunderX and power8 as CONFIG_RTE_CACHE_LINE_SIZE=128 targets
> based on existing configuration.
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

It looks good.
We are moving from a constant defined on CC command line in most cases
to a constant included in rte_config.h only.
Have you checked that the generated libraries are identical?
It would be good to have a check for POWER8. Chao?
Thanks

Re: [PATCH] config: cleanup existing RTE_CACHE_LINE_SIZE selection scheme

[Jerin Jacob]

On Mon, Dec 07, 2015 at 03:45:10PM +0100, Thomas Monjalon wrote:
> 2015-12-07 19:52, Jerin Jacob:
> > by default, all the targets will be configured with the 64-byte cache line
> > size, targets which have different cache line size can be overridden
> > through target specific config file.
> > 
> > Selected ThunderX and power8 as CONFIG_RTE_CACHE_LINE_SIZE=128 targets
> > based on existing configuration.
> > 
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> 
> It looks good.
> We are moving from a constant defined on CC command line in most cases
> to a constant included in rte_config.h only.
> Have you checked that the generated libraries are identical?

Yes, Tested with a 64byte(x86_64-native-linuxapp-gcc) and a 128byte
(arm64-thunderx-linuxapp-gcc) target.
md5sum build/app/test comes same with or without patch.

> It would be good to have a check for POWER8. Chao?
> Thanks

Re: [PATCH] config: cleanup existing RTE_CACHE_LINE_SIZE selection scheme

[Jerin Jacob]

On Tue, Dec 08, 2015 at 03:03:34PM +0530, Jerin Jacob wrote:
> On Mon, Dec 07, 2015 at 03:45:10PM +0100, Thomas Monjalon wrote:
> > 2015-12-07 19:52, Jerin Jacob:
> > > by default, all the targets will be configured with the 64-byte cache line
> > > size, targets which have different cache line size can be overridden
> > > through target specific config file.
> > > 
> > > Selected ThunderX and power8 as CONFIG_RTE_CACHE_LINE_SIZE=128 targets
> > > based on existing configuration.
> > > 
> > > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > 
> > It looks good.
> > We are moving from a constant defined on CC command line in most cases
> > to a constant included in rte_config.h only.
> > Have you checked that the generated libraries are identical?
> 
> Yes, Tested with a 64byte(x86_64-native-linuxapp-gcc) and a 128byte
> (arm64-thunderx-linuxapp-gcc) target.
> md5sum build/app/test comes same with or without patch.

ping

> 
> > It would be good to have a check for POWER8. Chao?
> > Thanks

Re: [PATCH] config: cleanup existing RTE_CACHE_LINE_SIZE selection scheme

[Jerin Jacob]

On Fri, Jan 29, 2016 at 09:54:55AM +0530, Jerin Jacob wrote:
> On Tue, Dec 08, 2015 at 03:03:34PM +0530, Jerin Jacob wrote:
> > On Mon, Dec 07, 2015 at 03:45:10PM +0100, Thomas Monjalon wrote:
> > > 2015-12-07 19:52, Jerin Jacob:
> > > > by default, all the targets will be configured with the 64-byte cache line
> > > > size, targets which have different cache line size can be overridden
> > > > through target specific config file.
> > > > 
> > > > Selected ThunderX and power8 as CONFIG_RTE_CACHE_LINE_SIZE=128 targets
> > > > based on existing configuration.
> > > > 
> > > > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > 
> > > It looks good.
> > > We are moving from a constant defined on CC command line in most cases
> > > to a constant included in rte_config.h only.
> > > Have you checked that the generated libraries are identical?
> > 
> > Yes, Tested with a 64byte(x86_64-native-linuxapp-gcc) and a 128byte
> > (arm64-thunderx-linuxapp-gcc) target.
> > md5sum build/app/test comes same with or without patch.
> 
> ping

Thomas, Can you review this, This is a long pending item.
This rework based on your request.

Jerin

> 
> > 
> > > It would be good to have a check for POWER8. Chao?
> > > Thanks

Re: [PATCH] config: cleanup existing RTE_CACHE_LINE_SIZE selection scheme

[Thomas Monjalon]

2015-12-08 15:03, Jerin Jacob:
> On Mon, Dec 07, 2015 at 03:45:10PM +0100, Thomas Monjalon wrote:
> > 2015-12-07 19:52, Jerin Jacob:
> > > by default, all the targets will be configured with the 64-byte cache line
> > > size, targets which have different cache line size can be overridden
> > > through target specific config file.
> > > 
> > > Selected ThunderX and power8 as CONFIG_RTE_CACHE_LINE_SIZE=128 targets
> > > based on existing configuration.
> > > 
> > > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > 
> > It looks good.
> > We are moving from a constant defined on CC command line in most cases
> > to a constant included in rte_config.h only.
> > Have you checked that the generated libraries are identical?
> 
> Yes, Tested with a 64byte(x86_64-native-linuxapp-gcc) and a 128byte
> (arm64-thunderx-linuxapp-gcc) target.
> md5sum build/app/test comes same with or without patch.
> 
> > It would be good to have a check for POWER8. Chao?
> > Thanks

It seems we won't have a test on POWER8.

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Re: [PATCH] config: cleanup existing RTE_CACHE_LINE_SIZE selection scheme

[Thomas Monjalon]

2016-02-08 12:44, Thomas Monjalon:
> 2015-12-08 15:03, Jerin Jacob:
> > On Mon, Dec 07, 2015 at 03:45:10PM +0100, Thomas Monjalon wrote:
> > > 2015-12-07 19:52, Jerin Jacob:
> > > > by default, all the targets will be configured with the 64-byte cache line
> > > > size, targets which have different cache line size can be overridden
> > > > through target specific config file.
> > > > 
> > > > Selected ThunderX and power8 as CONFIG_RTE_CACHE_LINE_SIZE=128 targets
> > > > based on existing configuration.
> > > > 
> > > > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > 
> > > It looks good.
> > > We are moving from a constant defined on CC command line in most cases
> > > to a constant included in rte_config.h only.
> > > Have you checked that the generated libraries are identical?
> > 
> > Yes, Tested with a 64byte(x86_64-native-linuxapp-gcc) and a 128byte
> > (arm64-thunderx-linuxapp-gcc) target.
> > md5sum build/app/test comes same with or without patch.
> > 
> > > It would be good to have a check for POWER8. Chao?
> > > Thanks
> 
> It seems we won't have a test on POWER8.
> 
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied, thanks