Re: Fix for SLUB? (was: Fwd: [PATCH v3] mm: make expand_downwards symmetrical to expand_upwards)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[Added some CCs]

On Sat, Apr 23, 2011 at 05:47, Michael Schmitz
<schmitzmic@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> wrote:
---------- Forwarded message ----------
From: David Rientjes <rientjes@xxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxx>
Cc: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>, Pekka Enberg
<penberg@xxxxxxxxxx>, Christoph Lameter <cl@xxxxxxxxx>, Michal Hocko
<mhocko@xxxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, Hugh
Dickins <hughd@xxxxxxxxxx>, linux-mm@xxxxxxxxx, LKML
<linux-kernel@xxxxxxxxxxxxxxx>, linux-parisc@xxxxxxxxxxxxxxx, Ingo
Molnar <mingo@xxxxxxx>, x86 maintainers <x86@xxxxxxxxxx>


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@xxxxxxxxxxxxxx

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-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux