| 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
| ||
|
Message-ID: <CAK1f24=3LmQMzgosB0o9OKV-P8f6cPO2ZCPBA64QXQqZYPk=ww@mail.gmail.com>
Date: Wed, 15 Jan 2025 13:01:16 +0800
From: Lance Yang <ioworker0@...il.com>
To: Barry Song <21cnbao@...il.com>
Cc: akpm@...ux-foundation.org, linux-mm@...ck.org,
baolin.wang@...ux.alibaba.com, chrisl@...nel.org, david@...hat.com,
kasong@...cent.com, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
lorenzo.stoakes@...cle.com, ryan.roberts@....com, v-songbaohua@...o.com,
x86@...nel.org, ying.huang@...el.com, zhengtangquan@...o.com
Subject: Re: [PATCH v3 4/4] mm: Avoid splitting pmd for lazyfree pmd-mapped
THP in try_to_unmap
On Wed, Jan 15, 2025 at 11:38 AM Barry Song <21cnbao@...il.com> wrote:
>
> From: Barry Song <v-songbaohua@...o.com>
>
> The try_to_unmap_one() function currently handles PMD-mapped THPs
> inefficiently. It first splits the PMD into PTEs, copies the dirty
> state from the PMD to the PTEs, iterates over the PTEs to locate
> the dirty state, and then marks the THP as swap-backed. This process
> involves unnecessary PMD splitting and redundant iteration. Instead,
> this functionality can be efficiently managed in
> __discard_anon_folio_pmd_locked(), avoiding the extra steps and
> improving performance.
>
> The following microbenchmark redirties folios after invoking MADV_FREE,
> then measures the time taken to perform memory reclamation (actually
> set those folios swapbacked again) on the redirtied folios.
>
> #include <stdio.h>
> #include <sys/mman.h>
> #include <string.h>
> #include <time.h>
>
> #define SIZE 128*1024*1024 // 128 MB
>
> int main(int argc, char *argv[])
> {
> while(1) {
> volatile int *p = mmap(0, SIZE, PROT_READ | PROT_WRITE,
> MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>
> memset((void *)p, 1, SIZE);
> madvise((void *)p, SIZE, MADV_FREE);
> /* redirty after MADV_FREE */
> memset((void *)p, 1, SIZE);
>
> clock_t start_time = clock();
> madvise((void *)p, SIZE, MADV_PAGEOUT);
> clock_t end_time = clock();
>
> double elapsed_time = (double)(end_time - start_time) / CLOCKS_PER_SEC;
> printf("Time taken by reclamation: %f seconds\n", elapsed_time);
>
> munmap((void *)p, SIZE);
> }
> return 0;
> }
>
> Testing results are as below,
> w/o patch:
> ~ # ./a.out
> Time taken by reclamation: 0.007300 seconds
> Time taken by reclamation: 0.007226 seconds
> Time taken by reclamation: 0.007295 seconds
> Time taken by reclamation: 0.007731 seconds
> Time taken by reclamation: 0.007134 seconds
> Time taken by reclamation: 0.007285 seconds
> Time taken by reclamation: 0.007720 seconds
> Time taken by reclamation: 0.007128 seconds
> Time taken by reclamation: 0.007710 seconds
> Time taken by reclamation: 0.007712 seconds
> Time taken by reclamation: 0.007236 seconds
> Time taken by reclamation: 0.007690 seconds
> Time taken by reclamation: 0.007174 seconds
> Time taken by reclamation: 0.007670 seconds
> Time taken by reclamation: 0.007169 seconds
> Time taken by reclamation: 0.007305 seconds
> Time taken by reclamation: 0.007432 seconds
> Time taken by reclamation: 0.007158 seconds
> Time taken by reclamation: 0.007133 seconds
> …
>
> w/ patch
>
> ~ # ./a.out
> Time taken by reclamation: 0.002124 seconds
> Time taken by reclamation: 0.002116 seconds
> Time taken by reclamation: 0.002150 seconds
> Time taken by reclamation: 0.002261 seconds
> Time taken by reclamation: 0.002137 seconds
> Time taken by reclamation: 0.002173 seconds
> Time taken by reclamation: 0.002063 seconds
> Time taken by reclamation: 0.002088 seconds
> Time taken by reclamation: 0.002169 seconds
> Time taken by reclamation: 0.002124 seconds
> Time taken by reclamation: 0.002111 seconds
> Time taken by reclamation: 0.002224 seconds
> Time taken by reclamation: 0.002297 seconds
> Time taken by reclamation: 0.002260 seconds
> Time taken by reclamation: 0.002246 seconds
> Time taken by reclamation: 0.002272 seconds
> Time taken by reclamation: 0.002277 seconds
> Time taken by reclamation: 0.002462 seconds
> …
>
> This patch significantly speeds up try_to_unmap_one() by allowing it
> to skip redirtied THPs without splitting the PMD.
>
> Suggested-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
> Suggested-by: Lance Yang <ioworker0@...il.com>
> Signed-off-by: Barry Song <v-songbaohua@...o.com>
> ---
> mm/huge_memory.c | 24 +++++++++++++++++-------
> mm/rmap.c | 13 ++++++++++---
> 2 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 3d3ebdc002d5..47cc8c3f8f80 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3070,8 +3070,12 @@ static bool __discard_anon_folio_pmd_locked(struct vm_area_struct *vma,
> int ref_count, map_count;
> pmd_t orig_pmd = *pmdp;
>
> - if (folio_test_dirty(folio) || pmd_dirty(orig_pmd))
> + if (pmd_dirty(orig_pmd))
> + folio_set_dirty(folio);
> + if (folio_test_dirty(folio) && !(vma->vm_flags & VM_DROPPABLE)) {
> + folio_set_swapbacked(folio);
> return false;
> + }
If either the PMD or the folio is dirty, should we just return false right away,
regardless of VM_DROPPABLE? There’s no need to proceed further in that
case, IMHO ;)
Thanks,
Lance
>
> orig_pmd = pmdp_huge_clear_flush(vma, addr, pmdp);
>
> @@ -3098,8 +3102,15 @@ static bool __discard_anon_folio_pmd_locked(struct vm_area_struct *vma,
> *
> * The only folio refs must be one from isolation plus the rmap(s).
> */
> - if (folio_test_dirty(folio) || pmd_dirty(orig_pmd) ||
> - ref_count != map_count + 1) {
> + if (pmd_dirty(orig_pmd))
> + folio_set_dirty(folio);
> + if (folio_test_dirty(folio) && !(vma->vm_flags & VM_DROPPABLE)) {
> + folio_set_swapbacked(folio);
> + set_pmd_at(mm, addr, pmdp, orig_pmd);
> + return false;
> + }
> +
> + if (ref_count != map_count + 1) {
> set_pmd_at(mm, addr, pmdp, orig_pmd);
> return false;
> }
> @@ -3119,12 +3130,11 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
> {
> VM_WARN_ON_FOLIO(!folio_test_pmd_mappable(folio), folio);
> VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio);
> + VM_WARN_ON_FOLIO(!folio_test_anon(folio), folio);
> + VM_WARN_ON_FOLIO(folio_test_swapbacked(folio), folio);
> VM_WARN_ON_ONCE(!IS_ALIGNED(addr, HPAGE_PMD_SIZE));
>
> - if (folio_test_anon(folio) && !folio_test_swapbacked(folio))
> - return __discard_anon_folio_pmd_locked(vma, addr, pmdp, folio);
> -
> - return false;
> + return __discard_anon_folio_pmd_locked(vma, addr, pmdp, folio);
> }
>
> static void remap_page(struct folio *folio, unsigned long nr, int flags)
> diff --git a/mm/rmap.c b/mm/rmap.c
> index be1978d2712d..a859c399ec7c 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1724,9 +1724,16 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> }
>
> if (!pvmw.pte) {
> - if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd,
> - folio))
> - goto walk_done;
> + if (folio_test_anon(folio) && !folio_test_swapbacked(folio)) {
> + if (unmap_huge_pmd_locked(vma, pvmw.address, pvmw.pmd, folio))
> + goto walk_done;
> + /*
> + * unmap_huge_pmd_locked has either already marked
> + * the folio as swap-backed or decided to retain it
> + * due to GUP or speculative references.
> + */
> + goto walk_abort;
> + }
>
> if (flags & TTU_SPLIT_HUGE_PMD) {
> /*
> --
> 2.39.3 (Apple Git-146)
>
Powered by blists - more mailing lists