On 8/10/26 11:25 PM, Rui Qi wrote:
On 8/10/26 10:53 PM, Waiman Long wrote:
On 8/9/26 4:21 AM, Rui Qi wrote:
The cpuset partition root state selftest checks several CPU hotplug
transitions. If writing to a CPU online file fails, the helper still
runs pause afterwards and returns the status of pause instead of the
failed write.

This hides the real hotplug failure and can make later checks run
against expectations for a transition that never happened. Move the
write before the bookkeeping and return when it fails, so callers can
observe the hotplug error and the test does not record a CPU as offline
unless the offline operation actually succeeded.

Fixes: a8c52eba880a ("kselftest/cgroup: Add cpuset v2 partition root state 
test")
Signed-off-by: Rui Qi <[email protected]>
I am aware that on x86-64, cpu0/online may not exist. Other than that,
under what condition will the write fail as only root is allowed to run
the script?

Cheers,
Longman
Hi Longman,

Right, the failure I wanted to preserve is not the normal permission
case. Running as root only gets us past the sysfs file permission check;
the write can still fail in the CPU hotplug path.

For example, cpu_down() can return -EBUSY when trying to offline the last
online CPU, when CPU hotplug is temporarily disabled, or when offlining
would leave no housekeeping CPU. cpu_up() can also fail if the CPU is not
possible/present, if hotplug is disabled, or if the CPU is not bootable
under the current SMT control state. Arch or registered cpuhp callbacks
may also reject the transition and return an error.

The current helper hides all of those cases because it does the write and
then calls pause(), so the function returns the status of pause rather
than the failed sysfs write. It also updates OFFLINE_CPUS before knowing
whether the offline request succeeded. So this change is mainly to avoid
continuing the test with bookkeeping that says a CPU was offlined when the
kernel actually rejected the hotplug operation.

Thank for the clarification. You are right that cpu_down() can be blocked in the hotplug path. That should rarely happen, but I agree that we catch that special case too.

Cheers,
Longman


Thanks,
Rui

---
   tools/testing/selftests/cgroup/test_cpuset_prs.sh | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh 
b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index 16c38eb11e63..a2f7208633bc 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -412,6 +412,7 @@ write_cpu_online()
        CPU=${1%=*}
        VAL=${1#*=}
        CPUFILE=//sys/devices/system/cpu/cpu${CPU}/online
+       echo $VAL > $CPUFILE || return 1
        if [[ $VAL -eq 0 ]]
        then
                OFFLINE_CPUS="$OFFLINE_CPUS $CPU"
@@ -421,7 +422,6 @@ write_cpu_online()
                                        sort | uniq -u)
                }
        fi
-       echo $VAL > $CPUFILE
        pause 0.05
   }
@@ -493,6 +493,7 @@ set_ctrl_state()
                        eval $COMM $REDIRECT
                        ;;
                    O*) VAL=${CMD#?}
+                       COMM="write_cpu_online $VAL"
                        write_cpu_online $VAL
                        ;;
                    T*) COMM="echo 0 > $TFILE"


Reply via email to