[v11,7/7] eal: keep per-lcore power intrinsics state in lcore variable

Message ID 20241014074348.821962-8-mattias.ronnblom@ericsson.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Lcore variables |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build fail github build: failed
ci/intel-Functional success Functional PASS

Commit Message

Mattias Rönnblom Oct. 14, 2024, 7:43 a.m. UTC
Keep per-lcore power intrinsics state in a lcore variable to reduce
cache working set size and avoid any CPU next-line-prefetching causing
false sharing.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/x86/rte_power_intrinsics.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
  

Comments

Stephen Hemminger Oct. 14, 2024, 4:30 p.m. UTC | #1
On Mon, 14 Oct 2024 09:43:48 +0200
Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:

> Keep per-lcore power intrinsics state in a lcore variable to reduce
> cache working set size and avoid any CPU next-line-prefetching causing
> false sharing.
> 
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

This looks like a problem.

-------------------------------BEGIN LOGS----------------------------
####################################################################################
#### [Begin job log] "ubuntu-22.04-clang-asan+doc+tests" at step Build and test
####################################################################################
+ configure_coredump
+ which gdb
+ ulimit -c unlimited
+ sudo sysctl -w kernel.core_pattern=/tmp/dpdk-core.%e.%p
kernel.core_pattern = /tmp/dpdk-core.%e.%p
+ devtools/test-null.sh
=================================================================
==67776==ERROR: AddressSanitizer: invalid alignment requested in aligned_alloc: 64, alignment must be a power of two and the requested size 0x8000008 must be a multiple of alignment (thread T0)
    #0 0x5562b2504042 in aligned_alloc (/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0xaad042) (BuildId: 731d8ec8ca4a6bf8e01bfd7548ebeb784aece6e3)
    #1 0x5562b37f671b in lcore_var_alloc /home/runner/work/dpdk/dpdk/build/../lib/eal/common/eal_common_lcore_var.c:77:20
    #2 0x5562b37f671b in rte_lcore_var_alloc /home/runner/work/dpdk/dpdk/build/../lib/eal/common/eal_common_lcore_var.c:123:9
    #3 0x5562b341b902 in rte_power_ethdev_pmgmt_init /home/runner/work/dpdk/dpdk/build/../lib/power/rte_power_pmd_mgmt.c:775:2
    #4 0x7f76b7829eba in call_init csu/../csu/libc-start.c:145:3
    #5 0x7f76b7829eba in __libc_start_main csu/../csu/libc-start.c:379:5

==67776==HINT: if you don't care about these errors you may set allocator_may_return_null=1
SUMMARY: AddressSanitizer: invalid-aligned-alloc-alignment (/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0xaad042) (BuildId: 731d8ec8ca4a6bf8e01bfd7548ebeb784aece6e3) in aligned_alloc
==67776==ABORTING
  
