From 73c62fc2bbb5e6957c446b424e4e56e25775bf9d Mon Sep 17 00:00:00 2001 From: rammsguns <6564405+rammsguns@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:57:47 -0600 Subject: [PATCH 1/2] Capture the prior state before tuning, and restore it on revert Closes #23. 10-os-tune.sh changed privileged machine state and wrote three files under /etc. 19-os-revert.sh "undid" that by writing fixed defaults -- THP=madvise, governor=schedutil, power at maximum, persistence off -- and by `rm -f`-ing three paths it had never proven it owned. On a machine that already had a governor policy, a THP setting, or its own 99-llm-inference.conf, that was not a rollback. It was a second round of configuration wearing a rollback's clothes, and in the file case a deletion of somebody else's work. The README promised the tuning was reversible; it was re-settable, which is a different claim. THE MODEL, in lib/ostune.sh Before the first mutation of a setting, its effective prior value is captured to /var/lib/llm-rig/os-tune.state -- root-owned, 0600, in a 0700 directory. 19-os-revert.sh restores from that file and from nothing else. Capture is append-once. A second tune must not re-record the values llm-rig itself set as the ones to restore, which is precisely how a rollback becomes a silent no-op. Capture is per setting and immediately precedes the change, so a crash halfway through leaves an exact record of what has been changed so far. A partial run is fully revertible, and there is a test that proves it by making step 5 refuse and then reverting steps 1-4. The power-limit case is worth naming. TUNING.md concludes 100% is right for this chassis. Someone who read that, disagreed, and capped their card at 120W had the decision quietly undone by a revert that "restored" the maximum -- it raised a limit that had been deliberately lowered. Per-CPU governors are captured and restored individually. A machine running different governors on different cores was being flattened to one value. An unreadable prior value is recorded as `unknown` and never "restored". Writing madvise because THP could not be read is a new decision, not a rollback. FILES A file that existed before llm-rig is backed up byte for byte, restored byte for byte, and never destroyed. If it cannot be backed up, it is not overwritten -- the tune refuses and says why. Existence is checked separately from hashing, because a path that exists and cannot be read is the moment we know least about it. An unreadable file, a directory, or a dangling symlink now refuses; previously the failed hash read as "absent" and the path was overwritten. -L as well as -e: -e follows symlinks and is false for a dangling one, which is exactly the case where a write replaces somebody's link with a regular file. A file we wrote is deleted only if it still holds what we wrote. An edit is a claim of ownership. Ownership recorded but never proven -- the hash is `unknown` because the run died between recording and hashing -- also blocks the delete. Unproven is treated as not ours. PLAN MODE Both scripts take --dry-run. It prints every intended mutation with its current value, marks the ones already correct, warns when a file of yours would be adopted, and uses no sudo at all. TESTS 35 new, in tests/cases/ostune_test.sh. OSTUNE_ROOT points at a sandbox and OSTUNE_SUDO is empty, so the scripts run their real code paths -- writing, hashing, backing up, restoring, refusing -- against a fake root. That is deliberately not mocked: a mock that always succeeds cannot show that a file came back byte for byte. Two mocks earn their keep here. cpupower actually writes the sandbox governor files, because a stub that only records the call cannot demonstrate that a revert put the previous governor back. And the dangling-symlink test uses the real ln rather than the mock on PATH, which would have created nothing and left the assertion measuring the stub. The backup-failure case is a dangling symlink rather than a chmod 000 file, because tests/isolated.sh runs inside unshare --map-root-user, where permission bits do not apply and an unreadable file is readable. 412 tests across 13 suites, ordinary and network-isolated both green. README and TUNING.md now state what the guarantee actually is, including a table of what the old revert wrote versus what it should have. Co-Authored-By: Claude Opus 5 --- 10-os-tune.sh | 271 ++++++++++++++++----- 19-os-revert.sh | 167 +++++++++++-- README.md | 53 +++- TUNING.md | 30 +++ lib/ostune.sh | 426 +++++++++++++++++++++++++++++++++ tests/cases/ostune_test.sh | 418 ++++++++++++++++++++++++++++++++ tests/mocks/bin/cpupower | 24 ++ tests/mocks/bin/sysctl | 5 + tests/mocks/bin/system76-power | 9 + 9 files changed, 1326 insertions(+), 77 deletions(-) create mode 100644 lib/ostune.sh create mode 100644 tests/cases/ostune_test.sh create mode 100755 tests/mocks/bin/cpupower create mode 100755 tests/mocks/bin/sysctl create mode 100755 tests/mocks/bin/system76-power diff --git a/10-os-tune.sh b/10-os-tune.sh index 43e779c..d3f976c 100755 --- a/10-os-tune.sh +++ b/10-os-tune.sh @@ -1,19 +1,41 @@ #!/usr/bin/env bash -# OS + GPU level tuning for LLM inference. Idempotent. Needs sudo. -# Everything here is reversible; see 19-os-revert.sh +# OS + GPU level tuning for LLM inference. Idempotent, transactional, and +# reversible against the state this machine was ACTUALLY in -- not against a +# set of defaults someone assumed. +# +# Every setting's prior value is captured to a root-owned state file before it +# is changed, one setting at a time, so a crash halfway through still leaves +# 19-os-revert.sh enough to undo exactly what happened. Files under /etc that +# existed before llm-rig are backed up byte for byte and never destroyed. See +# lib/ostune.sh for the ownership model. +# +# Usage: +# ./10-os-tune.sh # capture, then tune. Needs sudo. +# ./10-os-tune.sh --dry-run # print every intended mutation. No sudo. +# +# Undo with ./19-os-revert.sh set -uo pipefail -source "$(dirname "$0")/lib/detect.sh" -detect_hw +RIG_SRC_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "$RIG_SRC_DIR/lib/detect.sh" +source "$RIG_SRC_DIR/lib/ostune.sh" + +DRY=0 +while (( $# )); do + case "$1" in + --dry-run|--plan) DRY=1 ;; + -h|--help) sed -n '2,18p' "${BASH_SOURCE[0]}"; exit 0 ;; + *) die "unknown argument: $1" ;; + esac + shift +done +detect_hw c_info "Tuning for $GPU_NAME / ${RAM_GB}GB RAM / $PHYS_CORES cores" -# --- 1. GPU persistence mode ------------------------------------------------ -# Without this the driver tears down GPU state between processes, adding -# seconds of latency to the first request after an idle period. -c_info "GPU persistence mode" -sudo nvidia-smi -pm 1 >/dev/null && c_ok "persistence mode on" +# --- what we intend to set -------------------------------------------------- +# Computed before anything is touched, so --dry-run and the real run agree by +# construction rather than by two lists being kept in step by hand. -# --- 2. Power + clocks ------------------------------------------------------ # MEASURED on 2x RTX A4000 (see 70-thermal-sweep.sh), heat-soaked: # # W pp16384 t/s/W tg128 peakT sustClk @@ -34,38 +56,20 @@ sudo nvidia-smi -pm 1 >/dev/null && c_ok "persistence mode on" # # Override with POWER_PCT=85 if your chassis actually has thermal headroom. POWER_PCT="${POWER_PCT:-100}" -MAXW=$(nvidia-smi --query-gpu=power.max_limit --format=csv,noheader,nounits | head -1 | cut -d. -f1) -MINW=$(nvidia-smi --query-gpu=power.min_limit --format=csv,noheader,nounits | head -1 | cut -d. -f1) -TGT=$(( MAXW * POWER_PCT / 100 )); (( TGT < MINW )) && TGT=$MINW -c_info "Power limit: max ${MAXW}W -> setting ${TGT}W (${POWER_PCT}%)" -sudo nvidia-smi -pl "$TGT" >/dev/null 2>&1 && c_ok "power limit ${TGT}W" \ - || c_warn "could not set power limit (locked VBIOS?) -- harmless, skipping" - -# --- 3. CPU governor -------------------------------------------------------- -c_info "CPU governor -> performance" -if need system76-power; then - sudo system76-power profile performance >/dev/null 2>&1 && c_ok "system76 performance profile" -fi -if need cpupower; then - sudo cpupower frequency-set -g performance >/dev/null 2>&1 && c_ok "governor=performance" -else - for g in /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor; do - echo performance | sudo tee "$g" >/dev/null 2>&1 || true - done - c_ok "governor written directly" +MAXW=$(nvidia-smi --query-gpu=power.max_limit --format=csv,noheader,nounits 2>/dev/null | head -1 | cut -d. -f1) +MINW=$(nvidia-smi --query-gpu=power.min_limit --format=csv,noheader,nounits 2>/dev/null | head -1 | cut -d. -f1) +TGT="" +if [[ -n "${MAXW:-}" && -n "${MINW:-}" ]]; then + TGT=$(( MAXW * POWER_PCT / 100 )); (( TGT < MINW )) && TGT=$MINW fi -# --- 4. Transparent huge pages --------------------------------------------- -# Model weights are mmap'd in multi-GB contiguous ranges. THP=always measurably -# reduces TLB misses during prompt processing when layers live in system RAM. -c_info "Transparent huge pages -> always" -echo always | sudo tee /sys/kernel/mm/transparent_hugepage/enabled >/dev/null -echo defer+madvise | sudo tee /sys/kernel/mm/transparent_hugepage/defrag >/dev/null 2>&1 || true -c_ok "THP=always" +WANT_GOVERNOR=performance +WANT_S76=performance +WANT_THP=always +WANT_THP_DEFRAG='defer+madvise' -# --- 5. VM / memory sysctls ------------------------------------------------- -c_info "Kernel sysctls" -sudo tee /etc/sysctl.d/99-llm-inference.conf >/dev/null </dev/null 2>&1 -c_ok "/etc/sysctl.d/99-llm-inference.conf applied" +} -# --- 6. mlock limits -------------------------------------------------------- -# --mlock pins weights in RAM. Needs an unlimited memlock rlimit. -c_info "memlock rlimit -> unlimited" -sudo tee /etc/security/limits.d/99-llm-memlock.conf >/dev/null </dev/null </dev/null 2>&1 -c_ok "llm-gpu-tune.service enabled" +} + +# --- the plan --------------------------------------------------------------- +# Reading current state needs no privilege, which is what makes --dry-run +# honest: it reports the same values the real run will capture. + +ostune_build_plan() { + local idx pm pl f gov + + while IFS=$'\t' read -r idx pm pl; do + [[ -n "$idx" ]] || continue + ostune_plan_line "GPU $idx persistence" "$pm" "Enabled" + [[ -n "$TGT" ]] && ostune_plan_line "GPU $idx power limit (W)" "$pl" "$TGT" + done < <(ostune_gpu_state) + + while IFS=$'\t' read -r f gov; do + [[ -n "$f" ]] || continue + ostune_plan_line "governor $(basename "$(dirname "$(dirname "$f")")")" "$gov" "$WANT_GOVERNOR" + done < <(ostune_governors) + + need system76-power && ostune_plan_line "system76-power profile" "$(ostune_s76_profile)" "$WANT_S76" + + ostune_plan_line "THP enabled" "$(ostune_sysfs_choice "$OSTUNE_THP_ENABLED")" "$WANT_THP" + ostune_plan_line "THP defrag" "$(ostune_sysfs_choice "$OSTUNE_THP_DEFRAG")" "$WANT_THP_DEFRAG" + + local p + for p in "$OSTUNE_SYSCTL_FILE" "$OSTUNE_LIMITS_FILE" "$OSTUNE_UNIT_FILE"; do + case "$(ostune_file_status "$p")" in + absent) ostune_plan_line "$p" "absent" "created by llm-rig" ;; + created) ostune_plan_line "$p" "llm-rig's" "rewritten (unchanged)" ;; + adopted) ostune_plan_line "$p" "yours, backed up" "rewritten" ;; + foreign) ostune_plan_line "$p" "YOURS" "backed up, then overwritten" ;; + *-dirty) ostune_plan_line "$p" "EDITED SINCE" "REFUSED -- tune will stop here" ;; + esac + done +} + +if (( DRY )); then + echo + c_info "Planned mutations (nothing has been changed):" + ostune_build_plan | ostune_plan_render + echo + c_info "State file that would be written: $OSTUNE_STATE" + if ostune_state_exists; then + c_warn "A state file already exists -- prior values captured earlier are kept, + so a re-run cannot overwrite them with values llm-rig itself set." + fi + exit 0 +fi + +# --- capture, then mutate --------------------------------------------------- +# Order matters throughout: capture is always the statement BEFORE the change, +# never after and never in a batch at the end. + +ostune_state_init || die "cannot create the state directory $OSTUNE_STATE_DIR" +ostune_state_put state tuned_at "$(date -u +%Y-%m-%dT%H:%M:%SZ)" >/dev/null + +fail=0 + +# --- 1. GPU persistence mode + power ---------------------------------------- +# Without persistence the driver tears down GPU state between processes, adding +# seconds of latency to the first request after an idle period. +c_info "GPU persistence mode and power limit" +while IFS=$'\t' read -r idx pm pl; do + [[ -n "$idx" ]] || continue + ostune_state_put gpu "$idx.persistence" "$pm" + ostune_state_put gpu "$idx.power_limit" "$pl" +done < <(ostune_gpu_state) -# --- 8. Report -------------------------------------------------------------- +if ostune_priv nvidia-smi -pm 1 >/dev/null 2>&1; then + c_ok "persistence mode on" +else + c_warn "could not set persistence mode" +fi + +if [[ -n "$TGT" ]]; then + c_info "Power limit: max ${MAXW}W -> setting ${TGT}W (${POWER_PCT}%)" + ostune_priv nvidia-smi -pl "$TGT" >/dev/null 2>&1 && c_ok "power limit ${TGT}W" \ + || c_warn "could not set power limit (locked VBIOS?) -- harmless, skipping" +else + c_warn "no GPU power limits reported -- skipping power tuning" +fi + +# --- 2. CPU governor -------------------------------------------------------- +c_info "CPU governor -> $WANT_GOVERNOR" +if need system76-power; then + ostune_state_put cpu s76_profile "$(ostune_s76_profile)" + ostune_priv system76-power profile "$WANT_S76" >/dev/null 2>&1 \ + && c_ok "system76 $WANT_S76 profile" +fi + +while IFS=$'\t' read -r f gov; do + [[ -n "$f" ]] || continue + ostune_state_put cpu "$f" "$gov" +done < <(ostune_governors) + +if need cpupower; then + ostune_priv cpupower frequency-set -g "$WANT_GOVERNOR" >/dev/null 2>&1 \ + && c_ok "governor=$WANT_GOVERNOR" +else + for f in $OSTUNE_CPU_GLOB; do + [[ -f "$f" ]] || continue + printf '%s\n' "$WANT_GOVERNOR" | ostune_priv tee "$f" >/dev/null 2>&1 || true + done + c_ok "governor written directly" +fi + +# --- 3. Transparent huge pages --------------------------------------------- +# Model weights are mmap'd in multi-GB contiguous ranges. THP=always measurably +# reduces TLB misses during prompt processing when layers live in system RAM. +c_info "Transparent huge pages -> $WANT_THP" +ostune_state_put thp enabled "$(ostune_sysfs_choice "$OSTUNE_THP_ENABLED")" +ostune_state_put thp defrag "$(ostune_sysfs_choice "$OSTUNE_THP_DEFRAG")" +printf '%s\n' "$WANT_THP" | ostune_priv tee "$OSTUNE_THP_ENABLED" >/dev/null 2>&1 \ + && c_ok "THP=$WANT_THP" || c_warn "could not set THP" +printf '%s\n' "$WANT_THP_DEFRAG" | ostune_priv tee "$OSTUNE_THP_DEFRAG" >/dev/null 2>&1 || true + +# --- 4. VM / memory sysctls ------------------------------------------------- +c_info "Kernel sysctls" +if sysctl_content | ostune_install_file "$OSTUNE_SYSCTL_FILE" 644; then + ostune_priv sysctl --system >/dev/null 2>&1 + c_ok "$OSTUNE_SYSCTL_FILE applied" +else + c_err "${OSTUNE_LAST_ERROR:-could not write $OSTUNE_SYSCTL_FILE}" + fail=1 +fi + +# --- 5. mlock limits -------------------------------------------------------- +# --mlock pins weights in RAM. Needs an unlimited memlock rlimit. +c_info "memlock rlimit -> unlimited" +if limits_content | ostune_install_file "$OSTUNE_LIMITS_FILE" 644; then + c_ok "limits.d written (takes effect on next login)" +else + c_err "${OSTUNE_LAST_ERROR:-could not write $OSTUNE_LIMITS_FILE}" + fail=1 +fi + +# --- 6. Make the GPU tuning survive reboot ---------------------------------- +c_info "Persisting GPU settings across reboot" +if unit_content | ostune_install_file "$OSTUNE_UNIT_FILE" 644; then + ostune_state_put unit llm-gpu-tune.service enabled + ostune_priv systemctl daemon-reload + ostune_priv systemctl enable --now llm-gpu-tune.service >/dev/null 2>&1 + c_ok "llm-gpu-tune.service enabled" +else + c_err "${OSTUNE_LAST_ERROR:-could not write $OSTUNE_UNIT_FILE}" + fail=1 +fi + +# --- 7. Report -------------------------------------------------------------- echo +c_info "Prior state captured in $OSTUNE_STATE -- 19-os-revert.sh restores exactly that" +if (( fail )); then + c_warn "Some steps were refused. Everything captured above is still revertible; + nothing that was refused was changed." +fi + c_info "Post-tune state" nvidia-smi --query-gpu=name,persistence_mode,power.limit,temperature.gpu,clocks.max.sm \ - --format=csv -echo "Governor: $(cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor)" -echo "THP: $(cat /sys/kernel/mm/transparent_hugepage/enabled)" -echo "swappiness: $(cat /proc/sys/vm/swappiness)" + --format=csv 2>/dev/null +echo "Governor: $(cat "$OSTUNE_ROOT/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor" 2>/dev/null || echo "$OSTUNE_UNKNOWN")" +echo "THP: $(cat "$OSTUNE_THP_ENABLED" 2>/dev/null || echo "$OSTUNE_UNKNOWN")" +echo "swappiness: $(cat "$OSTUNE_ROOT/proc/sys/vm/swappiness" 2>/dev/null || echo "$OSTUNE_UNKNOWN")" echo c_warn "Log out and back in (or reboot) for the memlock limit to apply." + +exit "$fail" diff --git a/19-os-revert.sh b/19-os-revert.sh index 38a05cd..6e17d09 100755 --- a/19-os-revert.sh +++ b/19-os-revert.sh @@ -1,18 +1,153 @@ #!/usr/bin/env bash -# Undo everything 10-os-tune.sh did. +# Undo 10-os-tune.sh, restoring the values this machine actually had. +# +# This used to write hardcoded defaults -- THP=madvise, governor=schedutil, +# power=max, persistence off -- and `rm -f` three paths under /etc without ever +# having proven it owned them. On a machine that had its own governor policy, +# its own THP setting, or its own 99-llm-inference.conf, that was not a revert. +# It was a second, unannounced round of configuration, and in the file case a +# deletion of someone else's work. +# +# Now every value comes from the state file 10-os-tune.sh captured before it +# changed anything. Nothing is assumed, and nothing whose ownership cannot be +# proven is deleted. +# +# Usage: +# ./19-os-revert.sh # restore captured state. Needs sudo. +# ./19-os-revert.sh --dry-run # print what would be restored. No sudo. set -uo pipefail -source "$(dirname "$0")/lib/detect.sh" - -c_info "Reverting OS tuning" -sudo systemctl disable --now llm-gpu-tune.service 2>/dev/null || true -sudo rm -f /etc/systemd/system/llm-gpu-tune.service -sudo systemctl daemon-reload -sudo rm -f /etc/sysctl.d/99-llm-inference.conf /etc/security/limits.d/99-llm-memlock.conf -sudo sysctl --system >/dev/null 2>&1 -echo madvise | sudo tee /sys/kernel/mm/transparent_hugepage/enabled >/dev/null -need cpupower && sudo cpupower frequency-set -g schedutil >/dev/null 2>&1 || true -need system76-power && sudo system76-power profile balanced >/dev/null 2>&1 || true -MAXW=$(nvidia-smi --query-gpu=power.max_limit --format=csv,noheader,nounits | head -1 | cut -d. -f1) -sudo nvidia-smi -pl "$MAXW" >/dev/null 2>&1 || true -sudo nvidia-smi -pm 0 >/dev/null 2>&1 || true -c_ok "reverted (reboot to fully reset)" +RIG_SRC_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "$RIG_SRC_DIR/lib/detect.sh" +source "$RIG_SRC_DIR/lib/ostune.sh" + +DRY=0 +while (( $# )); do + case "$1" in + --dry-run|--plan) DRY=1 ;; + -h|--help) sed -n '2,18p' "${BASH_SOURCE[0]}"; exit 0 ;; + *) die "unknown argument: $1" ;; + esac + shift +done + +# Idempotent by construction: with nothing captured there is nothing this +# script is entitled to change, and saying so is the correct outcome rather +# than an error. +if ! ostune_state_exists; then + c_ok "No llm-rig tuning state at $OSTUNE_STATE -- nothing to revert." + exit 0 +fi + +version="$(ostune_state_get state version 2>/dev/null || printf '')" +if [[ "$version" != "$OSTUNE_STATE_VERSION" ]]; then + die "state file $OSTUNE_STATE is version '${version:-none}', this script understands $OSTUNE_STATE_VERSION. + Refusing to guess what its entries mean." +fi + +captured_at="$(ostune_state_get state tuned_at 2>/dev/null || printf 'unrecorded')" +c_info "Reverting to the state captured at $captured_at" + +fail=0 + +# --- files, before the services that depend on them ------------------------- +for p in "$OSTUNE_UNIT_FILE" "$OSTUNE_SYSCTL_FILE" "$OSTUNE_LIMITS_FILE"; do + ostune_state_has file "$p" || continue + if [[ "$p" == "$OSTUNE_UNIT_FILE" ]] && (( ! DRY )); then + ostune_priv systemctl disable --now llm-gpu-tune.service >/dev/null 2>&1 || true + fi + if (( DRY )); then + printf ' %-50s %s\n' "$p" "$(ostune_state_get file "$p" | cut -f1)" + continue + fi + OSTUNE_LAST_NOTE=""; OSTUNE_LAST_ERROR="" + if ostune_restore_file "$p"; then + c_ok "${OSTUNE_LAST_NOTE:-$p restored}" + else + c_warn "${OSTUNE_LAST_ERROR:-could not restore $p}" + fail=1 + fi +done + +if (( ! DRY )); then + ostune_priv systemctl daemon-reload >/dev/null 2>&1 || true + ostune_priv sysctl --system >/dev/null 2>&1 || true +fi + +# --- THP -------------------------------------------------------------------- +for key in enabled defrag; do + ostune_state_has thp "$key" || continue + want="$(ostune_state_get thp "$key")" + path="$OSTUNE_THP_ENABLED"; [[ "$key" == defrag ]] && path="$OSTUNE_THP_DEFRAG" + if [[ "$want" == "$OSTUNE_UNKNOWN" ]]; then + # It could not be read before the change, so there is no value to put + # back. Writing madvise here -- as this script used to -- would be a new + # decision wearing a rollback's clothes. + c_warn "THP $key was unreadable when tuning ran; leaving it as it is" + continue + fi + if (( DRY )); then printf ' %-50s -> %s\n' "THP $key" "$want"; continue; fi + printf '%s\n' "$want" | ostune_priv tee "$path" >/dev/null 2>&1 \ + && c_ok "THP $key restored to $want" \ + || { c_warn "could not restore THP $key"; fail=1; } +done + +# --- CPU governor, per CPU -------------------------------------------------- +while IFS=$'\t' read -r name value; do + [[ -n "$name" ]] || continue + case "$name" in + s76_profile) + [[ "$value" == "$OSTUNE_UNKNOWN" ]] && { c_warn "system76 profile was unreadable; leaving it"; continue; } + if (( DRY )); then printf ' %-50s -> %s\n' "system76-power profile" "$value"; continue; fi + need system76-power && ostune_priv system76-power profile "$value" >/dev/null 2>&1 \ + && c_ok "system76 profile restored to $value" + ;; + *) + # The name IS the governor file path, so a machine running different + # governors on different cores gets each of them back. + if (( DRY )); then printf ' %-50s -> %s\n' "$(basename "$(dirname "$(dirname "$name")")") governor" "$value"; continue; fi + printf '%s\n' "$value" | ostune_priv tee "$name" >/dev/null 2>&1 || fail=1 + ;; + esac +done < <(ostune_state_list cpu) +(( DRY )) || c_ok "CPU governors restored to their captured values" + +# --- GPU -------------------------------------------------------------------- +while IFS=$'\t' read -r name value; do + [[ -n "$name" ]] || continue + idx="${name%%.*}"; what="${name#*.}" + case "$what" in + persistence) + mode=0; [[ "${value,,}" == enabled || "$value" == 1 ]] && mode=1 + if (( DRY )); then printf ' %-50s -> %s\n' "GPU $idx persistence" "$value"; continue; fi + ostune_priv nvidia-smi -i "$idx" -pm "$mode" >/dev/null 2>&1 \ + && c_ok "GPU $idx persistence restored to $value" + ;; + power_limit) + [[ "$value" == "$OSTUNE_UNKNOWN" ]] && continue + if (( DRY )); then printf ' %-50s -> %sW\n' "GPU $idx power limit" "$value"; continue; fi + # The captured limit, not the maximum. Restoring "max" would raise the + # limit on a machine that had deliberately capped it. + ostune_priv nvidia-smi -i "$idx" -pl "$value" >/dev/null 2>&1 \ + && c_ok "GPU $idx power limit restored to ${value}W" + ;; + esac +done < <(ostune_state_list gpu) + +if (( DRY )); then + echo + c_info "Nothing has been changed. State file: $OSTUNE_STATE" + exit 0 +fi + +# --- retire the state file -------------------------------------------------- +# Only when everything in it was successfully undone. Keeping it after a +# partial revert is what makes a second run finish the job rather than lose +# the record of what is still outstanding. +if (( fail )); then + c_warn "Some entries could not be restored -- keeping $OSTUNE_STATE so a later + run (or a human) can finish. Re-running is safe." + exit 1 +fi + +ostune_priv rm -f "$OSTUNE_STATE" +c_ok "reverted; state file removed (reboot to fully reset)" diff --git a/README.md b/README.md index f498cd5..1a920d0 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,7 @@ chmod +x *.sh source ~/.bashrc ./00-specs.sh # read-only. writes ~/llm-specs.txt, prints the tier plan ./10-os-tune.sh # sudo. GPU persistence, power, governor, THP, sysctls + # --dry-run first if you want to see the plan ./20-build-llamacpp.sh # compiles for your compute capability. 5–20 min. out-of-tree ./30-models.sh # resolves + downloads 3 GGUFs matched to your budget ./40-serve.sh # llama-swap config + systemd + firewall @@ -59,7 +60,7 @@ claude | `./71-verify-runtime.sh` | Query the **running** server's `/props` — confirms live `n_ctx`, flash-attn, KV cache types. Trust this over the config file. Grades its evidence; see below. | | `./70-thermal-sweep.sh` | Re-derive the best power limit for your chassis under a heat-soaked load. | | `./80-try-bigger.sh [quant]` | Assess, download, auto-tune `--n-cpu-moe` and benchmark a model **larger than VRAM**. Empirically finds the lowest working offload level. `--list` sizes it without downloading. | -| `./19-os-revert.sh` | Undo `10-os-tune.sh`. | +| `./19-os-revert.sh` | Undo `10-os-tune.sh`, restoring the values captured before it ran — not assumed defaults. See [Rollback](#what-reversible-means). | `80-try-bigger.sh` exists because with lots of system RAM, an MoE far larger than VRAM is viable — attention stays on the GPU, expert tensors go to CPU, and only a few billion @@ -481,15 +482,61 @@ only if you know why the check is wrong. ## Rollback -- OS tuning: `./19-os-revert.sh` +- OS tuning: `./19-os-revert.sh` — see [what "reversible" means](#what-reversible-means) below - Services: `sudo systemctl disable --now llama-swap` (and `litellm`, if present) - Ollama: weights were moved to `~/.ollama.removed-` and `~/ollama-models.removed-`, not deleted +### What "reversible" means + +`10-os-tune.sh` captures the **effective prior value of every setting it +changes**, one at a time, immediately before changing it. The capture goes to +`/var/lib/llm-rig/os-tune.state`, root-owned and `0600`. `19-os-revert.sh` +restores from that file and from nothing else. + +This is a stronger claim than the one this README used to make. The old revert +wrote fixed defaults — `THP=madvise`, `governor=schedutil`, power at maximum, +persistence off — and `rm -f`'d three paths under `/etc` it had never proven it +owned. On a machine that already had a governor policy, a THP setting, or its +own `99-llm-inference.conf`, that was not a rollback. It was a second round of +configuration, and in the file case a deletion of somebody else's work. + +| Rule | What it prevents | +| --- | --- | +| Capture happens before the first mutation, per setting | A crash halfway through still leaves an exact record of what changed | +| Capture is append-once | A second tune re-recording *llm-rig's own* values as the ones to restore — which turns the rollback into a no-op | +| A pre-existing file is backed up byte for byte before being overwritten, and restored byte for byte | Losing configuration that was there first | +| A file that cannot be backed up is not overwritten at all | Overwriting something we could not preserve | +| A file we wrote is deleted only if it still holds what we wrote | Deleting your edits — an edit is a claim of ownership | +| Per-CPU governors are captured and restored individually | Flattening a machine that deliberately ran different governors on different cores | +| An unreadable prior value is recorded as `unknown` and never "restored" | Substituting a guess for a value nobody read | + +Both scripts take `--dry-run`, which prints every intended mutation with its +current value and needs no `sudo`: + +```bash +./10-os-tune.sh --dry-run +./19-os-revert.sh --dry-run +``` + +``` + THP enabled madvise -> always + governor cpu0 schedutil -> performance + GPU 0 power limit (W) 140 (already set) + /etc/sysctl.d/99-llm-inference.conf YOURS -> backed up, then overwritten +``` + +Reverting twice is a no-op rather than an error, and a revert that could not +finish keeps the state file so a later run can complete it. If a setting cannot +be restored, that is reported and the exit status is non-zero — the one thing +this must never do is report success for a rollback that did not happen. + ## License [MIT](LICENSE). Note that `10-os-tune.sh` takes `sudo` and changes system state — GPU power and persistence, CPU governor, transparent hugepages, and sysctls. Read it before running -it, as you should with any script that asks for root. `19-os-revert.sh` undoes it. +it, as you should with any script that asks for root. `./10-os-tune.sh --dry-run` prints +everything it would change without using `sudo` at all, and `19-os-revert.sh` puts back the +values it captured — see [what that guarantees](#what-reversible-means). diff --git a/TUNING.md b/TUNING.md index ee9117e..d134101 100644 --- a/TUNING.md +++ b/TUNING.md @@ -151,6 +151,36 @@ reaches equilibrium temperature. - **`--split-mode row`** — fails to load without P2P. - **Power capping** — measured strictly worse on this chassis. +## Rolling the OS tuning back + +Every setting on this page is applied by `10-os-tune.sh`, and every one of them +is captured **before** it is changed, to `/var/lib/llm-rig/os-tune.state` +(root-owned, `0600`). `19-os-revert.sh` restores from that capture and from +nothing else. + +That matters most for the settings whose "default" is not a fixed value: + +| Setting | What the old revert wrote | What it should be | +| --- | --- | --- | +| CPU governor | `schedutil`, always | Whatever each CPU had — which on a laptop under `system76-power` is not necessarily the same across cores | +| THP | `madvise`, always | Whatever the machine had; `never` is a legitimate prior state and used to be silently changed to `madvise` | +| GPU power limit | The **maximum** | The captured limit. Restoring the maximum *raises* the cap on a machine that had deliberately lowered it — the opposite of a rollback | +| GPU persistence | Off | Whatever it was. A box running `nvidia-persistenced` already had it on | +| `/etc/sysctl.d/99-llm-inference.conf` | `rm -f` | Removed only if llm-rig created it and it still holds what llm-rig wrote; a pre-existing file is restored byte for byte from a backup | + +The power-limit case is the one worth dwelling on, because this page is the +reason the tuning sets it at all: the measurements below conclude that 100% is +right *for this chassis*. Someone who read them, disagreed, and capped their +card at 120W would have had that decision quietly undone by a revert that +"restored" the maximum. + +Both scripts take `--dry-run`, which needs no `sudo` and changes nothing: + +```bash +./10-os-tune.sh --dry-run # every intended mutation, with its current value +./19-os-revert.sh --dry-run # everything that would be put back +``` + ## Known rough edges - **The single-card trick has no candidate.** See [Models](#models). diff --git a/lib/ostune.sh b/lib/ostune.sh new file mode 100644 index 0000000..87d89af --- /dev/null +++ b/lib/ostune.sh @@ -0,0 +1,426 @@ +#!/usr/bin/env bash +# Transactional, ownership-aware OS tuning. +# +# 10-os-tune.sh changes privileged machine state and writes files under /etc. +# 19-os-revert.sh used to "undo" that by writing hardcoded defaults -- +# THP=madvise, governor=schedutil, power=max, persistence off -- and by +# `rm -f`-ing three paths it had never proven it owned. On a machine that +# already had a governor policy, a THP setting, or its own +# 99-llm-inference.conf, "revert" meant "overwrite with someone else's idea of +# a default", and in the file case "delete". +# +# The README promised the tuning was reversible. It was not; it was +# re-settable. Those are different claims, and this file exists to make the +# first one true. +# +# THE MODEL +# +# 1. Before the FIRST mutation of a setting, its current effective value is +# captured to a root-owned state file. +# 2. Capture is append-once. Re-running the tune never re-captures, because +# the second capture would record OUR value as the prior one -- which is +# how a rollback quietly becomes a no-op. +# 3. Capture happens before the mutation, one setting at a time. A crash +# halfway through therefore leaves a state file describing exactly what +# has been changed so far, and 19-os-revert.sh can undo precisely that. +# 4. A file we did not write is never destroyed. It is backed up byte for +# byte first, and restored byte for byte on revert. If it cannot be backed +# up, the tune refuses rather than proceeding. +# 5. On revert, a file we DID write is deleted only if it still has the +# contents we wrote. If someone has edited it since, it is left alone with +# a message -- an edit is a claim of ownership. +# +# Every path is prefixed by $OSTUNE_ROOT, which is empty in production and a +# sandbox in the tests. That is what lets the whole flow be exercised for real +# -- writing, backing up, restoring, refusing -- without sudo and without a +# mock standing in for the filesystem. +# +# shellcheck shell=bash + +[[ -z "${_LLMRIG_OSTUNE_SH:-}" ]] || return 0 +_LLMRIG_OSTUNE_SH=1 + +# --- paths ------------------------------------------------------------------ + +OSTUNE_ROOT="${OSTUNE_ROOT:-}" + +OSTUNE_STATE_DIR="${OSTUNE_STATE_DIR:-$OSTUNE_ROOT/var/lib/llm-rig}" +OSTUNE_STATE="$OSTUNE_STATE_DIR/os-tune.state" +OSTUNE_BACKUP_DIR="$OSTUNE_STATE_DIR/backup" + +OSTUNE_SYSCTL_FILE="$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" +OSTUNE_LIMITS_FILE="$OSTUNE_ROOT/etc/security/limits.d/99-llm-memlock.conf" +OSTUNE_UNIT_FILE="$OSTUNE_ROOT/etc/systemd/system/llm-gpu-tune.service" + +OSTUNE_THP_ENABLED="$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/enabled" +OSTUNE_THP_DEFRAG="$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/defrag" +OSTUNE_CPU_GLOB="$OSTUNE_ROOT/sys/devices/system/cpu/cpu*/cpufreq/scaling_governor" + +# The state file format version. Bumped if the schema changes, and checked on +# revert: reverting from a state file this code cannot read is worse than +# refusing to. +OSTUNE_STATE_VERSION=1 + +# The value recorded when something could not be read. Never a plausible +# default -- a rollback that restores a guess is the bug being fixed. +OSTUNE_UNKNOWN='unknown' + +# --- privilege -------------------------------------------------------------- + +# Everything privileged goes through here. Tests set OSTUNE_SUDO='' and point +# OSTUNE_ROOT at a sandbox, so the real code paths run unprivileged against +# real (fake-rooted) files rather than against a mock that always says yes. +ostune_priv() { + local s="${OSTUNE_SUDO-sudo}" + if [[ -n "$s" ]]; then "$s" "$@"; else "$@"; fi +} + +# Record a refusal and say it out loud. +# +# Both channels, because these functions are called from the right-hand side +# of a pipe -- `content | ostune_install_file ...` -- which runs in a subshell, +# where an assignment to OSTUNE_LAST_ERROR is discarded the moment the function +# returns. The variable serves direct callers; stderr is what a user actually +# sees. +ostune_fail() { + # shellcheck disable=SC2034 # documented return channel, read by callers + OSTUNE_LAST_ERROR="$*" + printf '\033[1;31m XX\033[0m %s\n' "$*" >&2 + return 1 +} + +# --- the state file --------------------------------------------------------- +# Tab-separated: . Kinds are gpu, cpu, thp, file, unit. +# One line per captured setting, appended in the order they were captured. + +# The state file is 0600 root:root, so every read of it goes through the same +# privilege wrapper the writes do. Reading it with a plain `cat` works when +# testing and fails silently in production -- and a state read that silently +# returns nothing makes the tune re-capture its own values as the prior ones, +# which is precisely the bug that turns a rollback into a no-op. +ostune_state_exists() { ostune_priv test -f "$OSTUNE_STATE"; } +ostune_state_read() { ostune_priv cat "$OSTUNE_STATE" 2>/dev/null; } + +# Create the state directory and file with restrictive permissions. The +# captured state names what a machine was configured to do; it is root-only +# for the same reason the files it describes are. +ostune_state_init() { + ostune_priv mkdir -p "$OSTUNE_STATE_DIR" "$OSTUNE_BACKUP_DIR" || return 1 + ostune_priv chmod 700 "$OSTUNE_STATE_DIR" "$OSTUNE_BACKUP_DIR" || return 1 + if [[ ! -f "$OSTUNE_STATE" ]]; then + printf 'state\tversion\t%s\n' "$OSTUNE_STATE_VERSION" | ostune_priv tee "$OSTUNE_STATE" >/dev/null || return 1 + ostune_priv chmod 600 "$OSTUNE_STATE" || return 1 + fi + return 0 +} + +ostune_state_has() { + local kind="$1" name="$2" + ostune_state_exists || return 1 + ostune_state_read | awk -F'\t' -v k="$kind" -v n="$name" \ + '$1 == k && $2 == n { found = 1; exit } END { exit !found }' +} + +# All value fields of one entry, tab-separated, or nothing. +ostune_state_get() { + local kind="$1" name="$2" + ostune_state_exists || return 1 + local line + line="$(ostune_state_read | awk -F'\t' -v k="$kind" -v n="$name" '$1 == k && $2 == n { + out = "" + for (i = 3; i <= NF; i++) out = out (i > 3 ? "\t" : "") $i + print out; exit + }')" + [[ -n "$line" ]] || return 1 + printf '%s' "$line" +} + +# Append-once. The second capture of a setting would record the value WE set as +# the one to restore, so it is refused rather than overwritten -- silently +# keeping the first is the correct behaviour for a re-run of the tune. +ostune_state_put() { + local kind="$1" name="$2"; shift 2 + ostune_state_has "$kind" "$name" && return 0 + local line; line="$(printf '%s\t%s' "$kind" "$name")" + local v + for v in "$@"; do line+="$(printf '\t%s' "$v")"; done + printf '%s\n' "$line" | ostune_priv tee -a "$OSTUNE_STATE" >/dev/null +} + +# Every entry of a kind, as "\t" lines. +ostune_state_list() { + local kind="$1" + ostune_state_exists || return 1 + ostune_state_read | awk -F'\t' -v k="$kind" '$1 == k { + out = $2 + for (i = 3; i <= NF; i++) out = out "\t" $i + print out + }' +} + +# --- reading current state -------------------------------------------------- + +# The active value out of a sysfs multiple-choice file: "always [madvise] never" +# -> madvise. Prints `unknown` if the file is not readable, because a rollback +# needs to know the difference between "was madvise" and "could not tell". +ostune_sysfs_choice() { + local f="$1" raw v + raw="$(cat "$f" 2>/dev/null)" || { printf '%s' "$OSTUNE_UNKNOWN"; return 1; } + v="$(grep -o '\[[^]]*\]' <<<"$raw" | tr -d '[]')" + if [[ -z "$v" ]]; then + # No brackets. Either the file holds a single bare value -- which is what + # one of these looks like immediately after a write, and what a fixture + # holds -- or it is a list with nothing marked active, which tells us + # nothing and must not be guessed at. + read -r v _ <<<"$raw" + [[ "$raw" == "$v" ]] || v="" + fi + [[ -n "$v" ]] || { printf '%s' "$OSTUNE_UNKNOWN"; return 1; } + printf '%s' "$v" +} + +# "\t" for every CPU that has one. +# +# Per CPU, not one global value: a machine can legitimately run different +# governors on different cores, and restoring cpu0's to all of them would be a +# change dressed up as a rollback. +ostune_governors() { + local f gov found=0 + for f in $OSTUNE_CPU_GLOB; do + [[ -f "$f" ]] || continue + gov="$(cat "$f" 2>/dev/null)" || continue + printf '%s\t%s\n' "$f" "$gov" + found=1 + done + (( found )) +} + +# "\t\t" per GPU, from nvidia-smi. +ostune_gpu_state() { + need nvidia-smi || return 1 + nvidia-smi --query-gpu=index,persistence_mode,power.limit \ + --format=csv,noheader,nounits 2>/dev/null \ + | sed -e 's/[[:space:]]*,[[:space:]]*/\t/g' -e 's/[[:space:]]*$//' \ + | awk -F'\t' 'NF >= 3 { sub(/\..*$/, "", $3); print $1 "\t" $2 "\t" $3 }' +} + +ostune_s76_profile() { + need system76-power || { printf '%s' "$OSTUNE_UNKNOWN"; return 1; } + local out + out="$(system76-power profile 2>/dev/null | sed -n 's/.*[Pp]rofile:[[:space:]]*//p' | head -1)" + out="${out,,}" + [[ -n "$out" ]] || { printf '%s' "$OSTUNE_UNKNOWN"; return 1; } + printf '%s' "$out" +} + +# --- file ownership --------------------------------------------------------- + +# Privileged, because backups live in a 0700 directory: an unprivileged +# sha256sum there fails, and a failed hash reads as "no file" -- which would +# let a restore skip a file it should have put back. +ostune_sha() { + local out + out="$(ostune_priv sha256sum "$1" 2>/dev/null)" || return 1 + [[ -n "$out" ]] || return 1 + printf '%s' "${out%% *}" +} + +# Where a backup of an adopted file lives. The path is mangled into the name so +# two files with the same basename cannot collide. +ostune_backup_path() { + local path="$1" mangled + mangled="$(printf '%s' "${path#"$OSTUNE_ROOT"}" | sed -e 's#^/##' -e 's#/#_#g')" + printf '%s/%s' "$OSTUNE_BACKUP_DIR" "$mangled" +} + +# ostune_file_status -- what we are allowed to do to this file. +# +# absent nothing there; we may create it +# created we created it and it is unchanged; we may replace or delete it +# modified we created it and someone has edited it since; hands off +# adopted it existed first, we backed it up and overwrote it +# adopted-dirty it existed first and has been edited since we wrote it +# foreign it exists and we have no record of it; must be backed up first +ostune_file_status() { + local path="$1" entry kind recorded_sha current + current="$(ostune_sha "$path" 2>/dev/null)" || current="" + + # Present but unhashable -- unreadable, a directory, or a symlink pointing + # somewhere that no longer exists. Existence is checked separately from + # hashing on purpose: treating "cannot read it" as "not there" would + # overwrite the path precisely when we know least about it. + # + # -L as well as -e, because -e follows symlinks and is therefore false for a + # dangling one -- which is exactly the case where writing would replace + # somebody's link with a regular file. + if [[ -z "$current" && ( -e "$path" || -L "$path" ) ]]; then + printf 'unreadable' + return 0 + fi + + if entry="$(ostune_state_get file "$path")"; then + IFS=$'\t' read -r kind recorded_sha _ <<<"$entry" + # Recorded but gone. Nothing to protect and nothing to delete. + [[ -n "$current" ]] || { printf 'absent'; return 0; } + # `unknown` means we recorded ownership and then died before we could hash + # what we wrote. Ownership is therefore unproven, and unproven is treated + # as dirty: refuse, rather than delete a file we cannot show is ours. + if [[ "$recorded_sha" != "$OSTUNE_UNKNOWN" && "$current" == "$recorded_sha" ]]; then + printf '%s' "$kind" + else + printf '%s-dirty' "$kind" + fi + return 0 + fi + + [[ -n "$current" ]] && { printf 'foreign'; return 0; } + printf 'absent' +} + +# ostune_install_file (content on stdin) +# +# Captures ownership before writing, backs up anything pre-existing, and +# refuses rather than proceeding if the backup cannot be made. Records the +# hash of what WE wrote, which is what makes a later delete safe. +ostune_install_file() { + local path="$1" mode="$2" content status backup + content="$(cat)" + status="$(ostune_file_status "$path")" + + case "$status" in + created|adopted) ;; # ours, unchanged: replacing our own content is fine + absent) + # Ownership is recorded BEFORE the write, with the hash left `unknown` + # until there is something to hash. A crash in between therefore leaves a + # file whose ownership is recorded but unproven, and the revert refuses + # to delete it -- the safe direction. + ostune_state_put file "$path" created "$OSTUNE_UNKNOWN" || return 1 + ;; + created-dirty|adopted-dirty) + ostune_fail "$path has been edited since llm-rig wrote it -- refusing to overwrite it. + Reconcile it by hand, or move it aside, then re-run." + return 1 + ;; + unreadable) + ostune_fail "$path exists but cannot be read, so it cannot be backed up -- refusing + to overwrite it. Nothing has been changed." + return 1 + ;; + foreign) + # Pre-existing and not ours. Back it up byte for byte BEFORE touching it, + # and fail closed if that is not possible: proceeding would mean the + # rollback could not restore what was there. + backup="$(ostune_backup_path "$path")" + ostune_priv mkdir -p "$OSTUNE_BACKUP_DIR" || return 1 + if ! ostune_priv cp -p "$path" "$backup"; then + ostune_fail "cannot back up the existing $path -- refusing to overwrite it. + Nothing has been changed." + return 1 + fi + local orig_sha; orig_sha="$(ostune_sha "$path")" + ostune_state_put file "$path" adopted "$OSTUNE_UNKNOWN" "$backup" "$orig_sha" || return 1 + ;; + esac + + ostune_priv mkdir -p "$(dirname "$path")" || return 1 + printf '%s\n' "$content" | ostune_priv tee "$path" >/dev/null || return 1 + ostune_priv chmod "$mode" "$path" || return 1 + + # Now there is something to hash, so the ownership record can be completed. + local ours; ours="$(ostune_sha "$path")" || return 1 + ostune_state_rewrite_sha "$path" "$ours" +} + +# Update the recorded "what we wrote" hash in place, leaving the ownership kind +# and the backup fields alone. Field 4 of a file line: file, path, kind, sha, +# [backup, original-sha]. +ostune_state_rewrite_sha() { + local path="$1" sha="$2" tmp rc + tmp="$(mktemp)" + ostune_state_read | awk -F'\t' -v OFS='\t' -v p="$path" -v s="$sha" ' + $1 == "file" && $2 == p { $4 = s } + { print } + ' >"$tmp" || { rm -f "$tmp"; return 1; } + [[ -s "$tmp" ]] || { rm -f "$tmp"; return 1; } + ostune_priv cp "$tmp" "$OSTUNE_STATE" && ostune_priv chmod 600 "$OSTUNE_STATE" + rc=$? + rm -f "$tmp" + return $rc +} + +# ostune_restore_file -- the revert half. +# +# created: delete, but only if the contents are still ours. +# adopted: put the original back, byte for byte, and verify the hash. +ostune_restore_file() { + local path="$1" entry kind sha backup orig_sha current + entry="$(ostune_state_get file "$path")" || return 0 # never ours, never touched + IFS=$'\t' read -r kind sha backup orig_sha <<<"$entry" + current="$(ostune_sha "$path" 2>/dev/null)" || current="" + + if [[ -z "$current" ]]; then + OSTUNE_LAST_NOTE="$path is already gone" + return 0 + fi + + # An edit since we wrote it is a claim of ownership, and `unknown` is + # ownership we never managed to prove. Both leave the file alone. + if [[ "$sha" == "$OSTUNE_UNKNOWN" ]]; then + ostune_fail "llm-rig recorded $path but never recorded what it wrote, so it cannot prove + the file is unmodified -- leaving it in place. Check it and remove it by hand." + return 1 + fi + if [[ "$current" != "$sha" ]]; then + ostune_fail "$path has been edited since llm-rig wrote it -- leaving it in place. + Remove it by hand if you no longer want it." + return 1 + fi + + case "$kind" in + created) + ostune_priv rm -f "$path" || return 1 + OSTUNE_LAST_NOTE="removed $path" + ;; + adopted) + ostune_priv test -f "$backup" || { + ostune_fail "the backup of $path is missing ($backup) -- leaving the current file in place" + return 1 + } + ostune_priv cp -p "$backup" "$path" || return 1 + local now; now="$(ostune_sha "$path")" + if [[ -n "$orig_sha" && "$now" != "$orig_sha" ]]; then + ostune_fail "restored $path does not match the recorded original hash" + return 1 + fi + OSTUNE_LAST_NOTE="restored the original $path from $backup" + ;; + *) + ostune_fail "unknown ownership record for $path: $kind" + return 1 + ;; + esac + return 0 +} + +# --- plan ------------------------------------------------------------------- + +# One line per intended mutation: "\t\t". Printed by +# --dry-run, which must work without sudo, so nothing here mutates or requires +# privilege -- reading sysfs and querying nvidia-smi are both unprivileged. +ostune_plan_line() { + printf '%s\t%s\t%s\n' "$1" "${2:-$OSTUNE_UNKNOWN}" "$3" +} + +# A human-readable rendering of plan lines on stdin, with unchanged settings +# marked so a reader can see what the tune would actually do. +ostune_plan_render() { + local what from to + while IFS=$'\t' read -r what from to; do + [[ -n "$what" ]] || continue + if [[ "$from" == "$to" ]]; then + printf ' %-34s %s (already set)\n' "$what" "$to" + else + printf ' %-34s %s -> %s\n' "$what" "$from" "$to" + fi + done +} diff --git a/tests/cases/ostune_test.sh b/tests/cases/ostune_test.sh new file mode 100644 index 0000000..350d13d --- /dev/null +++ b/tests/cases/ostune_test.sh @@ -0,0 +1,418 @@ +#!/usr/bin/env bash +# OS tuning: capture before mutation, ownership before overwriting, and a +# revert that restores what was there rather than what someone assumed. +# +# Nothing here needs sudo and nothing touches the real machine. OSTUNE_ROOT +# points at a sandbox and OSTUNE_SUDO is empty, so the scripts run their real +# code paths -- writing, hashing, backing up, restoring, refusing -- against a +# fake root. That is deliberately not a mock: a mock that always succeeds +# cannot show that a file was preserved byte for byte. +set -uo pipefail + +TEST_ROOT="${TEST_ROOT:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}" +REPO_ROOT="${REPO_ROOT:-$(cd "$TEST_ROOT/.." && pwd)}" +source "$TEST_ROOT/lib/harness.sh" +source "$TEST_ROOT/lib/mockenv.sh" + +# shellcheck disable=SC2034 +SUITE_NAME="OS tuning rollback (#23)" + +setup_test() { + mock_init + export OSTUNE_ROOT="$SANDBOX/root" + export OSTUNE_SUDO="" # the sandbox is ours; no privilege needed + export MOCK_S76_PROFILE="Balanced" + mkdir -p "$OSTUNE_ROOT/etc/sysctl.d" \ + "$OSTUNE_ROOT/etc/security/limits.d" \ + "$OSTUNE_ROOT/etc/systemd/system" \ + "$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage" \ + "$OSTUNE_ROOT/proc/sys/vm" + # A machine with settings of its own, none of them llm-rig's defaults. + printf 'always [madvise] never\n' >"$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/enabled" + printf 'always defer [defer+madvise] madvise never\n' >"$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/defrag" + printf '60\n' >"$OSTUNE_ROOT/proc/sys/vm/swappiness" + set_governors schedutil powersave + # Re-source with the sandbox paths in place. + unset _LLMRIG_OSTUNE_SH + # shellcheck source=lib/ostune.sh + source "$REPO_ROOT/lib/ostune.sh" +} + +# Give cpu0..cpuN-1 the governors named, one per argument. +set_governors() { + local i=0 g + for g in "$@"; do + mkdir -p "$OSTUNE_ROOT/sys/devices/system/cpu/cpu$i/cpufreq" + printf '%s\n' "$g" >"$OSTUNE_ROOT/sys/devices/system/cpu/cpu$i/cpufreq/scaling_governor" + i=$(( i + 1 )) + done +} + +governor_of() { cat "$OSTUNE_ROOT/sys/devices/system/cpu/cpu$1/cpufreq/scaling_governor"; } +# The ACTIVE choice, not the raw text. Real sysfs answers a write of "madvise" +# with "always [madvise] never"; a sandbox file just holds "madvise". Comparing +# raw text would make the test assert a detail of the fake rather than the +# behaviour under test. +thp_now() { ostune_sysfs_choice "$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/enabled"; } +thp_raw() { cat "$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/enabled"; } + +tune() { run bash -c "cd '$REPO_ROOT' && OSTUNE_ROOT='$OSTUNE_ROOT' OSTUNE_SUDO='' HOME='$HOME' PATH='$PATH' bash ./10-os-tune.sh $*"; } +revert() { run bash -c "cd '$REPO_ROOT' && OSTUNE_ROOT='$OSTUNE_ROOT' OSTUNE_SUDO='' HOME='$HOME' PATH='$PATH' bash ./19-os-revert.sh $*"; } + +state_file() { printf '%s' "$OSTUNE_ROOT/var/lib/llm-rig/os-tune.state"; } + +# The real ln, not the mock on PATH. A test that needs a symlink to actually +# exist cannot use the stub that only records the call -- it would silently +# create nothing, and the assertion would be measuring the mock. +real_ln() { PATH=/usr/bin:/bin ln "$@"; } + +# --- the state file --------------------------------------------------------- + +test_capture_happens_before_the_first_mutation() { + tune + assert_ok "tune must complete: $RUN_OUTPUT" || return 1 + # THP was madvise before the run; the state file must say so, not `always`. + assert_eq "$(ostune_state_get thp enabled)" "madvise" "the PRIOR value" || return 1 + assert_eq "$(thp_now)" "always" "and the new value is applied" +} + +test_the_state_file_is_root_only() { + tune + local dir_mode file_mode + dir_mode="$(stat -c '%a' "$OSTUNE_ROOT/var/lib/llm-rig")" + file_mode="$(stat -c '%a' "$(state_file)")" + assert_eq "$dir_mode" "700" "state directory permissions" || return 1 + assert_eq "$file_mode" "600" "state file permissions" +} + +test_a_second_tune_does_not_recapture_its_own_values() { + # The failure this prevents: run twice, and the "prior" governor recorded is + # `performance` -- the value llm-rig set -- so the revert restores nothing. + tune + assert_eq "$(ostune_state_get cpu "$OSTUNE_ROOT/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor")" \ + "schedutil" "first capture" || return 1 + tune + assert_eq "$(ostune_state_get cpu "$OSTUNE_ROOT/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor")" \ + "schedutil" "still the original after a second run" +} + +test_the_state_file_records_a_version_and_a_timestamp() { + tune + assert_eq "$(ostune_state_get state version)" "1" "version" || return 1 + assert_matches "$(ostune_state_get state tuned_at)" '^[0-9]{4}-[0-9]{2}-[0-9]{2}T' "timestamp" +} + +test_a_state_file_from_a_future_version_is_refused() { + tune + sed -i 's/^state\tversion\t1$/state\tversion\t99/' "$(state_file)" + revert + assert_fails "refuse rather than guess" || return 1 + assert_contains "$RUN_OUTPUT" "version" "with the reason named" || return 1 + assert_eq "$(governor_of 0)" "performance" "and nothing is changed" +} + +# --- restoring values, not defaults ----------------------------------------- + +test_the_governor_goes_back_to_what_it_was() { + # schedutil is what this machine had. The old revert wrote schedutil too -- + # by coincidence, because it was hardcoded. This one reads it. + tune + assert_eq "$(governor_of 0)" "performance" "tuned" || return 1 + revert + assert_ok "revert: $RUN_OUTPUT" || return 1 + assert_eq "$(governor_of 0)" "schedutil" "restored" +} + +test_an_unusual_governor_is_restored_rather_than_normalised() { + # The case the hardcoded revert got wrong: a machine deliberately running + # `powersave` got `schedutil` back and nobody noticed. + set_governors ondemand ondemand + tune + revert + assert_eq "$(governor_of 0)" "ondemand" "cpu0" || return 1 + assert_eq "$(governor_of 1)" "ondemand" "cpu1" +} + +test_per_cpu_governors_are_restored_individually() { + set_governors schedutil powersave + tune + revert + assert_eq "$(governor_of 0)" "schedutil" "cpu0 keeps its own" || return 1 + assert_eq "$(governor_of 1)" "powersave" "cpu1 keeps its own" +} + +test_thp_is_restored_to_the_captured_value() { + printf 'always madvise [never]\n' >"$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/enabled" + tune + revert + assert_eq "$(thp_now)" "never" "restored to never, not to the assumed madvise" +} + +test_an_unreadable_prior_value_is_not_replaced_with_a_guess() { + rm -f "$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/enabled" + tune + assert_eq "$(ostune_state_get thp enabled)" "unknown" "recorded as unknown" || return 1 + printf 'always [madvise] never\n' >"$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/enabled" + revert + assert_contains "$RUN_OUTPUT" "unreadable" "says so" || return 1 + assert_contains "$(thp_raw)" "[madvise]" "and leaves the current setting alone" +} + +test_the_gpu_power_limit_is_restored_to_the_captured_watts() { + tune + revert + # dual_a4000 reports a 140W limit; the revert must ask for that, not for the + # maximum -- restoring "max" would raise a limit somebody had lowered. + assert_contains "$(cat "$MOCK_CALLS")" "nvidia-smi -i 0 -pl 140" "captured watts" || return 1 + assert_not_contains "$(cat "$MOCK_CALLS")" "-pm 0" "persistence is restored by value, not switched off" +} + +test_persistence_is_restored_to_its_captured_mode() { + tune + revert + # The fixture reports persistence Enabled, so the revert must set 1, not 0. + assert_contains "$(cat "$MOCK_CALLS")" "nvidia-smi -i 0 -pm 1" "restored to Enabled" +} + +# --- file ownership --------------------------------------------------------- + +test_a_file_we_created_is_removed_on_revert() { + tune + assert_ok "tune" || return 1 + [[ -f "$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" ]] \ + || { _fail "the sysctl file should have been created"; return 1; } + revert + assert_ok "revert: $RUN_OUTPUT" || return 1 + [[ -f "$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" ]] \ + && { _fail "our own file should have been removed"; return 1; } + return 0 +} + +test_a_pre_existing_file_is_backed_up_and_restored_byte_for_byte() { + local f="$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" + printf 'vm.swappiness = 42\n# my own tuning, do not touch\n' >"$f" + local before; before="$(sha256sum "$f" | cut -d' ' -f1)" + + tune + assert_ok "tune" || return 1 + assert_ne "$(sha256sum "$f" | cut -d' ' -f1)" "$before" "it was overwritten while tuned" || return 1 + + revert + assert_ok "revert: $RUN_OUTPUT" || return 1 + assert_eq "$(sha256sum "$f" | cut -d' ' -f1)" "$before" "byte-for-byte restore" || return 1 + assert_contains "$(cat "$f")" "do not touch" "including the comment" +} + +test_a_pre_existing_file_that_cannot_be_backed_up_stops_the_write() { + # Fail closed: if the original cannot be preserved, it must not be replaced. + # An unreadable file is also the case where "cannot hash it" must not be + # mistaken for "not there" -- that mistake would overwrite the file at + # exactly the moment we know least about it. + # + # A dangling symlink rather than an unreadable file, because the isolated + # suite runs inside `unshare --map-root-user` -- as root, where permission + # bits mean nothing and a chmod 000 file is readable. A link to something + # that no longer exists cannot be hashed whoever you are, and is a real + # shape for a config path to be in. + local f="$OSTUNE_ROOT/etc/security/limits.d/99-llm-memlock.conf" + real_ln -s "$SANDBOX/removed-by-a-package.conf" "$f" + + tune + [[ -L "$f" ]] || { _fail "the symlink must survive untouched"; return 1; } + [[ -f "$f" ]] && { _fail "it must not have been replaced by a regular file"; return 1; } + assert_contains "$RUN_OUTPUT" "cannot be read" "and the refusal must say why" +} + +test_a_file_edited_after_we_wrote_it_is_never_deleted() { + # An edit is a claim of ownership. + tune + local f="$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" + printf '\n# I changed this\n' >>"$f" + revert + assert_fails "revert must report that it could not finish" || return 1 + [[ -f "$f" ]] || { _fail "an edited file must not be deleted"; return 1; } + assert_contains "$(cat "$f")" "I changed this" "and must keep the edit" || return 1 + assert_contains "$RUN_OUTPUT" "edited" "with a message naming the problem" +} + +test_a_file_edited_after_we_wrote_it_is_never_overwritten_either() { + tune + local f="$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" + printf '\n# I changed this\n' >>"$f" + tune + assert_contains "$(cat "$f")" "I changed this" "a re-tune must not clobber the edit" || return 1 + assert_contains "$RUN_OUTPUT" "edited" "and must say so" +} + +test_ownership_that_was_never_proven_blocks_deletion() { + # 10-os-tune.sh records ownership before writing, with the hash filled in + # afterwards. A crash in between leaves `unknown`, and an unproven claim of + # ownership must not authorise a delete. + tune + local f="$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" + sed -i "s#^\(file\t$f\tcreated\t\).*#\1unknown#" "$(state_file)" + revert + assert_fails "cannot prove it is ours" || return 1 + [[ -f "$f" ]] || { _fail "must not delete a file whose ownership is unproven"; return 1; } + return 0 +} + +test_a_file_llm_rig_never_touched_is_left_alone_by_revert() { + local other="$OSTUNE_ROOT/etc/sysctl.d/50-somebody-else.conf" + printf 'vm.swappiness = 7\n' >"$other" + tune + revert + assert_eq "$(cat "$other")" "vm.swappiness = 7" "an unrelated file is not in scope" +} + +# --- partial failure and idempotence ---------------------------------------- + +test_a_partial_failure_still_leaves_a_usable_rollback() { + # The limits file is pre-existing and unreadable, so step 5 refuses. Steps + # 1-4 already happened, and every one of them must still be revertible. + local f="$OSTUNE_ROOT/etc/security/limits.d/99-llm-memlock.conf" + real_ln -s "$SANDBOX/removed-by-a-package.conf" "$f" + tune + + assert_eq "$(ostune_state_get thp enabled)" "madvise" "THP was captured before it changed" || return 1 + assert_eq "$(governor_of 0)" "performance" "and the governor was changed" || return 1 + + revert + assert_eq "$(governor_of 0)" "schedutil" "so the revert can still undo it" || return 1 + [[ -L "$f" && ! -f "$f" ]] || { _fail "the path that was never written must be untouched"; return 1; } + return 0 +} + +test_tune_revert_tune_revert_ends_where_it_started() { + local before_gov before_thp + before_gov="$(governor_of 0)"; before_thp="$(thp_now)" + tune; revert; tune; revert + assert_ok "the second revert must succeed: $RUN_OUTPUT" || return 1 + assert_eq "$(governor_of 0)" "$before_gov" "governor" || return 1 + assert_eq "$(thp_now)" "$before_thp" "THP" || return 1 + [[ -f "$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" ]] \ + && { _fail "the sysctl file should be gone again"; return 1; } + return 0 +} + +test_reverting_twice_is_not_an_error() { + tune + revert + assert_ok "first revert" || return 1 + revert + assert_ok "second revert must be a no-op, not a failure" || return 1 + assert_contains "$RUN_OUTPUT" "nothing to revert" "and must say so" +} + +test_reverting_without_ever_tuning_is_not_an_error() { + revert + assert_ok "nothing captured, nothing to do" || return 1 + assert_contains "$RUN_OUTPUT" "nothing to revert" "with an explanation" +} + +test_the_state_file_survives_a_partial_revert() { + # Otherwise the record of what is still outstanding is destroyed by the run + # that failed to finish. + tune + printf '\n# edited\n' >>"$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" + revert + assert_fails "the revert could not finish" || return 1 + [[ -f "$(state_file)" ]] || { _fail "state must be kept so a later run can finish"; return 1; } + return 0 +} + +test_a_clean_revert_removes_the_state_file() { + tune + revert + [[ -f "$(state_file)" ]] && { _fail "a completed revert has nothing left to describe"; return 1; } + return 0 +} + +# --- dry run ---------------------------------------------------------------- + +test_dry_run_changes_nothing_at_all() { + tune --dry-run + assert_ok "dry run: $RUN_OUTPUT" || return 1 + assert_eq "$(governor_of 0)" "schedutil" "governor untouched" || return 1 + assert_contains "$(thp_raw)" "[madvise]" "THP untouched" || return 1 + [[ -f "$(state_file)" ]] && { _fail "a plan must not write state"; return 1; } + [[ -f "$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" ]] \ + && { _fail "a plan must not write files"; return 1; } + return 0 +} + +test_dry_run_uses_no_privilege() { + tune --dry-run + assert_not_contains "$(cat "$MOCK_CALLS")" "sudo " "no sudo in a plan" +} + +test_dry_run_shows_each_intended_change_with_its_current_value() { + tune --dry-run + assert_contains "$RUN_OUTPUT" "THP enabled" "names the setting" || return 1 + assert_contains "$RUN_OUTPUT" "madvise -> always" "from and to" || return 1 + assert_contains "$RUN_OUTPUT" "schedutil -> performance" "the governor too" +} + +test_dry_run_warns_that_an_existing_file_would_be_overwritten() { + printf 'mine\n' >"$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" + tune --dry-run + assert_contains "$RUN_OUTPUT" "backed up, then overwritten" "the plan says what happens to it" +} + +test_dry_run_marks_settings_that_are_already_correct() { + printf '[always] madvise never\n' >"$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/enabled" + tune --dry-run + assert_contains "$RUN_OUTPUT" "already set" "no change is a legitimate plan entry" +} + +test_revert_has_a_dry_run_too() { + tune + revert --dry-run + assert_ok "revert plan" || return 1 + assert_contains "$RUN_OUTPUT" "schedutil" "shows what it would restore" || return 1 + assert_eq "$(governor_of 0)" "performance" "and changes nothing" || return 1 + [[ -f "$(state_file)" ]] || { _fail "a plan must not consume the state file"; return 1; } + return 0 +} + +# --- the library, directly -------------------------------------------------- + +test_file_status_distinguishes_the_four_cases() { + local f="$OSTUNE_ROOT/etc/sysctl.d/test.conf" + ostune_state_init + assert_eq "$(ostune_file_status "$f")" "absent" "nothing there" || return 1 + + printf 'yours\n' >"$f" + assert_eq "$(ostune_file_status "$f")" "foreign" "someone else's" || return 1 + + printf 'ours\n' | ostune_install_file "$f" 644 + assert_eq "$(ostune_file_status "$f")" "adopted" "adopted after backup" || return 1 + + printf 'edited\n' >>"$f" + assert_eq "$(ostune_file_status "$f")" "adopted-dirty" "edited since" +} + +test_the_backup_path_cannot_collide() { + local a b + a="$(ostune_backup_path /etc/sysctl.d/99-x.conf)" + b="$(ostune_backup_path /etc/security/limits.d/99-x.conf)" + assert_ne "$a" "$b" "two files with the same basename must back up separately" +} + +test_a_sysfs_choice_is_read_from_the_brackets() { + local f="$SANDBOX/choice" + printf 'always [madvise] never\n' >"$f" + assert_eq "$(ostune_sysfs_choice "$f")" "madvise" "the active choice" || return 1 + assert_eq "$(ostune_sysfs_choice "$SANDBOX/missing")" "unknown" "an unreadable file" +} + +test_the_state_file_is_append_once_per_setting() { + ostune_state_init + ostune_state_put thp enabled madvise + ostune_state_put thp enabled always + assert_eq "$(ostune_state_get thp enabled)" "madvise" "the first capture wins" || return 1 + assert_eq "$(grep -c $'^thp\tenabled' "$(state_file)")" "1" "and there is only one line" +} + +run_suite +suite_exit diff --git a/tests/mocks/bin/cpupower b/tests/mocks/bin/cpupower new file mode 100755 index 0000000..8db59cb --- /dev/null +++ b/tests/mocks/bin/cpupower @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# Mock cpupower. +# +# `frequency-set -g X` really writes the sandbox governor files, rather than +# only recording the call. A mock that merely says "yes" cannot demonstrate +# that a revert put the previous governor back -- which is the whole subject of +# issue #23 -- so this one models the effect the real tool has. +set -uo pipefail +printf 'cpupower %s\n' "$*" >>"${MOCK_CALLS:-/dev/null}" + +gov="" +prev="" +for a in "$@"; do + [[ "$prev" == "-g" || "$prev" == "--governor" ]] && { gov="$a"; break; } + prev="$a" +done + +if [[ -n "$gov" ]]; then + for f in "${OSTUNE_ROOT:-}"/sys/devices/system/cpu/cpu*/cpufreq/scaling_governor; do + [[ -f "$f" ]] || continue + printf '%s\n' "$gov" >"$f" 2>/dev/null || true + done +fi +exit 0 diff --git a/tests/mocks/bin/sysctl b/tests/mocks/bin/sysctl new file mode 100755 index 0000000..a89ceec --- /dev/null +++ b/tests/mocks/bin/sysctl @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +# Mock sysctl. Records the call; changes nothing. +set -uo pipefail +printf 'sysctl %s\n' "$*" >>"${MOCK_CALLS:-/dev/null}" +exit 0 diff --git a/tests/mocks/bin/system76-power b/tests/mocks/bin/system76-power new file mode 100755 index 0000000..c290fd3 --- /dev/null +++ b/tests/mocks/bin/system76-power @@ -0,0 +1,9 @@ +#!/usr/bin/env bash +# Mock system76-power. Records the call; changes nothing. +set -uo pipefail +printf 'system76-power %s\n' "$*" >>"${MOCK_CALLS:-/dev/null}" +# `system76-power profile` with no argument REPORTS; with one it SETS. +if [[ "${1:-}" == "profile" && -z "${2:-}" ]]; then + printf 'Power Profile: %s\n' "${MOCK_S76_PROFILE:-Balanced}" +fi +exit 0 From 04b7f67b40b8ff7a75454ddc69a100c6f029e3d6 Mon Sep 17 00:00:00 2001 From: rammsguns <6564405+rammsguns@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:07:01 -0600 Subject: [PATCH 2/2] Mark the cross-file state variables as intentionally external ShellCheck at --severity=warning flags SC2034 for the six variables lib/ostune.sh defines for 10-os-tune.sh and 19-os-revert.sh to read: the three /etc paths, the two THP paths, and OSTUNE_LAST_NOTE. Same convention as the rest of lib/ -- a directive at the assignment, naming it a documented return channel. OSTUNE_LAST_NOTE gets one directive scoped to the whole of ostune_restore_file rather than three copies inside it, matching what fdb0a2b did in the selector. No behaviour change. CI's fixture suites were already green; this is the lint half. Co-Authored-By: Claude Opus 5 --- lib/ostune.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/ostune.sh b/lib/ostune.sh index 87d89af..64b6d98 100644 --- a/lib/ostune.sh +++ b/lib/ostune.sh @@ -48,11 +48,18 @@ OSTUNE_STATE_DIR="${OSTUNE_STATE_DIR:-$OSTUNE_ROOT/var/lib/llm-rig}" OSTUNE_STATE="$OSTUNE_STATE_DIR/os-tune.state" OSTUNE_BACKUP_DIR="$OSTUNE_STATE_DIR/backup" +# The three files under /etc. Read by 10-os-tune.sh and 19-os-revert.sh, which +# is why shellcheck cannot see a use for them here. +# shellcheck disable=SC2034 # documented return channel, read by callers OSTUNE_SYSCTL_FILE="$OSTUNE_ROOT/etc/sysctl.d/99-llm-inference.conf" +# shellcheck disable=SC2034 # documented return channel, read by callers OSTUNE_LIMITS_FILE="$OSTUNE_ROOT/etc/security/limits.d/99-llm-memlock.conf" +# shellcheck disable=SC2034 # documented return channel, read by callers OSTUNE_UNIT_FILE="$OSTUNE_ROOT/etc/systemd/system/llm-gpu-tune.service" +# shellcheck disable=SC2034 # documented return channel, read by callers OSTUNE_THP_ENABLED="$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/enabled" +# shellcheck disable=SC2034 # documented return channel, read by callers OSTUNE_THP_DEFRAG="$OSTUNE_ROOT/sys/kernel/mm/transparent_hugepage/defrag" OSTUNE_CPU_GLOB="$OSTUNE_ROOT/sys/devices/system/cpu/cpu*/cpufreq/scaling_governor" @@ -352,6 +359,9 @@ ostune_state_rewrite_sha() { # # created: delete, but only if the contents are still ours. # adopted: put the original back, byte for byte, and verify the hash. +# shellcheck disable=SC2034 # OSTUNE_LAST_NOTE is a documented return channel, +# read by 19-os-revert.sh. Scoped to the whole function rather than repeated at +# each assignment. ostune_restore_file() { local path="$1" entry kind sha backup orig_sha current entry="$(ostune_state_get file "$path")" || return 0 # never ours, never touched