Re: [PATCH] powerpc: thp: Add write barrier after updating the valid bit

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

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> On Tue, 2014-07-15 at 20:22 +0530, Aneesh Kumar K.V wrote:
>> With hugepages, we store the hpte valid information in the pte page
>> whose address is stored in the second half of the PMD. Use a
>> write barrier to make sure that clearing pmd busy bit and updating
>> hpte valid info are ordered properly.
>> 
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/pgtable-ppc64.h | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/arch/powerpc/include/asm/pgtable-ppc64.h b/arch/powerpc/include/asm/pgtable-ppc64.h
>> index eb9261024f51..558beb760062 100644
>> --- a/arch/powerpc/include/asm/pgtable-ppc64.h
>> +++ b/arch/powerpc/include/asm/pgtable-ppc64.h
>> @@ -394,6 +394,12 @@ static inline void mark_hpte_slot_valid(unsigned char *hpte_slot_array,
>>  					unsigned int index, unsigned int hidx)
>>  {
>>  	hpte_slot_array[index] = hidx << 4 | 0x1 << 3;
>> +	/*
>> +	 * The hpte valid is stored in the pgtable whose address is in the
>> +	 * second half of the PMD. Order this against clearing of the busy bit in
>> +	 * huge pmd.
>> +	 */
>> +	smp_wmb();
>>  }
>
> A better place for this would be right before the last write to the PMD
> (that's also clearing BUSY) in __hash_page_thp(). Basically, it's the
> normal lock ordering that's missing here, nothing specific to
> mark_hpte_slot_valid() but instead, any state relative to the BUSY bit
> in the PMD (including the actual hash writes in update_pp etc...)
>

IIUC updatepp already have required barriers. ie in updatepp we do tlbie
which should take care of the ordering right ?

Now the reason i moved that spm_wmb() to mark_hpte_slot_valid was to
pair it with smb_rmb() in get_hpte_slot_array().

-aneesh