2023-11-22 01:42:13

by zhiguojiang

[permalink] [raw]
Subject: [PATCH v3] mm: ALLOC_HIGHATOMIC flag allocation issue

Update base on the latest Linus' tree or Andrew's tree.

Signed-off-by: Zhiguo Jiang <[email protected]>
---

Changelog:
v1:
In case that alloc_flags contains ALLOC_HIGHATOMIC and alloc order
is order1/2/3/10 in rmqueue(), if pages are alloced successfully
from pcplist, a free pageblock will be also moved from the alloced
migratetype freelist to MIGRATE_HIGHATOMIC freelist, rather than
alloc from MIGRATE_HIGHATOMIC freelist firstly, so this will result
in an increasing number of pages on the MIGRATE_HIGHATOMIC freelist,
pages in other migratetype freelist are reduced and more likely to
allocation failure.

Currently the sequence of ALLOC_HIGHATOMIC allocation is:
pcplist --> rmqueue_bulk() --> rmqueue_buddy() MIGRATE_HIGHATOMIC
--> rmqueue_buddy() allocation migratetype.

Due to the fact that requesting pages from the pcplist is faster than
buddy, the sequence of modifying the ALLOC_HIGHATOMIC allocation is:
pcplist --> rmqueue_buddy() MIGRATE_HIGHATOMIC --> rmqueue_buddy()
allocation migratetype.

This patch can solve the failure problem of allocating other types of
pages due to excessive MIGRATE_HIGHATOMIC freelist reservations.

In comparative testing, cat /proc/pagetypeinfo and the HighAtomic
freelist size is:
Test without this patch:
Node 0, zone Normal, type HighAtomic 2369 771 138 15 0 0 0 0 0 0 0
Test with this patch:
Node 0, zone Normal, type HighAtomic 206 82 4 2 1 0 0 0 0 0 0

v2:
Update comments and modify variable highatomic_allocation to highatomic.

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

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 49890d00cc3c..693e86fc9850 100755
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2851,9 +2851,9 @@ struct page *__rmqueue_pcplist(struct zone *zone, unsigned int order,
int alloced;

/*
- * If pcplist is empty and alloc_flags is with ALLOC_HIGHATOMIC,
- * it should alloc from buddy highatomic migrate freelist firstly
- * to ensure quick and successful allocation.
+ * If pcplist is empty and alloc_flags contains
+ * ALLOC_HIGHATOMIC, alloc from buddy highatomic
+ * freelist first.
*/
if (alloc_flags & ALLOC_HIGHATOMIC)
goto out;
@@ -2927,7 +2927,7 @@ static inline
struct page *rmqueue(struct zone *preferred_zone,
struct zone *zone, unsigned int order,
gfp_t gfp_flags, unsigned int alloc_flags,
- int migratetype, bool *highatomc_allocation)
+ int migratetype, bool *highatomic)
{
struct page *page;

@@ -2948,21 +2948,22 @@ struct page *rmqueue(struct zone *preferred_zone,
migratetype);

/*
- * The high-order atomic allocation pageblock reserved conditions:
+ * The high-order atomic allocation pageblock reserved:
*
- * If the high-order atomic allocation page is alloced from pcplist,
- * the highatomic pageblock does not need to be reserved, which can
- * void to migrate an increasing number of pages into buddy
- * MIGRATE_HIGHATOMIC freelist and lead to an increasing risk of
- * allocation failure on other buddy migrate freelists.
+ * If the high-order atomic page is allocated from pcplist,
+ * the highatomic pageblock does not need to be reserved,
+ * which can avoid migrating an increasing number of pages
+ * into buddy highatomic freelist and leading to an increased
+ * risk of allocation failure on other migrate freelists in
+ * buddy.
*
- * If the high-order atomic allocation page is alloced from buddy
- * highatomic migrate freelist, regardless of whether the allocation
- * is successful or not, the highatomic pageblock can try to be
- * reserved.
+ * If the high-order atomic page is allocated from buddy
+ * highatomic freelist, regardless of whether the allocation
+ * is successful or not, the highatomic pageblock can try to
+ * be reserved.
*/
if (unlikely(alloc_flags & ALLOC_HIGHATOMIC))
- *highatomc_allocation = true;
+ *highatomic = true;

out:
/* Separate test+clear to avoid unnecessary atomics */
@@ -3234,7 +3235,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
struct pglist_data *last_pgdat = NULL;
bool last_pgdat_dirty_ok = false;
bool no_fallback;
- bool highatomc_allocation = false;
+ bool highatomic = false;

retry:
/*
@@ -3366,7 +3367,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,

try_this_zone:
page = rmqueue(ac->preferred_zoneref->zone, zone, order,
- gfp_mask, alloc_flags, ac->migratetype, &highatomc_allocation);
+ gfp_mask, alloc_flags, ac->migratetype, &highatomic);
if (page) {
prep_new_page(page, order, gfp_mask, alloc_flags);

@@ -3374,7 +3375,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
* If this is a high-order atomic allocation then check
* if the pageblock should be reserved for the future
*/
- if (unlikely(highatomc_allocation))
+ if (unlikely(highatomic))
reserve_highatomic_pageblock(page, zone);

return page;
--
2.39.0


2023-11-22 06:21:11

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH v3] mm: ALLOC_HIGHATOMIC flag allocation issue

On Wed, Nov 22, 2023 at 09:39:25AM +0800, Zhiguo Jiang wrote:
> Update base on the latest Linus' tree or Andrew's tree.
> +++ b/mm/page_alloc.c
> @@ -2851,9 +2851,9 @@ struct page *__rmqueue_pcplist(struct zone *zone, unsigned int order,
> int alloced;
>
> /*
> - * If pcplist is empty and alloc_flags is with ALLOC_HIGHATOMIC,
> - * it should alloc from buddy highatomic migrate freelist firstly
> - * to ensure quick and successful allocation.
> + * If pcplist is empty and alloc_flags contains
> + * ALLOC_HIGHATOMIC, alloc from buddy highatomic
> + * freelist first.

No, it's still based on your earlier patch, is it really so hard for
you to read your patches before you send them?

2023-11-22 07:13:17

by zhiguojiang

[permalink] [raw]
Subject: Re: [PATCH v3] mm: ALLOC_HIGHATOMIC flag allocation issue



在 2023/11/22 14:20, Matthew Wilcox 写道:
> On Wed, Nov 22, 2023 at 09:39:25AM +0800, Zhiguo Jiang wrote:
>> Update base on the latest Linus' tree or Andrew's tree.
>> +++ b/mm/page_alloc.c
>> @@ -2851,9 +2851,9 @@ struct page *__rmqueue_pcplist(struct zone *zone, unsigned int order,
>> int alloced;
>>
>> /*
>> - * If pcplist is empty and alloc_flags is with ALLOC_HIGHATOMIC,
>> - * it should alloc from buddy highatomic migrate freelist firstly
>> - * to ensure quick and successful allocation.
>> + * If pcplist is empty and alloc_flags contains
>> + * ALLOC_HIGHATOMIC, alloc from buddy highatomic
>> + * freelist first.
> No, it's still based on your earlier patch, is it really so hard for
> you to read your patches before you send them?
Sorry, I didn't understand your meaning correctly before. I have updated
base on the latest linux-next tree in patch v4.
Thank you for your patient guidance.