[PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[Jinjiang Tu]

When calling alloc_contig_range() with __GFP_COMP and the order of
requested pfn range is pageblock_order, less than MAX_ORDER, I triggered
WARNING as follows:

 PFN range: requested [2150105088, 2150105600), allocated [2150105088, 2150106112)
 WARNING: CPU: 3 PID: 580 at mm/page_alloc.c:6877 alloc_contig_range+0x280/0x340

alloc_contig_range() marks pageblocks of the requested pfn range to be
isolated, migrate these pages if they are in use and will be freed to
MIGRATE_ISOLATED freelist.

Suppose two alloc_contig_range() calls at the same time and the requested
pfn range are [0x80280000, 0x80280200) and [0x80280200, 0x80280400)
respectively. Suppose the two memory range are in use, then
alloc_contig_range() will migrate and free these pages to MIGRATE_ISOLATED
freelist. __free_one_page() will merge MIGRATE_ISOLATE buddy to larger
buddy, resulting in a MAX_ORDER buddy. Finally, find_large_buddy() in
alloc_contig_range() returns a MAX_ORDER buddy and results in WARNING.

To fix it, call free_contig_range() to free the excess pfn range.

Fixes: e98337d11bbd ("mm/contig_alloc: support __GFP_COMP")
Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
---
Changelog since v1:
 * Add comment and remove redundant code, suggested by Zi Yan

 mm/page_alloc.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 579789600a3c..f0162ab991ad 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6440,6 +6440,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 		.alloc_contig = true,
 	};
 	INIT_LIST_HEAD(&cc.migratepages);
+	bool is_range_aligned;
 
 	gfp_mask = current_gfp_context(gfp_mask);
 	if (__alloc_contig_verify_gfp_mask(gfp_mask, (gfp_t *)&cc.gfp_mask))
@@ -6528,7 +6529,14 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 		goto done;
 	}
 
