On 7/4/22 04:16, Shin'ichiro Kawasaki wrote:
The helper function _offline_cpu() sets a value to RESTORE_CPUS_ONLINE. However, the commit bd6b882b2650 ("block/008: check CPU offline failure due to many IRQs") put _offline_cpu() call in sub-shell, then the set value to RESTORE_CPUS_ONLINE no longer affects function caller's environment. This resulted in offlined CPUs not restored by _cleanup() when the test case block/008 calls only _offline_cpu() and does not call _online_cpu(). To fix the issue, avoid _offline_cpu() call in sub-shell. Use file redirect to get output of _offline_cpu() instead of sub-shell execution. Fixes: bd6b882b2650 ("block/008: check CPU offline failure due to many IRQs") Reported-by: Yi Zhang <yi.zhang@xxxxxxxxxx> Tested-by: Yi Zhang <yi.zhang@xxxxxxxxxx> Link: https://lore.kernel.org/linux-block/20220703180956.2922025-1-yi.zhang@xxxxxxxxxx/ Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> --- tests/block/008 | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/block/008 b/tests/block/008 index 75aae65..cd09352 100755 --- a/tests/block/008 +++ b/tests/block/008 @@ -34,6 +34,7 @@ test_device() { local offline_cpus=() local offlining=1 local max_offline=${#HOTPLUGGABLE_CPUS[@]} + local o=$TMPDIR/offline_cpu_out if [[ ${#HOTPLUGGABLE_CPUS[@]} -eq ${#ALL_CPUS[@]} ]]; then (( max_offline-- )) fi @@ -60,18 +61,18 @@ test_device() {if (( offlining )); thenidx=$((RANDOM % ${#online_cpus[@]})) - if err=$(_offline_cpu "${online_cpus[$idx]}" 2>&1); then + if _offline_cpu "${online_cpus[$idx]}" > "$o" 2>&1; then offline_cpus+=("${online_cpus[$idx]}") unset "online_cpus[$idx]" online_cpus=("${online_cpus[@]}") - elif [[ $err =~ "No space left on device" ]]; then + elif [[ $(<"$o") =~ "No space left on device" ]]; then # ENOSPC means CPU offline failure due to IRQ # vector shortage. Keep current number of # offline CPUs as maximum CPUs to offline. max_offline=${#offline_cpus[@]} offlining=0 else - echo "Failed to offline CPU: $err" + echo "Failed to offline CPU: $(<"$o")" break fi fi
Has it been considered to move RESTORE_CPUS_ONLINE=1 from _online_cpu() / _offline_cpu() into the callers of these functions instead of making the above change?
Thanks, Bart.
