Subject: Re: [PATCH V4 03/18] powerpc/mm: add _PAGE_HASHPTE similar to 4K hash

From: "Aneesh Kumar K.V" <[email-masked]>
Date: Tue, 23 Feb 2016 14:52:07 +0530

Paul Mackerras <[email-masked]> writes:

> On Tue, Feb 23, 2016 at 10:18:05AM +0530, Aneesh Kumar K.V wrote:
>> The difference between 64K and 4K hash fault handling is confusing
>> with respect to when we set _PAGE_HASHPTE in the linux pte.
>> I was trying to find out whether we miss a hpte flush in any
>> scenario because of this. ie, a pte update on a linux pte, for which we
>> are doing a parallel hash pte insert. After looking at it closer my
>> understanding is this won't happen because pte update also look at
>> _PAGE_BUSY and we will wait for hash pte insert to finish before going
>> ahead with the pte update. But to avoid further confusion keep the
>> hash fault handler for all the page size similar to  __hash_page_4k.
>> 
>> This partially reverts commit 41743a4e34f0 ("powerpc: Free a PTE bit on ppc64 with 64K pages"
>
> In each of the functions you are modifying below, there is already an
> explicit setting of _PAGE_HASHPTE in new_pte.  So I don't think this
> is necessary, or if we do this, we can eliminate the separate setting
> of _PAGE_HASHPTE later on.
>
> In general I think it's better to leave the setting of _PAGE_HASHPTE
> until we know what slot the HPTE is going to go into.  That way we
> have less chance of ending up with _PAGE_HASHPTE set but bogus
> information in _PAGE_F_GIX and _PAGE_F_SECOND.
>

Ok. I will do a variant that remove _PAGE_HASHPTE setting from 4k. ie, 

diff --git a/arch/powerpc/mm/hash64_4k.c b/arch/powerpc/mm/hash64_4k.c
index e3e76b929f33..47d1b26effc6 100644
--- a/arch/powerpc/mm/hash64_4k.c
+++ b/arch/powerpc/mm/hash64_4k.c
@@ -44,7 +44,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
 		 * a write access. Since this is 4K insert of 64K page size
 		 * also add _PAGE_COMBO
 		 */
-		new_pte = old_pte | _PAGE_BUSY | _PAGE_ACCESSED | _PAGE_HASHPTE;
+		new_pte = old_pte | _PAGE_BUSY | _PAGE_ACCESSED;
 		if (access & _PAGE_RW)
 			new_pte |= _PAGE_DIRTY;
 	} while (old_pte != __cmpxchg_u64((unsigned long *)ptep,