Re: [PATCH 1/5] rtla: Add dependency on libcpupower

[Daniel Bristot de Oliveira <bristot@kernel.org>]

Hi Thomas

On 6/12/24 16:54, tglozar@redhat.com wrote:
> From: Tomas Glozar <tglozar@redhat.com>

I think we can split this into two patches, this first part on tools/Build:

> Add a test for libcpupower into feature tests and use it to add a
> dependency on libcpupower to rtla.
> 
> Signed-off-by: Tomas Glozar <tglozar@redhat.com>
> ---
>  tools/build/Makefile.feature           | 1 +
>  tools/build/feature/Makefile           | 4 ++++
>  tools/build/feature/test-libcpupower.c | 8 ++++++++
>  tools/tracing/rtla/Makefile            | 2 ++
>  tools/tracing/rtla/Makefile.config     | 9 +++++++++
>  5 files changed, 24 insertions(+)
>  create mode 100644 tools/build/feature/test-libcpupower.c
> 
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 1e2ab148d5db..e4fb0a1fbddf 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -53,6 +53,7 @@ FEATURE_TESTS_BASIC :=                  \
>          libslang-include-subdir         \
>          libtraceevent                   \
>          libtracefs                      \
> +        libcpupower                     \
>          libcrypto                       \
>          libunwind                       \
>          pthread-attr-setaffinity-np     \
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index ed54cef450f5..c93d62afc1e8 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -38,6 +38,7 @@ FILES=                                          \
>           test-libslang.bin                      \
>           test-libslang-include-subdir.bin       \
>           test-libtraceevent.bin                 \
> +         test-libcpupower.bin                   \
>           test-libtracefs.bin                    \
>           test-libcrypto.bin                     \
>           test-libunwind.bin                     \
> @@ -212,6 +213,9 @@ $(OUTPUT)test-libslang-include-subdir.bin:
>  $(OUTPUT)test-libtraceevent.bin:
>  	$(BUILD) -ltraceevent
>  
> +$(OUTPUT)test-libcpupower.bin:
> +	$(BUILD) -lcpupower
> +
>  $(OUTPUT)test-libtracefs.bin:
>  	 $(BUILD) $(shell $(PKG_CONFIG) --cflags libtraceevent 2>/dev/null) -ltracefs
>  
> diff --git a/tools/build/feature/test-libcpupower.c b/tools/build/feature/test-libcpupower.c
> new file mode 100644
> index 000000000000..a346aa332a71
> --- /dev/null
> +++ b/tools/build/feature/test-libcpupower.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <cpuidle.h>
> +
> +int main(void)
> +{
> +	int rv = cpuidle_state_count(0);
> +	return rv;
> +}
> diff --git a/tools/tracing/rtla/Makefile b/tools/tracing/rtla/Makefile
> index b5878be36125..a6a7dee16622 100644


And this part for rtla:

> --- a/tools/tracing/rtla/Makefile
> +++ b/tools/tracing/rtla/Makefile
> @@ -32,8 +32,10 @@ DOCSRC		:= ../../../Documentation/tools/rtla/
>  
>  FEATURE_TESTS	:= libtraceevent
>  FEATURE_TESTS	+= libtracefs
> +FEATURE_TESTS	+= libcpupower
>  FEATURE_DISPLAY	:= libtraceevent
>  FEATURE_DISPLAY	+= libtracefs
> +FEATURE_DISPLAY	+= libcpupower
>  
>  ifeq ($(V),1)
>    Q		=
> diff --git a/tools/tracing/rtla/Makefile.config b/tools/tracing/rtla/Makefile.config
> index 0b7ecfb30d19..8b6bc91e5dff 100644
> --- a/tools/tracing/rtla/Makefile.config
> +++ b/tools/tracing/rtla/Makefile.config
> @@ -42,6 +42,15 @@ else
>    $(info libtracefs is missing. Please install libtracefs-dev/libtracefs-devel)
>  endif
>  
> +$(call feature_check,libcpupower)
> +ifeq ($(feature-libcpupower), 1)
> +  $(call detected,CONFIG_LIBCPUPOWER)
> +  $(call lib_setup,cpupower)
> +else

Also, it is better to make it optional: if the system has this library,
set a CONFIG_HAS_LIBCPUPOWER, otherwise no..

> +  STOP_ERROR := 1
> +  $(info libcpupower is missing. Please install libcpupower-dev/kernel-tools-libs-devel)
> +endif

Then, place your code inside this option. If the option is called
but the tool was not compiled with CONFIG_HAS_LIBCPUPOWER, just
complain saying that the option is not supported because it was
compiled without it. Then put this info there, to compile
with these options enabled.

>  ifeq ($(STOP_ERROR),1)
>    $(error Please, check the errors above.)
>  endif