linux-kernel - Re: [v3 PATCH 0/8] Make khugepaged collapse readonly FS THP more consistent

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

Message-ID: <627a71f8-e879-69a5-ceb3-fc8d29d2f7f1@suse.cz>
Date:   Mon, 9 May 2022 18:05:13 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Yang Shi <shy828301@...il.com>, kirill.shutemov@...ux.intel.com,
        linmiaohe@...wei.com, songliubraving@...com, riel@...riel.com,
        willy@...radead.org, ziy@...dia.com, tytso@....edu,
        akpm@...ux-foundation.org
Cc:     linux-mm@...ck.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [v3 PATCH 0/8] Make khugepaged collapse readonly FS THP more
 consistent

On 4/4/22 22:02, Yang Shi wrote:
>  include/linux/huge_mm.h        | 14 ++++++++++++
>  include/linux/khugepaged.h     | 59 ++++++++++++---------------------------------------
>  include/linux/sched/coredump.h |  3 ++-
>  kernel/fork.c                  |  4 +---
>  mm/huge_memory.c               | 15 ++++---------
>  mm/khugepaged.c                | 76 +++++++++++++++++++++++++++++++++++++-----------------------------
>  mm/mmap.c                      | 14 ++++++++----
>  mm/shmem.c                     | 12 -----------
>  8 files changed, 88 insertions(+), 109 deletions(-)

Resending my general feedback from mm-commits thread to include the
public ML's:

There's modestly less lines in the end, some duplicate code removed,
special casing in shmem.c removed, that's all good as it is. Also patch 8/8
become quite boring in v3, no need to change individual filesystems and also
no hook in fault path, just the common mmap path. So I would just handle
patch 6 differently as I just replied to it, and acked the rest.

That said it's still unfortunately rather a mess of functions that have
similar names. transhuge_vma_enabled(vma). hugepage_vma_check(vma),
transparent_hugepage_active(vma), transhuge_vma_suitable(vma, addr)?
So maybe still some space for further cleanups. But the series is fine as it
is so we don't have to wait for it now.

We could also consider that the tracking of which mm is to be scanned is
modelled after ksm which has its own madvise flag, but also no "always"
mode. What if for THP we only tracked actual THP madvised mm's, and in
"always" mode just scanned all vm's, would that allow ripping out some code
perhaps, while not adding too many unnecessary scans? If some processes are
being scanned without any effect, maybe track success separately, and scan
them less frequently etc. That could be ultimately more efficinet than
painfully tracking just *eligibility* for scanning in "always" mode?

Even more radical thing to consider (maybe that's a LSF/MM level topic, too
bad :) is that we scan pagetables in ksm, khugepaged, numa balancing, soon
in MGLRU, and I probably forgot something else. Maybe time to think about
unifying those scanners?