Re: [PATCH v2 02/17] xen/arm: implement helpers to get and update NUMA status

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.01.2023 09:49, Wei Chen wrote:
> --- a/xen/arch/arm/include/asm/numa.h
> +++ b/xen/arch/arm/include/asm/numa.h
> @@ -22,6 +22,12 @@ typedef u8 nodeid_t;
>   */
>  #define NR_NODE_MEMBLKS NR_MEM_BANKS
>  
> +enum dt_numa_status {
> +    DT_NUMA_INIT,

I don't see any use of this. I also think the name isn't good, as INIT
can be taken for "initializer" as well as "initialized". Suggesting an
alternative would require knowing what the future plans with this are;
right now ...

> +    DT_NUMA_ON,
> +    DT_NUMA_OFF,
> +};

... the other two are also used only in a single file, at which point
their placing in a header is also questionable.

> --- /dev/null
> +++ b/xen/arch/arm/numa.c
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Arm Architecture support layer for NUMA.
> + *
> + * Copyright (C) 2021 Arm Ltd
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +#include <xen/init.h>
> +#include <xen/numa.h>
> +
> +static enum dt_numa_status __read_mostly device_tree_numa;

__ro_after_init?

> --- a/xen/arch/x86/include/asm/numa.h
> +++ b/xen/arch/x86/include/asm/numa.h
> @@ -12,7 +12,6 @@ extern unsigned int numa_node_to_arch_nid(nodeid_t n);
>  
>  #define ZONE_ALIGN (1UL << (MAX_ORDER+PAGE_SHIFT))
>  
> -extern bool numa_disabled(void);
>  extern nodeid_t setup_node(unsigned int pxm);
>  extern void srat_detect_node(int cpu);
>  
> --- a/xen/include/xen/numa.h
> +++ b/xen/include/xen/numa.h
> @@ -55,6 +55,7 @@ extern void numa_init_array(void);
>  extern void numa_set_node(unsigned int cpu, nodeid_t node);
>  extern void numa_initmem_init(unsigned long start_pfn, unsigned long 
> end_pfn);
>  extern void numa_fw_bad(void);
> +extern bool numa_disabled(void);
>  
>  extern int arch_numa_setup(const char *opt);
>  extern bool arch_numa_unavailable(void);

How is this movement of a declaration related to the subject of the patch?

Jan