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

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
+#else
 #define PAGE_SHIFT 12
 #define HPAGE_SHIFT 21
+#endif
 
 #define PAGE_SIZE (1 << PAGE_SHIFT)
 #define HPAGE_SIZE (1 << HPAGE_SHIFT)

-- 
2.34.1

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

Why not have this is in common code?

+#else
  #define PAGE_SHIFT 12
  #define HPAGE_SHIFT 21

Same here.

+#endif
  
  #define PAGE_SIZE (1 << PAGE_SHIFT)
  #define HPAGE_SIZE (1 << HPAGE_SHIFT)

Please cc linux-kselftest mailing list in the future.

With the above fixed.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

Shuah Khan [off-list ref] writes:

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

+#else
  #define PAGE_SHIFT 12
  #define HPAGE_SHIFT 21

Same here.

quoted

+#endif
  
  #define PAGE_SIZE (1 << PAGE_SHIFT)
  #define HPAGE_SIZE (1 << HPAGE_SHIFT)

Please cc linux-kselftest mailing list in the future.

With the above fixed.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

-aneesh

Aneesh Kumar K.V [off-list ref] writes:

Shuah Khan [off-list ref] writes:

quoted

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

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

quoted

+++ 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

quoted

+#else
  #define PAGE_SHIFT 12
  #define HPAGE_SHIFT 21

I can do a util.h for vm related test as below. 

commit 378fa2d73f1255d3045023875dd00d5b8486440e
Author: Aneesh Kumar K.V [off-list ref]
Date:   Thu Feb 10 10:22:27 2022 +0530

    selftest/vm: Add util.h and and move helper functions there
    
    Avoid code duplication by adding util.h
    
    Signed-off-by: Aneesh Kumar K.V [off-list ref]

diff --git a/tools/testing/selftests/vm/ksm_tests.c b/tools/testing/selftests/vm/ksm_tests.c
index 8200328ff018..fd85f15869d1 100644
--- a/tools/testing/selftests/vm/ksm_tests.c
+++ b/tools/testing/selftests/vm/ksm_tests.c

@@ -12,6 +12,7 @@
 
 #include "../kselftest.h"
 #include "../../../../include/vdso/time64.h"
+#include "util.h"
 
 #define KSM_SYSFS_PATH "/sys/kernel/mm/ksm/"
 #define KSM_FP(s) (KSM_SYSFS_PATH s)

@@ -22,23 +23,6 @@
 #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)
