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

Andy Whitcroft
2007-10-29T20:24:43Z

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