Re: [PATCH v2] selftests: cpufreq: stop CPU hotplug loop after cpufreq update test

[Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>]

Hi,

On 10/28/2026 6:02 PM, xiong yining wrote:
> Original test uses a fixed 5000 iteration loop to trigger CPU hotplug
> for verifying cpufreq synchronization between CPU hotplug and sysfs
> writes.
> 
> Two issues exist:
> 1. Background hotplug task keeps running after test completes, causing
> redundant CPU on/offline operations.
> 2. When cpufreq frequency setup is slow, the hotplug loop may exit
> before frequency updates finish, which breaks test purpose.
> 
> Introduce a file-based stop flag to control the infinite hotplug loop.
> After all cpufreq updates complete, create the stop flag file and
> wait for the hotplug task to exit.

The changelog still describes the v1 stop-file approach; please
update it.

> 
> This makes hotplug run only during cpufreq sysfs write test,
> align with test objective.
> 
> Signed-off-by: xiong yining <xiongyining1480@phytium.com.cn>
> ---
> Changes in v2:
> - Stop the hotplug child process by signal notification instead of
> a stop file, avoiding the predictable temporary path.
> - Add an exit trap so the hotplug child is also cleaned up when the
> main shell terminates abnormally.
> - Link to v1:
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2662696.html
> 
> This addresses the security and robustness concerns raised by Sashiko.
> ---
>   .../selftests/cpufreq/special-tests.sh        | 30 +++++++++++++++----
>   1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/cpufreq/special-tests.sh b/tools/testing/selftests/cpufreq/special-tests.sh
> index e87ed7c8e5e5..b3ebead1c52a 100755
> --- a/tools/testing/selftests/cpufreq/special-tests.sh
> +++ b/tools/testing/selftests/cpufreq/special-tests.sh
> @@ -16,6 +16,8 @@ source cpu.sh
>   source cpufreq.sh
>   source governor.sh
>   
> +HOTPLUG_PID=""
> +
>   # Test 1
>   # $1: policy
>   __simple_lockdep()
> @@ -92,20 +94,34 @@ governor_race()
>   	restore_all_governors
>   }
>   
> +hotplug_cleanup()
> +{
> +	if [[ -n "$HOTPLUG_PID" ]]; then
> +		kill -s SIGTERM "$HOTPLUG_PID"
> +		wait "$HOTPLUG_PID"
> +
> +		HOTPLUG_PID=""
> +	fi
> +}
> +
>   # Test 4
>   # $1: cpu
>   hotplug_with_updates_cpu()
>   {
>   	local filepath="$CPUROOT/$1/cpufreq"
>   
> +	(
> +		local quit=0
> +		trap 'quit=1' SIGTERM
> +		while [ "$quit" -eq 0 ]; do
> +			reboot_cpu "$1"
> +		done
> +	) &
> +	HOTPLUG_PID=$!
> +
>   	# switch to ondemand
>   	__switch_governor_for_cpu $1 "ondemand"
>   
> -	for i in `seq 1 5000`
> -	do
> -		reboot_cpu $1
> -	done &
> -
>   	local freqs=$(cat $filepath/scaling_available_frequencies)
>   	local oldfreq=$(cat $filepath/scaling_min_freq)


The background hotplug loop is now started before switching to ondemand
and before reading scaling_available_frequencies / scaling_min_freq. If
the CPU happens to be offline at that point (such as single-CPU policy),
those accesses return -EBUSY: the governor switch is lost, freqs is
empty so the update loop does nothing while the test still passes, and
oldfreq is empty so scaling_min_freq is not restored. Please start the
hotplug subshell only after that setup is done.

>   
> @@ -117,14 +133,16 @@ hotplug_with_updates_cpu()
>   		done
>   	done
>   
> +	hotplug_cleanup
> +
>   	# restore old freq
>   	echo $oldfreq > $filepath/scaling_min_freq
>   }
>   
>   hotplug_with_updates()
>   {
> +	trap 'hotplug_cleanup' EXIT


AFAIK, regarding the EXIT-trap-on-signal remark from Sashiko:

https://sashiko.dev/#/patchset/20261028100241.189085-1-xiongyining1480%40phytium.com.cn

Bash runs the EXIT trap on SIGINT, SIGTERM, and SIGHUP, so I don't think
additional traps are necessary. This issue occurs with Dash, but not
with Bash.

One sample simulation script：


#!/bin/bash

for sh in bash dash; do
	command -v $sh >/dev/null || continue
	for s in INT TERM HUP; do
		out=$(env --default-signal=INT $sh -c \
			'echo "$(readlink /proc/$$/exe)"
			 trap "echo EXIT-trap-ran" EXIT
			 kill -'$s' $$
			 sleep 1
			 echo not-killed' 2>/dev/null)
		rc=$?
		self=$(echo "$out" | sed -n 1p)
		res=$(echo "$out" | sed -n 2p)
		printf "%-14s %-4s rc=%-3s %s\n" "$self" $s $rc "${res:-<EXIT trap NOT 
run>}"
	done
done


output:
/usr/bin/bash  INT  rc=130 EXIT-trap-ran
/usr/bin/bash  TERM rc=143 EXIT-trap-ran
/usr/bin/bash  HUP  rc=129 EXIT-trap-ran
/usr/bin/dash  INT  rc=130 <EXIT trap NOT run>
/usr/bin/dash  TERM rc=143 <EXIT trap NOT run>
/usr/bin/dash  HUP  rc=129 <EXIT trap NOT run>


>   	save_all_governors
>   	for_each_non_boot_cpu hotplug_with_updates_cpu
> -	wait
>   	restore_all_governors
>   }


-- 
Thx and BRs,
Zhongqiu Han