Re: [PATCH V3 1/6] powerpc/mm/book3s: Update pmd_present to look at _PAGE_PRESENT bit

["Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>]

On 9/21/18 11:25 AM, Christophe LEROY wrote:
> 
> 
> Le 20/09/2018 à 20:09, Aneesh Kumar K.V a écrit :

>> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
>> index d71c7777669c..aee04b209b51 100644
>> --- a/arch/powerpc/mm/pgtable.c
>> +++ b/arch/powerpc/mm/pgtable.c
>> @@ -188,11 +188,10 @@ void set_pte_at(struct mm_struct *mm, unsigned 
>> long addr, pte_t *ptep,
>>           pte_t pte)
>>   {
>>       /*
>> -     * When handling numa faults, we already have the pte marked
>> -     * _PAGE_PRESENT, but we can be sure that it is not in hpte.
>> -     * Hence we can use set_pte_at for them.
>> +     * Make sure hardware valid bit is not set. We don't do
>> +     * tlb flush for this update.
>>        */
>> -    VM_WARN_ON(pte_present(*ptep) && !pte_protnone(*ptep));
>> +    VM_WARN_ON(pte_val(*ptep) & _PAGE_PRESENT);
> 
> Why not using pte_present() anymore ?
> 
> Also, you are removing the pte_protnone() check, won't it change the 
> behaviour ?
> 
> If we can't use pte_present(), can we create a new helper for that 
> (allthough _PAGE_PRESENT exists on all platforms).
> 
> Christophe
> 

This patch update a page table clear to clear _PAGE_PRESENT and mark it 
invalid via _PAGE_INVALID. The pte_present now looks at both the flag. 
That is we want these transient clear of pte to be considered as present 
pte even if _PAGE_PRESENT is cleared. What we are catching by the debug 
BUG_ON in these function is we are not using them to set a pte where the 
old entry has a hadware valid bit set. This is because we don't do any 
tlb flush with set_pte_at.


So the reason for pte_present -> pte_val() & _PAGE_PRESENT is because we 
swtiched the clear to clear _PAGE_PRESENT and set _PAGE_INVALID and 
pte_present now check both.

The reason for the removal of pte_protnone is because we dropped that 
set_pte_at usage from core autonuma code long time back.

Now Considering we are calling this from mm/pgtable.c With your approach 
of not using pte flags directly in core code we could switch this to 
pte_hw_valid(). May be we can do that as an addon patch?

-aneesh