Question about ptep_get_and_clear and TLB flush

Question about ptep_get_and_clear and TLB flush

[Joel Fernandes]

Hello friends,
I was trying to understand the safety of this piece of code in
move_ptes in mremap.c
Here we have some code that does this in a loop:

for (; old_addr < old_end; old_pte++, old_addr += PAGE_SIZE,
 new_pte++, new_addr += PAGE_SIZE) {
  if (pte_none(*old_pte))
       continue;
    pte = ptep_get_and_clear(mm, old_addr, old_pte);
    if (pte_present(pte) && pte_dirty(pte))
         force_flush = true;
    pte = move_pte(pte, new_vma->vm_page_prot, old_addr, new_addr);
    pte = move_soft_dirty_pte(pte);
    set_pte_at(mm, new_addr, new_pte, pte);
}

If I understand correctly, the ptep_get_and_clear is needed to
atomically get and clear the page table entry so that we do not miss
any other bits in PTE that may get set but have not been read, before
we clear it. Such as the dirty bit.

My question is, After the ptep_get_and_clear runs, what happens if
another CPU has a valid TLB entry for this old_addr and does a
memory-write *before* the TLBs are flushed. Would that not cause us to
lose the dirty bit? Once set_pte_at runs, it would be using the PTE
fetched earlier which did not have the dirty bit set. This seems wrong
to me. What do you think?

Thanks,
Joel

Re: Question about ptep_get_and_clear and TLB flush

[Joel Fernandes]

On Thu, Oct 18, 2018 at 11:04:02PM -0700, Joel Fernandes wrote:
> Hello friends,
> I was trying to understand the safety of this piece of code in
> move_ptes in mremap.c
> Here we have some code that does this in a loop:
> 
> for (; old_addr < old_end; old_pte++, old_addr += PAGE_SIZE,
>  new_pte++, new_addr += PAGE_SIZE) {
>   if (pte_none(*old_pte))
>        continue;
>     pte = ptep_get_and_clear(mm, old_addr, old_pte);
>     if (pte_present(pte) && pte_dirty(pte))
>          force_flush = true;
>     pte = move_pte(pte, new_vma->vm_page_prot, old_addr, new_addr);
>     pte = move_soft_dirty_pte(pte);
>     set_pte_at(mm, new_addr, new_pte, pte);
> }
> 
> If I understand correctly, the ptep_get_and_clear is needed to
> atomically get and clear the page table entry so that we do not miss
> any other bits in PTE that may get set but have not been read, before
> we clear it. Such as the dirty bit.
> 
> My question is, After the ptep_get_and_clear runs, what happens if
> another CPU has a valid TLB entry for this old_addr and does a
> memory-write *before* the TLBs are flushed. Would that not cause us to
> lose the dirty bit? Once set_pte_at runs, it would be using the PTE
> fetched earlier which did not have the dirty bit set. This seems wrong
> to me. What do you think?

Just for completeness of discussion, I'd like to say Ramon kindly helped me
understand this by explaining to me that the stores would not be affected by
stale TLB entries, and they would end up doing an actual page-table walk so
the issue I was hypothesizing would not arise.

thanks,

- Joel

Re: Question about ptep_get_and_clear and TLB flush

[Jerome Glisse]

On Thu, Oct 18, 2018 at 11:04:02PM -0700, Joel Fernandes wrote:
> Hello friends,
> I was trying to understand the safety of this piece of code in
> move_ptes in mremap.c
> Here we have some code that does this in a loop:
> 
> for (; old_addr < old_end; old_pte++, old_addr += PAGE_SIZE,
>  new_pte++, new_addr += PAGE_SIZE) {
>   if (pte_none(*old_pte))
>        continue;
>     pte = ptep_get_and_clear(mm, old_addr, old_pte);
>     if (pte_present(pte) && pte_dirty(pte))
>          force_flush = true;
>     pte = move_pte(pte, new_vma->vm_page_prot, old_addr, new_addr);
>     pte = move_soft_dirty_pte(pte);
>     set_pte_at(mm, new_addr, new_pte, pte);
> }
> 
> If I understand correctly, the ptep_get_and_clear is needed to
> atomically get and clear the page table entry so that we do not miss
> any other bits in PTE that may get set but have not been read, before
> we clear it. Such as the dirty bit.
> 
> My question is, After the ptep_get_and_clear runs, what happens if
> another CPU has a valid TLB entry for this old_addr and does a
> memory-write *before* the TLBs are flushed. Would that not cause us to
> lose the dirty bit? Once set_pte_at runs, it would be using the PTE
> fetched earlier which did not have the dirty bit set. This seems wrong
> to me. What do you think?
> 

https://yarchive.net/comp/linux/x86_tlb.html