On 08.01.2024 15:57, Julien Grall wrote: > Hi Jan, > > On 08/01/2024 14:47, Jan Beulich wrote: >> 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_*? > > Do you have any concrete example where this would be useful? If not, > then I suggest to go with this solution and we can cross the bridge when > we have an example. > > We don't have to solve everything at once and at least with the approach > I proposed we can start to use STATIC_IF() (or EXTERN_IF) a bit more > often without open-coding it.
Well. IS_ENABLED() is okay in this regard because you can combine multiple of them (with && or ||). The same isn't true here (afaict). After all I could equally well say that as long as we don't have a sufficient number of such examples, but just one, not introducing a special construct is going to be okay for the time being. Jan
