On 2/10/22 8:03 AM, Aneesh Kumar K V wrote:
On 2/10/22 20:09, Shuah Khan wrote:
On 2/9/22 9:12 PM, Aneesh Kumar K.V wrote:
Shuah Khan skhan@linuxfoundation.org writes:
On 2/9/22 8:43 AM, Aneesh Kumar K.V wrote:
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 aneesh.kumar@linux.ibm.com
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.
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
linux-kselftest-mirror@lists.linaro.org
-
Shuah Khan