Re: [PATCH v7 1/2] selftests/resctrl: Adjust effective L3 cache size with SNC enabled

[Reinette Chatre <reinette.chatre@intel.com>]

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@intel.com>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> ---

...

> 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 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?

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.

Reinette