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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4magarhopewnvypvj6kukva6gl33zwgpjmjomz7s3yrkntxnrw@idiykpyhep7k>
Date: Mon, 16 Dec 2024 15:23:39 +0100
From: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
To: Reinette Chatre <reinette.chatre@...el.com>
CC: <shuah@...nel.org>, <fenghua.yu@...el.com>,
	<linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
	<ilpo.jarvinen@...ux.intel.com>, <tony.luck@...el.com>
Subject: Re: [PATCH v7 1/2] selftests/resctrl: Adjust effective L3 cache size
 with SNC enabled

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