Re: [PATCH] CMA: treat free cma pages as non-free if not ALLOC_CMA on watermark checking

From: Hui Zhu
Date: Mon Jan 19 2015 - 02:37:52 EST

Next message: Lee Jones: "Re: [PATCH 08/10] mfd: rtsx: add support for rts524A"
Previous message: Stephen Rothwell: "Re: linux-next: build failure after merge of the akpm-current tree"
In reply to: Minchan Kim: "Re: [PATCH] CMA: treat free cma pages as non-free if not ALLOC_CMA on watermark checking"
Next in thread: Zhang Yanfei: "Re: [PATCH] CMA: treat free cma pages as non-free if not ALLOC_CMA on watermark checking"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, Jan 19, 2015 at 2:55 PM, Minchan Kim <minchan@xxxxxxxxxx> wrote:
> Hello,
>
> On Sun, Jan 18, 2015 at 04:32:59PM +0800, Hui Zhu wrote:
>> From: Hui Zhu <zhuhui@xxxxxxxxxx>
>>
>> The original of this patch [1] is part of Joonsoo's CMA patch series.
>> I made a patch [2] to fix the issue of this patch. Joonsoo reminded me
>> that this issue affect current kernel too. So made a new one for upstream.
>
> Recently, we found many problems of CMA and Joonsoo tried to add more
> hooks into MM like agressive allocation but I suggested adding new zone
> would be more desirable than more hooks in mm fast path in various aspect.
> (ie, remove lots of hooks in hot path of MM, don't need reclaim hooks
> for special CMA pages, don't need custom fair allocation for CMA).
>
> Joonsoo is investigating the direction so please wait.
> If it turns out we have lots of hurdle to go that way,
> this direction(ie, putting more hooks) should be second plan.

OK. Thanks.

Best,
Hui

>
> Thanks.
>
>>
>> Current code treat free cma pages as non-free if not ALLOC_CMA in the first
>> check:
>> if (free_pages - free_cma <= min + z->lowmem_reserve[classzone_idx])
>> return false;
>> But in the loop after that, it treat free cma pages as free memory even
>> if not ALLOC_CMA.
>> So this one substruct free_cma from free_pages before the loop if not
>> ALLOC_CMA to treat free cma pages as non-free in the loop.
>>
>> But there still have a issue is that CMA memory in each order is part
>> of z->free_area[o].nr_free, then the CMA page number of this order is
>> substructed twice. This bug will make __zone_watermark_ok return more false.
>> This patch add cma_nr_free to struct free_area that just record the number
>> of CMA pages. And add it back in the order loop to handle the substruct
>> twice issue.
>>
>> The last issue of this patch should handle is pointed by Joonsoo in [3].
>> If pageblock for CMA is isolated, cma_nr_free would be miscalculated.
>> This patch add two functions nr_free_inc and nr_free_dec to change the
>> values of nr_free and cma_nr_free. If the migratetype is MIGRATE_ISOLATE,
>> they will not change the value of nr_free.
>> Change __mod_zone_freepage_state to doesn't record isolated page to
>> NR_FREE_PAGES.
>> And add code to move_freepages to record the page number that isolated:
>> if (is_migrate_isolate(migratetype))
>> nr_free_dec(&zone->free_area[order],
>> get_freepage_migratetype(page));
>> else
>> nr_free_inc(&zone->free_area[order], migratetype);
>> Then the isolate issue is handled.
>>
>> This patchset is based on fc7f0dd381720ea5ee5818645f7d0e9dece41cb0.
>>
>> [1] https://lkml.org/lkml/2014/5/28/110
>> [2] https://lkml.org/lkml/2014/12/25/43
>> [3] https://lkml.org/lkml/2015/1/4/220
>>
>> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
>> Signed-off-by: Hui Zhu <zhuhui@xxxxxxxxxx>
>> Signed-off-by: Weixing Liu <liuweixing@xxxxxxxxxx>
>> ---
>> include/linux/mmzone.h | 3 +++
>> include/linux/vmstat.h | 4 +++-
>> mm/page_alloc.c | 59 +++++++++++++++++++++++++++++++++++++++++---------
>> 3 files changed, 55 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> index 2f0856d..094476b 100644
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -92,6 +92,9 @@ static inline int get_pfnblock_migratetype(struct page *page, unsigned long pfn)
>> struct free_area {
>> struct list_head free_list[MIGRATE_TYPES];
>> unsigned long nr_free;
>> +#ifdef CONFIG_CMA
>> + unsigned long cma_nr_free;
>> +#endif
>> };
>>
>> struct pglist_data;
>> diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
>> index 82e7db7..f18ef00 100644
>> --- a/include/linux/vmstat.h
>> +++ b/include/linux/vmstat.h
>> @@ -6,6 +6,7 @@
>> #include <linux/mm.h>
>> #include <linux/mmzone.h>
>> #include <linux/vm_event_item.h>
>> +#include <linux/page-isolation.h>
>> #include <linux/atomic.h>
>>
>> extern int sysctl_stat_interval;
>> @@ -280,7 +281,8 @@ static inline void drain_zonestat(struct zone *zone,
>> static inline void __mod_zone_freepage_state(struct zone *zone, int nr_pages,
>> int migratetype)
>> {
>> - __mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages);
>> + if (!is_migrate_isolate(migratetype))
>> + __mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages);
>> if (is_migrate_cma(migratetype))
>> __mod_zone_page_state(zone, NR_FREE_CMA_PAGES, nr_pages);
>> }
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 7633c50..9a2b6da 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -576,6 +576,28 @@ static inline int page_is_buddy(struct page *page, struct page *buddy,
>> return 0;
>> }
>>
>> +static inline void nr_free_inc(struct free_area *area, int migratetype)
>> +{
>> + if (!is_migrate_isolate(migratetype))
>> + area->nr_free++;
>> +
>> +#ifdef CONFIG_CMA
>> + if (is_migrate_cma(migratetype))
>> + area->cma_nr_free++;
>> +#endif
>> +}
>> +
>> +static inline void nr_free_dec(struct free_area *area, int migratetype)
>> +{
>> + if (!is_migrate_isolate(migratetype))
>> + area->nr_free--;
>> +
>> +#ifdef CONFIG_CMA
>> + if (is_migrate_cma(migratetype))
>> + area->cma_nr_free--;
>> +#endif
>> +}
>> +
>> /*
>> * Freeing function for a buddy system allocator.
>> *
>> @@ -649,7 +671,7 @@ static inline void __free_one_page(struct page *page,
>> clear_page_guard(zone, buddy, order, migratetype);
>> } else {
>> list_del(&buddy->lru);
>> - zone->free_area[order].nr_free--;
>> + nr_free_dec(&zone->free_area[order], migratetype);
>> rmv_page_order(buddy);
>> }
>> combined_idx = buddy_idx & page_idx;
>> @@ -682,7 +704,7 @@ static inline void __free_one_page(struct page *page,
>>
>> list_add(&page->lru, &zone->free_area[order].free_list[migratetype]);
>> out:
>> - zone->free_area[order].nr_free++;
>> + nr_free_inc(&zone->free_area[order], migratetype);
>> }
>>
>> static inline int free_pages_check(struct page *page)
>> @@ -936,7 +958,7 @@ static inline void expand(struct zone *zone, struct page *page,
>> continue;
>> }
>> list_add(&page[size].lru, &area->free_list[migratetype]);
>> - area->nr_free++;
>> + nr_free_inc(area, migratetype);
>> set_page_order(&page[size], high);
>> }
>> }
>> @@ -1019,7 +1041,7 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order,
>> struct page, lru);
>> list_del(&page->lru);
>> rmv_page_order(page);
>> - area->nr_free--;
>> + nr_free_dec(area, migratetype);
>> expand(zone, page, order, current_order, area, migratetype);
>> set_freepage_migratetype(page, migratetype);
>> return page;
>> @@ -1089,6 +1111,11 @@ int move_freepages(struct zone *zone,
>> order = page_order(page);
>> list_move(&page->lru,
>> &zone->free_area[order].free_list[migratetype]);
>> + if (is_migrate_isolate(migratetype))
>> + nr_free_dec(&zone->free_area[order],
>> + get_freepage_migratetype(page));
>> + else
>> + nr_free_inc(&zone->free_area[order], migratetype);
>> set_freepage_migratetype(page, migratetype);
>> page += 1 << order;
>> pages_moved += 1 << order;
>> @@ -1207,7 +1234,7 @@ __rmqueue_fallback(struct zone *zone, unsigned int order, int start_migratetype)
>>
>> page = list_entry(area->free_list[migratetype].next,
>> struct page, lru);
>> - area->nr_free--;
>> + nr_free_dec(area, migratetype);
>>
>> new_type = try_to_steal_freepages(zone, page,
>> start_migratetype,
>> @@ -1596,7 +1623,7 @@ int __isolate_free_page(struct page *page, unsigned int order)
>>
>> /* Remove page from free list */
>> list_del(&page->lru);
>> - zone->free_area[order].nr_free--;
>> + nr_free_dec(&zone->free_area[order], mt);
>> rmv_page_order(page);
>>
>> /* Set the pageblock if the isolated page is at least a pageblock */
>> @@ -1808,7 +1835,6 @@ static bool __zone_watermark_ok(struct zone *z, unsigned int order,
>> /* free_pages may go negative - that's OK */
>> long min = mark;
>> int o;
>> - long free_cma = 0;
>>
>> free_pages -= (1 << order) - 1;
>> if (alloc_flags & ALLOC_HIGH)
>> @@ -1818,15 +1844,24 @@ static bool __zone_watermark_ok(struct zone *z, unsigned int order,
>> #ifdef CONFIG_CMA
>> /* If allocation can't use CMA areas don't use free CMA pages */
>> if (!(alloc_flags & ALLOC_CMA))
>> - free_cma = zone_page_state(z, NR_FREE_CMA_PAGES);
>> + free_pages -= zone_page_state(z, NR_FREE_CMA_PAGES);
>> #endif
>>
>> - if (free_pages - free_cma <= min + z->lowmem_reserve[classzone_idx])
>> + if (free_pages <= min + z->lowmem_reserve[classzone_idx])
>> return false;
>> for (o = 0; o < order; o++) {
>> /* At the next order, this order's pages become unavailable */
>> free_pages -= z->free_area[o].nr_free << o;
>>
>> +#ifdef CONFIG_CMA
>> + /* If CMA's page number of this order was substructed as part
>> + of "zone_page_state(z, NR_FREE_CMA_PAGES)", subtracting
>> + "z->free_area[o].nr_free << o" substructed CMA's page
>> + number of this order again. So add it back. */
>> + if (!(alloc_flags & ALLOC_CMA)) {
>> + free_pages += z->free_area[o].cma_nr_free << o;
>> +#endif
>> +
>> /* Require fewer higher order pages to be free */
>> min >>= 1;
>>
>> @@ -4238,6 +4273,9 @@ static void __meminit zone_init_free_lists(struct zone *zone)
>> for_each_migratetype_order(order, t) {
>> INIT_LIST_HEAD(&zone->free_area[order].free_list[t]);
>> zone->free_area[order].nr_free = 0;
>> +#ifdef CONFIG_CMA
>> + zone->free_area[order].cma_nr_free = 0;
>> +#endif
>> }
>> }
>>
>> @@ -6609,7 +6647,8 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)
>> #endif
>> list_del(&page->lru);
>> rmv_page_order(page);
>> - zone->free_area[order].nr_free--;
>> + nr_free_dec(&zone->free_area[order],
>> + get_pageblock_migratetype(page));
>> for (i = 0; i < (1 << order); i++)
>> SetPageReserved((page+i));
>> pfn += (1 << order);
>> --
>> 1.9.3
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@xxxxxxxxxx For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Lee Jones: "Re: [PATCH 08/10] mfd: rtsx: add support for rts524A"
Previous message: Stephen Rothwell: "Re: linux-next: build failure after merge of the akpm-current tree"
In reply to: Minchan Kim: "Re: [PATCH] CMA: treat free cma pages as non-free if not ALLOC_CMA on watermark checking"
Next in thread: Zhang Yanfei: "Re: [PATCH] CMA: treat free cma pages as non-free if not ALLOC_CMA on watermark checking"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]