cpu/stressng_cpu: Add runtime scheduling-policy change test - #3238
cpu/stressng_cpu: Add runtime scheduling-policy change test#3238SamirMulani wants to merge 1 commit into
Conversation
|
|
||
| Background | ||
| ---------- | ||
| By default, processes run under CFS (SCHED_OTHER, policy=0). |
There was a problem hiding this comment.
I will suggest to only retain this content under background. Remaining content is not required and is too verbose
There was a problem hiding this comment.
Trimmed the Background section to a single essential paragraph. Removed the transition chain diagram, SCHED_DEADLINE explanation, Verification Method, and Test Flow sections.
| self.log.info("Test: Runtime Scheduling-Policy Change") | ||
| self.log.info("=" * 60) | ||
|
|
||
| # Aggregate results across all transitions and all PIDs. |
|
|
||
| # ---------------------------------------------------------------- | ||
| # Step 1: Spawn stress-ng in the background | ||
| # ---------------------------------------------------------------- |
|
|
||
| # ---------------------------------------------------------------- | ||
| # Step 2: Discover worker PIDs | ||
| # ---------------------------------------------------------------- |
|
|
||
| # ---------------------------------------------------------------- | ||
| # Step 3: Walk through the policy transition chain | ||
| # ---------------------------------------------------------------- |
| "pgrep found no children of PID %d", parent_pid) | ||
|
|
||
| # Always include the parent itself so we have at least one target | ||
| pids.add(parent_pid) |
There was a problem hiding this comment.
_get_stressng_worker_pids unconditionally adds parent_pid to pids with pids.add(parent_pid) at the end. This means worker_pids can never be empty. The guard:
if not worker_pids:
self._terminate_stressng()
self.fail("No stress-ng worker PIDs found — cannot proceed.")
…is dead code and will never trigger. The real case of interest — no children (workers) found — is silently swallowed, and the test proceeds exercising only the parent PID (the stress-ng manager process, not a CPU stressor).
Suggested to check for children separately from the parent.
|
|
||
| # Derived from POLICY_META so label strings are never duplicated. | ||
| # chrt -p <pid> uses "--<label>" as the policy flag. | ||
| CHRT_POLICY_FLAG = { |
There was a problem hiding this comment.
This is never used anywhere in the code. Is this required?
|
|
||
| def _read_proc_sched_policy(self, pid): | ||
| """ | ||
| Read the current scheduling policy integer for *pid* from |
There was a problem hiding this comment.
The docstring opens with "Read the current scheduling policy integer for pid from /proc//status" and then contradicts itself in the same sentence, describing what actually happens (using os.sched_getscheduler()). The /proc//status reference is wrong — that file does not contain the scheduling policy integer — and will confuse anyone reading this code.
Please change the docstring accordingly
| Parameters (from YAML) | ||
| ---------------------- | ||
| - ``policy_change_runtime`` : seconds stress-ng runs (default 120) | ||
| - ``policy_change_workers`` : number of CPU stressor threads |
There was a problem hiding this comment.
also this contradicts with default value mentioned in YAML
| except (subprocess.CalledProcessError, FileNotFoundError) as chrt_err: | ||
| self.log.warning( | ||
| "chrt verification skipped for PID %d: %s", pid, chrt_err) | ||
| return True, "chrt-unavailable" |
There was a problem hiding this comment.
When chrt is not installed (FileNotFoundError) or fails (CalledProcessError), the method returns (True, "chrt-unavailable"). This means the secondary verification silently succeeds — the test may pass while only the primary os.sched_getscheduler() check actually ran. This is particularly concerning because the PR's stated purpose is dual verification. There is no warning to the user that the secondary verification was skipped for the whole test run (not just one PID).
There was a problem hiding this comment.
Split the combined except (subprocess.CalledProcessError, FileNotFoundError) into separate handlers. FileNotFoundError now returns (False, "chrt-not-installed") and emits a warning indicating that secondary verification is unavailable for the entire test run. CalledProcessError returns (False, "chrt-error") with a per-PID warning. Both cases are now reported as actual failures in all_results instead of being treated as successful secondary verification.
6cc872e to
c5859f8
Compare
|
| child_pids = [p for p in worker_pids | ||
| if p != self._stressng_proc.pid] | ||
| if not child_pids: | ||
| self._terminate_stressng() |
There was a problem hiding this comment.
_terminate_stressng() sets self._stressng_proc = None as its first action (line 682) before doing anything else. After calling _terminate_stressng() on line 544, the very next line (548) dereferences self._stressng_proc.pid, which will raise AttributeError: 'NoneType' object has no attribute 'pid' and mask the real failure with an uncaught exception instead of the intended self.fail() message.
Current Code:
line 543-548
if not child_pids:
self._terminate_stressng() # sets self._stressng_proc = None !
self.fail(
"No stress-ng worker child PIDs found under PID %d — "
"cannot exercise CPU stressor scheduling policies."
% self._stressng_proc.pid) # <-- AttributeError: None.pid
Suggested Fix:
Capture the PID before terminating:
if not child_pids:
mgr_pid = self._stressng_proc.pid
self._terminate_stressng()
self.fail(
"No stress-ng worker child PIDs found under PID %d — "
"cannot exercise CPU stressor scheduling policies." % mgr_pid)
|
|
||
| Uses two independent sources: | ||
| 1. ``os.sched_getscheduler(pid)`` — kernel syscall (primary) | ||
| 2. ``chrt -p <pid>`` — userspace tool (secondary) |
There was a problem hiding this comment.
verify_policy_via_chrt() returns (False, "chrt-not-installed") or (False, "chrt-error") when chrt is absent or fails. In _verify_policy_for_pids(), any False from chrt_ok is unconditionally appended to failures[], which then causes self.fail(). This means the entire test will be marked FAILED on any system where chrt is not installed or where it temporarily fails (e.g. PID disappeared between set and verify). The PR description calls chrt a secondary check, but the code treats it as primary and mandatory. The FileNotFoundError warning at line 385 claims "secondary policy verification is unavailable for the entire test run" — but the loop never exits early after that; it will emit this warning for every PID of every transition, flooding the log.
Current Code:
_verify_policy_for_pids, lines 487-495
chrt_ok, chrt_actual = self._verify_policy_via_chrt(pid, expected_policy)
if not chrt_ok:
failures.append(
"PID %d: chrt mismatch — expected %s got %s"
% (pid, policy_name, chrt_actual))
Suggested Fix:
Distinguish between chrt tool errors (warn only) and actual policy mismatches:
chrt_ok, chrt_actual = self._verify_policy_via_chrt(pid, expected_policy)
if not chrt_ok:
if chrt_actual in ("chrt-not-installed", "chrt-error", "parse-error"):
# Secondary tool unavailable; primary syscall result is authoritative
self.log.warning(
"PID %d: chrt secondary check skipped (%s)",
pid, chrt_actual)
else:
failures.append(
"PID %d: chrt mismatch — expected %s got %s"
% (pid, policy_name, chrt_actual))
Also, track whether chrt is available once (e.g. a self._chrt_available flag set in setUp) and skip all secondary checks early rather than spawning a new chrt subprocess for every PID of every transition.
| proc = subprocess.Popen( | ||
| cmd, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, |
Add test_runtime_policy_change() to validate that the Linux kernel correctly applies sched_setscheduler(2) on live stress-ng worker threads at run-time. The test spawns stress-ng CPU worker threads in the background and walks the following policy transition chain on all worker PIDs: SCHED_OTHER -> SCHED_IDLE -> SCHED_BATCH -> SCHED_FIFO -> SCHED_RR -> SCHED_OTHER SCHED_DEADLINE is excluded as it requires sched_setattr(2) with runtime/deadline/period attributes which are not exposed via the Python os.sched_setscheduler() interface. Each transition is verified using two sources: - os.sched_getscheduler(pid) : direct kernel syscall (primary) - chrt -p <pid> : userspace scheduling tool (secondary) All transition results are collected in a list and reported at the end so a single run shows the complete picture of pass/fail/skip rather than stopping at the first failure. Also introduce POLICY_META as a single source of truth for policy attributes (name, label, rt_prio) and derive CHRT_POLICY_FLAG from it via a dict comprehension to avoid duplicate policy definitions. New YAML parameters: - policy_change_runtime: seconds stress-ng workers stay alive (default 120) - policy_change_workers: number of --cpu worker threads to spawn (default 4) Signed-off-by: Samir Mulani <samir@linux.ibm.com>
0a61576 to
423125d
Compare
|
3 participants
Add test_runtime_policy_change() to validate that the Linux kernel correctly applies sched_setscheduler(2) on live stress-ng worker threads at run-time.
The test spawns stress-ng CPU worker threads in the background and walks the following policy transition chain on all worker PIDs:
SCHED_OTHER -> SCHED_IDLE -> SCHED_BATCH -> SCHED_FIFO -> SCHED_RR -> SCHED_OTHER
SCHED_DEADLINE is excluded as it requires sched_setattr(2) with runtime/deadline/period attributes which are not exposed via the Python os.sched_setscheduler() interface.
Each transition is verified using two sources:
All transition results are collected in a list and reported at the end so a single run shows the complete picture of pass/fail/skip rather than stopping at the first failure.
Also introduce POLICY_META as a single source of truth for policy attributes (name, label, rt_prio) and derive CHRT_POLICY_FLAG from it via a dict comprehension to avoid duplicate policy definitions.
New YAML parameters: