On 02/07/2024 15:57, David Hildenbrand wrote:
On 02.07.24 16:46, Ryan Roberts wrote:
Since the introduction of mTHP, the docuementation has stated that khugepaged would be enabled when any mTHP size is enabled, and disabled when all mTHP sizes are disabled. There are 2 problems with this; 1. this is not what was implemented by the code and 2. this is not the desirable behavior.
Desirable behavior is for khugepaged to be enabled when any PMD-sized THP is enabled, anon or file. (Note that file THP is still controlled by the top-level control so we must always consider that, as well as the PMD-size mTHP control for anon). khugepaged only supports collapsing to PMD-sized THP so there is no value in enabling it when PMD-sized THP is disabled. So let's change the code and documentation to reflect this policy.
Further, per-size enabled control modification events were not previously forwarded to khugepaged to give it an opportunity to start or stop. Consequently the following was resulting in khugepaged eroneously not being activated:
echo never > /sys/kernel/mm/transparent_hugepage/enabled echo always > /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/enabled
Signed-off-by: Ryan Roberts ryan.roberts@arm.com Fixes: 3485b88390b0 ("mm: thp: introduce multi-size THP sysfs interface") Closes: https://lore.kernel.org/linux-mm/7a0bbe69-1e3d-4263-b206-da007791a5c4@redhat... Cc: stable@vger.kernel.org
Hi All,
Applies on top of today's mm-unstable (9bb8753acdd8). No regressions observed in mm selftests.
When fixing this I also noticed that khugepaged doesn't get (and never has been) activated/deactivated by `shmem_enabled=`. I'm not sure if khugepaged knows how to collapse shmem - perhaps it should be activated in this case?
Call me confused.
khugepaged_scan_mm_slot() and madvise_collapse() only all hpage_collapse_scan_file() with ... IS_ENABLED(CONFIG_SHMEM) ?
Looks like khugepaged_scan_mm_slot() was converted from:
if (shmem_file(vma->vm_file)) {
to:
if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
By 99cb0dbd47a15d395bf3faa78dc122bc5efe3fc0 which adds THP collapse support for non-shmem files. Clearly that looks wrong, but I guess never spotted in practice because noone disables shemem?
I guess madvise_collapse() was a copy/paste?
collapse_file() is only called by hpage_collapse_scan_file() ... and there we check "shmem_file(file)".
So why is the IS_ENABLED(CONFIG_SHMEM) check in there if collapse_file() seems to "collapse filemap/tmpfs/shmem pages into huge one".
Anyhow, we certainly can collapse shmem (that's how it all started IIUC).
Yes, thanks for pointing me at it. Should have just searched "shmem" in khugepaged.c :-/
Besides that, khugepaged only seems to collapse !shmem with VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
That makes sense. I guess I could use IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) to tighen the (non-shmem) file THP check in hugepage_pmd_enabled() (currently I'm unconditionally using the top-level enabled setting as a "is THP enabled for files" check).
But back to my original question, I think hugepage_pmd_enabled() should also be explicitly checking the appropriate shmem_enabled controls and ORing in the result? Otherwise in a situation where only shmem is THP enabled (and file/anon THP is disabled) khugepaged won't run.
The thp_vma_allowable_order() check tests if we are allowed to collapse a PMD_ORDER in that VMA.
I don't follow the relevance of this statement.