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

[Shuah Khan <skhan@linuxfoundation.org>]

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