From patchwork Wed Jul 4 11:19:35 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mel Gorman X-Patchwork-Id: 1155731 Return-Path: X-Original-To: patchwork-linux-acpi@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id D14ACDFF0F for ; Wed, 4 Jul 2012 11:19:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755024Ab2GDLTk (ORCPT ); Wed, 4 Jul 2012 07:19:40 -0400 Received: from gir.skynet.ie ([193.1.99.77]:56087 "EHLO gir.skynet.ie" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753793Ab2GDLTj (ORCPT ); Wed, 4 Jul 2012 07:19:39 -0400 Received: from csn.ul.ie (skynet.skynet.ie [193.1.99.74]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by gir.skynet.ie (Postfix) with ESMTPS id D7F541157B; Wed, 4 Jul 2012 12:19:36 +0100 (IST) Date: Wed, 4 Jul 2012 12:19:35 +0100 From: Mel Gorman To: Lai Jiangshan Cc: Chris Metcalf , Len Brown , Greg Kroah-Hartman , Andi Kleen , Julia Lawall , David Howells , Benjamin Herrenschmidt , Kay Sievers , Ingo Molnar , Paul Gortmaker , Daniel Kiper , Andrew Morton , Konrad Rzeszutek Wilk , Michal Hocko , KAMEZAWA Hiroyuki , Minchan Kim , Michal Nazarewicz , Marek Szyprowski , Rik van Riel , Bjorn Helgaas , Christoph Lameter , David Rientjes , linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, linux-mm@kvack.org Subject: Re: [RFC PATCH 1/3 V1] mm, page_alloc: use __rmqueue_smallest when borrow memory from MIGRATE_CMA Message-ID: <20120704111935.GN13141@csn.ul.ie> References: <1341386778-8002-1-git-send-email-laijs@cn.fujitsu.com> <1341386778-8002-2-git-send-email-laijs@cn.fujitsu.com> <20120704101737.GL13141@csn.ul.ie> <4FF41E63.4020303@cn.fujitsu.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4FF41E63.4020303@cn.fujitsu.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Wed, Jul 04, 2012 at 06:43:47PM +0800, Lai Jiangshan wrote: > On 07/04/2012 06:17 PM, Mel Gorman wrote: > > On Wed, Jul 04, 2012 at 03:26:16PM +0800, Lai Jiangshan wrote: > >> The pages of MIGRATE_CMA can't not be changed to the other type, > >> nor be moved to the other free list. > >> > >> ==> > >> So when we use __rmqueue_fallback() to borrow memory from MIGRATE_CMA, > >> one of the highest order page is borrowed and it is split. > >> But the free pages resulted by splitting can NOT > >> be moved to MIGRATE_MOVABLE. > >> > >> ==> > >> So in the next time of allocation, we NEED to borrow again, > >> another one of the highest order page is borrowed from CMA and it is split. > >> and results some other new split free pages. > >> > > > > Then special case __rmqueue_fallback() to move pages stolen from > > MIGRATE_CMA to the MIGRATE_MOVABLE lists but do not change the pageblock > > type. > > Because unmovable-page-requirement can allocate page from > MIGRATE_MOVABLE free list. So We can not move MIGRATE_CMA pages > to the MIGRATE_MOVABLE free list. > Ok, good point. > See here: > > MOVABLE list is empty > UNMOVABLE list is empty > movable-page-requirement > borrow from CMA list > split it, others are put into UNMOVABLE list > unmovable-page-requiremnt > borrow from UNMOVABLE list > NOW, it is BUG, we use CMA pages for unmovable usage. > The patch still looks unnecessarily complex for what you are trying to achieve and as a result I'm not reviewing it as carefully as I should. It looks like the entire patch boiled down to this hunk here +#ifdef CONFIG_CMA + if (unlikely(!page) && migratetype == MIGRATE_MOVABLE) + page = __rmqueue_smallest(zone, order, MIGRATE_CMA); +#endif + With that in place, this would would need to change from [MIGRATE_MOVABLE] = { MIGRATE_CMA, MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_RESERVE }, to [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_RESERVE }, because the fallback is already being handled as a special case. Leave the other fallback logic as it is. This is not tested at all and is only meant to illustrate why I think your patch looks excessively complex for what you are trying to achieve. --- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 4beb7ae..0063e93 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -895,11 +895,9 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order, static int fallbacks[MIGRATE_TYPES][4] = { [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE, MIGRATE_RESERVE }, [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE, MIGRATE_RESERVE }, + [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_RESERVE }, #ifdef CONFIG_CMA - [MIGRATE_MOVABLE] = { MIGRATE_CMA, MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_RESERVE }, [MIGRATE_CMA] = { MIGRATE_RESERVE }, /* Never used */ -#else - [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_RESERVE }, #endif [MIGRATE_RESERVE] = { MIGRATE_RESERVE }, /* Never used */ [MIGRATE_ISOLATE] = { MIGRATE_RESERVE }, /* Never used */ @@ -1076,6 +1074,20 @@ static struct page *__rmqueue(struct zone *zone, unsigned int order, retry_reserve: page = __rmqueue_smallest(zone, order, migratetype); +#ifdef CONFIG_CMA + if (!unlikely(!page) && migratetype == MIGRATE_MOVABLE) { + + /* + * CMA is a special case where we want to use + * the smallest available page instead of splitting + * the largest chunks. We still must avoid the pages + * moving to MIGRATE_MOVABLE where they might be + * used for UNRECLAIMABLE or UNMOVABLE allocations + */ + migratetype = MIGRATE_CMA; + goto retry_reserve; + } +#endif /* CONFIG_CMA */ if (unlikely(!page) && migratetype != MIGRATE_RESERVE) { page = __rmqueue_fallback(zone, order, migratetype);