From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752187Ab0CINMj (ORCPT ); Tue, 9 Mar 2010 08:12:39 -0500 Received: from mga09.intel.com ([134.134.136.24]:58046 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751255Ab0CINMg (ORCPT ); Tue, 9 Mar 2010 08:12:36 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.49,608,1262592000"; d="scan'208";a="499078160" Date: Tue, 9 Mar 2010 21:12:31 +0800 From: Wu Fengguang To: Yinghai Lu Cc: "mingo@redhat.com" , "hpa@zytor.com" , "linux-kernel@vger.kernel.org" , "sachinp@in.ibm.com" , "tglx@linutronix.de" , "kamezawa.hiroyu@jp.fujitsu.com" , "linux-tip-commits@vger.kernel.org" Subject: Re: [tip:x86/mm] resource: Fix generic page_is_ram() for partial RAM pages Message-ID: <20100309131231.GA7620@localhost> References: <20100301135551.GA9998@localhost> <4B8C51A3.8020307@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4B8C51A3.8020307@kernel.org> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 02, 2010 at 07:45:39AM +0800, Yinghai Lu wrote: > On 03/01/2010 11:00 AM, tip-bot for Wu Fengguang wrote: > > Commit-ID: 37b99dd5372cff42f83210c280f314f10f99138e > > Gitweb: http://git.kernel.org/tip/37b99dd5372cff42f83210c280f314f10f99138e > > Author: Wu Fengguang > > AuthorDate: Mon, 1 Mar 2010 21:55:51 +0800 > > Committer: H. Peter Anvin > > CommitDate: Mon, 1 Mar 2010 10:18:32 -0800 > > > > resource: Fix generic page_is_ram() for partial RAM pages > > > > The System RAM walk shall skip partial RAM pages and avoid calling > > func() on them. So that page_is_ram() return 0 for a partial RAM page. > > > > In particular, it shall not call func() with len=0. > > This fixes a boot time bug reported by Sachin and root caused by Thomas: > > > >>>>> WARNING: at arch/x86/mm/ioremap.c:111 __ioremap_caller+0x169/0x2f1() > >>>>> Hardware name: BladeCenter LS21 -[79716AA]- > >>>>> Modules linked in: > >>>>> Pid: 0, comm: swapper Not tainted 2.6.33-git6-autotest #1 > >>>>> Call Trace: > >>>>> [] ? __ioremap_caller+0x169/0x2f1 > >>>>> [] warn_slowpath_common+0x77/0xa4 > >>>>> [] warn_slowpath_null+0xf/0x11 > >>>>> [] __ioremap_caller+0x169/0x2f1 > >>>>> [] ? acpi_os_map_memory+0x12/0x1b > >>>>> [] ioremap_nocache+0x12/0x14 > >>>>> [] acpi_os_map_memory+0x12/0x1b > >>>>> [] acpi_tb_verify_table+0x29/0x5b > >>>>> [] acpi_load_tables+0x39/0x15a > >>>>> [] acpi_early_init+0x60/0xf5 > >>>>> [] start_kernel+0x397/0x3a7 > >>>>> [] x86_64_start_reservations+0xa5/0xa9 > >>>>> [] x86_64_start_kernel+0xe1/0xe8 > >>>>> ---[ end trace 4eaa2a86a8e2da22 ]--- > >>>>> ioremap reserve_memtype failed -22 > > > > The return code is -EINVAL, so it failed in the is_ram check, which is > > not too surprising > > > >> BIOS-provided physical RAM map: > >> BIOS-e820: 0000000000000000 - 000000000009c000 (usable) > >> BIOS-e820: 000000000009c000 - 00000000000a0000 (reserved) > >> BIOS-e820: 00000000000e0000 - 0000000000100000 (reserved) > >> BIOS-e820: 0000000000100000 - 00000000cffa3900 (usable) > >> BIOS-e820: 00000000cffa3900 - 00000000cffa7400 (ACPI data) > > > > The ACPI data is not starting on a page boundary and neither does the > > usable RAM area end on a page boundary. Very useful ! > > > >> ACPI: DSDT 00000000cffa3900 036CE (v01 IBM SERLEWIS 00001000 INTL 20060912) > > > > ACPI is trying to map DSDT at cffa3900, which results in a check > > vs. cffa3000 which is the relevant page boundary. The generic is_ram > > check correctly identifies that as RAM because it's in the usable > > resource area. The old e820 based is_ram check does not take > > overlapping resource areas into account. That's why it works. > > > > CC: Sachin Sant > > CC: Thomas Gleixner > > CC: KAMEZAWA Hiroyuki > > Signed-off-by: Wu Fengguang > > LKML-Reference: <20100301135551.GA9998@localhost> > > Signed-off-by: H. Peter Anvin > > --- > > kernel/resource.c | 9 +++++---- > > 1 files changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/kernel/resource.c b/kernel/resource.c > > index 03c897f..8f0e3d0 100644 > > --- a/kernel/resource.c > > +++ b/kernel/resource.c > > @@ -274,7 +274,7 @@ int walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages, > > void *arg, int (*func)(unsigned long, unsigned long, void *)) > > { > > struct resource res; > > - unsigned long pfn, len; > > + unsigned long pfn, end_pfn; > > u64 orig_end; > > int ret = -1; > > > > @@ -284,9 +284,10 @@ int walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages, > > orig_end = res.end; > > while ((res.start < res.end) && > > (find_next_system_ram(&res, "System RAM") >= 0)) { > > - pfn = (unsigned long)(res.start >> PAGE_SHIFT); > > - len = (unsigned long)((res.end + 1 - res.start) >> PAGE_SHIFT); > > - ret = (*func)(pfn, len, arg); > > + pfn = (res.start + PAGE_SIZE - 1) >> PAGE_SHIFT; > > + end_pfn = (res.end + 1) >> PAGE_SHIFT; > > + if (end_pfn > pfn) > > + ret = (*func)(pfn, end_pfn - pfn, arg); > > if (ret) > > break; > > res.start = res.end + 1; > > -- > > wonder if we should trim the ram earlier. It will actually yield more efficient walk_system_ram_range(). However the problem is, we have to ensure page alignment for _every_ arch in order to optimize away the alignment checks in walk_system_ram_range(), which makes it less feasible. Thanks, Fengguang