From: Zhaoyang Huang <[email protected]>
According to current CMA utilization policy, an alloc_pages(GFP_USER)
could 'steal' UNMOVABLE & RECLAIMABLE page blocks via the help of
CMA(pass zone_watermark_ok by counting CMA in but use U&R in rmqueue),
which could lead to following alloc_pages(GFP_KERNEL) fail.
Solving this by introducing second watermark checking for GFP_MOVABLE,
which could have the allocation use CMA when proper.
-- Free_pages(30MB)
|
|
-- WMARK_LOW(25MB)
|
-- Free_CMA(12MB)
|
|
--
Signed-off-by: Zhaoyang Huang <[email protected]>
---
v6: update comments
---
---
mm/page_alloc.c | 44 ++++++++++++++++++++++++++++++++++++++++----
1 file changed, 40 insertions(+), 4 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 452459836b71..5a146aa7c0aa 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2078,6 +2078,43 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype,
}
+#ifdef CONFIG_CMA
+/*
+ * GFP_MOVABLE allocation could drain UNMOVABLE & RECLAIMABLE page blocks via
+ * the help of CMA which makes GFP_KERNEL failed. Checking if zone_watermark_ok
+ * again without ALLOC_CMA to see if to use CMA first.
+ */
+static bool use_cma_first(struct zone *zone, unsigned int order, unsigned int alloc_flags)
+{
+ unsigned long watermark;
+ bool cma_first = false;
+
+ watermark = wmark_pages(zone, alloc_flags & ALLOC_WMARK_MASK);
+ /* check if GFP_MOVABLE pass previous zone_watermark_ok via the help of CMA */
+ if (zone_watermark_ok(zone, order, watermark, 0, alloc_flags & (~ALLOC_CMA))) {
+ /*
+ * Balance movable allocations between regular and CMA areas by
+ * allocating from CMA when over half of the zone's free memory
+ * is in the CMA area.
+ */
+ cma_first = (zone_page_state(zone, NR_FREE_CMA_PAGES) >
+ zone_page_state(zone, NR_FREE_PAGES) / 2);
+ } else {
+ /*
+ * watermark failed means UNMOVABLE & RECLAIMBLE is not enough
+ * now, we should use cma first to keep them stay around the
+ * corresponding watermark
+ */
+ cma_first = true;
+ }
+ return cma_first;
+}
+#else
+static bool use_cma_first(struct zone *zone, unsigned int order, unsigned int alloc_flags)
+{
+ return false;
+}
+#endif
/*
* Do the hard work of removing an element from the buddy allocator.
* Call me with the zone->lock already held.
@@ -2091,12 +2128,11 @@ __rmqueue(struct zone *zone, unsigned int order, int migratetype,
if (IS_ENABLED(CONFIG_CMA)) {
/*
* Balance movable allocations between regular and CMA areas by
- * allocating from CMA when over half of the zone's free memory
- * is in the CMA area.
+ * allocating from CMA base on judging zone_watermark_ok again
+ * to see if the latest check got pass via the help of CMA
*/
if (alloc_flags & ALLOC_CMA &&
- zone_page_state(zone, NR_FREE_CMA_PAGES) >
- zone_page_state(zone, NR_FREE_PAGES) / 2) {
+ use_cma_first(zone, order, alloc_flags)) {
page = __rmqueue_cma_fallback(zone, order);
if (page)
return page;
--
2.25.1
On Mon, 16 Oct 2023 15:12:45 +0800 "zhaoyang.huang" <[email protected]> wrote:
> From: Zhaoyang Huang <[email protected]>
>
> According to current CMA utilization policy, an alloc_pages(GFP_USER)
> could 'steal' UNMOVABLE & RECLAIMABLE page blocks via the help of
> CMA(pass zone_watermark_ok by counting CMA in but use U&R in rmqueue),
> which could lead to following alloc_pages(GFP_KERNEL) fail.
> Solving this by introducing second watermark checking for GFP_MOVABLE,
> which could have the allocation use CMA when proper.
>
> -- Free_pages(30MB)
> |
> |
> -- WMARK_LOW(25MB)
> |
> -- Free_CMA(12MB)
> |
> |
> --
>
> Signed-off-by: Zhaoyang Huang <[email protected]>
> ---
> v6: update comments
The patch itself is identical to the v5 patch. So either you meant
"update changelog" above or you sent the wrong diff?
Also, have we resolved any concerns regarding possible performance
impacts of this change?
On Tue, Oct 17, 2023 at 6:40 AM Andrew Morton <[email protected]> wrote:
>
> On Mon, 16 Oct 2023 15:12:45 +0800 "zhaoyang.huang" <[email protected]> wrote:
>
> > From: Zhaoyang Huang <[email protected]>
> >
> > According to current CMA utilization policy, an alloc_pages(GFP_USER)
> > could 'steal' UNMOVABLE & RECLAIMABLE page blocks via the help of
> > CMA(pass zone_watermark_ok by counting CMA in but use U&R in rmqueue),
> > which could lead to following alloc_pages(GFP_KERNEL) fail.
> > Solving this by introducing second watermark checking for GFP_MOVABLE,
> > which could have the allocation use CMA when proper.
> >
> > -- Free_pages(30MB)
> > |
> > |
> > -- WMARK_LOW(25MB)
> > |
> > -- Free_CMA(12MB)
> > |
> > |
> > --
> >
> > Signed-off-by: Zhaoyang Huang <[email protected]>
> > ---
> > v6: update comments
>
> The patch itself is identical to the v5 patch. So either you meant
> "update changelog" above or you sent the wrong diff?
sorry, should be update changelog
>
> Also, have we resolved any concerns regarding possible performance
> impacts of this change?
I don't think this commit could introduce performance impact as it
actually adds one more path for using CMA page blocks in advance.
__rmqueue(struct zone *zone, unsigned int order, int migratetype,
if (IS_ENABLED(CONFIG_CMA)) {
if (alloc_flags & ALLOC_CMA &&
- zone_page_state(zone, NR_FREE_CMA_PAGES) >
- zone_page_state(zone, NR_FREE_PAGES) / 2) {
+ use_cma_first(zone, order, alloc_flags)) {
//current '1/2' logic is kept while add a path for using CMA
in advance than now.
page = __rmqueue_cma_fallback(zone, order);
if (page)
return page;
}
}
retry:
//normal rmqueue_smallest path is not
affected which could deemed as a fallback path for
__rmqueue_cma_fallback failure
page = __rmqueue_smallest(zone, order, migratetype);
if (unlikely(!page)) {
if (alloc_flags & ALLOC_CMA)
page = __rmqueue_cma_fallback(zone, order);
if (!page && __rmqueue_fallback(zone, order, migratetype,
alloc_flags))
goto retry;
}
return page;
}
On Wed, 8 Nov 2023 16:55:19 +0800 Zhaoyang Huang <[email protected]> wrote:
> > > +static bool use_cma_first(struct zone *zone, unsigned int order, unsigned int alloc_flags)
> > > +{
> > > + unsigned long watermark;
> > > + bool cma_first = false;
> > > +
> > > + watermark = wmark_pages(zone, alloc_flags & ALLOC_WMARK_MASK);
> > > + /* check if GFP_MOVABLE pass previous zone_watermark_ok via the help of CMA */
> > > + if (zone_watermark_ok(zone, order, watermark, 0, alloc_flags & (~ALLOC_CMA))) {
> > > + /*
> > > + * Balance movable allocations between regular and CMA areas by
> > > + * allocating from CMA when over half of the zone's free memory
> > > + * is in the CMA area.
> > > + */
> ok, thanks for point out.
> Could we simple it like this, which will mis-judge kmalloc within
> ioctl as GFP_USER. I think it is ok as it is rare
> if (current_is_kswapd() || !current->mm)
> gfp_flags = GFP_KERNEL;
> else
> gfp_flags = GFP_USER;
> free_pages = zone_page_state(zone, NR_FREE_PAGES);
> free_pages -= zone->lowmem_reserve[gfp_zone(gfp_flags)];
> free_pages -= wmark_pages(zone, alloc_flags & ALLOC_WMARK_MASK);
> cma_first = free_pages > zone_page_state(zone, NR_FREE_PAGES) / 2);
>
This went all quiet. Do we feel that "mm: optimization on page
allocation when CMA enabled" should be merged as-is, or dropped in the
expectation that something based on Johannes's suggestion will be
developed?
On Sat, Dec 30, 2023 at 3:40 AM Andrew Morton <[email protected]> wrote:
>
> On Wed, 8 Nov 2023 16:55:19 +0800 Zhaoyang Huang <[email protected]> wrote:
>
> > > > +static bool use_cma_first(struct zone *zone, unsigned int order, unsigned int alloc_flags)
> > > > +{
> > > > + unsigned long watermark;
> > > > + bool cma_first = false;
> > > > +
> > > > + watermark = wmark_pages(zone, alloc_flags & ALLOC_WMARK_MASK);
> > > > + /* check if GFP_MOVABLE pass previous zone_watermark_ok via the help of CMA */
> > > > + if (zone_watermark_ok(zone, order, watermark, 0, alloc_flags & (~ALLOC_CMA))) {
> > > > + /*
> > > > + * Balance movable allocations between regular and CMA areas by
> > > > + * allocating from CMA when over half of the zone's free memory
> > > > + * is in the CMA area.
> > > > + */
> > ok, thanks for point out.
> > Could we simple it like this, which will mis-judge kmalloc within
> > ioctl as GFP_USER. I think it is ok as it is rare
> > if (current_is_kswapd() || !current->mm)
> > gfp_flags = GFP_KERNEL;
> > else
> > gfp_flags = GFP_USER;
> > free_pages = zone_page_state(zone, NR_FREE_PAGES);
> > free_pages -= zone->lowmem_reserve[gfp_zone(gfp_flags)];
> > free_pages -= wmark_pages(zone, alloc_flags & ALLOC_WMARK_MASK);
> > cma_first = free_pages > zone_page_state(zone, NR_FREE_PAGES) / 2);
> >
>
> This went all quiet. Do we feel that "mm: optimization on page
> allocation when CMA enabled" should be merged as-is, or dropped in the
> expectation that something based on Johannes's suggestion will be
> developed?
I just establish a v6.6 environment and will provide comparison
results with and without the patch
>