Re: [stable] 2.6.23 boot failures on x86-64.

[Andy Whitcroft <apw@shadowen.org>]

On Mon, Oct 29, 2007 at 11:37:40AM -0700, Linus Torvalds wrote:
> 
> 
> On Mon, 29 Oct 2007, Greg KH wrote:
> > 
> > I'll be glad to revert it in -stable, if it's also reverted in Linus's
> > tree first :)
> 
> We've had some changes since 2.6.23, and afaik, the 
> "alloc_bootmem_high_node()" code is alreadt effectively dead there. It's 
> only called if CONFIG_SPARSEMEM_VMEMMAP is *not* enabled, and I *think* we 
> enable it by force on x86-64 these days.

CONFIG_SPARSEMEM_VMEMMAP is the default when SPARSEMEM is enabled
on x86_64.   The overall default remains DISCONTIGMEM, mainly as a
safety measure while the i386/x86_64 => x86 merge stablises.  But yes
this code is only used when SPARSEMEM is enabled but VMEMMAP is not.
So it is effectivly redundant.

> More people added to Cc, just to clarify whether I'm just confused.
> 
> Andy, Christoph, Mel: commit 2e1c49db4c640b35df13889b86b9d62215ade4b6 aka 
> "x86_64: allocate sparsemem memmap above 4G" is the one that causes the 
> failures, just fyi.

That patch seems to have a laudable goal of trying to push the memory
which backs the sparsemem memmap out to non-dma memory.  I would have
expected that call to actually succeed as the bootmem allocator seems to
try at the goal which would likely be outside the node on a small
machine, and then retry without a goal.  Which is what the code without
the goal does.  Most illogical.

> Martin - it would be great if you could try out your failing machine with 
> 2.6.24-rc1 (or a nightly snapshot or current git.. the more recent the 
> better).
> 
> But if I'm right, that commit should be reverted from 2.6.24 just because 
> it's pointless (even if the bug itself is gone). And if I'm wrong, it 
> should be reverted. So something like the appended would make sense 
> regardless.
> 
> Can I get a "tested-by"? And/or ack/nack's on my half-arsed theory above?

This code is definatly only used when SPARSEMEM is enabled, and VMEMMAP
is not which is not a combination we see on x86_64.

Acked-by: Andy Whitcroft <apw@shadowen.org>

> 		Linus
> --
> From: Linus Torvalds <torvalds@woody.linux-foundation.org>
> 
> Revert "x86_64: allocate sparsemem memmap above 4G"
> 
> This reverts commit 2e1c49db4c640b35df13889b86b9d62215ade4b6, since
> testing in Fedora has shown it to cause boot failures, as per Dave
> Jones. Bisected down by Martin Ebourne.
> 
> Cc: Dave Jones <davej@redhat.com>
> Cc: Martin Ebourne <fedora@ebourne.me.uk>
> Cc: Zou Nan hai <nanhai.zou@intel.com>
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> 
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 1e3862e..a7308b2 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -728,12 +728,6 @@ int in_gate_area_no_task(unsigned long addr)
>  	return (addr >= VSYSCALL_START) && (addr < VSYSCALL_END);
>  }
>  
> -void * __init alloc_bootmem_high_node(pg_data_t *pgdat, unsigned long size)
> -{
> -	return __alloc_bootmem_core(pgdat->bdata, size,
> -			SMP_CACHE_BYTES, (4UL*1024*1024*1024), 0);
> -}
> -
>  const char *arch_vma_name(struct vm_area_struct *vma)
>  {
>  	if (vma->vm_mm && vma->vm_start == (long)vma->vm_mm->context.vdso)
> diff --git a/include/linux/bootmem.h b/include/linux/bootmem.h
> index c83534e..0365ec9 100644
> --- a/include/linux/bootmem.h
> +++ b/include/linux/bootmem.h
> @@ -59,7 +59,6 @@ extern void *__alloc_bootmem_core(struct bootmem_data *bdata,
>  				  unsigned long align,
>  				  unsigned long goal,
>  				  unsigned long limit);
> -extern void *alloc_bootmem_high_node(pg_data_t *pgdat, unsigned long size);
>  
>  #ifndef CONFIG_HAVE_ARCH_BOOTMEM_NODE
>  extern void reserve_bootmem(unsigned long addr, unsigned long size);
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 08fb14f..e06f514 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -220,12 +220,6 @@ static int __meminit sparse_init_one_section(struct mem_section *ms,
>  	return 1;
>  }
>  
> -__attribute__((weak)) __init
> -void *alloc_bootmem_high_node(pg_data_t *pgdat, unsigned long size)
> -{
> -	return NULL;
> -}
> -
>  static unsigned long usemap_size(void)
>  {
>  	unsigned long size_bytes;
> @@ -267,11 +261,6 @@ struct page __init *sparse_mem_map_populate(unsigned long pnum, int nid)
>  	if (map)
>  		return map;
>  
> -  	map = alloc_bootmem_high_node(NODE_DATA(nid),
> -                       sizeof(struct page) * PAGES_PER_SECTION);
> -	if (map)
> -		return map;
> -
>  	map = alloc_bootmem_node(NODE_DATA(nid),
>  			sizeof(struct page) * PAGES_PER_SECTION);
>  	return map;

-apw