Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751171AbaKDAmu (ORCPT ); Mon, 3 Nov 2014 19:42:50 -0500 Received: from LGEMRELSE7Q.lge.com ([156.147.1.151]:55800 "EHLO lgemrelse7q.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750901AbaKDAmt (ORCPT ); Mon, 3 Nov 2014 19:42:49 -0500 X-Original-SENDERIP: 10.177.222.213 X-Original-MAILFROM: iamjoonsoo.kim@lge.com Date: Tue, 4 Nov 2014 09:44:21 +0900 From: Joonsoo Kim To: Heesub Shin Cc: Andrew Morton , "Kirill A. Shutemov" , Rik van Riel , Peter Zijlstra , Mel Gorman , Johannes Weiner , Minchan Kim , Yasuaki Ishimatsu , Zhang Yanfei , Tang Chen , Naoya Horiguchi , Bartlomiej Zolnierkiewicz , Wen Congyang , Marek Szyprowski , Michal Nazarewicz , Laura Abbott , "Aneesh Kumar K.V" , Ritesh Harjani , Gioh Kim , Vlastimil Babka , linux-mm@kvack.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH v5 2/4] mm/page_alloc: add freepage on isolate pageblock to correct buddy list Message-ID: <20141104004421.GD8412@js1304-P5Q-DELUXE> References: <1414740330-4086-1-git-send-email-iamjoonsoo.kim@lge.com> <1414740330-4086-3-git-send-email-iamjoonsoo.kim@lge.com> <54573B3B.4070500@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54573B3B.4070500@samsung.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 03, 2014 at 05:22:19PM +0900, Heesub Shin wrote: > Hello, > > On 10/31/2014 04:25 PM, Joonsoo Kim wrote: > >In free_pcppages_bulk(), we use cached migratetype of freepage > >to determine type of buddy list where freepage will be added. > >This information is stored when freepage is added to pcp list, so > >if isolation of pageblock of this freepage begins after storing, > >this cached information could be stale. In other words, it has > >original migratetype rather than MIGRATE_ISOLATE. > > > >There are two problems caused by this stale information. One is that > >we can't keep these freepages from being allocated. Although this > >pageblock is isolated, freepage will be added to normal buddy list > >so that it could be allocated without any restriction. And the other > >problem is incorrect freepage accounting. Freepages on isolate pageblock > >should not be counted for number of freepage. > > > >Following is the code snippet in free_pcppages_bulk(). > > > >/* MIGRATE_MOVABLE list may include MIGRATE_RESERVEs */ > >__free_one_page(page, page_to_pfn(page), zone, 0, mt); > >trace_mm_page_pcpu_drain(page, 0, mt); > >if (likely(!is_migrate_isolate_page(page))) { > > __mod_zone_page_state(zone, NR_FREE_PAGES, 1); > > if (is_migrate_cma(mt)) > > __mod_zone_page_state(zone, NR_FREE_CMA_PAGES, 1); > >} > > > >As you can see above snippet, current code already handle second problem, > >incorrect freepage accounting, by re-fetching pageblock migratetype > >through is_migrate_isolate_page(page). But, because this re-fetched > >information isn't used for __free_one_page(), first problem would not be > >solved. This patch try to solve this situation to re-fetch pageblock > >migratetype before __free_one_page() and to use it for __free_one_page(). > > > >In addition to move up position of this re-fetch, this patch use > >optimization technique, re-fetching migratetype only if there is > >isolate pageblock. Pageblock isolation is rare event, so we can > >avoid re-fetching in common case with this optimization. > > > >This patch also correct migratetype of the tracepoint output. > > > >Cc: > >Acked-by: Minchan Kim > >Acked-by: Michal Nazarewicz > >Acked-by: Vlastimil Babka > >Signed-off-by: Joonsoo Kim > >--- > > mm/page_alloc.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > >diff --git a/mm/page_alloc.c b/mm/page_alloc.c > >index f7a867e..6df23fe 100644 > >--- a/mm/page_alloc.c > >+++ b/mm/page_alloc.c > >@@ -725,14 +725,17 @@ static void free_pcppages_bulk(struct zone *zone, int count, > > /* must delete as __free_one_page list manipulates */ > > list_del(&page->lru); > > mt = get_freepage_migratetype(page); > >+ if (unlikely(has_isolate_pageblock(zone))) { > > How about adding an additional check for 'mt == MIGRATE_MOVABLE' > here? Then, most of get_pageblock_migratetype() calls could be > avoided while the isolation is in progress. I am not sure this is > the case on memory offlining. How do you think? Hello, Isolation could be invoked to other migratetype pageblock. You can reference has_unmovable_pages() in page_alloc.c. So, additional check 'mt == MIGRATE_MOVABLE' should not be inserted. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/