Re: about the ptep_set_access_flags() for hardware AF/DBM

[Catalin Marinas <catalin.marinas@arm.com>]

On Sun, Oct 27, 2019 at 05:56:24PM +0800, FF wrote:
> i see a patch, commit id: 66dbd6e61a52 "arm64: Implement ptep_set_access_flags() for hardware AF/DBM"
> in this patch, the author show a insteresting case of the racy of hardware AF/DBM.
> 
> Here is the scenario:
> A more complex situation is possible when all CPUs support hardware
>    AF/DBM:
> 
>    a) Initial state: shareable + writable vma and pte_none(pte)
>    b) Read fault taken by two threads of the same process on different
>       CPUs
>    c) CPU0 takes the mmap_sem and proceeds to handling the fault. It
>       eventually reaches do_set_pte() which sets a writable + clean pte.
>       CPU0 releases the mmap_sem
>    d) CPU1 acquires the mmap_sem and proceeds to handle_pte_fault(). The
>       pte entry it reads is present, writable and clean and it continues
>       to pte_mkyoung()
>    e) CPU1 calls ptep_set_access_flags()
> 
>    If between (d) and (e) the hardware (another CPU) updates the dirty
>    state (clears PTE_RDONLY), CPU1 will override the PTR_RDONLY bit
>    marking the entry clean again.
> 
> my question is:
> 1. in step a, it say, the initial state vma is : sharable + writable +
> pte_none. let suppose this is a anon mapping by mmap() API.

What I had in mind at the time was a file mapping rather than anonymous
one (vma_is_anonymous() is false for shared mappings).

> so the vma->vm_page_prot should be : VM_READ |  VM_WRITE | VM_SHARED
> in vm_get_page_prot(), it will change to pte attribute，in linux
> kernel it has a protection_map[] array. in that case, it should be
> __S011 (PAGE_SHARED). for PAGE_SHARED, the pte attribute will set
> PTE_WRITE，so PTE_DBM is set, but the PTE_RDONLY should be zero,
> right?

PAGE_SHARED is indeed writable but how it ends up in the pte depends on
the mapping. For a shared memory mapping, I think you do get a writable
entry on a read fault.

For file mappings, the writable attribute is cleared from vm_page_prot
via the vma_set_page_prot() function because vma_wants_writenotify() is
true. Filesystem normally want to track which pages have been dirtied to
write back.

> in step c, CPU0 trigger read fault and handle the page fault, it will
> call do_anonymous_page(), and using  system_zero_page. i don't what is
> a clean pte?  but currently, the  PTE_RDONLY is zero, it means this
> pte is writable.

Note that we can't invoke do_anonymous_page() for VM_SHARED mappings.
This is only for private mappings. If you look at mmap_region(), the vma
is not set up as anonymous if MAP_SHARED is given but as a shmem.

> when the CPU2 write this memory, it will update the dirty state like
> clear PTE_RDONLY, but my questions, the PTE_RDONLY is still zero, in
> step a~d, so why CPU1 will override RT_RDONLY bit and marking the
> entry clean again.

As I said above, this scenario is for shared file mappings where you do
get a PTE_RDONLY set for clean mappings.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel