[PATCH v3 03/13] mm/debug_vm_pgtable/ppc64: Avoid setting top bits in radom value

[Aneesh Kumar K.V]

On 9/1/20 8:45 AM, Anshuman Khandual wrote:
> 
> 
> On 08/27/2020 01:34 PM, Aneesh Kumar K.V wrote:
>> ppc64 use bit 62 to indicate a pte entry (_PAGE_PTE). Avoid setting that bit in
>> random value.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar at linux.ibm.com>
>> ---
>>   mm/debug_vm_pgtable.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>> index 086309fb9b6f..bbf9df0e64c6 100644
>> --- a/mm/debug_vm_pgtable.c
>> +++ b/mm/debug_vm_pgtable.c
>> @@ -44,10 +44,17 @@
>>    * entry type. But these bits might affect the ability to clear entries with
>>    * pxx_clear() because of how dynamic page table folding works on s390. So
>>    * while loading up the entries do not change the lower 4 bits. It does not
>> - * have affect any other platform.
>> + * have affect any other platform. Also avoid the 62nd bit on ppc64 that is
>> + * used to mark a pte entry.
>>    */
>> -#define S390_MASK_BITS	4
>> -#define RANDOM_ORVALUE	GENMASK(BITS_PER_LONG - 1, S390_MASK_BITS)
>> +#define S390_SKIP_MASK		GENMASK(3, 0)
>> +#ifdef CONFIG_PPC_BOOK3S_64
>> +#define PPC64_SKIP_MASK		GENMASK(62, 62)
>> +#else
>> +#define PPC64_SKIP_MASK		0x0
>> +#endif
> 
> Please drop the #ifdef CONFIG_PPC_BOOK3S_64 here. We already accommodate skip
> bits for a s390 platform requirement and can also do so for ppc64 as well. As
> mentioned before, please avoid adding any platform specific constructs in the
> test.
> 


that is needed so that it can be built on 32 bit architectures.I did 
face build errors with arch-linux

>> +#define ARCH_SKIP_MASK (S390_SKIP_MASK | PPC64_SKIP_MASK)
>> +#define RANDOM_ORVALUE (GENMASK(BITS_PER_LONG - 1, 0) & ~ARCH_SKIP_MASK)
>>   #define RANDOM_NZVALUE	GENMASK(7, 0)
> 
> Please fix the alignments here. Feel free to consider following changes after
> this patch.
> 
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 122416464e0f..f969031152bb 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -48,14 +48,11 @@
>    * have affect any other platform. Also avoid the 62nd bit on ppc64 that is
>    * used to mark a pte entry.
>    */
> -#define S390_SKIP_MASK         GENMASK(3, 0)
> -#ifdef CONFIG_PPC_BOOK3S_64
> -#define PPC64_SKIP_MASK                GENMASK(62, 62)
> -#else
> -#define PPC64_SKIP_MASK                0x0
> -#endif
> -#define ARCH_SKIP_MASK (S390_SKIP_MASK | PPC64_SKIP_MASK)
> -#define RANDOM_ORVALUE (GENMASK(BITS_PER_LONG - 1, 0) & ~ARCH_SKIP_MASK)
> +#define S390_SKIP_MASK GENMASK(3, 0)
> +#define PPC64_SKIP_MASK        GENMASK(62, 62)
> +#define ARCH_SKIP_MASK (S390_SKIP_MASK | PPC64_SKIP_MASK)
> +#define RANDOM_ORVALUE (GENMASK(BITS_PER_LONG - 1, 0) & ~ARCH_SKIP_MASK)
> +
>   #define RANDOM_NZVALUE GENMASK(7, 0)
>   
>>   
>>   static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
>>
> 
> Besides, there is also one checkpatch.pl warning here.
> 
> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> #7:
> ppc64 use bit 62 to indicate a pte entry (_PAGE_PTE). Avoid setting that bit in
> 
> total: 0 errors, 1 warnings, 20 lines checked
> 


These warnings are not valid. They are mostly long lines (upto 100) . or 
some details mentioned in the () as above.

-aneesh