> > Could probably do: > > #ifdef HAVE_LIBCPUPOWER_SUPPORT > > + " --deepest-idle-state n: only go down to idle > > state n on cpus used by timerlat to reduce exit from idle latency", > #else > + " --deepest-idle-state n: [rtla built without > libcpupower, --deepest-idle-state is not supported]", > #endif > > > NULL, > > }; > >
I would still include what the option does, even if not building with libcpupower. I'm not too sure if the help is the place to say the option is unsupported either. I see two perspectives on this matter: one is that the binary does not support libcpupower and should state it in the help, the other is that the version of rtla as a whole does support it, you are just using a build that does not, and as such it should be in the help (you will know it is unsupported when trying to use the option). I suppose we can add a note like this, keeping the help message to inform the user what the option does so that they will rebuild if that want to use it: ``` #ifdef HAVE_LIBCPUPOWER_SUPPORT " --deepest-idle-state n: only go down to idle state n on cpus used by timerlat to reduce exit from idle latency", #else " --deepest-idle-state n: only go down to idle state n on cpus used by timerlat to reduce exit from idle latency (not supported due to rtla built without libcpupower)", #endif ``` What do you think about that? > > We could get rid of most of the ifdefs if you changed the header file to be: > That's a neat idea, thank you! I know this approach (with defining functions that do nothing when some feature is unavailable) is very commonly used in the kernel to keep the API consistent across different configs, but I didn't think about using it like this here in rtla. > >You would think gcc may optimize it, but I don't have that much confidence >it can or would. You may want to change that to: > > int nr_cpus = sysconf(_SC_NPROCESSORS_CONF); > > for (i = 0; i < nr_cpus; i++) { > >Otherwise you may be calling that sysconf() for each iteration of the loop. > Nah, that is simply an oversight. If GCC optimized that, it would be in fact a GCC bug, since the value of sysconf(_SC_NPROCESSORS_CONF) is external environment and can technically change during the runtime of a program (think of CRIU live migration of the process from one machine to another with a different number of CPUs). Tomas