Re: [PATCH v2] NUMA: limit first_valid_mfn exposure

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 08/01/2024 11:43, Jan Beulich wrote:
> On 08.01.2024 12:37, Julien Grall wrote:
> > On 08/01/2024 11:31, Jan Beulich wrote:
> > > Address the TODO regarding first_valid_mfn by making the variable static
> > > when NUMA=y, thus also addressing a Misra C:2012 rule 8.4 concern (on
> > > x86).
> > >
> > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > ---
> > > Julien suggests something like
> > >
> > > STATIC_IF(CONFIG_NUMA) unsigned long first_valid_mfn;
> > >
> > > but I view this as non-scalable (or at least I can't see how to
> > > implement such in a scalabale way) and hence undesirable to introduce.
> > I don't really see the scalability problem. Can you explain a bit more?
> Well, when seeing your original suggestion, I first considered it quite
> reasonable. But when thinking how to implement it, I couldn't see what
>
> #define STATIC_IF(cfg)
>
> should expand to. That's simply because a macro body cannot itself have
> pre-processor directives. Hence all I could think of was
>
> #ifdef CONFIG_NUMA
> # define static_if_CONFIG_NUMA static
> #else
> # define static_if_CONFIG_NUMA
> #endif
> #define STATIC_IF(cfg) static_if_ ## cfg
>
> And I think it is easy to see how this wouldn't scale across CONFIG_xyz.
> Plus that that point STATIC_IF() itself would be pretty much redundant.
> But maybe I'm simply overlooking the obvious ...
You can use the same trick as for IS_ENABLED. The code below will select 
static or nothing:
#define static_enabled(cfg) _static_enabled(cfg)
#define _static_enabled(value) __static_enabled(__ARG_PLACEHOLDER_##value)
#define __static_enabled(arg1_or_junk) ___static_enabled(arg1_or_junk 
static,)
#define ___static_enabled(__ignored, val, ...) val

#define STATIC_IF(option) static_enabled(option)

I have tested both with CONFIG_NUMA and !CONFIG_NUMA to confirm the 
visibility of the variable will be correct.
Cheers,

--
Julien Grall