[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/1] pc-dimm: Change PCDIMMDevice->node from UIN
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 1/1] pc-dimm: Change PCDIMMDevice->node from UINT32 to INT32, and initialize it as -1. |
Date: |
Mon, 18 Aug 2014 15:56:02 +0200 |
On Mon, Aug 18, 2014 at 08:12:14PM +0800, Tang Chen wrote:
> If user doesn't specify numa options, nb_numa_nodes will be 0. But
> PCDIMMDevice->node
> is also initialized to 0. As a result, the following check will fail:
>
> pc_dimm_realize()
> {
> ......
> if (dimm->node >= nb_numa_nodes) {
> error_setg(errp, "'DIMM property " PC_DIMM_NODE_PROP " has value
> %"
> PRIu32 "' which exceeds the number of numa nodes: %d",
> dimm->node, nb_numa_nodes);
> return;
> }
> ......
> }
>
> But this is not an error.
>
> PCDIMMDevice->node should be initialized to -1. This is for users who do not
> use
> NUMA.
>
> Signed-off-by: Tang Chen <address@hidden>
> ---
> hw/mem/pc-dimm.c | 2 +-
> include/hw/mem/pc-dimm.h | 2 +-
> include/sysemu/sysemu.h | 6 ++++++
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index ec8b1a3..4b26611 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -197,7 +197,7 @@ out:
>
> static Property pc_dimm_properties[] = {
> DEFINE_PROP_UINT64(PC_DIMM_ADDR_PROP, PCDIMMDevice, addr, 0),
> - DEFINE_PROP_UINT32(PC_DIMM_NODE_PROP, PCDIMMDevice, node, 0),
> + DEFINE_PROP_INT32(PC_DIMM_NODE_PROP, PCDIMMDevice, node, NO_NODE_ID),
> DEFINE_PROP_INT32(PC_DIMM_SLOT_PROP, PCDIMMDevice, slot,
> PC_DIMM_UNASSIGNED_SLOT),
> DEFINE_PROP_END_OF_LIST(),
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index 761eeef..82abb2f 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -53,7 +53,7 @@ typedef struct PCDIMMDevice {
>
> /* public */
> uint64_t addr;
> - uint32_t node;
> + int32_t node;
> int32_t slot;
> HostMemoryBackend *hostmem;
> } PCDIMMDevice;
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index d8539fd..c6b9bae 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -139,6 +139,12 @@ extern int mem_prealloc;
>
> #define MAX_NODES 128
>
> +/* Initialize PCDIMMDevice->node to -1 so that even if user doesn't specify
> + * any numa option, PCDIMMDevice->node won't be 0, which indicates node0.
> + * In this case, SRAT won't be created, and guest kernel will fake a node.
> + */
I think you simply mean
/* default value for PCDIMMDevice->node (unless specified by user) */
> +#define NO_NODE_ID -1
> +
So why do we want this macro here? Put the value near PCDIMMDevice where
it's used.
> /* The following shall be true for all CPUs:
> * cpu->cpu_index < max_cpus <= MAX_CPUMASK_BITS
> *
> --
> 1.8.4.2