Hi Maciej,
On 8/27/24 1:15 AM, Maciej Wieczor-Retman wrote:
On 2024-08-12 at 16:40:10 -0700, Reinette Chatre wrote:
On 7/12/24 2:04 AM, Maciej Wieczor-Retman wrote:
- if ((get_vendor() == ARCH_INTEL) && snc_unreliable) {
ksft_print_msg("Sub-NUMA Clustering could not be detected properly (see earlier messages for details).\n");ksft_print_msg("Intel CAT may be inaccurate.\n");- }
This is still relevant but unclear why previous message checked "ret" but above does not.
The above check tries to explain why a failure happened.
This check is a reminder about a false positive - the test passes but "snc_unreliable" was set. I guess we could make this check to test "!ret"?
Thinking about this more ... if the test results cannot be trusted at all (whether tests pass or fail) when snc_reliable is true then it seems more appropriate to just skip these tests when SNC detection is unreliable.
- return ret; }
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c index 0c045080d808..471e134face0 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -175,8 +175,14 @@ static int cmt_run_test(const struct resctrl_test *test, const struct user_param goto out; ret = check_results(¶m, span, n);
- if (ret && (get_vendor() == ARCH_INTEL))
ksft_print_msg("Intel CMT may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n");
- if (ret && (get_vendor() == ARCH_INTEL) && !snc_kernel_support())
ksft_print_msg("Kernel doesn't support Sub-NUMA Clustering but it is enabled on the system.\n");- if ((get_vendor() == ARCH_INTEL) && snc_unreliable) {
ksft_print_msg("Sub-NUMA Clustering could not be detected properly (see earlier messages for details).\n");ksft_print_msg("Intel CMT may be inaccurate.\n");- }
CMT may be inaccurate in both scenarios (no kernel support or unreliable detection). Why only check "ret" in case there is no kernel support?
I guess the same thing from above can apply here? Test "!ret"? Perhaps then make this check into "else if ()" instead of just "if" since they will be exclusive?
out: free(span_str); diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c index ab8496a4925b..a805c14fe04b 100644 --- a/tools/testing/selftests/resctrl/mba_test.c +++ b/tools/testing/selftests/resctrl/mba_test.c @@ -179,6 +179,13 @@ static int mba_run_test(const struct resctrl_test *test, const struct user_param return ret; ret = check_results();
- if (ret && (get_vendor() == ARCH_INTEL) && !snc_kernel_support())
ksft_print_msg("Kernel doesn't support Sub-NUMA Clustering but it is enabled on the system.\n");- if ((get_vendor() == ARCH_INTEL) && snc_unreliable) {
ksft_print_msg("Sub-NUMA Clustering could not be detected properly (see earlier messages for details).\n");ksft_print_msg("Intel MBA may be inaccurate.\n");- }
As I understand there is no change to MBA when SNC is enabled. These additions thus seem unnecessary.
I just rechecked by installing 6.9 kernel (no SNC kernel support) and using this series selftest. MBA seems to fail in these conditions. I think it is because MBA pulls values from resctrl and from iMC and then compares them. My guess is that iMC works on the halved cache while resctrl (without new SNC support) uses the whole cache. Here is the log from the MBA test I did:
Apologies, yes, while MBA is not impacted by SNC the MBA selftest relies on MBM that is impacted by SNC.
Reinette