diff mbox

[RFC,v3,02/24] x86: NUMA: Clean up: Fix coding styles and drop unused code

Message ID 1500378106-2620-3-git-send-email-vijay.kilari@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vijay Kilari July 18, 2017, 11:41 a.m. UTC 
From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

Fix coding style, trailing spaces, tabs in NUMA code.
Also drop unused macros and functions.
There is no functional change.

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
v3: - Change commit message
    - Changed VIRTUAL_BUG_ON to ASSERT
    - Dropped useless inner paranthesis for some macros
---
 xen/arch/x86/numa.c        | 55 +++++++++++++++++++++------------------------
 xen/arch/x86/srat.c        |  2 +-
 xen/include/asm-x86/numa.h | 56 +++++++++++++++++++++++-----------------------
 xen/include/xen/numa.h     |  3 ---
 4 files changed, 54 insertions(+), 62 deletions(-)

Comments

Julien Grall July 19, 2017, 4:23 p.m. UTC | #1 
Hi Vijay,

On 18/07/17 12:41, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> Fix coding style, trailing spaces, tabs in NUMA code.
> Also drop unused macros and functions.
> There is no functional change.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v3: - Change commit message
>     - Changed VIRTUAL_BUG_ON to ASSERT

Looking at the commit message you don't mention any renaming...

>     - Dropped useless inner paranthesis for some macros

[...]

> diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
> index 3cf26c2..c0de57b 100644
> --- a/xen/include/asm-x86/numa.h
> +++ b/xen/include/asm-x86/numa.h
> @@ -1,8 +1,11 @@
> -#ifndef _ASM_X8664_NUMA_H
> +#ifndef _ASM_X8664_NUMA_H
>  #define _ASM_X8664_NUMA_H 1
>
>  #include <xen/cpumask.h>
>
> +#define MAX_NUMNODES    NR_NODES
> +#define NR_NODE_MEMBLKS (MAX_NUMNODES * 2)

I don't understand why this suddenly appears in the code when you moved 
away in patch #1 in xen/numa.h.

[...]

> @@ -57,21 +55,23 @@ struct node_data {
>
>  extern struct node_data node_data[];
>
> -static inline __attribute__((pure)) nodeid_t phys_to_nid(paddr_t addr)
> -{
> -	nodeid_t nid;
> -	VIRTUAL_BUG_ON((paddr_to_pdx(addr) >> memnode_shift) >= memnodemapsize);
> -	nid = memnodemap[paddr_to_pdx(addr) >> memnode_shift];
> -	VIRTUAL_BUG_ON(nid >= MAX_NUMNODES || !node_data[nid]);
> -	return nid;
> -}
> -
> -#define NODE_DATA(nid)		(&(node_data[nid]))
> -
> -#define node_start_pfn(nid)	(NODE_DATA(nid)->node_start_pfn)
> -#define node_spanned_pages(nid)	(NODE_DATA(nid)->node_spanned_pages)
> -#define node_end_pfn(nid)       (NODE_DATA(nid)->node_start_pfn + \
> -				 NODE_DATA(nid)->node_spanned_pages)
> +static inline __attribute_pure__ nodeid_t phys_to_nid(paddr_t addr)
> +{
> +   nodeid_t nid;
> +
> +   ASSERT((paddr_to_pdx(addr) >> memnode_shift) < memnodemapsize);
> +   nid = memnodemap[paddr_to_pdx(addr) >> memnode_shift];
> +   ASSERT(nid <= MAX_NUMNODES || !node_data[nid].node_start_pfn);
> +
> +   return nid;
> +}
> +
> +#define NODE_DATA(nid)          (&(node_data[nid]))

I understand Jan asked to remove the inner parentheses here. And you 
didn't do it. However ...

> +
> +#define node_start_pfn(nid)     NODE_DATA(nid)->node_start_pfn
> +#define node_spanned_pages(nid) NODE_DATA(nid)->node_spanned_pages
> +#define node_end_pfn(nid)       NODE_DATA(nid)->node_start_pfn + \
> +                                 NODE_DATA(nid)->node_spanned_pages

