[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 02 Feb 2022 18:24:09 +0000
From: Valentin Schneider <valentin.schneider@....com>
To: Frederic Weisbecker <frederic@...nel.org>,
Peter Zijlstra <peterz@...radead.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Frederic Weisbecker <frederic@...nel.org>
Subject: Re: [PATCH] sched/preempt: Tell about PREEMPT_DYNAMIC on kernel headers
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@....com>
> Signed-off-by: Frederic Weisbecker <frederic@...nel.org>
> Cc: Valentin Schneider <valentin.schneider@....com>
> ---
> 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
Powered by blists - more mailing lists