mm/hugetlb: align cma on allocation order, not demotion order

[PATCH] mm/hugetlb: align cma on allocation order, not demotion order

Posted by Frank van der Linden 1 year, 7 months ago

Align the CMA area for hugetlb gigantic pages to their size, not the
size that they can be demoted to. Otherwise there might be misaligned
sections at the start and end of the CMA area that will never be used
for hugetlb page allocations.

Signed-off-by: Frank van der Linden <fvdl@google.com>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Fixes: a01f43901cfb ("hugetlb: be sure to free demoted CMA pages to CMA")
---
 mm/hugetlb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 5dc3f5ea3a2e..cfe7b025c576 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -7794,7 +7794,7 @@ void __init hugetlb_cma_reserve(int order)
 		 * huge page demotion.
 		 */
 		res = cma_declare_contiguous_nid(0, size, 0,
-					PAGE_SIZE << HUGETLB_PAGE_ORDER,
+					PAGE_SIZE << order,
 					HUGETLB_PAGE_ORDER, false, name,
 					&hugetlb_cma[nid], nid);
 		if (res) {
-- 
2.45.0.rc0.197.gbae5840b3b-goog

Re: [PATCH] mm/hugetlb: align cma on allocation order, not demotion order

Posted by Oscar Salvador 1 year, 7 months ago

On Tue, Apr 30, 2024 at 04:14:37PM +0000, Frank van der Linden wrote:
> Align the CMA area for hugetlb gigantic pages to their size, not the
> size that they can be demoted to. Otherwise there might be misaligned
> sections at the start and end of the CMA area that will never be used
> for hugetlb page allocations.
> 
> Signed-off-by: Frank van der Linden <fvdl@google.com>
> Cc: Roman Gushchin <roman.gushchin@linux.dev>
> Fixes: a01f43901cfb ("hugetlb: be sure to free demoted CMA pages to CMA")

Reviewed-by: Oscar Salvador <osalvador@suse.de>


-- 
Oscar Salvador
SUSE Labs

Re: [PATCH] mm/hugetlb: align cma on allocation order, not demotion order

Posted by Roman Gushchin 1 year, 7 months ago

On Tue, Apr 30, 2024 at 04:14:37PM +0000, Frank van der Linden wrote:
> Align the CMA area for hugetlb gigantic pages to their size, not the
> size that they can be demoted to. Otherwise there might be misaligned
> sections at the start and end of the CMA area that will never be used
> for hugetlb page allocations.
> 
> Signed-off-by: Frank van der Linden <fvdl@google.com>
> Cc: Roman Gushchin <roman.gushchin@linux.dev>
> Fixes: a01f43901cfb ("hugetlb: be sure to free demoted CMA pages to CMA")

Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>

Thanks!

Re: [PATCH] mm/hugetlb: align cma on allocation order, not demotion order

Posted by David Hildenbrand 1 year, 7 months ago

On 30.04.24 18:14, Frank van der Linden wrote:
> Align the CMA area for hugetlb gigantic pages to their size, not the
> size that they can be demoted to. Otherwise there might be misaligned
> sections at the start and end of the CMA area that will never be used
> for hugetlb page allocations.
> 
> Signed-off-by: Frank van der Linden <fvdl@google.com>
> Cc: Roman Gushchin <roman.gushchin@linux.dev>
> Fixes: a01f43901cfb ("hugetlb: be sure to free demoted CMA pages to CMA")
> ---
>   mm/hugetlb.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 5dc3f5ea3a2e..cfe7b025c576 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -7794,7 +7794,7 @@ void __init hugetlb_cma_reserve(int order)
>   		 * huge page demotion.
>   		 */
>   		res = cma_declare_contiguous_nid(0, size, 0,
> -					PAGE_SIZE << HUGETLB_PAGE_ORDER,
> +					PAGE_SIZE << order,
>   					HUGETLB_PAGE_ORDER, false, name,
>   					&hugetlb_cma[nid], nid);
>   		if (res) {

I was wondering how that worked when reviewing your other patch. 
Wondering why we never got a BUG report, maybe we were always lucky 
about the alignment we actually got?

We round up size to PAGE_SIZE << order, so that's the alignment we need.

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH] mm/hugetlb: align cma on allocation order, not demotion order

Posted by Frank van der Linden 1 year, 7 months ago

On Thu, May 2, 2024 at 6:15 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 30.04.24 18:14, Frank van der Linden wrote:
> > Align the CMA area for hugetlb gigantic pages to their size, not the
> > size that they can be demoted to. Otherwise there might be misaligned
> > sections at the start and end of the CMA area that will never be used
> > for hugetlb page allocations.
> >
> > Signed-off-by: Frank van der Linden <fvdl@google.com>
> > Cc: Roman Gushchin <roman.gushchin@linux.dev>
> > Fixes: a01f43901cfb ("hugetlb: be sure to free demoted CMA pages to CMA")
> > ---
> >   mm/hugetlb.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 5dc3f5ea3a2e..cfe7b025c576 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -7794,7 +7794,7 @@ void __init hugetlb_cma_reserve(int order)
> >                * huge page demotion.
> >                */
> >               res = cma_declare_contiguous_nid(0, size, 0,
> > -                                     PAGE_SIZE << HUGETLB_PAGE_ORDER,
> > +                                     PAGE_SIZE << order,
> >                                       HUGETLB_PAGE_ORDER, false, name,
> >                                       &hugetlb_cma[nid], nid);
> >               if (res) {
>
> I was wondering how that worked when reviewing your other patch.
> Wondering why we never got a BUG report, maybe we were always lucky
> about the alignment we actually got?

I think this issue was probably masked by the hugetlb allocator
falling back to direct alloc_contig_pages allocation if cma_alloc
fails. So if you're not under memory pressure, the failure to allocate
from the misaligned areas might not have been noticed.

I noticed it, because I was working with change I made: a flag that
prevents the fallback to straight alloc_contig_pages, as that behavior
may not be desired - you don't want to potentially eat in to
non-movable space that the kernel needs, it might be better to fail if
there's no CMA available.
>
> We round up size to PAGE_SIZE << order, so that's the alignment we need.
>
> Reviewed-by: David Hildenbrand <david@redhat.com>

Thanks!

Re: [PATCH] mm/hugetlb: align cma on allocation order, not demotion order

Posted by Frank van der Linden 1 year, 7 months ago

Note that this applies on top of:

https://lore.kernel.org/lkml/20240404162515.527802-2-fvdl@google.com/

..which is in mm-stable right now. It's not a fixup of that patch,
though, it's a separate issue. Although they could be combined in to a
"fix arguments to cma_declare_contiguous_nid" commit.

On Tue, Apr 30, 2024 at 9:14 AM Frank van der Linden <fvdl@google.com> wrote:
>
> Align the CMA area for hugetlb gigantic pages to their size, not the
> size that they can be demoted to. Otherwise there might be misaligned
> sections at the start and end of the CMA area that will never be used
> for hugetlb page allocations.
>
> Signed-off-by: Frank van der Linden <fvdl@google.com>
> Cc: Roman Gushchin <roman.gushchin@linux.dev>
> Fixes: a01f43901cfb ("hugetlb: be sure to free demoted CMA pages to CMA")
> ---
>  mm/hugetlb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 5dc3f5ea3a2e..cfe7b025c576 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -7794,7 +7794,7 @@ void __init hugetlb_cma_reserve(int order)
>                  * huge page demotion.
>                  */
>                 res = cma_declare_contiguous_nid(0, size, 0,
> -                                       PAGE_SIZE << HUGETLB_PAGE_ORDER,
> +                                       PAGE_SIZE << order,
>                                         HUGETLB_PAGE_ORDER, false, name,
>                                         &hugetlb_cma[nid], nid);
>                 if (res) {
> --
> 2.45.0.rc0.197.gbae5840b3b-goog
>