Re: [PATCH 4/6] mm: hugetlb_vmemmap: add missing smp_wmb() before set_pte_at()

[Miaohe Lin]

On 2022/8/18 10:47, Muchun Song wrote:
> 
> 
>> On Aug 18, 2022, at 10:00, Yin, Fengwei <fengwei.yin@xxxxxxxxx> wrote:
>>
>>
>>
>> On 8/18/2022 9:55 AM, Miaohe Lin wrote:
>>>>>> 	/*
>>>>>> 	 * The memory barrier inside __SetPageUptodate makes sure that
>>>>>> 	 * preceding stores to the page contents become visible before
>>>>>> 	 * the set_pte_at() write.
>>>>>> 	 */
>>>>>> 	__SetPageUptodate(page);
>>>>> IIUC, the case here we should make sure others (CPUs) can see new page’s
>>>>> contents after they have saw PG_uptodate is set. I think commit 0ed361dec369
>>>>> can tell us more details.
>>>>>
>>>>> I also looked at commit 52f37629fd3c to see why we need a barrier before
>>>>> set_pte_at(), but I didn’t find any info to explain why. I guess we want
>>>>> to make sure the order between the page’s contents and subsequent memory
>>>>> accesses using the corresponding virtual address, do you agree with this?
>>>> This is my understanding also. Thanks.
>>> That's also my understanding. Thanks both.
>> I have an unclear thing (not related with this patch directly): Who is response
>> for the read barrier in the read side in this case?
>>
>> For SetPageUptodate, there are paring write/read memory barrier.
>>
> 
> I have the same question. So I think the example proposed by Miaohe is a little
> difference from the case (hugetlb_vmemmap) here.

Per my understanding, memory barrier in PageUptodate() is needed because user might access the
page contents using page_address() (corresponding pagetable entry already exists) soon. But for
the above proposed case, if user wants to access the page contents, the corresponding pagetable
should be visible first or the page contents can't be accessed. So there should be a data dependency
acting as memory barrier between pagetable entry is loaded and page contents is accessed.
Or am I miss something?

Thanks,
Miaohe Lin