Re: [Xen-devel] [RFC PATCH v2 02/25] x86: NUMA: Fix datatypes and attributes

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.03.17 at 17:53, <vijay.kilari@xxxxxxxxx> wrote:
> Change u{8,32,64} to uint{8,32,64}_t and bool_t to bool.

Oh, I see you do this in a separate patch. Please disregard the
respective comments on patch 1 then.

> -bool_t numa_off = 0;
> +bool numa_off = 0;

"false" at the same time (and "true" wherever it is being set).

> @@ -201,19 +201,19 @@ void __init numa_init_array(void)
>  }
>  
>  #ifdef CONFIG_NUMA_EMU
> -static int numa_fake __initdata = 0;
> +static int __initdata numa_fake = 0;

unsigned int and the initializer can be dropped at once.

>  /* Numa emulation */
> -static int __init numa_emulation(u64 start_pfn, u64 end_pfn)
> +static int __init numa_emulation(uint64_t start_pfn, uint64_t end_pfn)
>  {
>      int i;
>      struct node nodes[MAX_NUMNODES];
> -    u64 sz = ((end_pfn - start_pfn) << PAGE_SHIFT) / numa_fake;
> +    uint64_t sz = ((end_pfn - start_pfn) << PAGE_SHIFT) / numa_fake;
>  
>      /* Kludge needed for the hash function */
>      if ( hweight64(sz) > 1 )
>      {
> -        u64 x = 1;
> +        uint64_t x = 1;
>          while ( (x << 1) < sz )

Please also add the missing blank line between declaration and
statements (same elsewhere).

> @@ -260,8 +260,8 @@ void __init numa_initmem_init(unsigned long start_pfn, 
> unsigned long end_pfn)
>  #endif
>  
>  #ifdef CONFIG_ACPI_NUMA
> -    if ( !numa_off && !acpi_scan_nodes((u64)start_pfn << PAGE_SHIFT,
> -         (u64)end_pfn << PAGE_SHIFT) )
> +    if ( !numa_off && !acpi_scan_nodes((uint64_t)start_pfn << PAGE_SHIFT,
> +         (uint64_t)end_pfn << PAGE_SHIFT) )

Instead of altering the casts please use pfn_to_paddr() (also further
down).

> --- a/xen/arch/x86/srat.c
> +++ b/xen/arch/x86/srat.c
> @@ -23,12 +23,12 @@
>  
>  static struct acpi_table_slit *__read_mostly acpi_slit;
>  
> -static nodemask_t memory_nodes_parsed __initdata;
> -static nodemask_t processor_nodes_parsed __initdata;
> -static struct node nodes[MAX_NUMNODES] __initdata;
> +static nodemask_t __initdata memory_nodes_parsed;
> +static nodemask_t __initdata processor_nodes_parsed;
> +static struct node __initdata nodes[MAX_NUMNODES];
>  
>  struct pxm2node {
> -     unsigned pxm;
> +     unsigned int pxm;
>       nodeid_t node;
>  };

How come there are still tabs left here after patch 1?

> @@ -64,9 +64,9 @@ nodeid_t pxm_to_node(unsigned pxm)
>  nodeid_t setup_node(unsigned pxm)
>  {
>       nodeid_t node;
> -     unsigned idx;
> -     static bool_t warned;
> -     static unsigned nodes_found;
> +     unsigned int idx;
> +     static bool warned;

Again together with the type change you should also change the
setting of the variable to use "true".

> @@ -134,7 +134,7 @@ static __init int conflicting_memblks(u64 start, u64 end)
>       return -1;
>  }
>  
> -static __init void cutoff_node(int i, u64 start, u64 end)
> +static void __init cutoff_node(int i, paddr_t start, paddr_t end)

The first parameter also wants changing (presumably nodeid_t).

> @@ -274,7 +274,7 @@ acpi_numa_processor_affinity_init(const struct 
> acpi_srat_cpu_affinity *pa)
>  void __init
>  acpi_numa_memory_affinity_init(const struct acpi_srat_mem_affinity *ma)
>  {
> -     u64 start, end;
> +     uint64_t start, end;

Why not paddr_t, like you do elsewhere?

> @@ -428,9 +428,9 @@ static int __init srat_parse_region(struct 
> acpi_subtable_header *header,
>       return 0;
>  }
>  
> -void __init srat_parse_regions(u64 addr)
> +void __init srat_parse_regions(uint64_t addr)

Same (at least) here.

Jan


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