Hello,
On 2024-07-08 at 09:39:02 -0700, Reinette Chatre wrote:
Hi Maciej,
On 7/4/24 12:23 AM, Maciej Wieczor-Retman wrote:
On 2024-07-03 at 13:51:03 -0700, Reinette Chatre wrote:
On 7/3/24 12:43 AM, Maciej Wieczór-Retman wrote:
On 3.07.2024 00:21, Reinette Chatre wrote:
On 7/1/24 7:18 AM, Maciej Wieczor-Retman wrote:
...
SNC might not be enabled at all so there would be no reason to send the user to check their BIOS - instead they can learn they have offline CPUs and they can work on fixing that. In my opinion it could be beneficial to have more specialized messages in the selftests to help users diagnose problems quicker.
My goal is indeed to have specialized messages. There cannot be a specialized message if snc_reliable == 1. In this case it needs to be generic since SNC may or may not be enabled and it is up to the user to investigate further.
How about this in cmt_run_test() for example:
if (snc_unreliable) ksft_print_msg("Intel CMT may be inaccurate or inefficient when Sub-NUMA Clustering is enabled and not properly detected.\n");
It is ok with me if you want to keep the message even if the test succeeds. Without seeing the new implementation it is unclear to me why the SNC check below is guarded by an ARCH_INTEL check while the one above is not. Ideally this should be consistent to not confuse how the architectures need to be treated here.
Right, I'll add the get_vendor() check to this too.
The message does sound a bit vague to me about being able to detect SNC. How about something like: Sub-NUMA Clustering could not be detected properly (see earlier messages for details). Intel CMT may be inaccurate.
It sounds good, I'll change the message to this.
else if (ret && (get_vendor() == ARCH_INTEL) && !snc_kernel_support()) ksft_print_msg("Kernel doesn't support Sub-NUMA Clustering but it is enabled. Check BIOS configuration.\n");
The "Check BIOS configuration" guidance is not clear to me. If the kernel does not support SNC then the user could also be guided to upgrade their kernel? Perhaps that snippet can just be dropped? To be more specific that SNC enabling is not a kernel configuration but a system configuration this can read (please feel free to improve): Kernel doesn't support Sub-NUMA Clustering but it is enabled on the system.
I suppose you're right, this does look better. Thanks!
Reinette