Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759173AbZD3N0E (ORCPT ); Thu, 30 Apr 2009 09:26:04 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1762098AbZD3NZt (ORCPT ); Thu, 30 Apr 2009 09:25:49 -0400 Received: from gir.skynet.ie ([193.1.99.77]:37824 "EHLO gir.skynet.ie" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761517AbZD3NZt (ORCPT ); Thu, 30 Apr 2009 09:25:49 -0400 Date: Thu, 30 Apr 2009 14:25:44 +0100 From: Mel Gorman To: Hugh Dickins Cc: Andrew Morton , Andi Kleen , David Miller , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH mmotm] mm: alloc_large_system_hash check order Message-ID: <20090430132544.GB21997@csn.ul.ie> References: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17+20080114 (2008-01-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2919 Lines: 83 On Wed, Apr 29, 2009 at 10:09:48PM +0100, Hugh Dickins wrote: > On an x86_64 with 4GB ram, tcp_init()'s call to alloc_large_system_hash(), > to allocate tcp_hashinfo.ehash, is now triggering an mmotm WARN_ON_ONCE on > order >= MAX_ORDER - it's hoping for order 11. alloc_large_system_hash() > had better make its own check on the order. > > Signed-off-by: Hugh Dickins Looks good Reviewed-by: Mel Gorman As I was looking there, it seemed that alloc_large_system_hash() should be using alloc_pages_exact() instead of having its own "give back the spare pages at the end of the buffer" logic. If alloc_pages_exact() was used, then the check for an order >= MAX_ORDER can be pushed down to alloc_pages_exact() where it may catch other unwary callers. How about adding the following patch on top of yours? ==== CUT HERE ==== Use alloc_pages_exact() in alloc_large_system_hash() to avoid duplicated logic alloc_large_system_hash() has logic for freeing unused pages at the end of an power-of-two-pages-aligned buffer that is a duplicate of what is in alloc_pages_exact(). This patch converts alloc_large_system_hash() to use alloc_pages_exact(). Signed-off-by: Mel Gorman --- mm/page_alloc.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 1b3da0f..c94b140 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1942,6 +1942,9 @@ void *alloc_pages_exact(size_t size, gfp_t gfp_mask) unsigned int order = get_order(size); unsigned long addr; + if (order >= MAX_ORDER) + return NULL; + addr = __get_free_pages(gfp_mask, order); if (addr) { unsigned long alloc_end = addr + (PAGE_SIZE << order); @@ -4755,28 +4758,8 @@ void *__init alloc_large_system_hash(const char *tablename, table = alloc_bootmem_nopanic(size); else if (hashdist) table = __vmalloc(size, GFP_ATOMIC, PAGE_KERNEL); - else { - unsigned long order = get_order(size); - - if (order < MAX_ORDER) - table = (void *)__get_free_pages(GFP_ATOMIC, - order); - /* - * If bucketsize is not a power-of-two, we may free - * some pages at the end of hash table. - */ - if (table) { - unsigned long alloc_end = (unsigned long)table + - (PAGE_SIZE << order); - unsigned long used = (unsigned long)table + - PAGE_ALIGN(size); - split_page(virt_to_page(table), order); - while (used < alloc_end) { - free_page(used); - used += PAGE_SIZE; - } - } - } + else + table = alloc_pages_exact(PAGE_ALIGN(size), GFP_ATOMIC); } while (!table && size > PAGE_SIZE && --log2qty); if (!table) -- 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/