Linux Power Management development
 help / color / mirror / Atom feed
* [PATCH v2] selftests: cpufreq: stop CPU hotplug loop after cpufreq update test
@ 2026-10-28 10:02 xiong yining
  2026-10-28 14:41 ` Zhongqiu Han
  0 siblings, 1 reply; 3+ messages in thread
From: xiong yining @ 2026-10-28 10:02 UTC (permalink / raw)
  To: Rafael J . Wysocki, Viresh Kumar, Shuah Khan
  Cc: linux-pm, linux-kselftest, linux-kernel, chenbaozi, xiong yining

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.

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)
 
@@ -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
 	save_all_governors
 	for_each_non_boot_cpu hotplug_with_updates_cpu
-	wait
 	restore_all_governors
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] selftests: cpufreq: stop CPU hotplug loop after cpufreq update test
  2026-10-28 10:02 [PATCH v2] selftests: cpufreq: stop CPU hotplug loop after cpufreq update test xiong yining
@ 2026-10-28 14:41 ` Zhongqiu Han
  2026-10-30 10:39   ` 回复: " xiongyining1480
  0 siblings, 1 reply; 3+ messages in thread
From: Zhongqiu Han @ 2026-10-28 14:41 UTC (permalink / raw)
  To: xiong yining, Rafael J . Wysocki, Viresh Kumar, Shuah Khan
  Cc: linux-pm, linux-kselftest, linux-kernel, chenbaozi, zhongqiu.han

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* 回复: Re: [PATCH v2] selftests: cpufreq: stop CPU hotplug loop after cpufreq update test
  2026-10-28 14:41 ` Zhongqiu Han
@ 2026-10-30 10:39   ` xiongyining1480
  0 siblings, 0 replies; 3+ messages in thread
From: xiongyining1480 @ 2026-10-30 10:39 UTC (permalink / raw)
  Cc: linux-pm, linux-kselftest, linux-kernel

Hi, 

Thank you for the review and the test script.

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

Right, thanks for catching that. I’ll update the changelog to describe the 
current approach.

> 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.

Agreed. Starting it before governor switching and sysfs frequency reads can 
hit -EBUSY if the target CPU is  offline. I will launch the hotplug child only 
after those setup steps finish.

> 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.

Thanks for the comparison script and the test results — that's very helpful.
I agree that Bash runs the EXIT trap on SIGINT, SIGTERM, and SIGHUP.  I tested 
both the subshell case and the main-shell case:

When the signal is sent to the do_test subshell, the EXIT trap runs and cleanup 
completes.

When this signal is sent to the main shell, the subshell does not receive it, keeps 
running,  and performs cleanup when it exits naturally.

Cleanup works correctly in both cases.

Sample simulation script:

#!/bin/bash


do_test() {
    trap 'echo "EXIT trap ran"' EXIT
    local sig="$1"
    local target="$2"


    case "$target" in
        1)
            echo "---- send $sig to do_test pid ----"
            kill -"$sig" "$BASHPID"
            ;;
        2)
            echo "---- send $sig to main pid ----"
            kill -"$sig" $$
            ;;
    esac

    echo "do_test finished"

}

 
export -f do_test

echo "---------sub shell-----------"
for sig in INT TERM HUP; do
        bash -c "do_test '$sig' 1 | tee test.log"
done
 
echo "---------main shell-----------"
for sig in INT TERM HUP; do
        bash -c "do_test '$sig' 2 | tee test.log" 2>/dev/null
done
 
output:
---------sub shell-----------
---- send INT to do_test pid ----
EXIT trap ran
---- send TERM to do_test pid ----
EXIT trap ran
---- send HUP to do_test pid ----
EXIT trap ran
---------main shell-----------
---- send INT to main pid ----
do_test finished
EXIT trap ran
Terminated
---- send TERM to main pid ----
do_test finished
EXIT trap ran
---- send HUP to main pid ----
do_test finished
EXIT trap ran
 

Thanks again for your useful test.
Best regards,
Yining Xiong

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-30 10:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-28 10:02 [PATCH v2] selftests: cpufreq: stop CPU hotplug loop after cpufreq update test xiong yining
2026-10-28 14:41 ` Zhongqiu Han
2026-10-30 10:39   ` 回复: " xiongyining1480

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox