Re: [RFC] [Patch] For booting a i386 numa system with no memory in a node

[Dave Hansen <haveblue@us.ibm.com>]

On Mon, 2005-02-21 at 14:03 -0800, keith wrote:
> On Mon, 2005-02-21 at 12:39, Dave Hansen wrote:
> > On Mon, 2005-02-21 at 12:17 -0800, keith wrote:
> > > +               if (node_has_online_mem(nid)){
> > > +                       if (start > low) {
> > 
> > Instead of indenting another level, can you just put a continue in the
> > loop?  I think it makes it much easier to read.  
> 
> I cannot put a continue here.  I know it makes ugly code worse but we
> have to call free area_init_node in all cases.   

If !node_has_online_mem(nid), then (node_start_pfn[nid] ==
node_end_pfn[nid]), and running through this if() won't hurt anything
here:

>                         if (start > low) {
> #ifdef CONFIG_HIGHMEM
>                                 BUG_ON(start > high);
>                                 zones_size[ZONE_HIGHMEM] = high - start;
> #endif
>                         }

high==start, so the bug won't trip, and it will set
zones_size[ZONE_HIGHMEM]=0, which is also OK.  Can you do this?

-               if (start > low) {
+               if (node_has_online_mem(nid) || (start > low)) {


> +#define node_has_online_mem(nid) !(node_start_pfn[nid] == node_end_pfn[nid]) 
> +/*
> +inline int __node_has_online_mem(int nid) {
> +        return !(node_start_pfn[nid]== node_end_pfn[nid]);
> +}
> +*/

You probably want to kill the extra definition.  Also, I prefer

	(node_start_pfn[nid] != node_end_pfn[nid])

to

	!(node_start_pfn[nid] == node_end_pfn[nid])

But, that's the most minor of nits.  

-- Dave

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>