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