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

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

[Rik van Riel]

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 checking in /proc/PID/maps, but only when THP is enabled
for that process.

Signed-off-by: Rik van Riel <riel@surriel.com>
---
 mm/mmap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/mmap.c b/mm/mmap.c
index c035020d0c89..3a9d19cec690 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2229,6 +2229,9 @@ get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
 		 */
 		pgoff = 0;
 		get_area = shmem_get_unmapped_area;
+	} else if (test_bit(MMF_VM_HUGEPAGE, &current->mm->flags)) {
+		/* Ensures that larger anonymous mappings are THP aligned. */
+		get_area = thp_get_unmapped_area;
 	}
 
 	addr = get_area(file, addr, len, pgoff, flags);
-- 
2.37.1

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

[Andrew Morton]

On Mon, 8 Aug 2022 16:46:58 -0400 Rik van Riel <riel@surriel.com> 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 checking in /proc/PID/maps, but only when THP is enabled
> for that process.
> 

What are the runtime effects of this change?

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

[Rik van Riel]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On Mon, 2022-08-08 at 15:09 -0700, Andrew Morton wrote:
> On Mon, 8 Aug 2022 16:46:58 -0400 Rik van Riel <riel@surriel.com>
> 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 checking in /proc/PID/maps, but only when THP is enabled
> > for that process.
> > 
> 
> What are the runtime effects of this change?

The runtime effect is that when a malloc library
allocates an arena that is a multiple of 2MB in size,
the entire arena can be mapped using THPs, resulting in
a lower TLB miss rate.

-- 
All Rights Reversed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Yang Shi]

On Mon, Aug 8, 2022 at 1:47 PM Rik van Riel <riel@surriel.com> 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 checking in /proc/PID/maps, but only when THP is enabled
> for that process.
>
> Signed-off-by: Rik van Riel <riel@surriel.com>
> ---
>  mm/mmap.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index c035020d0c89..3a9d19cec690 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2229,6 +2229,9 @@ get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
>                  */
>                 pgoff = 0;
>                 get_area = shmem_get_unmapped_area;
> +       } else if (test_bit(MMF_VM_HUGEPAGE, &current->mm->flags)) {

There is a kind of chicken & egg problem here, MMF_VM_HUGEPAGE is not
going to be set if there is no THP eligible vma even though THP is
enabled.

> +               /* Ensures that larger anonymous mappings are THP aligned. */
> +               get_area = thp_get_unmapped_area;
>         }
>
>         addr = get_area(file, addr, len, pgoff, flags);
> --
> 2.37.1
>
>

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

[Rik van Riel]

[-- Attachment #1: Type: text/plain, Size: 1735 bytes --]

On Tue, 2022-08-09 at 10:16 -0700, Yang Shi wrote:
> On Mon, Aug 8, 2022 at 1:47 PM Rik van Riel <riel@surriel.com> 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 checking in /proc/PID/maps, but only when THP is enabled
> > for that process.
> > 
> > Signed-off-by: Rik van Riel <riel@surriel.com>
> > ---
> >  mm/mmap.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > index c035020d0c89..3a9d19cec690 100644
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -2229,6 +2229,9 @@ get_unmapped_area(struct file *file, unsigned
> > long addr, unsigned long len,
> >                  */
> >                 pgoff = 0;
> >                 get_area = shmem_get_unmapped_area;
> > +       } else if (test_bit(MMF_VM_HUGEPAGE, &current->mm->flags))
> > {
> 
> There is a kind of chicken & egg problem here, MMF_VM_HUGEPAGE is not
> going to be set if there is no THP eligible vma even though THP is
> enabled.

With THP enabled=always, we should always end up with getting
MMF_VM_HUGEPAGE set after the first VMA, but you are right that:

1) The first VMA might not get the proper alignment, and
2) with THP enabled=madvise we might get a whole bunch of
   mis-aligned VMAs on which a subsequent madvise would
   not create the desired THPs.

You are right that that condition should just be removed,
and we should decide alignment based on VMA size alone.

Let me send a v2.

-- 
All Rights Reversed.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]