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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 13 Dec 2023, Jan Beulich wrote:
> 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.

Hi Jan,

Nicola was just following a review comment by Julien. NUMA has been
pending for a while and I wouldn't hold this patch back because of it.
I suggest we go with Julien's request (this version of the patch).

If you feel strongly about it, please suggest an alternative.


> 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.

Can be removed on commit