Re: [Qemu-devel] [PATCH V16 09/11] NUMA: set guest numa nodes memory policy

[Paolo Bonzini <pbonzini@redhat.com>]

Il 22/11/2013 03:08, Wanlong Gao ha scritto:
> +static int set_node_mem_policy(int nodeid)
> +{
> +#ifdef __linux__
> +    void *ram_ptr;
> +    RAMBlock *block;
> +    ram_addr_t len, ram_offset = 0;
> +    int bind_mode;
> +    int i;
> +
> +    QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> +        if (!strcmp(block->mr->name, "pc.ram")) {

This is not acceptable, "pc.ram" is a board-specific name.

I think instead set_node_mem_policy could be something like

int memory_region_set_mem_policy(MemoryRegion *mr,
                                 uint64_t start, uint64_t length,
                                 uint64_t offset);

that applies the NUMA policies specified for [offset, offset+length) to
the host physical address [ptr+start, ptr+start+length), where ptr is
memory_region_get_ram_ptr(mr).

Each board then can call the function after it adds RAM with
memory_region_add_subregion.

Paolo

> +            break;
> +        }
> +    }
> +
> +    if (block->host == NULL) {
> +        return -1;
> +    }
> +
> +    ram_ptr = block->host;
> +    for (i = 0; i < nodeid; i++) {
> +        len = numa_info[i].node_mem;
> +        ram_offset += len;
> +    }
> +
> +    len = numa_info[nodeid].node_mem;
> +    bind_mode = node_parse_bind_mode(nodeid);
> +    unsigned long *nodes = numa_info[nodeid].host_mem;
> +
> +    /* This is a workaround for a long standing bug in Linux'
> +     * mbind implementation, which cuts off the last specified
> +     * node. To stay compatible should this bug be fixed, we
> +     * specify one more node and zero this one out.
> +     */
> +    unsigned long maxnode = find_last_bit(nodes, MAX_NODES);
> +    if (syscall(SYS_mbind, ram_ptr + ram_offset, len, bind_mode,
> +                nodes, maxnode + 2, 0)) {
> +            perror("mbind");
> +            return -1;
> +    }

Also, it's still not clear to me why we're not using libnuma.

> +#endif
> +
> +    return 0;
> +}
> +
>  void set_numa_modes(void)
>  {
>      CPUState *cpu;
> @@ -240,4 +319,11 @@ void set_numa_modes(void)
>              }
>          }
>      }
> +
> +    for (i = 0; i < nb_numa_nodes; i++) {
> +        if (set_node_mem_policy(i) == -1) {
> +            fprintf(stderr,
> +                    "qemu: can not set host memory policy for node%d\n", i);
> +        }
> +    }
>  }
>