Re: [PATCH v2] mm: align larger anonymous mappings on THP boundaries

From: Jiri Slaby
Date: Tue Jan 16 2024 - 07:09:49 EST


On 16. 01. 24, 12:53, Jiri Slaby wrote:
Hi,

On 09. 08. 22, 20:24, Rik van Riel wrote:
Align larger anonymous memory mappings on THP boundaries by
going through thp_get_unmapped_area if THPs are enabled for
the current process.

With this patch, larger anonymous mappings are now THP aligned.
When a malloc library allocates a 2MB or larger arena, that
arena can now be mapped with THPs right from the start, which
can result in better TLB hit rates and execution time.

This appears to break 32bit processes on x86_64 (at least). In particular, 32bit kernel or firefox builds in our build system.

Reverting this on top of 6.7 makes it work again.

Downstream report:
 https://bugzilla.suse.com/show_bug.cgi?id=1218841

So running:
pahole -J --btf_gen_floats -j --lang_exclude=rust --skip_encoding_btf_inconsistent_proto --btf_gen_optimized .tmp_vmlinux.btf

crashes or errors out with some random errors:
[182671] STRUCT idr's field 'idr_next' offset=128 bit_size=0 type=181346 Error emitting field

strace shows mmap() fails with ENOMEM right before the errors:
1223  mmap2(NULL, 5783552, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0 <unfinished ...>
...
1223  <... mmap2 resumed>)              = -1 ENOMEM (Cannot allocate memory)

Note the .tmp_vmlinux.btf above can be arbitrary, but likely large enough. For reference, one is available at:
https://decibel.fi.muni.cz/~xslaby/n/btf

Any ideas?

This works around the problem, of course (but is a band-aid, not a fix):

--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1829,7 +1829,7 @@ get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
*/
pgoff = 0;
get_area = shmem_get_unmapped_area;
- } else if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) {
+ } else if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && !in_32bit_syscall()) {
/* Ensures that larger anonymous mappings are THP aligned. */
get_area = thp_get_unmapped_area;
}


thp_get_unmapped_area() does not take care of the legacy stuff...

regards,
--
js
suse labs