Hi Vijay,
On 18/07/17 12:41, vijay.kil...@gmail.com wrote:
From: Vijaya Kumar K <vijaya.ku...@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.ku...@cavium.com>
Reviewed-by: Wei Liu <wei.l...@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,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
