linux-kernel - Re: [PATCH 4/6] mm: hugetlb_vmemmap: add missing smp_wmb() before set_pte

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 18 Aug 2022 15:52:02 +0800
From:   Miaohe Lin <linmiaohe@...wei.com>
To:     Muchun Song <muchun.song@...ux.dev>,
        "Yin, Fengwei" <fengwei.yin@...el.com>
CC:     Andrew Morton <akpm@...ux-foundation.org>,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Muchun Song <songmuchun@...edance.com>,
        Linux MM <linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/6] mm: hugetlb_vmemmap: add missing smp_wmb() before
 set_pte_at()

On 2022/8/18 10:47, Muchun Song wrote:
> 
> 
>> On Aug 18, 2022, at 10:00, Yin, Fengwei <fengwei.yin@...el.com> 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