linux-kernel - Re: [PATCH v2] mm: Respect mmap hint address when aligning for THP

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] [day] [month] [year] [list]
Message-ID: <CAC_TJvfK8Rtf2tAtG-1QMK_SCjUAQeGZWkCR-aA1s69Pae7vEQ@mail.gmail.com>
Date: Wed, 20 Nov 2024 10:11:31 -0800
From: Kalesh Singh <kaleshsingh@...gle.com>
To: David Hildenbrand <david@...hat.com>
Cc: kernel-team@...roid.com, android-mm@...gle.com, 
	Andrew Morton <akpm@...ux-foundation.org>, Vlastimil Babka <vbabka@...e.cz>, 
	Yang Shi <yang@...amperecomputing.com>, Rik van Riel <riel@...riel.com>, 
	Ryan Roberts <ryan.roberts@....com>, Suren Baghdasaryan <surenb@...gle.com>, 
	Minchan Kim <minchan@...nel.org>, Hans Boehm <hboehm@...gle.com>, 
	Lokesh Gidra <lokeshgidra@...gle.com>, stable@...r.kernel.org, 
	"Liam R. Howlett" <Liam.Howlett@...cle.com>, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, 
	Jann Horn <jannh@...gle.com>, Yang Shi <shy828301@...il.com>, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] mm: Respect mmap hint address when aligning for THP

On Tue, Nov 19, 2024 at 6:35 AM David Hildenbrand <david@...hat.com> wrote:
>
> On 18.11.24 22:46, Kalesh Singh wrote:
> > Commit efa7df3e3bb5 ("mm: align larger anonymous mappings on THP
> > boundaries") updated __get_unmapped_area() to align the start address
> > for the VMA to a PMD boundary if CONFIG_TRANSPARENT_HUGEPAGE=y.
> >
> > It does this by effectively looking up a region that is of size,
> > request_size + PMD_SIZE, and aligning up the start to a PMD boundary.
> >
> > Commit 4ef9ad19e176 ("mm: huge_memory: don't force huge page alignment
> > on 32 bit") opted out of this for 32bit due to regressions in mmap base
> > randomization.
> >
> > Commit d4148aeab412 ("mm, mmap: limit THP alignment of anonymous
> > mappings to PMD-aligned sizes") restricted this to only mmap sizes that
> > are multiples of the PMD_SIZE due to reported regressions in some
> > performance benchmarks -- which seemed mostly due to the reduced spatial
> > locality of related mappings due to the forced PMD-alignment.
> >
> > Another unintended side effect has emerged: When a user specifies an mmap
> > hint address, the THP alignment logic modifies the behavior, potentially
> > ignoring the hint even if a sufficiently large gap exists at the requested
> > hint location.
> >
> > Example Scenario:
> >
> > Consider the following simplified virtual address (VA) space:
> >
> >      ...
> >
> >      0x200000-0x400000 --- VMA A
> >      0x400000-0x600000 --- Hole
> >      0x600000-0x800000 --- VMA B
> >
> >      ...
> >
> > A call to mmap() with hint=0x400000 and len=0x200000 behaves differently:
> >
> >    - Before THP alignment: The requested region (size 0x200000) fits into
> >      the gap at 0x400000, so the hint is respected.
> >
> >    - After alignment: The logic searches for a region of size
> >      0x400000 (len + PMD_SIZE) starting at 0x400000.
> >      This search fails due to the mapping at 0x600000 (VMA B), and the hint
> >      is ignored, falling back to arch_get_unmapped_area[_topdown]().
> >
> > In general the hint is effectively ignored, if there is any
> > existing mapping in the below range:
> >
> >       [mmap_hint + mmap_size, mmap_hint + mmap_size + PMD_SIZE)
> >
> > This changes the semantics of mmap hint; from ""Respect the hint if a
> > sufficiently large gap exists at the requested location" to "Respect the
> > hint only if an additional PMD-sized gap exists beyond the requested size".
> >
> > This has performance implications for allocators that allocate their heap
> > using mmap but try to keep it "as contiguous as possible" by using the
> > end of the exisiting heap as the address hint. With the new behavior
> > it's more likely to get a much less contiguous heap, adding extra
> > fragmentation and performance overhead.
> >
> > To restore the expected behavior; don't use thp_get_unmapped_area_vmflags()
> > when the user provided a hint address, for anonymous mappings.
> >
> > Note: As, Yang Shi, pointed out: the issue still remains for filesystems
> > which are using thp_get_unmapped_area() for their get_unmapped_area() op.
> > It is unclear what worklaods will regress for if we ignore THP alignment
> > when the hint address is provided for such file backed mappings -- so this
> > fix will be handled separately.
> >
> > Cc: Andrew Morton <akpm@...ux-foundation.org>
> > Cc: Vlastimil Babka <vbabka@...e.cz>
> > Cc: Yang Shi <yang@...amperecomputing.com>
> > Cc: Rik van Riel <riel@...riel.com>
> > Cc: Ryan Roberts <ryan.roberts@....com>
> > Cc: Suren Baghdasaryan <surenb@...gle.com>
> > Cc: Minchan Kim <minchan@...nel.org>
> > Cc: Hans Boehm <hboehm@...gle.com>
> > Cc: Lokesh Gidra <lokeshgidra@...gle.com>
> > Cc: <stable@...r.kernel.org>
> > Fixes: efa7df3e3bb5 ("mm: align larger anonymous mappings on THP boundaries")
> > Signed-off-by: Kalesh Singh <kaleshsingh@...gle.com>
> > Reviewed-by: Rik van Riel <riel@...riel.com>
> > Reviewed-by: Vlastimil Babka <vbabka@...e.cz>
> > ---
>
> LGTM. Hopefully that's the end of this story :)
>
> Reviewed-by: David Hildenbrand <david@...hat.com>

Thanks David.

Andrew, when you have a chance, could you take this for mm fixes please.

Thanks,
Kalesh

>
> --
> Cheers,
>
> David / dhildenb
>