Re: [PATCH - resend] Memory-Hotplug: Fix the bug on interface/dev/mem for 64-bit kernel(v1)

[Wu Fengguang]

On Wed, Jan 13, 2010 at 07:01:47AM +0800, Yinghai Lu wrote:
> On Tue, Jan 12, 2010 at 5:35 AM, Wu Fengguang <fengguang.wu@xxxxxxxxx> wrote:
> > On Tue, Jan 12, 2010 at 10:39:03AM +0800, KAMEZAWA Hiroyuki wrote:
> >> On Tue, 12 Jan 2010 10:33:08 +0800
> >> Wu Fengguang <fengguang.wu@xxxxxxxxx> wrote:
> >>
> >> > Sure, here it is :)
> >> > ---
> >> > x86: use the generic page_is_ram()
> >> >
> >> > The generic resource based page_is_ram() works better with memory
> >> > hotplug/hotremove. So switch the x86 e820map based code to it.
> >> >
> >> > CC: Andi Kleen <andi@xxxxxxxxxxxxxx>
> >> > CC: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> >> > Signed-off-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
> >>
> >> Ack.
> >
> > Thank you.
> >
> >>
> >> > +#ifdef CONFIG_X86
> >> > + Â /*
> >> > + Â Â* A special case is the first 4Kb of memory;
> >> > + Â Â* This is a BIOS owned area, not kernel ram, but generally
> >> > + Â Â* not listed as such in the E820 table.
> >> > + Â Â*/
> >> > + Â if (pfn == 0)
> >> > + Â Â Â Â Â return 0;
> >> > +
> >> > + Â /*
> >> > + Â Â* Second special case: Some BIOSen report the PC BIOS
> >> > + Â Â* area (640->1Mb) as ram even though it is not.
> >> > + Â Â*/
> >> > + Â if (pfn >= (BIOS_BEGIN >> PAGE_SHIFT) &&
> >> > + Â Â Â pfn < Â(BIOS_END Â >> PAGE_SHIFT))
> >> > + Â Â Â Â Â return 0;
> >> > +#endif
> >>
> >> I'm glad if this part is sorted out in clean way ;)
> >
> > Two possible solutions are:
> >
> > - to exclude the above two ranges directly in e820 map;
> > - to not add the above two ranges into iomem_resource.
> >
> > Yinghai, do you have any suggestions?
> > We want to get rid of the two explicit tests from page_is_ram().
> 
> please check attached patch.
> 
> YH

Thank you, it works!

Content-Description: remove_bios_begin_end.patch
> [PATCH] x86: remove bios data range from e820
> 
> to prepare move page_is_ram as generic one
> 
> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxxx

Malformed email address..

> ---
>  arch/x86/kernel/e820.c   |    8 ++++++++
>  arch/x86/kernel/head32.c |    2 --
>  arch/x86/kernel/head64.c |    2 --
>  arch/x86/kernel/setup.c  |   19 ++++++++++++++++++-
>  arch/x86/mm/ioremap.c    |   16 ----------------
>  5 files changed, 26 insertions(+), 21 deletions(-)
> 
> Index: linux-2.6/arch/x86/kernel/setup.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/setup.c
> +++ linux-2.6/arch/x86/kernel/setup.c
> @@ -657,6 +657,23 @@ static struct dmi_system_id __initdata b
>  	{}
>  };
>  
> +static void __init trim_bios_range(void)

How about e820_trim_bios_range() ?

> +{
> +	/*
> +	 * A special case is the first 4Kb of memory;
> +	 * This is a BIOS owned area, not kernel ram, but generally
> +	 * not listed as such in the E820 table.
> +	 */
> +	e820_update_range(0, PAGE_SIZE, E820_RAM, E820_RESERVED);
> +	/*
> +	 * special case: Some BIOSen report the PC BIOS
> +	 * area (640->1Mb) as ram even though it is not.
> +	 * take them out.
> +	 */
> +	e820_remove_range(BIOS_BEGIN, BIOS_END - BIOS_BEGIN, E820_RAM, 1);
> +	sanitize_e820_map(e820.map, ARRAY_SIZE(e820.map), &e820.nr_map);
> +}
> +


> Index: linux-2.6/arch/x86/kernel/head32.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/head32.c
> +++ linux-2.6/arch/x86/kernel/head32.c
> @@ -29,8 +29,6 @@ static void __init i386_default_early_se
>  
>  void __init i386_start_kernel(void)
>  {
> -	reserve_early_overlap_ok(0, PAGE_SIZE, "BIOS data page");
> -
>  #ifdef CONFIG_X86_TRAMPOLINE
>  	/*
>  	 * But first pinch a few for the stack/trampoline stuff
> Index: linux-2.6/arch/x86/kernel/head64.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/head64.c
> +++ linux-2.6/arch/x86/kernel/head64.c
> @@ -98,8 +98,6 @@ void __init x86_64_start_reservations(ch
>  {
>  	copy_bootdata(__va(real_mode_data));
>  
> -	reserve_early_overlap_ok(0, PAGE_SIZE, "BIOS data page");
> -
>  	reserve_early(__pa_symbol(&_text), __pa_symbol(&__bss_stop), "TEXT DATA BSS");
>  
>  #ifdef CONFIG_BLK_DEV_INITRD

The above two trunks don't apply in latest linux-next.
Not a big problem for my test though.

Thanks,
Fengguang
--
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/