From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54017) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UwGGn-0002Rb-A0 for qemu-devel@nongnu.org; Mon, 08 Jul 2013 14:34:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UwGGl-0001Ge-Ry for qemu-devel@nongnu.org; Mon, 08 Jul 2013 14:34:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34170) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UwGGl-0001Ga-Jn for qemu-devel@nongnu.org; Mon, 08 Jul 2013 14:34:39 -0400 Date: Mon, 8 Jul 2013 14:34:31 -0400 From: Luiz Capitulino Message-ID: <20130708143431.057b1d14@redhat.com> In-Reply-To: <20130708142514.714dade6@redhat.com> References: <1372931597-28115-1-git-send-email-gaowanlong@cn.fujitsu.com> <1372931597-28115-9-git-send-email-gaowanlong@cn.fujitsu.com> <20130708142514.714dade6@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V4 08/10] NUMA: add qmp command set-mpol to set memory policy for NUMA node List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wanlong Gao Cc: aliguori@us.ibm.com, ehabkost@redhat.com, qemu-devel@nongnu.org, bsd@redhat.com, y-goto@jp.fujitsu.com, pbonzini@redhat.com, afaerber@suse.de On Mon, 8 Jul 2013 14:25:14 -0400 Luiz Capitulino wrote: > On Thu, 4 Jul 2013 17:53:15 +0800 > Wanlong Gao wrote: > > > The QMP command let it be able to set node's memory policy > > through the QMP protocol. The qmp-shell command is like: > > set-mpol nodeid=0 mem-policy=membind mem-hostnode=0-1 > > > > Signed-off-by: Wanlong Gao > > --- > > cpus.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > qapi-schema.json | 15 +++++++++++++++ > > qmp-commands.hx | 35 +++++++++++++++++++++++++++++++++++ > > 3 files changed, 104 insertions(+) > > > > diff --git a/cpus.c b/cpus.c > > index 7240de7..ff42b9d 100644 > > --- a/cpus.c > > +++ b/cpus.c > > @@ -1417,3 +1417,57 @@ void qmp_inject_nmi(Error **errp) > > error_set(errp, QERR_UNSUPPORTED); > > #endif > > } > > + > > +void qmp_set_mpol(int64_t nodeid, bool has_mpol, const char *mpol, > > + bool has_hostnode, const char *hostnode, Error **errp) > > +{ > > + unsigned int flags; > > + DECLARE_BITMAP(host_mem, MAX_CPUMASK_BITS); > > + > > + if (nodeid >= nb_numa_nodes) { > > + error_setg(errp, "Only has '%d' NUMA nodes", nb_numa_nodes); > > + return; > > + } > > + > > + bitmap_copy(host_mem, numa_info[nodeid].host_mem, MAX_CPUMASK_BITS); > > + flags = numa_info[nodeid].flags; > > + > > + numa_info[nodeid].flags = NODE_HOST_NONE; > > + bitmap_zero(numa_info[nodeid].host_mem, MAX_CPUMASK_BITS); > > + > > + if (!has_mpol) { > > + if (set_node_mpol(nodeid) == -1) { > > + error_setg(errp, "Failed to set memory policy for node%lu", nodeid); > > + goto error; > > + } > > + return; > > + } > > + > > + numa_node_parse_mpol(nodeid, mpol, errp); > > + if (error_is_set(errp)) { > > + goto error; > > + } > > + > > + if (!has_hostnode) { > > + bitmap_fill(numa_info[nodeid].host_mem, MAX_CPUMASK_BITS); > > + } > > + > > + if (hostnode) { > > + numa_node_parse_hostnode(nodeid, hostnode, errp); > > + if (error_is_set(errp)) { > > + goto error; > > + } > > + } > > + > > + if (set_node_mpol(nodeid) == -1) { > > + error_setg(errp, "Failed to set memory policy for node%lu", nodeid); > > + goto error; > > + } > > + > > + return; > > + > > +error: > > + bitmap_copy(numa_info[nodeid].host_mem, host_mem, MAX_CPUMASK_BITS); > > + numa_info[nodeid].flags = flags; > > + return; > > +} > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 5c32528..0870da2 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -3712,3 +3712,18 @@ > > '*cpuid-input-ecx': 'int', > > 'cpuid-register': 'X86CPURegister32', > > 'features': 'int' } } > > + > > +# @set-mpol: > > +# > > +# Set the host memory binding policy for guest NUMA node. > > +# > > +# @nodeid: The node ID of guest NUMA node to set memory policy to. > > +# > > +# @mem-policy: The memory policy string to set. > > Shouldn't this be an enum? Also, optional members have a leading '#optional' > string and if a default value is used it should be documented. > > > +# > > +# @mem-hostnode: The host node or node range for memory policy. > > It doesn't seem appropriate to use a string here. Maybe we could > use a list with only to values (like [0,2] for 0-2) or maybe a > list of nodes if that makes sense (like [0,1,2]). I forgot to bike-shed on the naming. I'd suggest the following: - s/set-mpol/set-numa-policy - s/mem-policy/policy - s/mem-hostnode/host-nodes > > > +# > > +# Since: 1.6.0 > > +## > > +{ 'command': 'set-mpol', 'data': {'nodeid': 'int', '*mem-policy': 'str', > > + '*mem-hostnode': 'str'} } > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index 362f0e1..ccab51b 100644 > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -3043,3 +3043,38 @@ Example: > > <- { "return": {} } > > > > EQMP > > + > > + { > > + .name = "set-mpol", > > + .args_type = "nodeid:i,mem-policy:s?,mem-hostnode:s?", > > + .help = "Set the host memory binding policy for guest NUMA node", > > + .mhandler.cmd_new = qmp_marshal_input_set_mpol, > > + }, > > + > > +SQMP > > +set-mpol > > +------ > > + > > +Set the host memory binding policy for guest NUMA node > > + > > +Arguments: > > + > > +- "nodeid": The nodeid of guest NUMA node to set memory policy to. > > + (json-int) > > +- "mem-policy": The memory policy string to set. > > + (json-string, optional) > > +- "mem-hostnode": The host nodes contained to mpol. > > + (json-string, optional) > > + > > +Example: > > + > > +-> { "execute": "set-mpol", "arguments": { "nodeid": 0, "mem-policy": "membind", > > + "mem-hostnode": "0-1" }} > > +<- { "return": {} } > > + > > +Notes: > > + 1. If "mem-policy" is not set, the memory policy of this "nodeid" will be set > > + to "default". > > + 2. If "mem-hostnode" is not set, the node mask of this "mpol" will be set > > + to "all". > > +EQMP >