|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC PATCH v2 06/25] x86: NUMA: Add accessors for nodes[] and node_memblk_range[] structs
On Mon, May 8, 2017 at 8:09 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
> Hi Vijay,
>
> On 28/03/17 16:53, vijay.kilari@xxxxxxxxx wrote:
>>
>> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> ---
>> 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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |