Re: [PATCH] sched/preempt: Tell about PREEMPT_DYNAMIC on kernel headers

[Valentin Schneider]

On 02/02/22 15:59, Frederic Weisbecker wrote:
> Displaying "PREEMPT" on kernel headers when CONFIG_PREEMPT_DYNAMIC=y
> can be misleading for anybody involved in remote debugging because it
> is then not guaranteed that there is an actual preemption behaviour. It
> depends on default Kconfig or boot defined choices.
>
> Therefore, tell about PREEMPT_DYNAMIC on static kernel headers and leave
> the search for the actual preemption behaviour to browsing dmesg.
>

Looks sensible. One small further cleanup nit below, otherwise:

Reviewed-by: Valentin Schneider <valentin.schneider@xxxxxxx>

> Signed-off-by: Frederic Weisbecker <frederic@xxxxxxxxxx>
> Cc: Valentin Schneider <valentin.schneider@xxxxxxx>
> ---
>  init/Makefile       |  3 ++-
>  scripts/mkcompile_h | 15 +++++++++++----
>  2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/init/Makefile b/init/Makefile
> index 06326e304384..d82623d7fc8e 100644
> --- a/init/Makefile
> +++ b/init/Makefile
> @@ -31,7 +31,8 @@ quiet_cmd_compile.h = CHK     $@
>        cmd_compile.h = \
>       $(CONFIG_SHELL) $(srctree)/scripts/mkcompile_h $@	\
>       "$(UTS_MACHINE)" "$(CONFIG_SMP)" "$(CONFIG_PREEMPT_BUILD)"	\
> -	"$(CONFIG_PREEMPT_RT)" "$(CONFIG_CC_VERSION_TEXT)" "$(LD)"
> +	"$(CONFIG_PREEMPT_DYNAMIC)" "$(CONFIG_PREEMPT_RT)" \
> +	"$(CONFIG_CC_VERSION_TEXT)" "$(LD)"
>
>  include/generated/compile.h: FORCE
>       $(call cmd,compile.h)
> diff --git a/scripts/mkcompile_h b/scripts/mkcompile_h
> index 6a2a04d92f42..f4d6ca3c6fd7 100755
> --- a/scripts/mkcompile_h
> +++ b/scripts/mkcompile_h
> @@ -5,9 +5,10 @@ TARGET=$1
>  ARCH=$2
>  SMP=$3
>  PREEMPT=$4
> -PREEMPT_RT=$5
> -CC_VERSION="$6"
> -LD=$7
> +PREEMPT_DYNAMIC=$5
> +PREEMPT_RT=$6
> +CC_VERSION="$7"
> +LD=$8
>
>  # Do not expand names
>  set -f
> @@ -41,7 +42,13 @@ fi
>  UTS_VERSION="#$VERSION"
>  CONFIG_FLAGS=""
>  if [ -n "$SMP" ] ; then CONFIG_FLAGS="SMP"; fi
> -if [ -n "$PREEMPT" ] ; then CONFIG_FLAGS="$CONFIG_FLAGS PREEMPT"; fi
> +if [ -n "$PREEMPT" ] ; then
> +	if [ -n "$PREEMPT_DYNAMIC" ] ; then
> +		CONFIG_FLAGS="$CONFIG_FLAGS PREEMPT_DYNAMIC";
> +	else
> +		CONFIG_FLAGS="$CONFIG_FLAGS PREEMPT";
> +	fi
> +fi
>  if [ -n "$PREEMPT_RT" ] ; then CONFIG_FLAGS="$CONFIG_FLAGS PREEMPT_RT"; fi
>

I got suspicious of that PREEMPT_RT line, but it works because
PREEMPT_BUILD and PREEMPT_RT are mutually exclusive. Nevertheless, could we
clear out the ambiguity and make that into:

if   [ -n "$PREEMPT_RT" ] ;      then CONFIG_FLAGS="$CONFIG_FLAGS PREEMPT_RT";
elif [ -n "$PREEMPT_DYNAMIC" ] ; then CONFIG_FLAGS="$CONFIG_FLAGS PREEMPT_DYNAMIC";
elif [ -n "$PREEMPT" ] ;         then CONFIG_FLAGS="$CONFIG_FLAGS PREEMPT";
fi


>  # Truncate to maximum length
> --
> 2.25.1