* [PATCH] selftest/vm: Use correct PAGE_SHIFT value for ppc64
@ 2022-02-09 15:43 Aneesh Kumar K.V
2022-02-09 19:53 ` Shuah Khan
0 siblings, 1 reply; 7+ messages in thread
From: Aneesh Kumar K.V @ 2022-02-09 15:43 UTC (permalink / raw)
To: linux-mm, akpm; +Cc: mpe, linuxppc-dev, Aneesh Kumar K.V, Shuah Khan
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
+#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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] selftest/vm: Use correct PAGE_SHIFT value for ppc64
2022-02-09 15:43 [PATCH] selftest/vm: Use correct PAGE_SHIFT value for ppc64 Aneesh Kumar K.V
@ 2022-02-09 19:53 ` Shuah Khan
2022-02-10 4:12 ` Aneesh Kumar K.V
0 siblings, 1 reply; 7+ messages in thread
From: Shuah Khan @ 2022-02-09 19:53 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm
Cc: mpe, linuxppc-dev, Shuah Khan, Shuah Khan
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?
> +#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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] selftest/vm: Use correct PAGE_SHIFT value for ppc64
2022-02-09 19:53 ` Shuah Khan
@ 2022-02-10 4:12 ` Aneesh Kumar K.V
2022-02-10 5:35 ` Aneesh Kumar K.V
2022-02-10 14:39 ` Shuah Khan
0 siblings, 2 replies; 7+ messages in thread
From: Aneesh Kumar K.V @ 2022-02-10 4:12 UTC (permalink / raw)
To: Shuah Khan, linux-mm, akpm; +Cc: mpe, linuxppc-dev, Shuah Khan, Shuah Khan
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.
>
>> +#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
-aneesh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] selftest/vm: Use correct PAGE_SHIFT value for ppc64
2022-02-10 4:12 ` Aneesh Kumar K.V
@ 2022-02-10 5:35 ` Aneesh Kumar K.V
2022-02-10 14:39 ` Shuah Khan
1 sibling, 0 replies; 7+ messages in thread
From: Aneesh Kumar K.V @ 2022-02-10 5:35 UTC (permalink / raw)
To: Shuah Khan, linux-mm, akpm; +Cc: mpe, linuxppc-dev, Shuah Khan, Shuah Khan
Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> writes:
> Shuah Khan <skhan@linuxfoundation.org> writes:
>
>> On 2/9/22 8:43 AM, Aneesh Kumar K.V wrote:
>> --- 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.
>
>>
>>> +#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 <aneesh.kumar@linux.ibm.com>
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 <aneesh.kumar@linux.ibm.com>
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] selftest/vm: Use correct PAGE_SHIFT value for ppc64
2022-02-10 4:12 ` Aneesh Kumar K.V
2022-02-10 5:35 ` Aneesh Kumar K.V
@ 2022-02-10 14:39 ` Shuah Khan
2022-02-10 15:03 ` Aneesh Kumar K V
1 sibling, 1 reply; 7+ messages in thread
From: Shuah Khan @ 2022-02-10 14:39 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-mm, akpm
Cc: mpe, linuxppc-dev, Shuah Khan, Shuah Khan
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
Hope this helps.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] selftest/vm: Use correct PAGE_SHIFT value for ppc64
2022-02-10 14:39 ` Shuah Khan
@ 2022-02-10 15:03 ` Aneesh Kumar K V
2022-02-10 15:08 ` Shuah Khan
0 siblings, 1 reply; 7+ messages in thread
From: Aneesh Kumar K V @ 2022-02-10 15:03 UTC (permalink / raw)
To: Shuah Khan, linux-mm, akpm; +Cc: mpe, linuxppc-dev, Shuah Khan
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.
-aneesh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] selftest/vm: Use correct PAGE_SHIFT value for ppc64
2022-02-10 15:03 ` Aneesh Kumar K V
@ 2022-02-10 15:08 ` Shuah Khan
0 siblings, 0 replies; 7+ messages in thread
From: Shuah Khan @ 2022-02-10 15:08 UTC (permalink / raw)
To: Aneesh Kumar K V, linux-mm, akpm
Cc: mpe, linuxppc-dev, Shuah Khan, Shuah Khan, linux-kselftest
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-02-10 15:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09 15:43 [PATCH] selftest/vm: Use correct PAGE_SHIFT value for ppc64 Aneesh Kumar K.V
2022-02-09 19:53 ` Shuah Khan
2022-02-10 4:12 ` Aneesh Kumar K.V
2022-02-10 5:35 ` Aneesh Kumar K.V
2022-02-10 14:39 ` Shuah Khan
2022-02-10 15:03 ` Aneesh Kumar K V
2022-02-10 15:08 ` Shuah Khan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).