Re: [PATCH] selftest/vm: Use correct PAGE_SHIFT value for ppc64

On 2/10/22 20:09, Shuah Khan wrote:

On 2/9/22 9:12 PM, Aneesh Kumar K.V wrote:

quoted

Shuah Khan [off-list ref] writes:

quoted

On 2/9/22 8:43 AM, Aneesh Kumar K.V wrote:

quoted

Keep it simple by using a #define and limiting hugepage size to 2M.
This keeps the test simpler instead of dynamically finding the page 
size
and huge page size.

Without this tests are broken w.r.t reading /proc/self/pagemap

    if (pread(pagemap_fd, ent, sizeof(ent),
            (uintptr_t)ptr >> (PAGE_SHIFT - 3)) != sizeof(ent))
        err(2, "read pagemap");

Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Aneesh Kumar K.V <redacted>
---
   tools/testing/selftests/vm/ksm_tests.c        | 8 ++++++++
   tools/testing/selftests/vm/transhuge-stress.c | 8 ++++++++
   2 files changed, 16 insertions(+)

diff --git a/tools/testing/selftests/vm/ksm_tests.c 

b/tools/testing/selftests/vm/ksm_tests.c
index 1436e1a9a3d3..8200328ff018 100644

--- a/tools/testing/selftests/vm/ksm_tests.c
+++ b/tools/testing/selftests/vm/ksm_tests.c

@@ -22,8 +22,16 @@

   #define KSM_MERGE_ACROSS_NODES_DEFAULT true
   #define MB (1ul << 20)
+#ifdef __powerpc64__
+#define PAGE_SHIFT    16
+/*
+ * This will only work with radix 2M hugepage size
+ */
+#define HPAGE_SHIFT 21
+#else
   #define PAGE_SHIFT 12
   #define HPAGE_SHIFT 21
+#endif
   #define PAGE_SIZE (1 << PAGE_SHIFT)
   #define HPAGE_SIZE (1 << HPAGE_SHIFT)

diff --git a/tools/testing/selftests/vm/transhuge-stress.c 

b/tools/testing/selftests/vm/transhuge-stress.c
index 5e4c036f6ad3..f04c8aa4bcf6 100644

--- a/tools/testing/selftests/vm/transhuge-stress.c
+++ b/tools/testing/selftests/vm/transhuge-stress.c

@@ -16,8 +16,16 @@

   #include <string.h>
   #include <sys/mman.h>
+#ifdef __powerpc64__
+#define PAGE_SHIFT    16
+/*
+ * This will only work with radix 2M hugepage size
+ */
+#define HPAGE_SHIFT 21

Why not have this is in common code?

Can you suggest where I can move that. We also have helper functions
like allocate_transhuge() duplicated between tests. I didn't find
libutil.a or anything similar supported by the selftets build.

quoted

I noticed that HPAGE_SHIFT is defined in #ifdef __powerpc64__ block
as well as #else. I am asking is it necessary to be part of both
blocks.

+#ifdef __powerpc64__
+#define PAGE_SHIFT    16
+/*
+ * This will only work with radix 2M hugepage size
+ */
+#define HPAGE_SHIFT 21  --- this one
+#else
   #define PAGE_SHIFT 12
   #define HPAGE_SHIFT 21   --- this one
+#endif

The reason I did that was to add the comment which is relevant only for 
ppc64. ppc64 supports two hugepage sizes, 2M and 16M. The test won't 
work correctly with 16M hugepage size. We do have other tests in 
selftest/vm/ with similar restrictions.


-aneesh