2024-04-30 16:14:51

by Frank van der Linden

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

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 <[email protected]>
Cc: Roman Gushchin <[email protected]>
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



2024-05-02 13:16:09

by David Hildenbrand

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

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 <[email protected]>
> Cc: Roman Gushchin <[email protected]>
> 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 <[email protected]>

--
Cheers,

David / dhildenb


2024-05-02 17:02:16

by Roman Gushchin

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

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 <[email protected]>
> Cc: Roman Gushchin <[email protected]>
> Fixes: a01f43901cfb ("hugetlb: be sure to free demoted CMA pages to CMA")

Reviewed-by: Roman Gushchin <[email protected]>

Thanks!

2024-05-02 17:32:07

by Frank van der Linden

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

On Thu, May 2, 2024 at 6:15 AM David Hildenbrand <[email protected]> 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 <[email protected]>
> > Cc: Roman Gushchin <[email protected]>
> > 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 <[email protected]>

Thanks!

2024-05-08 10:27:46

by Oscar Salvador

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

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 <[email protected]>
> Cc: Roman Gushchin <[email protected]>
> Fixes: a01f43901cfb ("hugetlb: be sure to free demoted CMA pages to CMA")

Reviewed-by: Oscar Salvador <[email protected]>


--
Oscar Salvador
SUSE Labs