From: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
To: xiong yining <xiongyining1480@phytium.com.cn>,
"Rafael J . Wysocki" <rafael@kernel.org>,
Viresh Kumar <viresh.kumar@linaro.org>,
Shuah Khan <shuah@kernel.org>
Cc: linux-pm@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-kernel@vger.kernel.org, chenbaozi@phytium.com.cn,
zhongqiu.han@oss.qualcomm.com
Subject: Re: [PATCH v3] selftests: cpufreq: stop CPU hotplug loop after cpufreq update test
Date: Fri, 30 Oct 2026 23:11:51 +0800 [thread overview]
Message-ID: <a6840be7-bd50-4e64-af1b-0ab9334d8ec7@oss.qualcomm.com> (raw)
In-Reply-To: <20261030104213.2030283-1-xiongyining1480@phytium.com.cn>
On 10/30/2026 6:42 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.
>
> Use signal notification to terminate the infinite hotplug loop, and
> add an EXIT trap to clean up the hotplug child when this test function
> terminates abnormally.
>
> This makes hotplug run only during cpufreq sysfs write test,
> align with test objective.
>
> Signed-off-by: xiong yining <xiongyining1480@phytium.com.cn>
Reviewed-by: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
> ---
> Changes in v3:
> - Start the hotplug child after governor switch and sysfs frequency reads,
> to avoid -EBUSY errors when the target CPU is offline.
>
> 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..0665da8f2444 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,6 +94,16 @@ 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()
> @@ -101,14 +113,18 @@ hotplug_with_updates_cpu()
> # 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)
>
> + (
> + local quit=0
> + trap 'quit=1' SIGTERM
> + while [ "$quit" -eq 0 ]; do
> + reboot_cpu "$1"
> + done
> + ) &
> + HOTPLUG_PID=$!
> +
> for j in `seq 1 5000`
> do
> # Set all frequencies one-by-one
> @@ -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
> }
--
Thx and BRs,
Zhongqiu Han
prev parent reply other threads:[~2026-10-30 15:12 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-30 10:42 xiong yining
2026-10-30 15:11 ` Zhongqiu Han [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a6840be7-bd50-4e64-af1b-0ab9334d8ec7@oss.qualcomm.com \
--to=zhongqiu.han@oss.qualcomm.com \
--cc=chenbaozi@phytium.com.cn \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=shuah@kernel.org \
--cc=viresh.kumar@linaro.org \
--cc=xiongyining1480@phytium.com.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®