-	if (!(gfp_mask & __GFP_COMP)) {
+	/*
+	 * With __GFP_COMP and the requested order < MAX_PAGE_ORDER,
+	 * isolated free pages can have higher order than the requested
+	 * one. Use split_free_pages() to free out of range pages.
+	 */
+	is_range_aligned = is_power_of_2(end - start);
+	if (!(gfp_mask & __GFP_COMP) ||
+		(is_range_aligned && ilog2(end - start) < MAX_PAGE_ORDER)) {
 		split_free_pages(cc.freepages, gfp_mask);
 
 		/* Free head and tail (if any) */
@@ -6536,7 +6544,15 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 			free_contig_range(outer_start, start - outer_start);
 		if (end != outer_end)
 			free_contig_range(end, outer_end - end);
-	} else if (start == outer_start && end == outer_end && is_power_of_2(end - start)) {
+
+		outer_start = start;
+		outer_end = end;
+
+		if (!(gfp_mask & __GFP_COMP))
+			goto done;
+	}
+
+	if (start == outer_start && end == outer_end && is_range_aligned) {
 		struct page *head = pfn_to_page(start);
 		int order = ilog2(end - start);
 
-- 
2.43.0

Re: [PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[Zi Yan]

On 20 Apr 2025, at 21:36, Jinjiang Tu wrote:

> When calling alloc_contig_range() with __GFP_COMP and the order of
> requested pfn range is pageblock_order, less than MAX_ORDER, I triggered
> WARNING as follows:
>
>  PFN range: requested [2150105088, 2150105600), allocated [2150105088, 2150106112)
>  WARNING: CPU: 3 PID: 580 at mm/page_alloc.c:6877 alloc_contig_range+0x280/0x340
>
> alloc_contig_range() marks pageblocks of the requested pfn range to be
> isolated, migrate these pages if they are in use and will be freed to
> MIGRATE_ISOLATED freelist.
>
> Suppose two alloc_contig_range() calls at the same time and the requested
> pfn range are [0x80280000, 0x80280200) and [0x80280200, 0x80280400)
> respectively. Suppose the two memory range are in use, then
> alloc_contig_range() will migrate and free these pages to MIGRATE_ISOLATED
> freelist. __free_one_page() will merge MIGRATE_ISOLATE buddy to larger
> buddy, resulting in a MAX_ORDER buddy. Finally, find_large_buddy() in
> alloc_contig_range() returns a MAX_ORDER buddy and results in WARNING.
>
> To fix it, call free_contig_range() to free the excess pfn range.
>
> Fixes: e98337d11bbd ("mm/contig_alloc: support __GFP_COMP")
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> ---
> Changelog since v1:
>  * Add comment and remove redundant code, suggested by Zi Yan
>
>  mm/page_alloc.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>

LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[David Hildenbrand]

On 21.04.25 03:36, Jinjiang Tu wrote:
> When calling alloc_contig_range() with __GFP_COMP and the order of
> requested pfn range is pageblock_order, less than MAX_ORDER, I triggered
> WARNING as follows:
> 
>   PFN range: requested [2150105088, 2150105600), allocated [2150105088, 2150106112)
>   WARNING: CPU: 3 PID: 580 at mm/page_alloc.c:6877 alloc_contig_range+0x280/0x340
> 

Just to verify: there is no such in-tree user, right?

> alloc_contig_range() marks pageblocks of the requested pfn range to be
> isolated, migrate these pages if they are in use and will be freed to
> MIGRATE_ISOLATED freelist.
> 
> Suppose two alloc_contig_range() calls at the same time and the requested
> pfn range are [0x80280000, 0x80280200) and [0x80280200, 0x80280400)
> respectively. Suppose the two memory range are in use, then
> alloc_contig_range() will migrate and free these pages to MIGRATE_ISOLATED
> freelist. __free_one_page() will merge MIGRATE_ISOLATE buddy to larger
> buddy, resulting in a MAX_ORDER buddy. Finally, find_large_buddy() in
> alloc_contig_range() returns a MAX_ORDER buddy and results in WARNING.
> 
> To fix it, call free_contig_range() to free the excess pfn range.
> 
> Fixes: e98337d11bbd ("mm/contig_alloc: support __GFP_COMP")
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> ---
> Changelog since v1:
>   * Add comment and remove redundant code, suggested by Zi Yan
> 
>   mm/page_alloc.c | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 579789600a3c..f0162ab991ad 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6440,6 +6440,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
>   		.alloc_contig = true,
>   	};
>   	INIT_LIST_HEAD(&cc.migratepages);
> +	bool is_range_aligned;

is "aligned" the right word? Aligned to what?

I do wonder if we could do the following on top, checking that the range is suitable for __GFP_COMP earlier.


 From 6c414d786db74b1494f7cf66ebf911c01995d20a Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Fri, 25 Apr 2025 12:32:15 +0200
Subject: [PATCH] tmp

Signed-off-by: David Hildenbrand <david@redhat.com>
---
  mm/page_alloc.c | 24 ++++++++++++------------
  1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 57aa64dc74a05..85312903dcd8c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6682,6 +6682,7 @@ static int __alloc_contig_verify_gfp_mask(gfp_t gfp_mask, gfp_t *gfp_cc_mask)
  int alloc_contig_range_noprof(unsigned long start, unsigned long end,
  		       unsigned migratetype, gfp_t gfp_mask)
  {
+	const int range_order = ilog2(end - start);
  	unsigned long outer_start, outer_end;
  	int ret = 0;
  
@@ -6695,12 +6696,19 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
  		.alloc_contig = true,
  	};
  	INIT_LIST_HEAD(&cc.migratepages);
-	bool is_range_aligned;
  
  	gfp_mask = current_gfp_context(gfp_mask);
  	if (__alloc_contig_verify_gfp_mask(gfp_mask, (gfp_t *)&cc.gfp_mask))
  		return -EINVAL;
  
+	/* __GFP_COMP may only be used for certain aligned+sized ranges. */
+	if ((gfp_mask & __GFP_COMP) &&
+	    (!is_power_of_2(end - start) || !IS_ALIGNED(start, 1 << range_order))) {
+		WARN_ONCE(true, "PFN range: requested [%lu, %lu) is not suitable for __GFP_COMP\n",
+			  start, end);
+		return -EINVAL;
+	}
+
  	/*
  	 * What we do here is we mark all pageblocks in range as
  	 * MIGRATE_ISOLATE.  Because pageblock and max order pages may
@@ -6789,9 +6797,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
  	 * isolated free pages can have higher order than the requested
  	 * one. Use split_free_pages() to free out of range pages.
  	 */
-	is_range_aligned = is_power_of_2(end - start);
-	if (!(gfp_mask & __GFP_COMP) ||
-		(is_range_aligned && ilog2(end - start) < MAX_PAGE_ORDER)) {
+	if (!(gfp_mask & __GFP_COMP) || range_order < MAX_PAGE_ORDER) {
  		split_free_pages(cc.freepages, gfp_mask);
  
  		/* Free head and tail (if any) */
@@ -6802,22 +6808,16 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
  
  		outer_start = start;
  		outer_end = end;
-
-		if (!(gfp_mask & __GFP_COMP))
-			goto done;
  	}
  
-	if (start == outer_start && end == outer_end && is_range_aligned) {
+	if (gfp_mask & __GFP_COMP) {
  		struct page *head = pfn_to_page(start);
  		int order = ilog2(end - start);
  
+		VM_WARN_ON_ONCE(outer_start != start || outer_end != end);
  		check_new_pages(head, order);
  		prep_new_page(head, order, gfp_mask, 0);
  		set_page_refcounted(head);
-	} else {
-		ret = -EINVAL;
-		WARN(true, "PFN range: requested [%lu, %lu), allocated [%lu, %lu)\n",
-		     start, end, outer_start, outer_end);
  	}
  done:
  	undo_isolate_page_range(start, end, migratetype);
-- 
2.49.0


-- 
Cheers,

David / dhildenb

Re: [PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[Zi Yan]

On 25 Apr 2025, at 6:33, David Hildenbrand wrote:

> On 21.04.25 03:36, Jinjiang Tu wrote:
>> When calling alloc_contig_range() with __GFP_COMP and the order of
>> requested pfn range is pageblock_order, less than MAX_ORDER, I triggered
>> WARNING as follows:
>>
>>   PFN range: requested [2150105088, 2150105600), allocated [2150105088, 2150106112)
>>   WARNING: CPU: 3 PID: 580 at mm/page_alloc.c:6877 alloc_contig_range+0x280/0x340
>>
>
> Just to verify: there is no such in-tree user, right?
>
>> alloc_contig_range() marks pageblocks of the requested pfn range to be
>> isolated, migrate these pages if they are in use and will be freed to
>> MIGRATE_ISOLATED freelist.
>>
>> Suppose two alloc_contig_range() calls at the same time and the requested
>> pfn range are [0x80280000, 0x80280200) and [0x80280200, 0x80280400)
>> respectively. Suppose the two memory range are in use, then
>> alloc_contig_range() will migrate and free these pages to MIGRATE_ISOLATED
>> freelist. __free_one_page() will merge MIGRATE_ISOLATE buddy to larger
>> buddy, resulting in a MAX_ORDER buddy. Finally, find_large_buddy() in
>> alloc_contig_range() returns a MAX_ORDER buddy and results in WARNING.
>>
>> To fix it, call free_contig_range() to free the excess pfn range.
>>
>> Fixes: e98337d11bbd ("mm/contig_alloc: support __GFP_COMP")
>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
>> ---
>> Changelog since v1:
>>   * Add comment and remove redundant code, suggested by Zi Yan
>>
>>   mm/page_alloc.c | 20 ++++++++++++++++++--
>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 579789600a3c..f0162ab991ad 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -6440,6 +6440,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
>>   		.alloc_contig = true,
>>   	};
>>   	INIT_LIST_HEAD(&cc.migratepages);
>> +	bool is_range_aligned;
>
> is "aligned" the right word? Aligned to what?
>
> I do wonder if we could do the following on top, checking that the range is suitable for __GFP_COMP earlier.
>

The change below makes the code cleaner. Acked-by: Zi Yan <ziy@nvidia.com>

>
> From 6c414d786db74b1494f7cf66ebf911c01995d20a Mon Sep 17 00:00:00 2001
> From: David Hildenbrand <david@redhat.com>
> Date: Fri, 25 Apr 2025 12:32:15 +0200
> Subject: [PATCH] tmp
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  mm/page_alloc.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 57aa64dc74a05..85312903dcd8c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6682,6 +6682,7 @@ static int __alloc_contig_verify_gfp_mask(gfp_t gfp_mask, gfp_t *gfp_cc_mask)
>  int alloc_contig_range_noprof(unsigned long start, unsigned long end,
>  		       unsigned migratetype, gfp_t gfp_mask)
>  {
> +	const int range_order = ilog2(end - start);
>  	unsigned long outer_start, outer_end;
>  	int ret = 0;
>  @@ -6695,12 +6696,19 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
>  		.alloc_contig = true,
>  	};
>  	INIT_LIST_HEAD(&cc.migratepages);
> -	bool is_range_aligned;
>   	gfp_mask = current_gfp_context(gfp_mask);
>  	if (__alloc_contig_verify_gfp_mask(gfp_mask, (gfp_t *)&cc.gfp_mask))
>  		return -EINVAL;
>  +	/* __GFP_COMP may only be used for certain aligned+sized ranges. */
> +	if ((gfp_mask & __GFP_COMP) &&
> +	    (!is_power_of_2(end - start) || !IS_ALIGNED(start, 1 << range_order))) {
> +		WARN_ONCE(true, "PFN range: requested [%lu, %lu) is not suitable for __GFP_COMP\n",
> +			  start, end);
> +		return -EINVAL;
> +	}
> +
>  	/*
>  	 * What we do here is we mark all pageblocks in range as
>  	 * MIGRATE_ISOLATE.  Because pageblock and max order pages may
> @@ -6789,9 +6797,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
>  	 * isolated free pages can have higher order than the requested
>  	 * one. Use split_free_pages() to free out of range pages.
>  	 */
> -	is_range_aligned = is_power_of_2(end - start);
> -	if (!(gfp_mask & __GFP_COMP) ||
> -		(is_range_aligned && ilog2(end - start) < MAX_PAGE_ORDER)) {
> +	if (!(gfp_mask & __GFP_COMP) || range_order < MAX_PAGE_ORDER) {
>  		split_free_pages(cc.freepages, gfp_mask);
>   		/* Free head and tail (if any) */
> @@ -6802,22 +6808,16 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
>   		outer_start = start;
>  		outer_end = end;
> -
> -		if (!(gfp_mask & __GFP_COMP))
> -			goto done;
>  	}
>  -	if (start == outer_start && end == outer_end && is_range_aligned) {
> +	if (gfp_mask & __GFP_COMP) {
>  		struct page *head = pfn_to_page(start);
>  		int order = ilog2(end - start);
>  +		VM_WARN_ON_ONCE(outer_start != start || outer_end != end);
>  		check_new_pages(head, order);
>  		prep_new_page(head, order, gfp_mask, 0);
>  		set_page_refcounted(head);
> -	} else {
> -		ret = -EINVAL;
> -		WARN(true, "PFN range: requested [%lu, %lu), allocated [%lu, %lu)\n",
> -		     start, end, outer_start, outer_end);
>  	}
>  done:
>  	undo_isolate_page_range(start, end, migratetype);
> -- 
> 2.49.0
>
>
> -- 
> Cheers,
>
> David / dhildenb


--
Best Regards,
Yan, Zi

Re: [PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[David Hildenbrand]

On 25.04.25 13:04, Zi Yan wrote:
> On 25 Apr 2025, at 6:33, David Hildenbrand wrote:
> 
>> On 21.04.25 03:36, Jinjiang Tu wrote:
>>> When calling alloc_contig_range() with __GFP_COMP and the order of
>>> requested pfn range is pageblock_order, less than MAX_ORDER, I triggered
>>> WARNING as follows:
>>>
>>>    PFN range: requested [2150105088, 2150105600), allocated [2150105088, 2150106112)
>>>    WARNING: CPU: 3 PID: 580 at mm/page_alloc.c:6877 alloc_contig_range+0x280/0x340
>>>
>>
>> Just to verify: there is no such in-tree user, right?
>>
>>> alloc_contig_range() marks pageblocks of the requested pfn range to be
>>> isolated, migrate these pages if they are in use and will be freed to
>>> MIGRATE_ISOLATED freelist.
>>>
>>> Suppose two alloc_contig_range() calls at the same time and the requested
>>> pfn range are [0x80280000, 0x80280200) and [0x80280200, 0x80280400)
>>> respectively. Suppose the two memory range are in use, then
>>> alloc_contig_range() will migrate and free these pages to MIGRATE_ISOLATED
>>> freelist. __free_one_page() will merge MIGRATE_ISOLATE buddy to larger
>>> buddy, resulting in a MAX_ORDER buddy. Finally, find_large_buddy() in
>>> alloc_contig_range() returns a MAX_ORDER buddy and results in WARNING.
>>>
>>> To fix it, call free_contig_range() to free the excess pfn range.
>>>
>>> Fixes: e98337d11bbd ("mm/contig_alloc: support __GFP_COMP")
>>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
>>> ---
>>> Changelog since v1:
>>>    * Add comment and remove redundant code, suggested by Zi Yan
>>>
>>>    mm/page_alloc.c | 20 ++++++++++++++++++--
>>>    1 file changed, 18 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index 579789600a3c..f0162ab991ad 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -6440,6 +6440,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
>>>    		.alloc_contig = true,
>>>    	};
>>>    	INIT_LIST_HEAD(&cc.migratepages);
>>> +	bool is_range_aligned;
>>
>> is "aligned" the right word? Aligned to what?
>>
>> I do wonder if we could do the following on top, checking that the range is suitable for __GFP_COMP earlier.
>>
> 
> The change below makes the code cleaner. Acked-by: Zi Yan <ziy@nvidia.com>

Jinjiang, can you integrate that into your patch and resend?

Thanks!

-- 
Cheers,

David / dhildenb

Re: [PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[Jinjiang Tu]

在 2025/5/11 16:04, David Hildenbrand 写道:
> On 25.04.25 13:04, Zi Yan wrote:
>> On 25 Apr 2025, at 6:33, David Hildenbrand wrote:
>>
>>> On 21.04.25 03:36, Jinjiang Tu wrote:
>>>> When calling alloc_contig_range() with __GFP_COMP and the order of
>>>> requested pfn range is pageblock_order, less than MAX_ORDER, I 
>>>> triggered
>>>> WARNING as follows:
>>>>
>>>>    PFN range: requested [2150105088, 2150105600), allocated 
>>>> [2150105088, 2150106112)
>>>>    WARNING: CPU: 3 PID: 580 at mm/page_alloc.c:6877 
>>>> alloc_contig_range+0x280/0x340
>>>>
>>>
>>> Just to verify: there is no such in-tree user, right?
>>>
>>>> alloc_contig_range() marks pageblocks of the requested pfn range to be
>>>> isolated, migrate these pages if they are in use and will be freed to
>>>> MIGRATE_ISOLATED freelist.
>>>>
>>>> Suppose two alloc_contig_range() calls at the same time and the 
>>>> requested
>>>> pfn range are [0x80280000, 0x80280200) and [0x80280200, 0x80280400)
>>>> respectively. Suppose the two memory range are in use, then
>>>> alloc_contig_range() will migrate and free these pages to 
>>>> MIGRATE_ISOLATED
>>>> freelist. __free_one_page() will merge MIGRATE_ISOLATE buddy to larger
>>>> buddy, resulting in a MAX_ORDER buddy. Finally, find_large_buddy() in
>>>> alloc_contig_range() returns a MAX_ORDER buddy and results in WARNING.
>>>>
>>>> To fix it, call free_contig_range() to free the excess pfn range.
>>>>
>>>> Fixes: e98337d11bbd ("mm/contig_alloc: support __GFP_COMP")
>>>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
>>>> ---
>>>> Changelog since v1:
>>>>    * Add comment and remove redundant code, suggested by Zi Yan
>>>>
>>>>    mm/page_alloc.c | 20 ++++++++++++++++++--
>>>>    1 file changed, 18 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>>> index 579789600a3c..f0162ab991ad 100644
>>>> --- a/mm/page_alloc.c
>>>> +++ b/mm/page_alloc.c
>>>> @@ -6440,6 +6440,7 @@ int alloc_contig_range_noprof(unsigned long 
>>>> start, unsigned long end,
>>>>            .alloc_contig = true,
>>>>        };
>>>>        INIT_LIST_HEAD(&cc.migratepages);
>>>> +    bool is_range_aligned;
>>>
>>> is "aligned" the right word? Aligned to what?
>>>
>>> I do wonder if we could do the following on top, checking that the 
>>> range is suitable for __GFP_COMP earlier.
>>>
>>
>> The change below makes the code cleaner. Acked-by: Zi Yan 
>> <ziy@nvidia.com>
>
> Jinjiang, can you integrate that into your patch and resend?

Sorry for late reply. I will do it today.

Thanks for review.

>
> Thanks!
>

Re: [PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[Andrew Morton]

On Mon, 12 May 2025 09:13:31 +0800 Jinjiang Tu <tujinjiang@huawei.com> wrote:

> 
> >>
> >> The change below makes the code cleaner. Acked-by: Zi Yan 
> >> <ziy@nvidia.com>
> >
> > Jinjiang, can you integrate that into your patch and resend?
> 
> Sorry for late reply. I will do it today.
> 

As far as I know, this is still pending.  The patch does address a
WARN() and it's cc:stable, so it's rather important.

I'll move the v2 patch back into mm-new and shall keep it there to
remind me to bug people about it ;)


David (and everyone!), please don't be afraid to grab someone else's
patch, apply changes and send it in.  Stuff happens, and doing this is well
within accepted processes.

Re: [PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[Zi Yan]

On 27 May 2025, at 22:19, Andrew Morton wrote:

> On Mon, 12 May 2025 09:13:31 +0800 Jinjiang Tu <tujinjiang@huawei.com> wrote:
>
>>
>>>>
>>>> The change below makes the code cleaner. Acked-by: Zi Yan
>>>> <ziy@nvidia.com>
>>>
>>> Jinjiang, can you integrate that into your patch and resend?
>>
>> Sorry for late reply. I will do it today.
>>
>
> As far as I know, this is still pending.  The patch does address a
> WARN() and it's cc:stable, so it's rather important.
>
> I'll move the v2 patch back into mm-new and shall keep it there to
> remind me to bug people about it ;)
>
>
> David (and everyone!), please don't be afraid to grab someone else's
> patch, apply changes and send it in.  Stuff happens, and doing this is well
> within accepted processes.

Hi Andrew,

Based on my understanding, this patch is fixing an out-of-tree driver
issue, so Jinjiang is responsible to resolve David’s concerns.

Hi Jinjiang,

Let me know if I miss anything. And if you are not planning to keep
working on it, please let Andrew know so he can drop it.

Thanks.

Best Regards,
Yan, Zi

Re: [PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[Jinjiang Tu]

在 2025/5/28 10:25, Zi Yan 写道:
> On 27 May 2025, at 22:19, Andrew Morton wrote:
>
>> On Mon, 12 May 2025 09:13:31 +0800 Jinjiang Tu <tujinjiang@huawei.com> wrote:
>>
>>>>> The change below makes the code cleaner. Acked-by: Zi Yan
>>>>> <ziy@nvidia.com>
>>>> Jinjiang, can you integrate that into your patch and resend?
>>> Sorry for late reply. I will do it today.
>>>
>> As far as I know, this is still pending.  The patch does address a
>> WARN() and it's cc:stable, so it's rather important.
>>
>> I'll move the v2 patch back into mm-new and shall keep it there to
>> remind me to bug people about it ;)
>>
>>
>> David (and everyone!), please don't be afraid to grab someone else's
>> patch, apply changes and send it in.  Stuff happens, and doing this is well
>> within accepted processes.
> Hi Andrew,
>
> Based on my understanding, this patch is fixing an out-of-tree driver
> issue, so Jinjiang is responsible to resolve David’s concerns.
>
> Hi Jinjiang,
>
> Let me know if I miss anything. And if you are not planning to keep
> working on it, please let Andrew know so he can drop it.

Yes, this issue doesn't exist for any in-tree users. I found v2 patch is buggy
and reply in the v1 patch (https://lore.kernel.org/all/80039e40-a518-a85b-b955-96fb048a2dd0@huawei.com/). The
fixed patch is little complicated.

Since there is no in-tree user currently, it is enough if we simply fail if the range order < MAX_ORDER with __GFP_COMP.

If it is valuable to handle this situation, in case in-tree users in the future need to allocate folio with order < MAX_ORDER,
I would like to keep working on it.

> Thanks.
>
> Best Regards,
> Yan, Zi

Re: [PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[David Hildenbrand]

On 28.05.25 04:19, Andrew Morton wrote:
> On Mon, 12 May 2025 09:13:31 +0800 Jinjiang Tu <tujinjiang@huawei.com> wrote:
> 
>>
>>>>
>>>> The change below makes the code cleaner. Acked-by: Zi Yan
>>>> <ziy@nvidia.com>
>>>
>>> Jinjiang, can you integrate that into your patch and resend?
>>
>> Sorry for late reply. I will do it today.
>>
> 
> As far as I know, this is still pending.  The patch does address a
> WARN() and it's cc:stable, so it's rather important.
> 
> I'll move the v2 patch back into mm-new and shall keep it there to
> remind me to bug people about it ;)
> 
> 
> David (and everyone!), please don't be afraid to grab someone else's
> patch, apply changes and send it in.  Stuff happens, and doing this is well
> within accepted processes.

I would usually do that if (a) there would be a reproducer that I can 
test with and (b) I would find some spare time.

Given that I don't consider this urgent (no in-tree user ...), I would 
hope that the original submitter can just ... send a properly tested 
patch that includes review feedback.

Maybe best to drop this patch for now completely. In that case, I might 
just send a patch to fail any non-aligned / too small GFP_COMP 
allocation early, instead of adding support for it.

-- 
Cheers,

David / dhildenb

Re: [PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[Jinjiang Tu]

在 2025/5/28 16:43, David Hildenbrand 写道:
> On 28.05.25 04:19, Andrew Morton wrote:
>> On Mon, 12 May 2025 09:13:31 +0800 Jinjiang Tu 
>> <tujinjiang@huawei.com> wrote:
>>
>>>
>>>>>
>>>>> The change below makes the code cleaner. Acked-by: Zi Yan
>>>>> <ziy@nvidia.com>
>>>>
>>>> Jinjiang, can you integrate that into your patch and resend?
>>>
>>> Sorry for late reply. I will do it today.
>>>
>>
>> As far as I know, this is still pending.  The patch does address a
>> WARN() and it's cc:stable, so it's rather important.
>>
>> I'll move the v2 patch back into mm-new and shall keep it there to
>> remind me to bug people about it ;)
>>
>>
>> David (and everyone!), please don't be afraid to grab someone else's
>> patch, apply changes and send it in.  Stuff happens, and doing this 
>> is well
>> within accepted processes.
>
> I would usually do that if (a) there would be a reproducer that I can 
> test with and (b) I would find some spare time.
>
> Given that I don't consider this urgent (no in-tree user ...), I would 
> hope that the original submitter can just ... send a properly tested 
> patch that includes review feedback.
>
> Maybe best to drop this patch for now completely. In that case, I 
> might just send a patch to fail any non-aligned / too small GFP_COMP 
> allocation early, instead of adding support for it.
Thanks, I will send a patch to fail in such case simply.