Re: [PATCH v2] NUMA: limit first_valid_mfn exposure

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.01.2024 15:13, Julien Grall wrote:
> 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.

Hmm, okay. Then my 2nd scalability concern, in another dimension: What
if static-ness ends up depending on two (or more) CONFIG_*?

Jan