Re: [PATCH 5/5] mm: clear N_HIGH_MEMORY map before se set it again-v2

[Andrew Morton]

On Thu, 14 May 2009 10:05:43 -0700
Yinghai Lu <yinghai@xxxxxxxxxx> wrote:

> Andrew Morton wrote:
> > On Thu, 14 May 2009 09:43:22 -0700
> > Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
> > 
> >> incase some system strange SRAT table. some kind of small range.
> >> or with mem= etc
> >>
> > 
> > That description is very hard to understand.  Please provide more details.
> 
> if the wrong SRAT table, have small range for some node. that node will not be onlined.
> In the early checking, the bit in node_states[N_HIGH_MEMORY] for the node is set even 
> that node has less RAM like 1M, and it is not cleared before the bit is set again in 
> the following loop according online nodes.

Where in the kernel does this setting of the bit in node_states[]
occur?  early_calculate_totalpages()?

Where in the kernel is it later decided to _not_ use these pages in
that node?  Perhaps that's the place where the problem should be fixed.


> > 
> >> Signed-off-by: Yinghai Lu <Yinghai@xxxxxxxxxx>
> >> Tested-by: Jack Steiner <steiner@xxxxxxx>
> > 
> > What reason did Jack have to test this?  Perhaps he hit some bug? 
> > If so, please fully describe that bug in the changelog.
> 
> for some memmoryless node and strange memmap.

That's not a very good problem description.

Put yourself in the position of a distro engineer whose customer
reports a 2.6.26 problem.  He's going to look at your patch wondering
whether it might fix his customer's problem.  We should provide him
with sufficient information to be able to determine this.

> > 
> > 
> >> Index: linux-2.6/mm/page_alloc.c
> >> ===================================================================
> >> --- linux-2.6.orig/mm/page_alloc.c
> >> +++ linux-2.6/mm/page_alloc.c
> >> @@ -4041,6 +4047,11 @@ void __init free_area_init_nodes(unsigne
> >>  						early_node_map[i].start_pfn,
> >>  						early_node_map[i].end_pfn);
> >>  
> >> +	/*
> >> +	 * find_zone_movable_pfns_for_nodes/early_calculate_totalpages init
> >> +	 * that node_mask, clear it at first
> >> +	 */
> >> +	nodes_clear(node_states[N_HIGH_MEMORY]);
> >>  	/* Initialise every node */
> >>  	mminit_verify_pageflags_layout();
> >>  	setup_nr_node_ids();
> > 
> > If CONFIG_HIGHMEM=n, this will clear the N_NORMAL_MEMORY entry in
> > node_states[].  Why is this correct and desirable?
> 
> then N_NORMAL_MEMORY == N_HIGH_MEMORY

I know.

But it's unobvious that this change is correct and desirable with both
CONFIG_HIGHMEM=n and CONFIG_HIGHMEM=y.

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