From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Jackson <Ian.Jackson@eu.citrix.com>
Subject: Re: [PATCH RESEND 12/12] xl: numa-sched: enable
 specifying node-affinity in VM config file
Date: Thu, 7 Nov 2013 18:35:35 +0000
Message-ID: <21115.56695.827216.459048@mariner.uk.xensource.com>
References: <20131105142844.30446.78671.stgit@Solace>
	<20131105143617.30446.53267.stgit@Solace>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <20131105143617.30446.53267.stgit@Solace>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Marcus Granado <Marcus.Granado@eu.citrix.com>, Keir Fraser <keir@xen.org>, Ian Campbell <Ian.Campbell@citrix.com>, Li Yechen <lccycc123@gmail.com>, George Dunlap <george.dunlap@eu.citrix.com>, Andrew Cooper <Andrew.Cooper3@citrix.com>, Juergen Gross <juergen.gross@ts.fujitsu.com>, xen-devel@lists.xen.org, Jan Beulich <JBeulich@suse.com>, Justin Weaver <jtweaver@hawaii.edu>, Daniel De Graaf <dgdegra@tycho.nsa.gov>, Matt Wilson <msw@amazon.com>, Elena Ufimtseva <ufimtseva@gmail.com>
List-Id: xen-devel@lists.xenproject.org

Dario Faggioli writes ("[PATCH RESEND 12/12] xl: numa-sched: enable specifying node-affinity in VM config file"):
> in a similar way to how it is possible to specify vcpu-affinity.
...
>      /*
> -     * Check if the domain has any CPU affinity. If not, try to build
> -     * up one. In case numa_place_domain() find at least a suitable
> -     * candidate, it will affect info->nodemap accordingly; if it
> -     * does not, it just leaves it as it is. This means (unless
> -     * some weird error manifests) the subsequent call to
> -     * libxl_domain_set_nodeaffinity() will do the actual placement,
> +     * Check if the domain has any pinning or node-affinity and, if not, try
> +     * to build up one.
> +     *
> +     * In case numa_place_domain() find at least a suitable candidate, it will
> +     * affect info->nodemap accordingly; if it does not, it just leaves it as
> +     * it is. This means (unless some weird error manifests) the subsequent
> +     * call to libxl_domain_set_nodeaffinity() will do the actual placement,
>       * whatever that turns out to be.
>       */
>      if (libxl_defbool_val(info->numa_placement)) {
>  
> -        if (!libxl_bitmap_is_full(&info->cpumap)) {
> +        if (!libxl_bitmap_is_full(&info->cpumap) ||
> +            !libxl_bitmap_is_full(&info->nodemap)) {
>              LOG(ERROR, "Can run NUMA placement only if no vcpu "
> -                       "affinity is specified");
> +                       "pinning or node-affinity is specified");

I appreciate I may be a bit late with this complaint, but surely this
approach (setting the cpumap and nodemap when they aren't
all-bits-set) means that it's not possible to explicitly set "all
cpus".

If I say in the config file "all" for cpus and "all" for nodes, this
code will override that.  I don't think that can be right.

Ian.