lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:	Sat, 23 Apr 2011 15:08:23 +0200
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Michael Schmitz <schmitzmic@...glemail.com>
Cc:	Thorsten Glaser <tg@...bsd.de>,
	"Linux/m68k" <linux-m68k@...r.kernel.org>,
	David Rientjes <rientjes@...gle.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: Fix for SLUB? (was: Fwd: [PATCH v3] mm: make expand_downwards
 symmetrical to expand_upwards)

[Added some CCs]

On Sat, Apr 23, 2011 at 05:47, Michael Schmitz
<schmitzmic@...glemail.com> wrote:
> Hi,
>
> node_present_pages(node) returns false:
>
> m68k_setup_node: node 0 addr 0 size 14680064
> m68k_setup_node: node 0 not present!
> m68k_setup_node: node 1 addr 16777216 size 268435456
> m68k_setup_node: node 1 not present!
>
> Changing the patch to
>
> diff --git a/arch/m68k/mm/init_mm.c b/arch/m68k/mm/init_mm.c
> --- a/arch/m68k/mm/init_mm.c
> +++ b/arch/m68k/mm/init_mm.c
> @@ -59,6 +59,7 @@ void __init m68k_setup_node(int node)
>       }
>  #endif
>       pg_data_map[node].bdata = bootmem_node_data + node;
> +       node_set_state(node, N_NORMAL_MEMORY);
>       node_set_online(node);
>  }
>
> i.e. ignoring the node_present_pages return value does result in a
> booting kernel even with the problematic commit included.
>
> I'll leave it to the mm experts to explain why node_present_pages
> returns zero here.
>
> Cheers,
>
>  Michael
>
>
>
> On Sat, Apr 23, 2011 at 2:14 PM, Michael Schmitz
> <schmitzmic@...glemail.com> wrote:
>> Looks like that wasn't helping after all. I still need to revert said
>> commit. Guess I'll have to check what node_present_pages(node) returns
>> in each case ...
>>
>> Cheers,
>>
>>  MIchael
>>
>>
>> On Sat, Apr 23, 2011 at 1:31 PM, Michael Schmitz
>> <schmitzmic@...glemail.com> wrote:
>>> I'll check this out - might well be the correct fix for our problems.
>>>
>>> Cheers,
>>>
>>>  Michael
>>>
>>>
>>> On Thu, Apr 21, 2011 at 8:19 PM, Geert Uytterhoeven
>>> <geert@...ux-m68k.org> wrote:
>>>> ---------- Forwarded message ----------
>>>> From: David Rientjes <rientjes@...gle.com>
>>>> Date: Thu, Apr 21, 2011 at 01:12
>>>> Subject: Re: [PATCH v3] mm: make expand_downwards symmetrical to expand_upwards
>>>> To: James Bottomley <James.Bottomley@...senpartnership.com>
>>>> Cc: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>, Pekka Enberg
>>>> <penberg@...nel.org>, Christoph Lameter <cl@...ux.com>, Michal Hocko
>>>> <mhocko@...e.cz>, Andrew Morton <akpm@...ux-foundation.org>, Hugh
>>>> Dickins <hughd@...gle.com>, linux-mm@...ck.org, LKML
>>>> <linux-kernel@...r.kernel.org>, linux-parisc@...r.kernel.org, Ingo
>>>> Molnar <mingo@...e.hu>, x86 maintainers <x86@...nel.org>
>>>>
>>>>
>>>> On Wed, 20 Apr 2011, James Bottomley wrote:
>>>>
>>>>> > This is probably because the parisc's DISCONTIGMEM memory ranges don't
>>>>> > have bits set in N_NORMAL_MEMORY.
>>>>> >
>>>>> > diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
>>>>> > --- a/arch/parisc/mm/init.c
>>>>> > +++ b/arch/parisc/mm/init.c
>>>>> > @@ -266,8 +266,10 @@ static void __init setup_bootmem(void)
>>>>> >     }
>>>>> >     memset(pfnnid_map, 0xff, sizeof(pfnnid_map));
>>>>> >
>>>>> > -   for (i = 0; i < npmem_ranges; i++)
>>>>> > +   for (i = 0; i < npmem_ranges; i++) {
>>>>> > +           node_set_state(i, N_NORMAL_MEMORY);
>>>>> >             node_set_online(i);
>>>>> > +   }
>>>>> >  #endif
>>>>>
>>>>> Yes, this seems to be the missing piece that gets it to boot.  We really
>>>>> need this in generic code, unless someone wants to run through all the
>>>>> other arch's doing it ...
>>>>>
>>>>
>>>> Looking at all other architectures that allow ARCH_DISCONTIGMEM_ENABLE, we
>>>> already know x86 is fine, avr32 disables ARCH_DISCONTIGMEM_ENABLE entirely
>>>> because its code only brings online node 0, and tile already sets the bit
>>>> in N_NORMAL_MEMORY correctly when bringing a node online, probably because
>>>> it was introduced after the various node state masks were added in
>>>> 7ea1530ab3fd back in October 2007.
>>>>
>>>> So we're really only talking about alpha, ia64, m32r, m68k, and mips and
>>>> it only seems to matter when using CONFIG_SLUB, which isn't surprising
>>>> when greping for it:
>>>>
>>>>        $ grep -r N_NORMAL_MEMORY mm/*
>>>>        mm/memcontrol.c:        if (!node_state(node, N_NORMAL_MEMORY))
>>>>        mm/memcontrol.c:                if (!node_state(node, N_NORMAL_MEMORY))
>>>>        mm/page_alloc.c:        [N_NORMAL_MEMORY] = { { [0] = 1UL } },
>>>>        mm/page_alloc.c:
>>>> node_set_state(zone_to_nid(zone), N_NORMAL_MEMORY);
>>>>        mm/slub.c:      for_each_node_state(node, N_NORMAL_MEMORY) {
>>>>        mm/slub.c:      for_each_node_state(node, N_NORMAL_MEMORY) {
>>>>        mm/slub.c:      for_each_node_state(node, N_NORMAL_MEMORY) {
>>>>        mm/slub.c:      for_each_node_state(node, N_NORMAL_MEMORY) {
>>>>        mm/slub.c:      for_each_node_state(node, N_NORMAL_MEMORY) {
>>>>        mm/slub.c:      for_each_node_state(node, N_NORMAL_MEMORY) {
>>>>        mm/slub.c:      for_each_node_state(node, N_NORMAL_MEMORY) {
>>>>        mm/slub.c:              for_each_node_state(node, N_NORMAL_MEMORY) {
>>>>        mm/slub.c:              for_each_node_state(node, N_NORMAL_MEMORY) {
>>>>        mm/slub.c:      for_each_node_state(node, N_NORMAL_MEMORY)
>>>>
>>>> Those memory controller occurrences only result in it passing a node id of
>>>> -1 to kmalloc_node() which means no specific node target, and that's fine
>>>> for DISCONTIGMEM since we don't care about any proximity between memory
>>>> ranges.
>>>>
>>>> This should fix the remaining architectures so they can use CONFIG_SLUB,
>>>> but I hope it can be tested by the individual arch maintainers like you
>>>> did for parisc.
>>>>
>>>> diff --git a/arch/alpha/mm/numa.c b/arch/alpha/mm/numa.c
>>>> --- a/arch/alpha/mm/numa.c
>>>> +++ b/arch/alpha/mm/numa.c
>>>> @@ -245,6 +245,7 @@ setup_memory_node(int nid, void *kernel_end)
>>>>                        bootmap_size, BOOTMEM_DEFAULT);
>>>>        printk(" reserving pages %ld:%ld\n", bootmap_start,
>>>> bootmap_start+PFN_UP(bootmap_size));
>>>>
>>>> +       node_set_state(nid, N_NORMAL_MEMORY);
>>>>        node_set_online(nid);
>>>>  }
>>>>
>>>> diff --git a/arch/ia64/mm/discontig.c b/arch/ia64/mm/discontig.c
>>>> --- a/arch/ia64/mm/discontig.c
>>>> +++ b/arch/ia64/mm/discontig.c
>>>> @@ -573,6 +573,8 @@ void __init find_memory(void)
>>>>                                  map>>PAGE_SHIFT,
>>>>                                  bdp->node_min_pfn,
>>>>                                  bdp->node_low_pfn);
>>>> +               if (node_present_pages(node))
>>>> +                       node_set_state(node, N_NORMAL_MEMORY);
>>>>        }
>>>>
>>>>        efi_memmap_walk(filter_rsvd_memory, free_node_bootmem);
>>>> diff --git a/arch/m32r/kernel/setup.c b/arch/m32r/kernel/setup.c
>>>> --- a/arch/m32r/kernel/setup.c
>>>> +++ b/arch/m32r/kernel/setup.c
>>>> @@ -247,7 +247,9 @@ void __init setup_arch(char **cmdline_p)
>>>>
>>>>  #ifdef CONFIG_DISCONTIGMEM
>>>>        nodes_clear(node_online_map);
>>>> +       node_set_state(0, N_NORMAL_MEMORY);     /* always has memory */
>>>>        node_set_online(0);
>>>> +       node_set_state(1, N_NORMAL_MEMORY);     /* always has memory */
>>>>        node_set_online(1);
>>>>  #endif /* CONFIG_DISCONTIGMEM */
>>>>
>>>> diff --git a/arch/m68k/mm/init_mm.c b/arch/m68k/mm/init_mm.c
>>>> --- a/arch/m68k/mm/init_mm.c
>>>> +++ b/arch/m68k/mm/init_mm.c
>>>> @@ -59,6 +59,8 @@ void __init m68k_setup_node(int node)
>>>>        }
>>>>  #endif
>>>>        pg_data_map[node].bdata = bootmem_node_data + node;
>>>> +       if (node_present_pages(node))
>>>> +               node_set_state(node, N_NORMAL_MEMORY);
>>>>        node_set_online(node);
>>>>  }
>>>>
>>>> diff --git a/arch/mips/sgi-ip27/ip27-memory.c b/arch/mips/sgi-ip27/ip27-memory.c
>>>> --- a/arch/mips/sgi-ip27/ip27-memory.c
>>>> +++ b/arch/mips/sgi-ip27/ip27-memory.c
>>>> @@ -471,6 +471,8 @@ void __init paging_init(void)
>>>>
>>>>                if (end_pfn > max_low_pfn)
>>>>                        max_low_pfn = end_pfn;
>>>> +               if (end_pfn > start_pfn)
>>>> +                       node_set_state(node, N_NORMAL_MEMORY);
>>>>        }
>>>>        zones_size[ZONE_NORMAL] = max_low_pfn;
>>>>        free_area_init_nodes(zones_size);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