... here it is totally wrong to remove the parenthesis. Imagine you do:

node_end_pfn(nid) * 2

This will now turned into

NODE_DATA(nid)->node_start_pfn + NODE_DATA(nid)->node_spanned_pages * 2

The parenthesis is not correct anymore and will result to wrong 
computation. You should keep the outer parenthesis *everywhere* for 
safety and remove only the inner one in NODE_DATA.

This is also more than cosmetics and I think the reviewed-by from Wei 
should have been carried.

>
>  extern int valid_numa_range(u64 start, u64 end, nodeid_t node);
>
> diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
> index 6bba29e..3bb4afc 100644
> --- a/xen/include/xen/numa.h
> +++ b/xen/include/xen/numa.h
> @@ -6,9 +6,6 @@
>  #define NUMA_NO_NODE     0xFF
>  #define NUMA_NO_DISTANCE 0xFF
>
> -#define MAX_NUMNODES    NR_NODES
> -#define NR_NODE_MEMBLKS (MAX_NUMNODES * 2)
> -

See my comment above.

>  #define vcpu_to_node(v) (cpu_to_node((v)->processor))
>
>  #define domain_to_node(d) \
>

Cheers,
Wei Liu July 19, 2017, 4:27 p.m. UTC | #2 
On Wed, Jul 19, 2017 at 05:23:43PM +0100, Julien Grall wrote:
> 
> This is also more than cosmetics and I think the reviewed-by from Wei should
> have been carried.

should *not* have been carried.

And I agree.
Julien Grall July 19, 2017, 4:34 p.m. UTC | #3 
Hi,

On 19/07/17 17:27, Wei Liu wrote:
> On Wed, Jul 19, 2017 at 05:23:43PM +0100, Julien Grall wrote:
>>
>> This is also more than cosmetics and I think the reviewed-by from Wei should
>> have been carried.
>
> should *not* have been carried.

That's what I meant but failed to write the not.

>
> And I agree.
>

Cheers,
Vijay Kilari July 20, 2017, 7 a.m. UTC | #4 
On Wed, Jul 19, 2017 at 9:53 PM, Julien Grall <julien.grall@arm.com> wrote:
> Hi Vijay,
>
> On 18/07/17 12:41, vijay.kilari@gmail.com wrote:
>>
>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> Fix coding style, trailing spaces, tabs in NUMA code.
>> Also drop unused macros and functions.
>> There is no functional change.
>>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
>> ---
>> v3: - Change commit message
>>     - Changed VIRTUAL_BUG_ON to ASSERT
>
>
> Looking at the commit message you don't mention any renaming...
>
>>     - Dropped useless inner paranthesis for some macros
>
>
> [...]
>
>> diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
>> index 3cf26c2..c0de57b 100644
>> --- a/xen/include/asm-x86/numa.h
>> +++ b/xen/include/asm-x86/numa.h
>> @@ -1,8 +1,11 @@
>> -#ifndef _ASM_X8664_NUMA_H
>> +#ifndef _ASM_X8664_NUMA_H
>>  #define _ASM_X8664_NUMA_H 1
>>
>>  #include <xen/cpumask.h>
>>
>> +#define MAX_NUMNODES    NR_NODES
>> +#define NR_NODE_MEMBLKS (MAX_NUMNODES * 2)
>
>
> I don't understand why this suddenly appears in the code when you moved away
> in patch #1 in xen/numa.h.

Particularly MAX_NUMNODES required by this header file with this
patch changes for compilation.
Though I can include xen/numa.h here but xen/numa.h is including
asm/numa.h back.

I will add separate patch for this defines movement and drop from
this patch.

>
> [...]
>
>
>> @@ -57,21 +55,23 @@ struct node_data {
>>
>>  extern struct node_data node_data[];
>>
>> -static inline __attribute__((pure)) nodeid_t phys_to_nid(paddr_t addr)
>> -{
>> -       nodeid_t nid;
>> -       VIRTUAL_BUG_ON((paddr_to_pdx(addr) >> memnode_shift) >=
>> memnodemapsize);
>> -       nid = memnodemap[paddr_to_pdx(addr) >> memnode_shift];
>> -       VIRTUAL_BUG_ON(nid >= MAX_NUMNODES || !node_data[nid]);
>> -       return nid;
>> -}
>> -
>> -#define NODE_DATA(nid)         (&(node_data[nid]))
>> -
>> -#define node_start_pfn(nid)    (NODE_DATA(nid)->node_start_pfn)
>> -#define node_spanned_pages(nid)
>> (NODE_DATA(nid)->node_spanned_pages)
>> -#define node_end_pfn(nid)       (NODE_DATA(nid)->node_start_pfn + \
>> -                                NODE_DATA(nid)->node_spanned_pages)
>> +static inline __attribute_pure__ nodeid_t phys_to_nid(paddr_t addr)
>> +{
>> +   nodeid_t nid;
>> +
>> +   ASSERT((paddr_to_pdx(addr) >> memnode_shift) < memnodemapsize);
>> +   nid = memnodemap[paddr_to_pdx(addr) >> memnode_shift];
>> +   ASSERT(nid <= MAX_NUMNODES || !node_data[nid].node_start_pfn);
>> +
>> +   return nid;
>> +}
>> +
>> +#define NODE_DATA(nid)          (&(node_data[nid]))
>
>
> I understand Jan asked to remove the inner parentheses here. And you didn't
> do it. However ...
>
>> +
>> +#define node_start_pfn(nid)     NODE_DATA(nid)->node_start_pfn
>> +#define node_spanned_pages(nid) NODE_DATA(nid)->node_spanned_pages
>> +#define node_end_pfn(nid)       NODE_DATA(nid)->node_start_pfn + \
>> +                                 NODE_DATA(nid)->node_spanned_pages
>
>
> ... here it is totally wrong to remove the parenthesis. Imagine you do:
>
> node_end_pfn(nid) * 2
>
> This will now turned into
>
> NODE_DATA(nid)->node_start_pfn + NODE_DATA(nid)->node_spanned_pages * 2
>
> The parenthesis is not correct anymore and will result to wrong computation.
> You should keep the outer parenthesis *everywhere* for safety and remove
> only the inner one in NODE_DATA.

OK.

>
> This is also more than cosmetics and I think the reviewed-by from Wei should
> have been carried.

OK.

>
>>
>>  extern int valid_numa_range(u64 start, u64 end, nodeid_t node);
>>
>> diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
>> index 6bba29e..3bb4afc 100644
>> --- a/xen/include/xen/numa.h
>> +++ b/xen/include/xen/numa.h
>> @@ -6,9 +6,6 @@
>>  #define NUMA_NO_NODE     0xFF
>>  #define NUMA_NO_DISTANCE 0xFF
>>
>> -#define MAX_NUMNODES    NR_NODES
>> -#define NR_NODE_MEMBLKS (MAX_NUMNODES * 2)
>> -
>
>
> See my comment above.
>
>>  #define vcpu_to_node(v) (cpu_to_node((v)->processor))
>>
>>  #define domain_to_node(d) \
>>
>
> Cheers,
>
> --
> Julien Grall
Julien Grall July 20, 2017, 11 a.m. UTC | #5 
Hi Vijay,