-
-#define PAGEMAP_PRESENT(ent)	(((ent) & (1ull << 63)) != 0)
-#define PAGEMAP_PFN(ent)	((ent) & ((1ull << 55) - 1))
-
 struct ksm_sysfs {
 	unsigned long max_page_sharing;
 	unsigned long merge_across_nodes;

@@ -464,34 +448,6 @@ static int check_ksm_numa_merge(int mapping, int prot, int timeout, bool merge_a
 	return KSFT_FAIL;
 }
 
-int64_t allocate_transhuge(void *ptr, int pagemap_fd)
-{
-	uint64_t ent[2];
-
-	/* drop pmd */
-	if (mmap(ptr, HPAGE_SIZE, PROT_READ | PROT_WRITE,
-				MAP_FIXED | MAP_ANONYMOUS |
-				MAP_NORESERVE | MAP_PRIVATE, -1, 0) != ptr)
-		errx(2, "mmap transhuge");
-
-	if (madvise(ptr, HPAGE_SIZE, MADV_HUGEPAGE))
-		err(2, "MADV_HUGEPAGE");
-
-	/* allocate transparent huge page */
-	*(volatile void **)ptr = ptr;
-
-	if (pread(pagemap_fd, ent, sizeof(ent),
-			(uintptr_t)ptr >> (PAGE_SHIFT - 3)) != sizeof(ent))
-		err(2, "read pagemap");
-
-	if (PAGEMAP_PRESENT(ent[0]) && PAGEMAP_PRESENT(ent[1]) &&
-	    PAGEMAP_PFN(ent[0]) + 1 == PAGEMAP_PFN(ent[1]) &&
-	    !(PAGEMAP_PFN(ent[0]) & ((1 << (HPAGE_SHIFT - PAGE_SHIFT)) - 1)))
-		return PAGEMAP_PFN(ent[0]);
-
-	return -1;
-}
-
 static int ksm_merge_hugepages_time(int mapping, int prot, int timeout, size_t map_size)
 {
 	void *map_ptr, *map_ptr_orig;

diff --git a/tools/testing/selftests/vm/transhuge-stress.c b/tools/testing/selftests/vm/transhuge-stress.c
index f04c8aa4bcf6..0da4aa10746a 100644
--- a/tools/testing/selftests/vm/transhuge-stress.c
+++ b/tools/testing/selftests/vm/transhuge-stress.c

@@ -16,52 +16,7 @@
 #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
-#else
-#define PAGE_SHIFT 12
-#define HPAGE_SHIFT 21
-#endif
-
-#define PAGE_SIZE (1 << PAGE_SHIFT)
-#define HPAGE_SIZE (1 << HPAGE_SHIFT)
-
-#define PAGEMAP_PRESENT(ent)	(((ent) & (1ull << 63)) != 0)
-#define PAGEMAP_PFN(ent)	((ent) & ((1ull << 55) - 1))
-
-int pagemap_fd;
-
-int64_t allocate_transhuge(void *ptr)
-{
-	uint64_t ent[2];
-
-	/* drop pmd */
-	if (mmap(ptr, HPAGE_SIZE, PROT_READ | PROT_WRITE,
-				MAP_FIXED | MAP_ANONYMOUS |
-				MAP_NORESERVE | MAP_PRIVATE, -1, 0) != ptr)
-		errx(2, "mmap transhuge");
-
-	if (madvise(ptr, HPAGE_SIZE, MADV_HUGEPAGE))
-		err(2, "MADV_HUGEPAGE");
-
-	/* allocate transparent huge page */
-	*(volatile void **)ptr = ptr;
-
-	if (pread(pagemap_fd, ent, sizeof(ent),
-			(uintptr_t)ptr >> (PAGE_SHIFT - 3)) != sizeof(ent))
-		err(2, "read pagemap");
-
-	if (PAGEMAP_PRESENT(ent[0]) && PAGEMAP_PRESENT(ent[1]) &&
-	    PAGEMAP_PFN(ent[0]) + 1 == PAGEMAP_PFN(ent[1]) &&
-	    !(PAGEMAP_PFN(ent[0]) & ((1 << (HPAGE_SHIFT - PAGE_SHIFT)) - 1)))
-		return PAGEMAP_PFN(ent[0]);
-
-	return -1;
-}
+#include "util.h"
 
 int main(int argc, char **argv)
 {

@@ -71,6 +26,7 @@ int main(int argc, char **argv)
 	double s;
 	uint8_t *map;
 	size_t map_len;
+	int pagemap_fd;
 
 	ram = sysconf(_SC_PHYS_PAGES);
 	if (ram > SIZE_MAX / sysconf(_SC_PAGESIZE) / 4)

@@ -117,7 +73,7 @@ int main(int argc, char **argv)
 		for (p = ptr; p < ptr + len; p += HPAGE_SIZE) {
 			int64_t pfn;
 
-			pfn = allocate_transhuge(p);
+			pfn = allocate_transhuge(p, pagemap_fd);
 
 			if (pfn < 0) {
 				nr_failed++;

diff --git a/tools/testing/selftests/vm/util.h b/tools/testing/selftests/vm/util.h
new file mode 100644
index 000000000000..1461a96f0bc0
--- /dev/null
+++ b/tools/testing/selftests/vm/util.h

@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __KSELFTEST_VM_UTIL_H
+#define __KSELFTEST_VM_UTIL_H
+
+#include <sys/mman.h>
+#include <err.h>
+
+#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)
+
+#define PAGEMAP_PRESENT(ent)	(((ent) & (1ull << 63)) != 0)
+#define PAGEMAP_PFN(ent)	((ent) & ((1ull << 55) - 1))
+
+
+static inline int64_t allocate_transhuge(void *ptr, int pagemap_fd)
+{
+	uint64_t ent[2];
+
+	/* drop pmd */
+	if (mmap(ptr, HPAGE_SIZE, PROT_READ | PROT_WRITE,
+		 MAP_FIXED | MAP_ANONYMOUS |
+		 MAP_NORESERVE | MAP_PRIVATE, -1, 0) != ptr)
+		errx(2, "mmap transhuge");
+
+	if (madvise(ptr, HPAGE_SIZE, MADV_HUGEPAGE))
+		err(2, "MADV_HUGEPAGE");
+
+	/* allocate transparent huge page */
+	*(volatile void **)ptr = ptr;
+
+	if (pread(pagemap_fd, ent, sizeof(ent),
+		  (uintptr_t)ptr >> (PAGE_SHIFT - 3)) != sizeof(ent))
+		err(2, "read pagemap");
+
+	if (PAGEMAP_PRESENT(ent[0]) && PAGEMAP_PRESENT(ent[1]) &&
+	    PAGEMAP_PFN(ent[0]) + 1 == PAGEMAP_PFN(ent[1]) &&
+	    !(PAGEMAP_PFN(ent[0]) & ((1 << (HPAGE_SHIFT - PAGE_SHIFT)) - 1)))
+		return PAGEMAP_PFN(ent[0]);
+
+	return -1;
+}
+
+#endif

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

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


Hope this helps.

thanks,
-- Shuah

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

On 2/10/22 8:03 AM, Aneesh Kumar K V wrote:

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

quoted

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.

Right. You don't have to duplicate code for the comment. You can add the
comment and then clarify in the comment that it is only relevant to ppc64.

This way the comment is clear and we can avoid duplicate code that makes it
hard to maintain in the future.

thanks,
-- Shuah