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 <jbeul...@suse.com> >>>> --- >>>> 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