On 20/07/17 08:00, Vijay Kilari wrote:
> On Wed, Jul 19, 2017 at 9:53 PM, Julien Grall <julien.grall@arm.com> wrote:
>> Hi Vijay,
>>
>> On 18/07/17 12:41, vijay.kilari@gmail.com wrote:
>>>
>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>
>>> Fix coding style, trailing spaces, tabs in NUMA code.
>>> Also drop unused macros and functions.
>>> There is no functional change.
>>>
>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
>>> ---
>>> v3: - Change commit message
>>>     - Changed VIRTUAL_BUG_ON to ASSERT
>>
>>
>> Looking at the commit message you don't mention any renaming...
>>
>>>     - Dropped useless inner paranthesis for some macros
>>
>>
>> [...]
>>
>>> diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
>>> index 3cf26c2..c0de57b 100644
>>> --- a/xen/include/asm-x86/numa.h
>>> +++ b/xen/include/asm-x86/numa.h
>>> @@ -1,8 +1,11 @@
>>> -#ifndef _ASM_X8664_NUMA_H
>>> +#ifndef _ASM_X8664_NUMA_H
>>>  #define _ASM_X8664_NUMA_H 1
>>>
>>>  #include <xen/cpumask.h>
>>>
>>> +#define MAX_NUMNODES    NR_NODES
>>> +#define NR_NODE_MEMBLKS (MAX_NUMNODES * 2)
>>
>>
>> I don't understand why this suddenly appears in the code when you moved away
>> in patch #1 in xen/numa.h.
>
> Particularly MAX_NUMNODES required by this header file with this
> patch changes for compilation.
> Though I can include xen/numa.h here but xen/numa.h is including
> asm/numa.h back.
>
> I will add separate patch for this defines movement and drop from
> this patch.

Why adding a separate patch? The code should not have been moved away in 
patch #1 as you did.

But I still don't understand what is the exact error here... If it fails 
on this patch, likely this should have failed after applying patch #1. 
And *all* patch should be able to build without the rest of the series.

Cheers,
Vijay Kilari July 20, 2017, 12:05 p.m. UTC | #6 
On Thu, Jul 20, 2017 at 4:30 PM, Julien Grall <julien.grall@arm.com> wrote:
> Hi Vijay,
>
>
> On 20/07/17 08:00, Vijay Kilari wrote:
>>
>> On Wed, Jul 19, 2017 at 9:53 PM, Julien Grall <julien.grall@arm.com>
>> wrote:
>>>
>>> Hi Vijay,
>>>
>>> On 18/07/17 12:41, vijay.kilari@gmail.com wrote:
>>>>
>>>>
>>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>>
>>>> Fix coding style, trailing spaces, tabs in NUMA code.
>>>> Also drop unused macros and functions.
>>>> There is no functional change.
>>>>
>>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
>>>> ---
>>>> v3: - Change commit message
>>>>     - Changed VIRTUAL_BUG_ON to ASSERT
>>>
>>>
>>>
>>> Looking at the commit message you don't mention any renaming...
>>>
>>>>     - Dropped useless inner paranthesis for some macros
>>>
>>>
>>>
>>> [...]
>>>
>>>> diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
>>>> index 3cf26c2..c0de57b 100644
>>>> --- a/xen/include/asm-x86/numa.h
>>>> +++ b/xen/include/asm-x86/numa.h
>>>> @@ -1,8 +1,11 @@
>>>> -#ifndef _ASM_X8664_NUMA_H
>>>> +#ifndef _ASM_X8664_NUMA_H
>>>>  #define _ASM_X8664_NUMA_H 1
>>>>
>>>>  #include <xen/cpumask.h>
>>>>
>>>> +#define MAX_NUMNODES    NR_NODES
>>>> +#define NR_NODE_MEMBLKS (MAX_NUMNODES * 2)
>>>
>>>
>>>
>>> I don't understand why this suddenly appears in the code when you moved
>>> away
>>> in patch #1 in xen/numa.h.
>>
>>
>> Particularly MAX_NUMNODES required by this header file with this
>> patch changes for compilation.
>> Though I can include xen/numa.h here but xen/numa.h is including
>> asm/numa.h back.
>>
>> I will add separate patch for this defines movement and drop from
>> this patch.
>
>
> Why adding a separate patch? The code should not have been moved away in
> patch #1 as you did.

In patch#1 , I have not moved MAX_NUMNODES. It is kept in xen/numa.h file
In this patch, when VIRTUAL_BUG_ON is changed to ASSERT, in asm-x86/numa.h,
it requires MAX_NUMNODES define. So I have moved it from xen/numa.h to
asm-x86/numa.h

