Re: [RFC PATCH v2 01/25] x86: NUMA: Clean up: Drop trailing spaces

From: "Jan Beulich" <JBeulich@suse.com>
To: vijay.kilari@gmail.com
Cc: tim@xen.org, sstabellini@kernel.org, wei.liu2@citrix.com,
	George.Dunlap@eu.citrix.com, andrew.cooper3@citrix.com,
	ian.jackson@eu.citrix.com,
	Vijaya Kumar K <Vijaya.Kumar@cavium.com>,
	julien.grall@arm.com, xen-devel@lists.xen.org
Subject: Re: [RFC PATCH v2 01/25] x86: NUMA: Clean up: Drop trailing spaces
Date: Wed, 31 May 2017 04:20:20 -0600	[thread overview]
Message-ID: <592EB504020000780015E298@prv-mh.provo.novell.com> (raw)
In-Reply-To: <1490716413-19796-2-git-send-email-vijay.kilari@gmail.com>

>>> On 28.03.17 at 17:53, <vijay.kilari@gmail.com> wrote:
> --- a/xen/include/asm-x86/numa.h
> +++ b/xen/include/asm-x86/numa.h
> @@ -1,4 +1,4 @@
> -#ifndef _ASM_X8664_NUMA_H 
> +#ifndef _ASM_X8664_NUMA_H
>  #define _ASM_X8664_NUMA_H 1
>  
>  #include <xen/cpumask.h>
> @@ -12,21 +12,20 @@ extern int srat_rev;
>  extern nodeid_t      cpu_to_node[NR_CPUS];
>  extern cpumask_t     node_to_cpumask[];
>  
> -#define cpu_to_node(cpu)		(cpu_to_node[cpu])
> -#define parent_node(node)		(node)
> +#define cpu_to_node(cpu)         (cpu_to_node[cpu])
> +#define parent_node(node)        (node)
>  #define node_to_first_cpu(node)  (__ffs(node_to_cpumask[node]))
>  #define node_to_cpumask(node)    (node_to_cpumask[node])
>  
> -struct node { 
> -	u64 start,end; 
> +struct node {
> +    u64 start,end;

You want to add a blank after the comma. Also at least where you
touch lines anyway, please also switch to uint64_t and alike.

> @@ -42,14 +41,8 @@ extern void setup_node_bootmem(nodeid_t nodeid, u64 start, u64 end);
>  extern nodeid_t apicid_to_node[];
>  extern void init_cpu_to_node(void);
>  
> -static inline void clear_node_cpumask(int cpu)
> -{
> -	cpumask_clear_cpu(cpu, &node_to_cpumask[cpu_to_node(cpu)]);
> -}
> -
>  /* Simple perfect hash to map pdx to node numbers */
> -extern int memnode_shift; 
> -extern unsigned long memnodemapsize;
> +extern int memnode_shift;

If you remove an extern declaration from a header, the
corresponding definition should become static.

> @@ -60,20 +53,16 @@ struct node_data {
>  extern struct node_data node_data[];
>  
>  static inline __attribute__((pure)) nodeid_t phys_to_nid(paddr_t addr)
> -{ 
> -	nodeid_t nid;
> -	VIRTUAL_BUG_ON((paddr_to_pdx(addr) >> memnode_shift) >= memnodemapsize);
> -	nid = memnodemap[paddr_to_pdx(addr) >> memnode_shift]; 
> -	VIRTUAL_BUG_ON(nid >= MAX_NUMNODES || !node_data[nid]); 
> -	return nid; 
> -} 
> -
> -#define NODE_DATA(nid)		(&(node_data[nid]))
> -
> -#define node_start_pfn(nid)	(NODE_DATA(nid)->node_start_pfn)
> -#define node_spanned_pages(nid)	(NODE_DATA(nid)->node_spanned_pages)
> +{
> +    return memnodemap[paddr_to_pdx(addr) >> memnode_shift];

I think it would be a good idea to convert the ineffective
VIRTUAL_BUG_ON()s to ASSERT()s.

> +}
> +
> +#define NODE_DATA(nid)          (&(node_data[nid]))

Please drop the pointless inner parentheses.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel