Re: [PATCH 10/11] x86, mem-hotplug: Support initialize page tablesfrom low to high.

[Tang Chen]

Hi Wanpeng,

On 09/06/2013 10:16 AM, Wanpeng Li wrote:
......
> > > > +#ifdef CONFIG_MOVABLE_NODE
> > > > +	unsigned long kernel_end;
> > > > +
> > > > +	if (movablenode_enable_srat&&
> > > > +	    memblock.current_order == MEMBLOCK_ORDER_LOW_TO_HIGH) {
> > >
> > > I think memblock.current_order == MEMBLOCK_ORDER_LOW_TO_HIGH is always
> > > true if config MOVABLE_NODE and movablenode_enable_srat == true if PATCH
> > > 11/11 is applied.
> >
> > memblock.current_order == MEMBLOCK_ORDER_LOW_TO_HIGH is true here if
> > MOVABLE_NODE
> > is configured, and it will be reset after SRAT is parsed. But
> > movablenode_enable_srat
> > could only be true when users specify movablenode boot option in the
> > kernel commandline.
>
> You are right.
>
> I mean the change should be:
>
> +#ifdef CONFIG_MOVABLE_NODE
> +       unsigned long kernel_end;
> +
> +       if (movablenode_enable_srat) {
>
> The is unnecessary to check memblock.current_order since it is always true
> if movable_node is configured and movablenode_enable_srat is true.

But I think, memblock.current_order is set outside init_mem_mapping(). And
the path in the if statement could only be run when current order is from
low to high. So I think it is safe to check it here.

I prefer to keep it at least in the next version patch-set. If others also
think it is unnecessary, I'm OK with removing the checking. :)

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/