Re: [patch] m68k, mm: set all online nodes in N_NORMAL_MEMORY

[KOSAKI Motohiro]

2011/4/24 Michael Schmitz <schmitzmic@xxxxxxxxxxxxxx>:
> David Rientjes wrote:
>>
>> For m68k, N_NORMAL_MEMORY represents all nodes that have present memory
>> since it does not support HIGHMEM.  This patch sets the bit at the time the
>> node is brought online.
>>
>> If N_NORMAL_MEMORY is not accurate, slub may encounter errors since it
>> uses this nodemask to setup per-cache kmem_cache_node data structures.
>>
>> Signed-off-by: David Rientjes <rientjes@xxxxxxxxxx>
>> ---
>>  arch/m68k/mm/init_mm.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> 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);
>>  }
>>
>
> As Andreas pointed out, node_present_pages is set in free_area_init_node
> which only gets called at the very end of m68k mm paging_init.
>
> The correct patch would be something like this - the need for the
> conditional is perhaps debatable, seeing as we set the pages present just
> before node_set_state.
>
> Tested on my ARAnyM test setup so far. I'd like to wait for an independent
> kernel image built by Thorsten before I test on the actual hardware. Sorry
> but you'll have to restart your build Thorsten :-)
>
> Signed-off-by: Michael Schmitz <schmitz@xxxxxxxxxx>
> --
> diff --git a/arch/m68k/mm/motorola.c b/arch/m68k/mm/motorola.c
> index 02b7a03..b806c19 100644
> --- a/arch/m68k/mm/motorola.c
> +++ b/arch/m68k/mm/motorola.c
> @@ -300,6 +300,8 @@ void __init paging_init(void)
>               zones_size[ZONE_DMA] = m68k_memory[i].size >> PAGE_SHIFT;
>               free_area_init_node(i, zones_size,
>                                   m68k_memory[i].addr >> PAGE_SHIFT, NULL);
> +                if (node_present_pages(i))
> +                        node_set_state(i, N_NORMAL_MEMORY);
>       }
> }

I think you are right. however I doubt m68k need to care memoryless node check.
probably following patch just work.

------------------------------------
 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;
        node_set_online(node);
+      node_set_state(node, N_NORMAL_MEMORY);


Maybe, the correct solution is to use CONFIG_ARCH_POPULATES_NODE_MAP feature.
the usage is simple, adding add_active_range() and replacing
free_area_init_node() with free_area_init_nodes().

Thanks.
--
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/