lkml.org 
[lkml]   [2024]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [RFC PATCH v1 3/4] mm/memory: Use ptep_get_lockless_norecency() for orig_pte
From
On 15.02.24 13:17, Ryan Roberts wrote:
> Let's convert handle_pte_fault()'s use of ptep_get_lockless() to
> ptep_get_lockless_norecency() to save orig_pte.
>
> There are a number of places that follow this model:
>
> orig_pte = ptep_get_lockless(ptep)
> ...
> <lock>
> if (!pte_same(orig_pte, ptep_get(ptep)))
> // RACE!
> ...
> <unlock>
>
> So we need to be careful to convert all of those to use
> pte_same_norecency() so that the access and dirty bits are excluded from
> the comparison.
>
> Additionally there are a couple of places that genuinely rely on the
> access and dirty bits of orig_pte, but with some careful refactoring, we
> can use ptep_get() once we are holding the lock to achieve equivalent
> logic.

We really should document that changed behavior somewhere where it can
be easily found: that orig_pte might have incomplete/stale
accessed/dirty information.


> @@ -5343,7 +5356,7 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
> vmf->address, &vmf->ptl);
> if (unlikely(!vmf->pte))
> return 0;
> - vmf->orig_pte = ptep_get_lockless(vmf->pte);
> + vmf->orig_pte = ptep_get_lockless_norecency(vmf->pte);
> vmf->flags |= FAULT_FLAG_ORIG_PTE_VALID;
>
> if (pte_none(vmf->orig_pte)) {
> @@ -5363,7 +5376,7 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
>
> spin_lock(vmf->ptl);
> entry = vmf->orig_pte;
> - if (unlikely(!pte_same(ptep_get(vmf->pte), entry))) {
> + if (unlikely(!pte_same_norecency(ptep_get(vmf->pte), entry))) {
> update_mmu_tlb(vmf->vma, vmf->address, vmf->pte);
> goto unlock;

I was wondering about the following:

Assume the PTE is not dirty.

Thread 1 does

vmf->orig_pte = ptep_get_lockless_norecency(vmf->pte)
/* not dirty */

/* Now, thread 2 ends up setting the PTE dirty under PT lock. */

spin_lock(vmf->ptl);
entry = vmf->orig_pte;
if (unlikely(!pte_same(ptep_get(vmf->pte), entry))) {
...
}
..
entry = pte_mkyoung(entry);
if (ptep_set_access_flags(vmf->vma, ...)
..
pte_unmap_unlock(vmf->pte, vmf->ptl);


Generic ptep_set_access_flags() will do another pte_same() check and
realize "hey, there was a change!" let's update the PTE!

set_pte_at(vma->vm_mm, address, ptep, entry);

would overwrite the dirty bit set by thread 2.

--
Cheers,

David / dhildenb


\
 
 \ /
  Last update: 2024-05-27 16:09    [W:0.203 / U:0.508 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site