Mattias Rönnblom Oct. 15, 2024, 6:48 a.m. UTC | #2
On 2024-10-14 18:30, Stephen Hemminger wrote:
> On Mon, 14 Oct 2024 09:43:48 +0200
> Mattias Rönnblom <mattias.ronnblom@ericsson.com> wrote:
> 
>> Keep per-lcore power intrinsics state in a lcore variable to reduce
>> cache working set size and avoid any CPU next-line-prefetching causing
>> false sharing.
>>
>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>> Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> This looks like a problem.
> 
> -------------------------------BEGIN LOGS----------------------------
> ####################################################################################
> #### [Begin job log] "ubuntu-22.04-clang-asan+doc+tests" at step Build and test
> ####################################################################################
> + configure_coredump
> + which gdb
> + ulimit -c unlimited
> + sudo sysctl -w kernel.core_pattern=/tmp/dpdk-core.%e.%p
> kernel.core_pattern = /tmp/dpdk-core.%e.%p
> + devtools/test-null.sh
> =================================================================
> ==67776==ERROR: AddressSanitizer: invalid alignment requested in aligned_alloc: 64, alignment must be a power of two and the requested size 0x8000008 must be a multiple of alignment (thread T0)
>      #0 0x5562b2504042 in aligned_alloc (/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0xaad042) (BuildId: 731d8ec8ca4a6bf8e01bfd7548ebeb784aece6e3)
>      #1 0x5562b37f671b in lcore_var_alloc /home/runner/work/dpdk/dpdk/build/../lib/eal/common/eal_common_lcore_var.c:77:20
>      #2 0x5562b37f671b in rte_lcore_var_alloc /home/runner/work/dpdk/dpdk/build/../lib/eal/common/eal_common_lcore_var.c:123:9
>      #3 0x5562b341b902 in rte_power_ethdev_pmgmt_init /home/runner/work/dpdk/dpdk/build/../lib/power/rte_power_pmd_mgmt.c:775:2
>      #4 0x7f76b7829eba in call_init csu/../csu/libc-start.c:145:3
>      #5 0x7f76b7829eba in __libc_start_main csu/../csu/libc-start.c:379:5
> 
> ==67776==HINT: if you don't care about these errors you may set allocator_may_return_null=1
> SUMMARY: AddressSanitizer: invalid-aligned-alloc-alignment (/home/runner/work/dpdk/dpdk/build/app/dpdk-testpmd+0xaad042) (BuildId: 731d8ec8ca4a6bf8e01bfd7548ebeb784aece6e3) in aligned_alloc
> ==67776==ABORTING

Yes. Thanks.
  

Patch

diff --git a/lib/eal/x86/rte_power_intrinsics.c b/lib/eal/x86/rte_power_intrinsics.c
index 6d9b64240c..f4ba2c8ecb 100644
--- a/lib/eal/x86/rte_power_intrinsics.c
+++ b/lib/eal/x86/rte_power_intrinsics.c
@@ -6,6 +6,7 @@ 
 
 #include <rte_common.h>
 #include <rte_lcore.h>
+#include <rte_lcore_var.h>
 #include <rte_rtm.h>
 #include <rte_spinlock.h>
 
@@ -14,10 +15,14 @@ 
 /*
  * Per-lcore structure holding current status of C0.2 sleeps.
  */
-static alignas(RTE_CACHE_LINE_SIZE) struct power_wait_status {
+struct power_wait_status {
 	rte_spinlock_t lock;
 	volatile void *monitor_addr; /**< NULL if not currently sleeping */
-} wait_status[RTE_MAX_LCORE];
+};
+
+RTE_LCORE_VAR_HANDLE(struct power_wait_status, wait_status);
+
+RTE_LCORE_VAR_INIT(wait_status);
 
 /*
  * This function uses UMONITOR/UMWAIT instructions and will enter C0.2 state.
@@ -172,7 +177,7 @@  rte_power_monitor(const struct rte_power_monitor_cond *pmc,
 	if (pmc->fn == NULL)
 		return -EINVAL;
 
-	s = &wait_status[lcore_id];
+	s = RTE_LCORE_VAR_LCORE_VALUE(lcore_id, wait_status);
 
 	/* update sleep address */
 	rte_spinlock_lock(&s->lock);
@@ -264,7 +269,7 @@  rte_power_monitor_wakeup(const unsigned int lcore_id)
 	if (lcore_id >= RTE_MAX_LCORE)
 		return -EINVAL;
 
-	s = &wait_status[lcore_id];
+	s = RTE_LCORE_VAR_LCORE_VALUE(lcore_id, wait_status);
 
 	/*
 	 * There is a race condition between sleep, wakeup and locking, but we
@@ -303,8 +308,8 @@  int
 rte_power_monitor_multi(const struct rte_power_monitor_cond pmc[],
 		const uint32_t num, const uint64_t tsc_timestamp)
 {
-	const unsigned int lcore_id = rte_lcore_id();
-	struct power_wait_status *s = &wait_status[lcore_id];
+	struct power_wait_status *s = RTE_LCORE_VAR_VALUE(wait_status);
+
 	uint32_t i, rc;
 
 	/* check if supported */