Hello! On 2024-12-09 at 11:18:37 -0800, Reinette Chatre wrote: >Hi Maciej, > >On 12/9/24 3:09 AM, Maciej Wieczor-Retman wrote: >> Sub-NUMA Cluster divides CPUs sharing an L3 cache into separate NUMA >> nodes. Systems may support splitting into either two, three, four or six >> nodes. When SNC mode is enabled the effective amount of L3 cache >> available for allocation is divided by the number of nodes per L3. >> >> It's possible to detect which SNC mode is active by comparing the number >> of CPUs that share a cache with CPU0, with the number of CPUs on node0. >> >> Detect SNC mode once and let other tests inherit that information. >> >> Change top_srcdir in the Makefile so fallthrough macro can be used in >> the switch statement. >> >> To check if SNC detection is reliable one can check the >> /sys/devices/system/cpu/offline file. If it's empty, it means all cores >> are operational and the ratio should be calculated correctly. If it has >> any contents, it means the detected SNC mode can't be trusted and should >> be disabled. >> >> Check if detection was not reliable due to offline cpus. If it was skip >> running tests since the results couldn't be trusted. >> >> Co-developed-by: Tony Luck <tony.luck@xxxxxxxxx> >> Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx> >> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@xxxxxxxxx> >> --- > >... > >> diff --git a/tools/testing/selftests/resctrl/Makefile b/tools/testing/selftests/resctrl/Makefile >> index f408bd6bfc3d..6b03bab6fc20 100644 >> --- a/tools/testing/selftests/resctrl/Makefile >> +++ b/tools/testing/selftests/resctrl/Makefile >> @@ -1,6 +1,7 @@ >> # SPDX-License-Identifier: GPL-2.0 >> +top_srcdir = ../../../.. > >top_srcdir is already defined in lib.mk, it should not be necessary to >redefine it. > >> >> -CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2 >> +CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2 -I$(top_srcdir)/tools/include > >It is not clear to me why my suggestion to set this after including lib.mk >was ignored. I'm sorry it looked like it, I read your suggestion and then I looked at tools/testing/selftests/rseq/Makefile which does exactly what I did (I looked for selftests that use the fallthrough attribute and it was the one with the simplest Makefile). >I am thinking about a change like below but looking at the other >selftest Makefiles I do see very few actually modify CFLAGS after including >lib.mk. Closest one I could find to this problem was >tools/testing/selftests/vDSO/Makefile. Do you see an issue with modifying CFLAGS >after including lib.mk? It looks good, it compiles fine and neither I nor my neovim's LSP can find any issues with it. > >diff --git a/tools/testing/selftests/resctrl/Makefile b/tools/testing/selftests/resctrl/Makefile >index 6b03bab6fc20..984534cfbf1b 100644 >--- a/tools/testing/selftests/resctrl/Makefile >+++ b/tools/testing/selftests/resctrl/Makefile >@@ -1,7 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0 >-top_srcdir = ../../../.. > >-CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2 -I$(top_srcdir)/tools/include >+CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2 > CFLAGS += $(KHDR_INCLUDES) > > TEST_GEN_PROGS := resctrl_tests >@@ -9,5 +8,6 @@ TEST_GEN_PROGS := resctrl_tests > LOCAL_HDRS += $(wildcard *.h) > > include ../lib.mk >+CFLAGS += -I$(top_srcdir)/tools/include > > $(OUTPUT)/resctrl_tests: $(wildcard *.c) > >> CFLAGS += $(KHDR_INCLUDES) >> >> TEST_GEN_PROGS := resctrl_tests > >Rest of the patch looks good to me. Thanks, I'll resend the corrected version today :) > >Reinette > -- Kind regards Maciej Wieczór-Retman
