On Tue, Jun 07, 2022 at 07:37:10PM +1200, Barry Song wrote:

On Mon, Jun 6, 2022 at 9:25 PM Barry Song [off-list ref] wrote:

quoted

On Wed, May 18, 2022 at 4:49 PM Yu Zhao [off-list ref] wrote:

quoted

diff --git a/mm/rmap.c b/mm/rmap.c
index fedb82371efe..7cb7ef29088a 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c

@@ -73,6 +73,7 @@
 #include <linux/page_idle.h>
 #include <linux/memremap.h>
 #include <linux/userfaultfd_k.h>
+#include <linux/mm_inline.h>

 #include <asm/tlbflush.h>

@@ -821,6 +822,12 @@ static bool folio_referenced_one(struct folio *folio,
                }

                if (pvmw.pte) {
+                       if (lru_gen_enabled() && pte_young(*pvmw.pte) &&
+                           !(vma->vm_flags & (VM_SEQ_READ | VM_RAND_READ))) {
+                               lru_gen_look_around(&pvmw);
+                               referenced++;
+                       }
+
                        if (ptep_clear_flush_young_notify(vma, address,

Hello, Yu.
look_around() is calling ptep_test_and_clear_young(pvmw->vma, addr, pte + i)
only without flush and notify. for flush, there is a tlb operation for arm64:
static inline int ptep_clear_flush_young(struct vm_area_struct *vma,
                                         unsigned long address, pte_t *ptep)
{
        int young = ptep_test_and_clear_young(vma, address, ptep);

        if (young) {
                /*
                 * We can elide the trailing DSB here since the worst that can
                 * happen is that a CPU continues to use the young entry in its
                 * TLB and we mistakenly reclaim the associated page. The
                 * window for such an event is bounded by the next
                 * context-switch, which provides a DSB to complete the TLB
                 * invalidation.
                 */
                flush_tlb_page_nosync(vma, address);
        }

        return young;
}

Does it mean the current kernel is over cautious?  is it
safe to call ptep_test_and_clear_young() only?

I can't really explain why we are getting a random app/java vm crash in monkey
test by using ptep_test_and_clear_young() only in lru_gen_look_around() on an
armv8-a machine without hardware PTE young support.

Moving to  ptep_clear_flush_young() in look_around can make the random
hang disappear according to zhanyuan(Cc-ed).

On x86, ptep_clear_flush_young() is exactly ptep_test_and_clear_young()
after
 'commit b13b1d2d8692 ("x86/mm: In the PTE swapout page reclaim case clear
the accessed bit instead of flushing the TLB")'

But on arm64, they are different. according to Will's comments in this
thread which
tried to make arm64 same with x86,
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1793881.html

"
This is blindly copied from x86 and isn't true for us: we don't invalidate
the TLB on context switch. That means our window for keeping the stale
entries around is potentially much bigger and might not be a great idea.

If we roll a TLB invalidation routine without the trailing DSB, what sort of
performance does that get you?
"
We shouldn't think ptep_clear_flush_young() is safe enough in LRU to
clear PTE young? Any comments from Will?

Given that this issue is specific to the multi-gen LRU work, I think Yu is
the best person to comment. However, looking quickly at your analysis above,
I wonder if the code is relying on this sequence:


	ptep_test_and_clear_young(vma, address, ptep);
	ptep_clear_flush_young(vma, address, ptep);


to invalidate the TLB. On arm64, that won't be the case, as the invalidation
in ptep_clear_flush_young() is predicated on the pte being young (and this
patches the generic implementation in mm/pgtable-generic.c. In fact, that
second function call is always going to be a no-op unless the pte became
young again in the middle.

Will

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