[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4DB3A09A.70308@gmail.com>
Date: Sun, 24 Apr 2011 16:01:30 +1200
From: Michael Schmitz <schmitzmic@...glemail.com>
To: KOSAKI Motohiro <kosaki.motohiro@...il.com>
CC: Michael Schmitz <schmitzmic@...glemail.com>,
David Rientjes <rientjes@...gle.com>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Roman Zippel <zippel@...ux-m68k.org>,
linux-m68k@...r.kernel.org, linux-kernel@...r.kernel.org,
schwab@...ux-m68k.org,
"tg@...bsd.de >> Thorsten Glaser" <tg@...bsd.de>
Subject: Re: [patch] m68k, mm: set all online nodes in N_NORMAL_MEMORY
KOSAKI Motohiro wrote:
> 2011/4/24 Michael Schmitz <schmitzmic@...glemail.com>:
>
>> 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@...gle.com>
>>> ---
>>> 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@...ian.org>
>> --
>> 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);
>
It does in fact - that's what I had as a workaround before.
Memory node information is provided by the respective m68k bootstrap so
memoryless nodes won't be possible indeed.
> 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().
>
Either of the patches should work as long as there is not a potential
problem with setting the node state before setting node_present_pages.
Your suggestion may work as well but I cannot judge whether there may be
side effects in relation to special case handling of node zero on m68k.
Do we need to call add_active_range at the time init_bootmem_node is
called? That would be a bit earlier in paging_init, and achieve much the
same thing as the proposed patch or the earlier workaround.
Besides, your solution would set N_HIGH_MEMORY not N_NORMAL_MEMORY if I
read free_area_init_nodes() right. Does that matter?
Cheers,
Michael
--
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