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

From: "Aneesh Kumar K.V" <[email-masked]>
Date: Tue, 29 Jul 2014 12:25:51 +0530

Benjamin Herrenschmidt <[email-masked]> writes:

> On Wed, 2014-07-23 at 00:23 +0530, Aneesh Kumar K.V wrote:
>> > 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 ?
>
> Only if it succeeds but that doesn't matter, I'd rather we get the
> semantics right. The clearing of the busy bit is an unlock, it should
> have the appropriate barriers like it does in other variants of hash
> page.

ok

>> 
>> 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().
>
> Which is also probably in the wrong place. Care to explain to me the
> exact relationship ?

We want to make sure for usage like below we don't reorder the  load.

if (pmd_trans_huge(*pmdp)){

   get_hpte_slot_array(pmdp)
}

-aneesh