linux-kernel - [PATCH] arm64: mm: drop tlb flush operation when clearing the access bit

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-next>] [day] [month] [year] [list]

Message-Id: <ae3115778a3fa10ec77152e18beed54fafe0f6e7.1698151516.git.baolin.wang@linux.alibaba.com>
Date:   Tue, 24 Oct 2023 20:56:35 +0800
From:   Baolin Wang <baolin.wang@...ux.alibaba.com>
To:     catalin.marinas@....com, will@...nel.org
Cc:     akpm@...ux-foundation.org, v-songbaohua@...o.com,
        yuzhao@...gle.com, baolin.wang@...ux.alibaba.com,
        linux-mm@...ck.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH] arm64: mm: drop tlb flush operation when clearing the access bit

Now ptep_clear_flush_young() is only called by folio_referenced() to
check if the folio was referenced, and now it will call a tlb flush on
ARM64 architecture. However the tlb flush can be expensive on ARM64
servers, especially for the systems with a large CPU numbers.

Similar to the x86 architecture, below comments also apply equally to
ARM64 architecture. So we can drop the tlb flush operation in
ptep_clear_flush_young() on ARM64 architecture to improve the performance.
"
/* Clearing the accessed bit without a TLB flush
 * doesn't cause data corruption. [ It could cause incorrect
 * page aging and the (mistaken) reclaim of hot pages, but the
 * chance of that should be relatively low. ]
 *
 * So as a performance optimization don't flush the TLB when
 * clearing the accessed bit, it will eventually be flushed by
 * a context switch or a VM operation anyway. [ In the rare
 * event of it not getting flushed for a long time the delay
 * shouldn't really matter because there's no real memory
 * pressure for swapout to react to. ]
 */
"
Running the thpscale to show some obvious improvements for compaction
latency with this patch:
                             base                   patched
Amean     fault-both-1      1093.19 (   0.00%)     1084.57 *   0.79%*
Amean     fault-both-3      2566.22 (   0.00%)     2228.45 *  13.16%*
Amean     fault-both-5      3591.22 (   0.00%)     3146.73 *  12.38%*
Amean     fault-both-7      4157.26 (   0.00%)     4113.67 *   1.05%*
Amean     fault-both-12     6184.79 (   0.00%)     5218.70 *  15.62%*
Amean     fault-both-18     9103.70 (   0.00%)     7739.71 *  14.98%*
Amean     fault-both-24    12341.73 (   0.00%)    10684.23 *  13.43%*
Amean     fault-both-30    15519.00 (   0.00%)    13695.14 *  11.75%*
Amean     fault-both-32    16189.15 (   0.00%)    14365.73 *  11.26%*
                       base       patched
Duration User         167.78      161.03
Duration System      1836.66     1673.01
Duration Elapsed     2074.58     2059.75

Barry Song submitted a similar patch [1] before, that replaces the
ptep_clear_flush_young_notify() with ptep_clear_young_notify() in
folio_referenced_one(). However, I'm not sure if removing the tlb flush
operation is applicable to every architecture in kernel, so dropping
the tlb flush for ARM64 seems a sensible change.

Note: I am okay for both approach, if someone can help to ensure that
all architectures do not need the tlb flush when clearing the accessed
bit, then I also think Barry's patch is better (hope Barry can resend
his patch).

[1] https://lore.kernel.org/lkml/20220617070555.344368-1-21cnbao@gmail.com/
Signed-off-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
---
 arch/arm64/include/asm/pgtable.h | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 0bd18de9fd97..2979d796ba9d 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -905,21 +905,22 @@ static inline int ptep_test_and_clear_young(struct vm_area_struct *vma,
 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;
+	/*
+	 * This comment is borrowed from x86, but applies equally to ARM64:
+	 *
+	 * Clearing the accessed bit without a TLB flush doesn't cause
+	 * data corruption. [ It could cause incorrect page aging and
+	 * the (mistaken) reclaim of hot pages, but the chance of that
+	 * should be relatively low. ]
+	 *
+	 * So as a performance optimization don't flush the TLB when
+	 * clearing the accessed bit, it will eventually be flushed by
+	 * a context switch or a VM operation anyway. [ In the rare
+	 * event of it not getting flushed for a long time the delay
+	 * shouldn't really matter because there's no real memory
+	 * pressure for swapout to react to. ]
+	 */
+	return ptep_test_and_clear_young(vma, address, ptep);
 }
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-- 
2.39.3