So, I was thinking of  adding small patch to move both MAX_NUMNODES and
NR_NODE_MEMBLKS to asm-x86/numa.h

And in code movement patch, I will move to xen/numa.h along with ASSERT code.

>
> But I still don't understand what is the exact error here... If it fails on
> this patch, likely this should have failed after applying patch #1. And
> *all* patch should be able to build without the rest of the series.

Yes, all patches are tested for compilation individually.

>
> Cheers,
>
> --
> Julien Grall
Julien Grall July 20, 2017, 12:09 p.m. UTC | #7 
On 20/07/17 13:05, Vijay Kilari wrote:
> On Thu, Jul 20, 2017 at 4:30 PM, Julien Grall <julien.grall@arm.com> wrote:
>> Hi Vijay,
>>
>>
>> On 20/07/17 08:00, Vijay Kilari wrote:
>>>
>>> On Wed, Jul 19, 2017 at 9:53 PM, Julien Grall <julien.grall@arm.com>
>>> wrote:
>>>>
>>>> Hi Vijay,
>>>>
>>>> On 18/07/17 12:41, vijay.kilari@gmail.com wrote:
>>>>>
>>>>>
>>>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>>>
>>>>> Fix coding style, trailing spaces, tabs in NUMA code.
>>>>> Also drop unused macros and functions.
>>>>> There is no functional change.
>>>>>
>>>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>>> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
>>>>> ---
>>>>> v3: - Change commit message
>>>>>     - Changed VIRTUAL_BUG_ON to ASSERT
>>>>
>>>>
>>>>
>>>> Looking at the commit message you don't mention any renaming...
>>>>
>>>>>     - Dropped useless inner paranthesis for some macros
>>>>
>>>>
>>>>
>>>> [...]
>>>>
>>>>> diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
>>>>> index 3cf26c2..c0de57b 100644
>>>>> --- a/xen/include/asm-x86/numa.h
>>>>> +++ b/xen/include/asm-x86/numa.h
>>>>> @@ -1,8 +1,11 @@
>>>>> -#ifndef _ASM_X8664_NUMA_H
>>>>> +#ifndef _ASM_X8664_NUMA_H
>>>>>  #define _ASM_X8664_NUMA_H 1
>>>>>
>>>>>  #include <xen/cpumask.h>
>>>>>
>>>>> +#define MAX_NUMNODES    NR_NODES
>>>>> +#define NR_NODE_MEMBLKS (MAX_NUMNODES * 2)
>>>>
>>>>
>>>>
>>>> I don't understand why this suddenly appears in the code when you moved
>>>> away
>>>> in patch #1 in xen/numa.h.
>>>
>>>
>>> Particularly MAX_NUMNODES required by this header file with this
>>> patch changes for compilation.
>>> Though I can include xen/numa.h here but xen/numa.h is including
>>> asm/numa.h back.
>>>
>>> I will add separate patch for this defines movement and drop from
>>> this patch.
>>
>>
>> Why adding a separate patch? The code should not have been moved away in
>> patch #1 as you did.
>
> In patch#1 , I have not moved MAX_NUMNODES. It is kept in xen/numa.h file
> In this patch, when VIRTUAL_BUG_ON is changed to ASSERT, in asm-x86/numa.h,
> it requires MAX_NUMNODES define. So I have moved it from xen/numa.h to
> asm-x86/numa.h

I am sorry but looked at your patch #1. You moved NR_NODE_MEMBLKS in 
patch #1 from asm-x86/numa.h to xen/numa.h. And then you moved it again 
here.

>
> So, I was thinking of  adding small patch to move both MAX_NUMNODES and
> NR_NODE_MEMBLKS to asm-x86/numa.h

Or better, you can do in xen/numa.h:

#define MAX_NUMNODES ...
#define NR_NODE_...

#include <asm/numa.h>

Cheers,
diff mbox

Patch

diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index d45196fa..444d7ad 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -1,8 +1,8 @@ 
-/* 
+/*
  * Generic VM initialization for x86-64 NUMA setups.
  * Copyright 2002,2003 Andi Kleen, SuSE Labs.
  * Adapted for Xen: Ryan Harper <ryanh@us.ibm.com>
- */ 
+ */
 
 #include <xen/mm.h>
 #include <xen/string.h>
@@ -21,13 +21,6 @@ 
 static int numa_setup(char *s);
 custom_param("numa", numa_setup);
 
-#ifndef Dprintk
-#define Dprintk(x...)
-#endif
-
-/* from proto.h */
-#define round_up(x,y) ((((x)+(y))-1) & (~((y)-1)))
-
 struct node_data node_data[MAX_NUMNODES];
 
 /* Mapping from pdx to node id */
@@ -144,8 +137,9 @@  static int __init extract_lsb_from_nodes(const struct node *nodes,
     if ( nodes_used <= 1 )
         i = BITS_PER_LONG - 1;
     else
-        i = find_first_bit(&bitfield, sizeof(unsigned long)*8);
+        i = find_first_bit(&bitfield, sizeof(unsigned long) * 8);
     memnodemapsize = (memtop >> i) + 1;
+
     return i;
 }
 
@@ -173,7 +167,7 @@  int __init compute_hash_shift(struct node *nodes, int numnodes,
 }
 /* initialize NODE_DATA given nodeid and start/end */
 void __init setup_node_bootmem(nodeid_t nodeid, u64 start, u64 end)
-{ 
+{
     unsigned long start_pfn, end_pfn;
 
     start_pfn = start >> PAGE_SHIFT;
@@ -183,7 +177,7 @@  void __init setup_node_bootmem(nodeid_t nodeid, u64 start, u64 end)
     NODE_DATA(nodeid)->node_spanned_pages = end_pfn - start_pfn;
 
     node_set_online(nodeid);
-} 
+}
 
 void __init numa_init_array(void)
 {
@@ -214,7 +208,7 @@  static int __init numa_emulation(u64 start_pfn, u64 end_pfn)
 {
     int i;
     struct node nodes[MAX_NUMNODES];
-    u64 sz = ((end_pfn - start_pfn)<<PAGE_SHIFT) / numa_fake;
+    u64 sz = ((end_pfn - start_pfn) << PAGE_SHIFT) / numa_fake;
 
     /* Kludge needed for the hash function */
     if ( hweight64(sz) > 1 )
@@ -222,21 +216,22 @@  static int __init numa_emulation(u64 start_pfn, u64 end_pfn)
         u64 x = 1;
         while ( (x << 1) < sz )
             x <<= 1;
-        if ( x < sz/2 )
-            printk(KERN_ERR "Numa emulation unbalanced. Complain to maintainer\n");
+        if ( x < sz / 2 )
+            printk(KERN_ERR
+                   "Numa emulation unbalanced. Complain to maintainer\n");
         sz = x;
     }
 
     memset(&nodes,0,sizeof(nodes));
     for ( i = 0; i < numa_fake; i++ )
     {
-        nodes[i].start = (start_pfn<<PAGE_SHIFT) + i*sz;
+        nodes[i].start = (start_pfn << PAGE_SHIFT) + i * sz;
         if ( i == numa_fake - 1 )
-            sz = (end_pfn<<PAGE_SHIFT) - nodes[i].start;
+            sz = (end_pfn << PAGE_SHIFT) - nodes[i].start;
         nodes[i].end = nodes[i].start + sz;
-        printk(KERN_INFO "Faking node %d at %"PRIx64"-%"PRIx64" (%"PRIu64"MB)\n",
-               i,
-               nodes[i].start, nodes[i].end,
+        printk(KERN_INFO
+               "Faking node %d at %"PRIx64"-%"PRIx64" (%"PRIu64"MB)\n",
+               i, nodes[i].start, nodes[i].end,
                (nodes[i].end - nodes[i].start) >> 20);
         node_set_online(i);
     }
@@ -256,7 +251,7 @@  static int __init numa_emulation(u64 start_pfn, u64 end_pfn)
 #endif
 
 void __init numa_initmem_init(unsigned long start_pfn, unsigned long end_pfn)
-{ 
+{
     int i;
 
 #ifdef CONFIG_NUMA_EMU
@@ -291,7 +286,7 @@  void __init numa_initmem_init(unsigned long start_pfn, unsigned long end_pfn)
 void numa_add_cpu(int cpu)
 {
     cpumask_set_cpu(cpu, &node_to_cpumask[cpu_to_node(cpu)]);
-} 
+}
 
 void numa_set_node(int cpu, nodeid_t node)
 {
@@ -299,23 +294,23 @@  void numa_set_node(int cpu, nodeid_t node)
 }
 
 /* [numa=off] */
-static __init int numa_setup(char *opt) 
-{ 
-    if ( !strncmp(opt,"off",3) )
+static __init int numa_setup(char *opt)
+{
+    if ( !strncmp(opt, "off", 3) )
         numa_off = true;
-    if ( !strncmp(opt,"on",2) )
+    if ( !strncmp(opt, "on", 2) )
         numa_off = false;
 #ifdef CONFIG_NUMA_EMU
     if ( !strncmp(opt, "fake=", 5) )
     {
         numa_off = false;
-        numa_fake = simple_strtoul(opt+5,NULL,0);
+        numa_fake = simple_strtoul(opt + 5, NULL, 0);
         if ( numa_fake >= MAX_NUMNODES )
             numa_fake = MAX_NUMNODES;
     }
 #endif
 #ifdef CONFIG_ACPI_NUMA
-    if ( !strncmp(opt,"noacpi",6) )
+    if ( !strncmp(opt,"noacpi", 6) )
     {
         numa_off = false;
         acpi_numa = -1;
@@ -323,7 +318,7 @@  static __init int numa_setup(char *opt)
 #endif
 
     return 1;
-} 
+}
 
 /*
  * Setup early cpu_to_node.
@@ -385,7 +380,7 @@  static void dump_numa(unsigned char key)
     const struct vnuma_info *vnuma;
 
     printk("'%c' pressed -> dumping numa info (now-0x%X:%08X)\n", key,
-           (u32)(now>>32), (u32)now);
+           (u32)(now >> 32), (u32)now);
 
     for_each_online_node ( i )
     {
diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c
index cd1283e..ec08112 100644
--- a/xen/arch/x86/srat.c
+++ b/xen/arch/x86/srat.c
@@ -7,7 +7,7 @@ 
  * Called from acpi_numa_init while reading the SRAT and SLIT tables.
  * Assumes all memory regions belonging to a single proximity domain
  * are in one chunk. Holes between them will be included in the node.
- * 
+ *
  * Adapted for Xen: Ryan Harper <ryanh@us.ibm.com>
  */
 
diff --git a/xen/include/asm-x86/numa.h b/xen/include/asm-x86/numa.h
index 3cf26c2..c0de57b 100644
--- a/xen/include/asm-x86/numa.h
+++ b/xen/include/asm-x86/numa.h
@@ -1,8 +1,11 @@ 
-#ifndef _ASM_X8664_NUMA_H 
+#ifndef _ASM_X8664_NUMA_H
 #define _ASM_X8664_NUMA_H 1
 
 #include <xen/cpumask.h>
 
+#define MAX_NUMNODES    NR_NODES
+#define NR_NODE_MEMBLKS (MAX_NUMNODES * 2)
+
 typedef u8 nodeid_t;
 
 extern int srat_rev;
@@ -10,21 +13,21 @@  extern int srat_rev;
 extern nodeid_t      cpu_to_node[NR_CPUS];
 extern cpumask_t     node_to_cpumask[];
 
-#define cpu_to_node(cpu)		(cpu_to_node[cpu])
-#define parent_node(node)		(node)
+#define cpu_to_node(cpu)         (cpu_to_node[cpu])
+#define parent_node(node)        (node)
 #define node_to_first_cpu(node)  (__ffs(node_to_cpumask[node]))
 #define node_to_cpumask(node)    (node_to_cpumask[node])
 
-struct node { 
-	u64 start,end; 
+struct node {
+    u64 start;
+    u64 end;
 };
 
 extern int compute_hash_shift(struct node *nodes, int numnodes,
-			      nodeid_t *nodeids);
+                              nodeid_t *nodeids);
 extern nodeid_t pxm_to_node(unsigned int pxm);
 
 #define ZONE_ALIGN (1UL << (MAX_ORDER+PAGE_SHIFT))
-#define VIRTUAL_BUG_ON(x) 
 
 extern void numa_add_cpu(int cpu);
 extern void numa_init_array(void);
@@ -40,13 +43,8 @@  extern void setup_node_bootmem(nodeid_t nodeid, u64 start, u64 end);
 extern nodeid_t apicid_to_node[];
 extern void init_cpu_to_node(void);
 
-static inline void clear_node_cpumask(int cpu)
-{
-	cpumask_clear_cpu(cpu, &node_to_cpumask[cpu_to_node(cpu)]);
-}
-
 /* Simple perfect hash to map pdx to node numbers */
-extern int memnode_shift; 
+extern int memnode_shift;
 extern unsigned long memnodemapsize;
 extern u8 *memnodemap;
 
@@ -57,21 +55,23 @@  struct node_data {
 
 extern struct node_data node_data[];
 
-static inline __attribute__((pure)) nodeid_t phys_to_nid(paddr_t addr)
-{ 
-	nodeid_t nid;
-	VIRTUAL_BUG_ON((paddr_to_pdx(addr) >> memnode_shift) >= memnodemapsize);
-	nid = memnodemap[paddr_to_pdx(addr) >> memnode_shift]; 
-	VIRTUAL_BUG_ON(nid >= MAX_NUMNODES || !node_data[nid]); 
-	return nid; 
-} 
-
-#define NODE_DATA(nid)		(&(node_data[nid]))
-
-#define node_start_pfn(nid)	(NODE_DATA(nid)->node_start_pfn)
-#define node_spanned_pages(nid)	(NODE_DATA(nid)->node_spanned_pages)
-#define node_end_pfn(nid)       (NODE_DATA(nid)->node_start_pfn + \
-				 NODE_DATA(nid)->node_spanned_pages)
+static inline __attribute_pure__ nodeid_t phys_to_nid(paddr_t addr)
+{
+   nodeid_t nid;
+
+   ASSERT((paddr_to_pdx(addr) >> memnode_shift) < memnodemapsize);
+   nid = memnodemap[paddr_to_pdx(addr) >> memnode_shift];
+   ASSERT(nid <= MAX_NUMNODES || !node_data[nid].node_start_pfn);
+
+   return nid;
+}
+
+#define NODE_DATA(nid)          (&(node_data[nid]))
+
+#define node_start_pfn(nid)     NODE_DATA(nid)->node_start_pfn
+#define node_spanned_pages(nid) NODE_DATA(nid)->node_spanned_pages
+#define node_end_pfn(nid)       NODE_DATA(nid)->node_start_pfn + \
+                                 NODE_DATA(nid)->node_spanned_pages
 
 extern int valid_numa_range(u64 start, u64 end, nodeid_t node);
 
diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
index 6bba29e..3bb4afc 100644
--- a/xen/include/xen/numa.h
+++ b/xen/include/xen/numa.h
@@ -6,9 +6,6 @@ 
 #define NUMA_NO_NODE     0xFF
 #define NUMA_NO_DISTANCE 0xFF
 
-#define MAX_NUMNODES    NR_NODES
-#define NR_NODE_MEMBLKS (MAX_NUMNODES * 2)
-
 #define vcpu_to_node(v) (cpu_to_node((v)->processor))
 
 #define domain_to_node(d) \