Skip to contentSkip to conversation

selftests/mm: Skip test for non-LPA2 and non-LVA systems

PATCH Superseded Dev Jain 2 participants 4 messages Jul 17, 2024

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Dev Jain

author
#0 lore raw

PATCH 1/1 selftests/mm: Skip test for non-LPA2 and non-LVA systems

Post my improvement of the test:
https://lore.kernel.org/all/[email protected]/
The test begins to fail on 4k and 16k pages, on non-LPA2 systems. To reduce noise in the CI systems, let us skip the test when higher address space is not implemented.

---
The patch applies on linux-next.

 tools/testing/selftests/mm/va_high_addr_switch.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
tools/testing/selftests/mm/va_high_addr_switch.c +13 -1
index fa7eabfaf841..c6040e1d6e53 100644
@@ -293,6 +293,18 @@ static int run_test(struct testcase *test, int count)
 	return ret;
 }
 
+/* Check if userspace VA > 48 bits */
+static int high_address_present(void)
+{
+	void *ptr = mmap((void *)(1UL << 50), 1, PROT_READ | PROT_WRITE,
+			 MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
+	if (ptr == MAP_FAILED)
+		return 0;
+
+	munmap(ptr, 1);
+	return 1;
+}
+
 static int supported_arch(void)
 {
 #if defined(__powerpc64__)
@@ -300,7 +312,7 @@ static int supported_arch(void)
 #elif defined(__x86_64__)
 	return 1;
 #elif defined(__aarch64__)
-	return 1;
+	return high_address_present();
 #else
 	return 0;
 #endif
Signature · 1 line
2.34.1
#1 lore raw
Quoted 18 lines · Dev Jain

Post my improvement of the test:
https://lore.kernel.org/all/[email protected]/
The test begins to fail on 4k and 16k pages, on non-LPA2 systems. To reduce noise in the CI systems, let us skip the test when higher address space is not implemented.

---
The patch applies on linux-next.

 tools/testing/selftests/mm/va_high_addr_switch.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
tools/testing/selftests/mm/va_high_addr_switch.c +0 -0
index fa7eabfaf841..c6040e1d6e53 100644

tools/testing/selftests/mm/va_high_addr_switch.c static int run_test(struct testcase *test, int count) · line 300

Quoted 8 lines
tools/testing/selftests/mm/va_high_addr_switch.c +5 -0
@@ -293,6 +293,18 @@ static int run_test(struct testcase *test, int count)
 	return ret;
 }
 
+/* Check if userspace VA > 48 bits */
+static int high_address_present(void)
+{
+	void *ptr = mmap((void *)(1UL << 50), 1, PROT_READ | PROT_WRITE,
+			 MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);

I think there is (very unlikely) possibility that something is already mapped at this address so it will be replaced due to MAP_FIXED. That could break the test. But the only way something could be already mapped is if ARM64_FORCE_52BIT is set and in that case, the test will fail anyway, right? So I think this is fine.

tools/testing/selftests/mm/va_high_addr_switch.c static int run_test(struct testcase *test, int count) · line 306

Quoted 6 lines
tools/testing/selftests/mm/va_high_addr_switch.c +6 -0
+	if (ptr == MAP_FAILED)
+		return 0;
+
+	munmap(ptr, 1);
+	return 1;
+}

I'm guessing this will cause a function-not-used warning on arches other than arm64? Perhaps wrap it in #ifdef __aarch64__?

Thanks,
Ryan

Quoted 13 lines
tools/testing/selftests/mm/va_high_addr_switch.c +2 -1
+
 static int supported_arch(void)
 {
 #if defined(__powerpc64__)
@@ -300,7 +312,7 @@ static int supported_arch(void)
 #elif defined(__x86_64__)
 	return 1;
 #elif defined(__aarch64__)
-	return 1;
+	return high_address_present();
 #else
 	return 0;
 #endif
author
#2 lore raw
Quoted 22 lines · Ryan Roberts
Quoted 21 lines · Dev Jain

Post my improvement of the test:
https://lore.kernel.org/all/[email protected]/
The test begins to fail on 4k and 16k pages, on non-LPA2 systems. To reduce noise in the CI systems, let us skip the test when higher address space is not implemented.

---
The patch applies on linux-next.

  tools/testing/selftests/mm/va_high_addr_switch.c | 14 +++++++++++++-
  1 file changed, 13 insertions(+), 1 deletion(-)
tools/testing/selftests/mm/va_high_addr_switch.c +0 -0
index fa7eabfaf841..c6040e1d6e53 100644
@@ -293,6 +293,18 @@ static int run_test(struct testcase *test, int count)
  	return ret;
  }
 
Quoted 9 lines

tools/testing/selftests/mm/va_high_addr_switch.c static int run_test(struct testcase *test, int count) · line 300

Quoted 5 lines
tools/testing/selftests/mm/va_high_addr_switch.c +5 -0
+/* Check if userspace VA > 48 bits */
+static int high_address_present(void)
+{
+	void *ptr = mmap((void *)(1UL << 50), 1, PROT_READ | PROT_WRITE,
+			 MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);

I think there is (very unlikely) possibility that something is already mapped at this address so it will be replaced due to MAP_FIXED. That could break the test. But the only way something could be already mapped is if ARM64_FORCE_52BIT is set and in that case, the test will fail anyway, right? So I think this is fine.

The testcases already assume that high addresses must be empty. Yes, FORCE_52BIT is the only way something could already be mapped at high addresses, but in that case the test fails trivially.

Quoted 9 lines

tools/testing/selftests/mm/va_high_addr_switch.c static int run_test(struct testcase *test, int count) · line 306

Quoted 7 lines
tools/testing/selftests/mm/va_high_addr_switch.c +6 -0
+	if (ptr == MAP_FAILED)
+		return 0;
+
+	munmap(ptr, 1);
+	return 1;
+}

I'm guessing this will cause a function-not-used warning on arches other than arm64? Perhaps wrap it in #ifdef __aarch64__?

Ah yes, I just checked and that is true. I shall post v2 in some time, shall wait if any more comments are there.

Quoted 17 lines

Thanks,
Ryan

Quoted 13 lines
tools/testing/selftests/mm/va_high_addr_switch.c +2 -1
+
  static int supported_arch(void)
  {
  #if defined(__powerpc64__)
@@ -300,7 +312,7 @@ static int supported_arch(void)
  #elif defined(__x86_64__)
  	return 1;
  #elif defined(__aarch64__)
-	return 1;
+	return high_address_present();
  #else
  	return 0;
  #endif
Reviewed-by
#3 lore raw
Quoted 34 lines · Dev Jain
Quoted 23 lines · Ryan Roberts
Quoted 22 lines · Dev Jain

Post my improvement of the test:
https://lore.kernel.org/all/[email protected]/
The test begins to fail on 4k and 16k pages, on non-LPA2 systems. To reduce noise in the CI systems, let us skip the test when higher address space is not implemented.

---
The patch applies on linux-next.

  tools/testing/selftests/mm/va_high_addr_switch.c | 14 +++++++++++++-

  1 file changed, 13 insertions(+), 1 deletion(-)
tools/testing/selftests/mm/va_high_addr_switch.c +0 -0
index fa7eabfaf841..c6040e1d6e53 100644
@@ -293,6 +293,18 @@ static int run_test(struct testcase *test, int count)
       return ret;
   }

  +/* Check if userspace VA > 48 bits */

Quoted 8 lines

tools/testing/selftests/mm/va_high_addr_switch.c static int run_test(struct testcase *test, int count) · line 300

Quoted 4 lines
tools/testing/selftests/mm/va_high_addr_switch.c +4 -0
+static int high_address_present(void)
+{
+    void *ptr = mmap((void *)(1UL << 50), 1, PROT_READ | PROT_WRITE,
+             MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);

I think there is (very unlikely) possibility that something is already mapped at this address so it will be replaced due to MAP_FIXED. That could break the test. But the only way something could be already mapped is if ARM64_FORCE_52BIT is set and in that case, the test will fail anyway, right? So I think this is fine.

Quoted 3 lines

The testcases already assume that high addresses must be empty. Yes, FORCE_52BIT is the only way something could already be mapped at high addresses, but in that case the test fails trivially.

agreed.

Quoted 11 lines
Quoted 10 lines

tools/testing/selftests/mm/va_high_addr_switch.c static int run_test(struct testcase *test, int count) · line 306

Quoted 8 lines

+    if (ptr == MAP_FAILED)
+        return 0;

tools/testing/selftests/mm/va_high_addr_switch.c +4 -0
+
+    munmap(ptr, 1);
+    return 1;
+}

I'm guessing this will cause a function-not-used warning on arches other than arm64? Perhaps wrap it in #ifdef __aarch64__?

Quoted 2 lines

Ah yes, I just checked and that is true. I shall post v2 in some time, shall wait if any more comments are there.

With this fixup:

Quoted 18 lines
Quoted 18 lines

Thanks,
Ryan

Quoted 13 lines

+
  static int supported_arch(void)
  {
  #if defined(__powerpc64__)

tools/testing/selftests/mm/va_high_addr_switch.c +1 -1
@@ -300,7 +312,7 @@ static int supported_arch(void)
   #elif defined(__x86_64__)
       return 1;
   #elif defined(__aarch64__)
-    return 1;
+    return high_address_present();
   #else
       return 0;
   #endif