| Message ID | 1490716413-19796-7-git-send-email-vijay.kilari@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Vijay, On 28/03/17 16:53, vijay.kilari@gmail.com wrote: > From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> > > Add accessor for nodes[] and other static variables and s/accessor/accessors/ > used those accessors. Also, I am not sure to understand the usefulness of those accessors over a global variable. > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com> > --- > xen/arch/x86/srat.c | 108 +++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 82 insertions(+), 26 deletions(-) > > diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c > index ccacbcd..983e1d8 100644 > --- a/xen/arch/x86/srat.c > +++ b/xen/arch/x86/srat.c > @@ -41,7 +41,45 @@ static struct node node_memblk_range[NR_NODE_MEMBLKS]; > static nodeid_t memblk_nodeid[NR_NODE_MEMBLKS]; > static __initdata DECLARE_BITMAP(memblk_hotplug, NR_NODE_MEMBLKS); > > -static inline bool node_found(unsigned idx, unsigned pxm) > +static struct node *get_numa_node(int id) unsigned int. > +{ > + return &nodes[id]; > +} > + > +static nodeid_t get_memblk_nodeid(int id) unsigned int. > +{ > + return memblk_nodeid[id]; > +} > + > +static nodeid_t *get_memblk_nodeid_map(void) > +{ > + return &memblk_nodeid[0]; > +} > + > +static struct node *get_node_memblk_range(int memblk) unsigned int. > +{ > + return &node_memblk_range[memblk]; > +} > + > +static int get_num_node_memblks(void) > +{ > + return num_node_memblks; > +} > + > +static int __init numa_add_memblk(nodeid_t nodeid, paddr_t start, uint64_t size) > +{ > + if (nodeid >= NR_NODE_MEMBLKS) > + return -EINVAL; > + > + node_memblk_range[num_node_memblks].start = start; > + node_memblk_range[num_node_memblks].end = start + size; > + memblk_nodeid[num_node_memblks] = nodeid; > + num_node_memblks++; > + > + return 0; > +} > + > +static inline bool node_found(unsigned int idx, unsigned int pxm) Please don't make unrelated change in the same patch. In this case I don't see why you switch from "unsigned" to "unsigned int". > { > return ((pxm2node[idx].pxm == pxm) && > (pxm2node[idx].node != NUMA_NO_NODE)); > @@ -107,11 +145,11 @@ int valid_numa_range(paddr_t start, paddr_t end, nodeid_t node) > { > int i; > > - for (i = 0; i < num_node_memblks; i++) { > - struct node *nd = &node_memblk_range[i]; > + for (i = 0; i < get_num_node_memblks(); i++) { > + struct node *nd = get_node_memblk_range(i); > > if (nd->start <= start && nd->end > end && > - memblk_nodeid[i] == node ) > + get_memblk_nodeid(i) == node) Why the indentation changed here? > return 1; > } > > @@ -122,8 +160,8 @@ static int __init conflicting_memblks(paddr_t start, paddr_t end) > { > int i; > > - for (i = 0; i < num_node_memblks; i++) { > - struct node *nd = &node_memblk_range[i]; > + for (i = 0; i < get_num_node_memblks(); i++) { > + struct node *nd = get_node_memblk_range(i); > if (nd->start == nd->end) > continue; > if (nd->end > start && nd->start < end) > @@ -136,7 +174,8 @@ static int __init conflicting_memblks(paddr_t start, paddr_t end) > > static void __init cutoff_node(int i, paddr_t start, paddr_t end) > { > - struct node *nd = &nodes[i]; > + struct node *nd = get_numa_node(i); > + > if (nd->start < start) { > nd->start = start; > if (nd->end < nd->start) > @@ -278,6 +317,7 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma) > unsigned pxm; > nodeid_t node; > int i; > + struct node *memblk; > > if (srat_disabled()) > return; > @@ -288,7 +328,7 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma) > if (!(ma->flags & ACPI_SRAT_MEM_ENABLED)) > return; > > - if (num_node_memblks >= NR_NODE_MEMBLKS) > + if (get_num_node_memblks() >= NR_NODE_MEMBLKS) > { > dprintk(XENLOG_WARNING, > "Too many numa entry, try bigger NR_NODE_MEMBLKS \n"); > @@ -310,27 +350,31 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma) > i = conflicting_memblks(start, end); > if (i < 0) > /* everything fine */; > - else if (memblk_nodeid[i] == node) { > + else if (get_memblk_nodeid(i) == node) { > bool mismatch = !(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) != > !test_bit(i, memblk_hotplug); > > + memblk = get_node_memblk_range(i); > + > printk("%sSRAT: PXM %u (%"PRIx64"-%"PRIx64") overlaps with itself (%"PRIx64"-%"PRIx64")\n", > mismatch ? KERN_ERR : KERN_WARNING, pxm, start, end, > - node_memblk_range[i].start, node_memblk_range[i].end); > + memblk->start, memblk->end); > if (mismatch) { > bad_srat(); > return; > } > } else { > + memblk = get_node_memblk_range(i); > + > printk(KERN_ERR > "SRAT: PXM %u (%"PRIx64"-%"PRIx64") overlaps with PXM %u (%"PRIx64"-%"PRIx64")\n", > - pxm, start, end, node_to_pxm(memblk_nodeid[i]), > - node_memblk_range[i].start, node_memblk_range[i].end); > + pxm, start, end, node_to_pxm(get_memblk_nodeid(i)), > + memblk->start, memblk->end); > bad_srat(); > return; > } > if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) { > - struct node *nd = &nodes[node]; > + struct node *nd = get_numa_node(node); > > if (!node_test_and_set(node, memory_nodes_parsed)) { > nd->start = start; > @@ -346,15 +390,17 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma) > node, pxm, start, end, > ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE ? " (hotplug)" : ""); > > - node_memblk_range[num_node_memblks].start = start; > - node_memblk_range[num_node_memblks].end = end; > - memblk_nodeid[num_node_memblks] = node; > + if (numa_add_memblk(node, start, ma->length)) { > + printk(KERN_ERR "SRAT: node-id %u out of range\n", node); > + bad_srat(); > + return; > + } > + > if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) { > - __set_bit(num_node_memblks, memblk_hotplug); > + __set_bit(get_num_node_memblks(), memblk_hotplug); > if (end > mem_hotplug) > mem_hotplug = end; > } > - num_node_memblks++; > } > > /* Sanity check to catch more bad SRATs (they are amazingly common). > @@ -377,17 +423,21 @@ static int __init nodes_cover_memory(void) > do { > found = 0; > for_each_node_mask(j, memory_nodes_parsed) > - if (start < nodes[j].end > - && end > nodes[j].start) { > - if (start >= nodes[j].start) { > - start = nodes[j].end; > + { > + struct node *nd = get_numa_node(j); > + > + if (start < nd->end > + && end > nd->start) { > + if (start >= nd->start) { > + start = nd->end; > found = 1; > } > - if (end <= nodes[j].end) { > - end = nodes[j].start; > + if (end <= nd->end) { > + end = nd->start; > found = 1; > } > } > + } > } while (found && start < end); > > if (start < end) { > @@ -457,6 +507,8 @@ int __init acpi_scan_nodes(uint64_t start, uint64_t end) > { > int i; > nodemask_t all_nodes_parsed; > + struct node *memblks; > + nodeid_t *nodeids; > > /* First clean up the node list */ > for (i = 0; i < MAX_NUMNODES; i++) > @@ -470,6 +522,8 @@ int __init acpi_scan_nodes(uint64_t start, uint64_t end) > return -1; > } > > + memblks = get_node_memblk_range(0); > + nodeids = get_memblk_nodeid_map(); > if (compute_memnode_shift(node_memblk_range, num_node_memblks, > memblk_nodeid, &memnode_shift)) { > memnode_shift = 0; > @@ -484,12 +538,14 @@ int __init acpi_scan_nodes(uint64_t start, uint64_t end) > /* Finally register nodes */ > for_each_node_mask(i, all_nodes_parsed) > { > - uint64_t size = nodes[i].end - nodes[i].start; > + struct node *nd = get_numa_node(i); > + uint64_t size = nd->end - nd->start; > + > if ( size == 0 ) > printk(KERN_WARNING "SRAT: Node %u has no memory. " > "BIOS Bug or mis-configured hardware?\n", i); > > - setup_node_bootmem(i, nodes[i].start, nodes[i].end); > + setup_node_bootmem(i, nd->start, nd->end); > } > for (i = 0; i < nr_cpu_ids; i++) { > if (cpu_to_node[i] == NUMA_NO_NODE) > Cheers,
On Mon, May 8, 2017 at 8:09 PM, Julien Grall <julien.grall@arm.com> wrote: > Hi Vijay, > > On 28/03/17 16:53, vijay.kilari@gmail.com wrote: >> >> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >> >> Add accessor for nodes[] and other static variables and > > > s/accessor/accessors/ > >> used those accessors. > > > Also, I am not sure to understand the usefulness of those accessors over a > global variable. These are static variables which needs to accessed from other files and later moved to generic file. > >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >> --- >> xen/arch/x86/srat.c | 108 >> +++++++++++++++++++++++++++++++++++++++------------- >> 1 file changed, 82 insertions(+), 26 deletions(-) >> >> diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c >> index ccacbcd..983e1d8 100644 >> --- a/xen/arch/x86/srat.c >> +++ b/xen/arch/x86/srat.c >> @@ -41,7 +41,45 @@ static struct node node_memblk_range[NR_NODE_MEMBLKS]; >> static nodeid_t memblk_nodeid[NR_NODE_MEMBLKS]; >> static __initdata DECLARE_BITMAP(memblk_hotplug, NR_NODE_MEMBLKS); >> >> -static inline bool node_found(unsigned idx, unsigned pxm) >> +static struct node *get_numa_node(int id) > > > unsigned int. OK > >> +{ >> + return &nodes[id]; >> +} >> + >> +static nodeid_t get_memblk_nodeid(int id) > > > unsigned int. > >> +{ >> + return memblk_nodeid[id]; >> +} >> + >> +static nodeid_t *get_memblk_nodeid_map(void) >> +{ >> + return &memblk_nodeid[0]; >> +} >> + >> +static struct node *get_node_memblk_range(int memblk) > > > unsigned int. > >> +{ >> + return &node_memblk_range[memblk]; >> +} >> + >> +static int get_num_node_memblks(void) >> +{ >> + return num_node_memblks; >> +} >> + >> +static int __init numa_add_memblk(nodeid_t nodeid, paddr_t start, >> uint64_t size) >> +{ >> + if (nodeid >= NR_NODE_MEMBLKS) >> + return -EINVAL; >> + >> + node_memblk_range[num_node_memblks].start = start; >> + node_memblk_range[num_node_memblks].end = start + size; >> + memblk_nodeid[num_node_memblks] = nodeid; >> + num_node_memblks++; >> + >> + return 0; >> +} >> + >> +static inline bool node_found(unsigned int idx, unsigned int pxm) > > > Please don't make unrelated change in the same patch. In this case I don't > see why you switch from "unsigned" to "unsigned int". > >> { >> return ((pxm2node[idx].pxm == pxm) && >> (pxm2node[idx].node != NUMA_NO_NODE)); >> @@ -107,11 +145,11 @@ int valid_numa_range(paddr_t start, paddr_t end, >> nodeid_t node) >> { >> int i; >> >> - for (i = 0; i < num_node_memblks; i++) { >> - struct node *nd = &node_memblk_range[i]; >> + for (i = 0; i < get_num_node_memblks(); i++) { >> + struct node *nd = get_node_memblk_range(i); >> >> if (nd->start <= start && nd->end > end && >> - memblk_nodeid[i] == node ) >> + get_memblk_nodeid(i) == node) > > > Why the indentation changed here? OK. will wrap these changes in other patches. > > >> return 1; >> } >> >> @@ -122,8 +160,8 @@ static int __init conflicting_memblks(paddr_t start, >> paddr_t end) >> { >> int i; >> >> - for (i = 0; i < num_node_memblks; i++) { >> - struct node *nd = &node_memblk_range[i]; >> + for (i = 0; i < get_num_node_memblks(); i++) { >> + struct node *nd = get_node_memblk_range(i); >> if (nd->start == nd->end) >> continue; >> if (nd->end > start && nd->start < end) >> @@ -136,7 +174,8 @@ static int __init conflicting_memblks(paddr_t start, >> paddr_t end) >> >> static void __init cutoff_node(int i, paddr_t start, paddr_t end) >> { >> - struct node *nd = &nodes[i]; >> + struct node *nd = get_numa_node(i); >> + >> if (nd->start < start) { >> nd->start = start; >> if (nd->end < nd->start) >> @@ -278,6 +317,7 @@ acpi_numa_memory_affinity_init(const struct >> acpi_srat_mem_affinity *ma) >> unsigned pxm; >> nodeid_t node; >> int i; >> + struct node *memblk; >> >> if (srat_disabled()) >> return; >> @@ -288,7 +328,7 @@ acpi_numa_memory_affinity_init(const struct >> acpi_srat_mem_affinity *ma) >> if (!(ma->flags & ACPI_SRAT_MEM_ENABLED)) >> return; >> >> - if (num_node_memblks >= NR_NODE_MEMBLKS) >> + if (get_num_node_memblks() >= NR_NODE_MEMBLKS) >> { >> dprintk(XENLOG_WARNING, >> "Too many numa entry, try bigger NR_NODE_MEMBLKS \n"); >> @@ -310,27 +350,31 @@ acpi_numa_memory_affinity_init(const struct >> acpi_srat_mem_affinity *ma) >> i = conflicting_memblks(start, end); >> if (i < 0) >> /* everything fine */; >> - else if (memblk_nodeid[i] == node) { >> + else if (get_memblk_nodeid(i) == node) { >> bool mismatch = !(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) >> != >> !test_bit(i, memblk_hotplug); >> >> + memblk = get_node_memblk_range(i); >> + >> printk("%sSRAT: PXM %u (%"PRIx64"-%"PRIx64") overlaps with >> itself (%"PRIx64"-%"PRIx64")\n", >> mismatch ? KERN_ERR : KERN_WARNING, pxm, start, >> end, >> - node_memblk_range[i].start, >> node_memblk_range[i].end); >> + memblk->start, memblk->end); >> if (mismatch) { >> bad_srat(); >> return; >> } >> } else { >> + memblk = get_node_memblk_range(i); >> + >> printk(KERN_ERR >> "SRAT: PXM %u (%"PRIx64"-%"PRIx64") overlaps with >> PXM %u (%"PRIx64"-%"PRIx64")\n", >> - pxm, start, end, node_to_pxm(memblk_nodeid[i]), >> - node_memblk_range[i].start, >> node_memblk_range[i].end); >> + pxm, start, end, node_to_pxm(get_memblk_nodeid(i)), >> + memblk->start, memblk->end); >> bad_srat(); >> return; >> } >> if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) { >> - struct node *nd = &nodes[node]; >> + struct node *nd = get_numa_node(node); >> >> if (!node_test_and_set(node, memory_nodes_parsed)) { >> nd->start = start; >> @@ -346,15 +390,17 @@ acpi_numa_memory_affinity_init(const struct >> acpi_srat_mem_affinity *ma) >> node, pxm, start, end, >> ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE ? " (hotplug)" : >> ""); >> >> - node_memblk_range[num_node_memblks].start = start; >> - node_memblk_range[num_node_memblks].end = end; >> - memblk_nodeid[num_node_memblks] = node; >> + if (numa_add_memblk(node, start, ma->length)) { >> + printk(KERN_ERR "SRAT: node-id %u out of range\n", node); >> + bad_srat(); >> + return; >> + } >> + >> if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) { >> - __set_bit(num_node_memblks, memblk_hotplug); >> + __set_bit(get_num_node_memblks(), memblk_hotplug); >> if (end > mem_hotplug) >> mem_hotplug = end; >> } >> - num_node_memblks++; >> } >> >> /* Sanity check to catch more bad SRATs (they are amazingly common). >> @@ -377,17 +423,21 @@ static int __init nodes_cover_memory(void) >> do { >> found = 0; >> for_each_node_mask(j, memory_nodes_parsed) >> - if (start < nodes[j].end >> - && end > nodes[j].start) { >> - if (start >= nodes[j].start) { >> - start = nodes[j].end; >> + { >> + struct node *nd = get_numa_node(j); >> + >> + if (start < nd->end >> + && end > nd->start) { >> + if (start >= nd->start) { >> + start = nd->end; >> found = 1; >> } >> - if (end <= nodes[j].end) { >> - end = nodes[j].start; >> + if (end <= nd->end) { >> + end = nd->start; >> found = 1; >> } >> } >> + } >> } while (found && start < end); >> >> if (start < end) { >> @@ -457,6 +507,8 @@ int __init acpi_scan_nodes(uint64_t start, uint64_t >> end) >> { >> int i; >> nodemask_t all_nodes_parsed; >> + struct node *memblks; >> + nodeid_t *nodeids; >> >> /* First clean up the node list */ >> for (i = 0; i < MAX_NUMNODES; i++) >> @@ -470,6 +522,8 @@ int __init acpi_scan_nodes(uint64_t start, uint64_t >> end) >> return -1; >> } >> >> + memblks = get_node_memblk_range(0); >> + nodeids = get_memblk_nodeid_map(); >> if (compute_memnode_shift(node_memblk_range, num_node_memblks, >> memblk_nodeid, &memnode_shift)) { >> memnode_shift = 0; >> @@ -484,12 +538,14 @@ int __init acpi_scan_nodes(uint64_t start, uint64_t >> end) >> /* Finally register nodes */ >> for_each_node_mask(i, all_nodes_parsed) >> { >> - uint64_t size = nodes[i].end - nodes[i].start; >> + struct node *nd = get_numa_node(i); >> + uint64_t size = nd->end - nd->start; >> + >> if ( size == 0 ) >> printk(KERN_WARNING "SRAT: Node %u has no memory. >> " >> "BIOS Bug or mis-configured hardware?\n", >> i); >> >> - setup_node_bootmem(i, nodes[i].start, nodes[i].end); >> + setup_node_bootmem(i, nd->start, nd->end); >> } >> for (i = 0; i < nr_cpu_ids; i++) { >> if (cpu_to_node[i] == NUMA_NO_NODE) >> > > Cheers, > > -- > Julien Grall
On 05/09/2017 08:02 AM, Vijay Kilari wrote: > On Mon, May 8, 2017 at 8:09 PM, Julien Grall <julien.grall@arm.com> wrote: >> Hi Vijay, >> >> On 28/03/17 16:53, vijay.kilari@gmail.com wrote: >>> >>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >>> >>> Add accessor for nodes[] and other static variables and >> >> >> s/accessor/accessors/ >> >>> used those accessors. >> >> >> Also, I am not sure to understand the usefulness of those accessors over a >> global variable. > > These are static variables which needs to accessed from other files and > later moved to generic file. 101 of a contributor, always explaining in the commit message why you do something. Also, I am quite confused why sometimes you decide to use static and helper, other time you will use global variables. Cheers,
diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c index ccacbcd..983e1d8 100644 --- a/xen/arch/x86/srat.c +++ b/xen/arch/x86/srat.c @@ -41,7 +41,45 @@ static struct node node_memblk_range[NR_NODE_MEMBLKS]; static nodeid_t memblk_nodeid[NR_NODE_MEMBLKS]; static __initdata DECLARE_BITMAP(memblk_hotplug, NR_NODE_MEMBLKS); -static inline bool node_found(unsigned idx, unsigned pxm) +static struct node *get_numa_node(int id) +{ + return &nodes[id]; +} + +static nodeid_t get_memblk_nodeid(int id) +{ + return memblk_nodeid[id]; +} + +static nodeid_t *get_memblk_nodeid_map(void) +{ + return &memblk_nodeid[0]; +} + +static struct node *get_node_memblk_range(int memblk) +{ + return &node_memblk_range[memblk]; +} + +static int get_num_node_memblks(void) +{ + return num_node_memblks; +} + +static int __init numa_add_memblk(nodeid_t nodeid, paddr_t start, uint64_t size) +{ + if (nodeid >= NR_NODE_MEMBLKS) + return -EINVAL; + + node_memblk_range[num_node_memblks].start = start; + node_memblk_range[num_node_memblks].end = start + size; + memblk_nodeid[num_node_memblks] = nodeid; + num_node_memblks++; + + return 0; +} + +static inline bool node_found(unsigned int idx, unsigned int pxm) { return ((pxm2node[idx].pxm == pxm) && (pxm2node[idx].node != NUMA_NO_NODE)); @@ -107,11 +145,11 @@ int valid_numa_range(paddr_t start, paddr_t end, nodeid_t node) { int i; - for (i = 0; i < num_node_memblks; i++) { - struct node *nd = &node_memblk_range[i]; + for (i = 0; i < get_num_node_memblks(); i++) { + struct node *nd = get_node_memblk_range(i); if (nd->start <= start && nd->end > end && - memblk_nodeid[i] == node ) + get_memblk_nodeid(i) == node) return 1; } @@ -122,8 +160,8 @@ static int __init conflicting_memblks(paddr_t start, paddr_t end) { int i; - for (i = 0; i < num_node_memblks; i++) { - struct node *nd = &node_memblk_range[i]; + for (i = 0; i < get_num_node_memblks(); i++) { + struct node *nd = get_node_memblk_range(i); if (nd->start == nd->end) continue; if (nd->end > start && nd->start < end) @@ -136,7 +174,8 @@ static int __init conflicting_memblks(paddr_t start, paddr_t end) static void __init cutoff_node(int i, paddr_t start, paddr_t end) { - struct node *nd = &nodes[i]; + struct node *nd = get_numa_node(i); + if (nd->start < start) { nd->start = start; if (nd->end < nd->start) @@ -278,6 +317,7 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma) unsigned pxm; nodeid_t node; int i; + struct node *memblk; if (srat_disabled()) return; @@ -288,7 +328,7 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma) if (!(ma->flags & ACPI_SRAT_MEM_ENABLED)) return; - if (num_node_memblks >= NR_NODE_MEMBLKS) + if (get_num_node_memblks() >= NR_NODE_MEMBLKS) { dprintk(XENLOG_WARNING, "Too many numa entry, try bigger NR_NODE_MEMBLKS \n"); @@ -310,27 +350,31 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma) i = conflicting_memblks(start, end); if (i < 0) /* everything fine */; - else if (memblk_nodeid[i] == node) { + else if (get_memblk_nodeid(i) == node) { bool mismatch = !(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) != !test_bit(i, memblk_hotplug); + memblk = get_node_memblk_range(i); + printk("%sSRAT: PXM %u (%"PRIx64"-%"PRIx64") overlaps with itself (%"PRIx64"-%"PRIx64")\n", mismatch ? KERN_ERR : KERN_WARNING, pxm, start, end, - node_memblk_range[i].start, node_memblk_range[i].end); + memblk->start, memblk->end); if (mismatch) { bad_srat(); return; } } else { + memblk = get_node_memblk_range(i); + printk(KERN_ERR "SRAT: PXM %u (%"PRIx64"-%"PRIx64") overlaps with PXM %u (%"PRIx64"-%"PRIx64")\n", - pxm, start, end, node_to_pxm(memblk_nodeid[i]), - node_memblk_range[i].start, node_memblk_range[i].end); + pxm, start, end, node_to_pxm(get_memblk_nodeid(i)), + memblk->start, memblk->end); bad_srat(); return; } if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE)) { - struct node *nd = &nodes[node]; + struct node *nd = get_numa_node(node); if (!node_test_and_set(node, memory_nodes_parsed)) { nd->start = start; @@ -346,15 +390,17 @@ acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma) node, pxm, start, end, ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE ? " (hotplug)" : ""); - node_memblk_range[num_node_memblks].start = start; - node_memblk_range[num_node_memblks].end = end; - memblk_nodeid[num_node_memblks] = node; + if (numa_add_memblk(node, start, ma->length)) { + printk(KERN_ERR "SRAT: node-id %u out of range\n", node); + bad_srat(); + return; + } + if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) { - __set_bit(num_node_memblks, memblk_hotplug); + __set_bit(get_num_node_memblks(), memblk_hotplug); if (end > mem_hotplug) mem_hotplug = end; } - num_node_memblks++; } /* Sanity check to catch more bad SRATs (they are amazingly common). @@ -377,17 +423,21 @@ static int __init nodes_cover_memory(void) do { found = 0; for_each_node_mask(j, memory_nodes_parsed) - if (start < nodes[j].end - && end > nodes[j].start) { - if (start >= nodes[j].start) { - start = nodes[j].end; + { + struct node *nd = get_numa_node(j); + + if (start < nd->end + && end > nd->start) { + if (start >= nd->start) { + start = nd->end; found = 1; } - if (end <= nodes[j].end) { - end = nodes[j].start; + if (end <= nd->end) { + end = nd->start; found = 1; } } + } } while (found && start < end); if (start < end) { @@ -457,6 +507,8 @@ int __init acpi_scan_nodes(uint64_t start, uint64_t end) { int i; nodemask_t all_nodes_parsed; + struct node *memblks; + nodeid_t *nodeids; /* First clean up the node list */ for (i = 0; i < MAX_NUMNODES; i++) @@ -470,6 +522,8 @@ int __init acpi_scan_nodes(uint64_t start, uint64_t end) return -1; } + memblks = get_node_memblk_range(0); + nodeids = get_memblk_nodeid_map(); if (compute_memnode_shift(node_memblk_range, num_node_memblks, memblk_nodeid, &memnode_shift)) { memnode_shift = 0; @@ -484,12 +538,14 @@ int __init acpi_scan_nodes(uint64_t start, uint64_t end) /* Finally register nodes */ for_each_node_mask(i, all_nodes_parsed) { - uint64_t size = nodes[i].end - nodes[i].start; + struct node *nd = get_numa_node(i); + uint64_t size = nd->end - nd->start; + if ( size == 0 ) printk(KERN_WARNING "SRAT: Node %u has no memory. " "BIOS Bug or mis-configured hardware?\n", i); - setup_node_bootmem(i, nodes[i].start, nodes[i].end); + setup_node_bootmem(i, nd->start, nd->end); } for (i = 0; i < nr_cpu_ids; i++) { if (cpu_to_node[i] == NUMA_NO_NODE)