Re: [XEN PATCH v3 3/3] xen/mm: add declaration for first_valid_mfn

From: Jan Beulich <jbeulich@suse.com>
To: Nicola Vetrini <nicola.vetrini@bugseng.com>
Cc: consulting@bugseng.com,
	Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Michal Orzel <michal.orzel@amd.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>, Wei Liu <wl@xen.org>,
	Shawn Anastasio <sanastasio@raptorengineering.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [XEN PATCH v3 3/3] xen/mm: add declaration for first_valid_mfn
Date: Wed, 13 Dec 2023 09:26:55 +0100	[thread overview]
Message-ID: <6c717bd4-db1b-4e19-9b98-0776a8359085@suse.com> (raw)
In-Reply-To: <d80309f31fea24ea75c4994e924da069472811fc.1702285639.git.nicola.vetrini@bugseng.com>

On 11.12.2023 10:14, Nicola Vetrini wrote:
> --- a/xen/arch/arm/include/asm/numa.h
> +++ b/xen/arch/arm/include/asm/numa.h
> @@ -2,8 +2,9 @@
>  #define __ARCH_ARM_NUMA_H
>  
>  #include <xen/mm.h>

With this, ...

> +#include <xen/types.h>
>  
> -typedef u8 nodeid_t;
> +typedef uint8_t nodeid_t;
>  
>  #ifndef CONFIG_NUMA
>  
> @@ -12,10 +13,9 @@ typedef u8 nodeid_t;
>  #define node_to_cpumask(node)   (cpu_online_map)
>  
>  /*
> - * TODO: make first_valid_mfn static when NUMA is supported on Arm, this
> - * is required because the dummy helpers are using it.
> + * TODO: define here first_valid_mfn as static when NUMA is supported on Arm,
> + * this is required because the dummy helpers are using it.
>   */
> -extern mfn_t first_valid_mfn;

... and this declaration moved to xen/mm.h, how is it going to be possible
to do as the adjusted comment says? The compiler will choke on the static
after having seen the extern.

I also firmly disagree with the entirely unrelated u8 -> uint8_t change
above. Such tidying can be done when somewhat related to what a patch is
doing anyway, but here there's (imo) not even a vague connection.

Jan