From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v6 02/23] xen: move NUMA_NO_NODE to public
 memory.h as XEN_NUMA_NO_NODE
Date: Fri, 27 Feb 2015 11:38:51 +0000
Message-ID: <54F0574B.50302@citrix.com>
References: <1424966166-2388-1-git-send-email-wei.liu2@citrix.com>
	<1424966166-2388-3-git-send-email-wei.liu2@citrix.com>
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: <1424966166-2388-3-git-send-email-wei.liu2@citrix.com>
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: Wei Liu <wei.liu2@citrix.com>, xen-devel@lists.xen.org
Cc: Jan Beulich <JBeulich@suse.com>
List-Id: xen-devel@lists.xenproject.org

On 26/02/15 15:55, Wei Liu wrote:
> Update NUMA_NO_NODE in Xen code to use the new macro.
>
> No functional change introduced.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
>
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index 595f953..0d8c85f 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -29,6 +29,8 @@
>  
>  #include "xen.h"
>  
> +#define XEN_NUMA_NO_NODE    0xFF
> +

I would suggest moving the define to beside the XENMEMF_{,get_}node()
definitions, which is where it is used.

(Also, someone in the future is going to have a fun time extending Xen's
node ids beyond 255, seeing as this is baked into our ABI)

>  /*
>   * Increase or decrease the specified domain's memory reservation. Returns the
>   * number of extents successfully allocated or freed.
> diff --git a/xen/include/xen/numa.h b/xen/include/xen/numa.h
> index ac4b391..60daaa5 100644
> --- a/xen/include/xen/numa.h
> +++ b/xen/include/xen/numa.h
> @@ -1,20 +1,19 @@
>  #ifndef _XEN_NUMA_H
>  #define _XEN_NUMA_H
>  
> +#include <public/memory.h>
>  #include <asm/numa.h>
>  
>  #ifndef NODES_SHIFT
>  #define NODES_SHIFT     0
>  #endif
>  
> -#define NUMA_NO_NODE    0xFF
> -

An option would be to

#define NUMA_NO_NODE XEN_NUMA_NO_NODE

Which would avoid changing any other code. We have a precedent for this
with the errno public header.

I would welcome the opinions of other maintainers.

~Andrew

>  #define MAX_NUMNODES    (1 << NODES_SHIFT)
>  
>  #define vcpu_to_node(v) (cpu_to_node((v)->processor))
>  
>  #define domain_to_node(d) \
>    (((d)->vcpu != NULL && (d)->vcpu[0] != NULL) \
> -   ? vcpu_to_node((d)->vcpu[0]) : NUMA_NO_NODE)
> +   ? vcpu_to_node((d)->vcpu[0]) : XEN_NUMA_NO_NODE)
>  
>  #endif /* _XEN_NUMA_H */