[Fix] (Trace, Bench, CI): compute p50 by the device's own median rule, fail the profile guard closed, and give the new control step its sibling's environment

- format_benchmark printed a p50 taken with the nearest-rank rule beside a medianFrameCpuMs the
  device computes as the average of the two middle frames, and documented the two as one rule; on
  an even window they differ (the pre-flight printed p50=8.261ms next to medianCpuMs=271.766).
  p50 now goes through series_median, which is SummarizeSeries' rule transcribed; p95 and p99 stay
  nearest rank, which is the device's rule for p95 and the honest extension of it for the p99 the
  device does not compute at all
- require_verified_profile treated a profile that simply omits PROFILE_VERIFIED as verified, which
  is the fail-open default a profile written by copying another one inherits - exactly the case the
  guard exists for. It defaults to unverified now, odinlite.env carries PROFILE_VERIFIED=1
  explicitly (it is the one profile that earned it), and the refusal says "says 0, or says nothing"
- the two new profiles claimed profile.sh refuses an unverified profile; it has no such check and
  needs none - it records a simpleperf profile and pins nothing. The claim is corrected in both
  profiles and in the README rather than a guard added where there is nothing to guard
- the handle-ABA / CSO control step in test.yml set only MOBILEGL_ITEST_REQUIRE_GPU while its
  sibling verify step sets the three MOBILEGL_MAGMA_* fixes and arms core dumps. It runs the same
  DirectVulkan binary on the same runner, so a crash there left no core; it now carries both
This commit is contained in:
2026-09-07 23:18:09 -04:00
parent a5d1136c02
commit af20dba6db
8 changed files with 73 additions and 20 deletions
+11
View File
@@ -577,11 +577,22 @@ jobs:
# An arm whose subsystem has not landed on this tree SKIPS with the reason (never absent,
# never a green that asserted nothing), so this step is green through the P2 landing order
# and starts asserting as each package arrives.
#
# The environment is the sibling step's, deliberately and in full: these entries run the
# same DirectVulkan binary through the same runner, so the three MOBILEGL_MAGMA_* fixes it
# needs apply here too, and a crash here has to leave a core for the same black-box flow.
# The step above is the only reason those lines exist in this job; a control that crashed
# without one would be the hardest failure in the job to diagnose.
- name: The handle-ABA and CSO-content-addressing controls (G8, G12)
working-directory: build-verify
env:
MOBILEGL_ITEST_REQUIRE_GPU: "1"
MOBILEGL_MAGMA_FIX_ITERATIONRP_SUBGROUP_SCRATCH: "1"
MOBILEGL_MAGMA_DERIVE_NUM_SUBGROUPS: "1"
MOBILEGL_MAGMA_ITERATIONRP_FIX_BARRIER: "1"
run: |
ulimit -c unlimited
sudo sysctl -w kernel.core_pattern='/tmp/core.%e.%p'
ctest --output-on-failure -L integration-gpu \
-R 'HandleRecycle|CsoContentAddressing' --no-tests=error -j 4
+3
View File
@@ -35,6 +35,9 @@ frequency-pin integrity.
(`big_cur`/`little_cur`/`gpu_cur_khz` in the result JSON must match the pins). Until then
it says `PROFILE_VERIFIED=0` and `bench.sh` / `session.sh` refuse to run against it unless
`--allow-unverified-profile` is passed, which labels the run unpinned in the warning.
**A profile that omits the key entirely is refused the same way** - the guard defaults to
unverified, so copying a verified profile and editing the serial cannot inherit its verdict.
(`profile.sh` pins nothing - it records a simpleperf profile - so it carries no such guard.)
That refusal exists because the pin path is silent when it is wrong: the harness writes
through `/proc/ppm/policy/hard_userlimit_*` and `/proc/gpufreq/gpufreq_opp_freq`, which are
+9 -4
View File
@@ -69,15 +69,20 @@ done
# TYPE off THIS device, ran one pinned window, and checked big_cur/little_cur/gpu_cur_khz in the
# result JSON against the pins. Nothing else earns it.
require_verified_profile() {
if [ "${PROFILE_VERIFIED:-1}" = "1" ]; then return 0; fi
# The default is UNVERIFIED. A profile that simply omits the key is a profile nobody has
# confirmed against its device, and defaulting it to "verified" would hand exactly the
# fail-open behaviour this guard exists to prevent to the most likely way a new profile is
# written - by copying an existing one and editing the serial.
if [ "${PROFILE_VERIFIED:-0}" = "1" ]; then return 0; fi
if [ "$ALLOW_UNVERIFIED_PROFILE" = "1" ]; then
echo "[warn] $DEVICE_ENV declares PROFILE_VERIFIED=0 and --allow-unverified-profile was passed:" >&2
echo "[warn] $DEVICE_ENV does not carry PROFILE_VERIFIED=1 and --allow-unverified-profile was passed:" >&2
echo "[warn] the frequency pins and the thermal gate in it are UNCONFIRMED, so any number this" >&2
echo "[warn] run produces is not comparable with a pinned one." >&2
return 0
fi
echo "$DEVICE_ENV declares PROFILE_VERIFIED=0: its sysfs nodes and OPPs have not been read off" >&2
echo "the device, so pinning would fail silently and the run would look pinned but not be." >&2
echo "$DEVICE_ENV does not carry PROFILE_VERIFIED=1 (it says 0, or says nothing at all): its" >&2
echo "sysfs nodes and OPPs have not been read off the device, so pinning would fail silently" >&2
echo "and the run would look pinned but not be." >&2
echo "Fill in the TODO_VERIFY_ON_DEVICE fields, confirm one pinned window, set PROFILE_VERIFIED=1 -" >&2
echo "or pass --allow-unverified-profile to measure anyway and label the result unpinned." >&2
exit 2
+6
View File
@@ -3,6 +3,12 @@
# Panel: 1080x1920 @ 60Hz (presented FPS caps at 60 - render-side FPS comes from the
# FCLFPS logcat tag, which counts eglSwapBuffers; it is NOT vsync-capped when the
# game runs with vsync off).
# Read off this device and confirmed against one pinned window (big_cur/little_cur/gpu_cur_khz
# in the result JSON matched the pins), which is what earns the key. bench.sh / session.sh refuse
# a profile without it: the default is UNVERIFIED, so a profile written by copying this one starts
# out refused until somebody repeats that check on the new device.
PROFILE_VERIFIED=1
DEVICE_SERIAL=MTK0002207301023500
# Frequency pins, enforced via /proc/ppm/policy/hard_userlimit_* (plain cpufreq
+3 -2
View File
@@ -5,8 +5,9 @@
# repository rather than in one operator's shell history.
#
# ============================ NOT YET DEVICE-VERIFIED ============================
# PROFILE_VERIFIED=0, and bench.sh / session.sh / profile.sh refuse to run against it unless
# --allow-unverified-profile is passed. This part is a MediaTek SoC, so unlike the Adreno
# PROFILE_VERIFIED=0, and bench.sh / session.sh refuse to run against it - or against a profile
# that omits the key - unless --allow-unverified-profile is passed. (profile.sh only records a
# simpleperf profile and pins nothing, so it carries no such guard.) This part is a MediaTek SoC, so unlike the Adreno
# profile the harness's existing /proc/ppm + /proc/gpufreq pin path is probably the right one -
# but "probably" is exactly the state a measurement profile must not ship in. The cluster
# indices, the available OPPs, the top GPU OPP and the thermal zone TYPE all differ between
@@ -6,8 +6,9 @@
# history, and so that a `--device` argument names something reviewable.
#
# ============================ NOT YET DEVICE-VERIFIED ============================
# PROFILE_VERIFIED=0 below, and bench.sh / session.sh / profile.sh REFUSE to run against a
# profile that says so unless --allow-unverified-profile is passed. Two of the values here are
# PROFILE_VERIFIED=0 below, and bench.sh / session.sh REFUSE to run against a profile that says
# so - or that omits the key - unless --allow-unverified-profile is passed. (profile.sh is not in
# that list: it records a simpleperf profile and pins nothing, so it has nothing to pin wrongly.) Two of the values here are
# protocol constants that are known (the campaign pins big 1.96 GHz / little 1.55 GHz and gates
# at 40 C), but the sysfs node names and the exact available OPPs are NOT: this is a Qualcomm
# part and the harness was written against MediaTek, where the pin goes through
+9 -4
View File
@@ -52,15 +52,20 @@ done
# TYPE off THIS device, ran one pinned window, and checked big_cur/little_cur/gpu_cur_khz in the
# result JSON against the pins. Nothing else earns it.
require_verified_profile() {
if [ "${PROFILE_VERIFIED:-1}" = "1" ]; then return 0; fi
# The default is UNVERIFIED. A profile that simply omits the key is a profile nobody has
# confirmed against its device, and defaulting it to "verified" would hand exactly the
# fail-open behaviour this guard exists to prevent to the most likely way a new profile is
# written - by copying an existing one and editing the serial.
if [ "${PROFILE_VERIFIED:-0}" = "1" ]; then return 0; fi
if [ "$ALLOW_UNVERIFIED_PROFILE" = "1" ]; then
echo "[warn] $DEVICE_ENV declares PROFILE_VERIFIED=0 and --allow-unverified-profile was passed:" >&2
echo "[warn] $DEVICE_ENV does not carry PROFILE_VERIFIED=1 and --allow-unverified-profile was passed:" >&2
echo "[warn] the frequency pins and the thermal gate in it are UNCONFIRMED, so any number this" >&2
echo "[warn] run produces is not comparable with a pinned one." >&2
return 0
fi
echo "$DEVICE_ENV declares PROFILE_VERIFIED=0: its sysfs nodes and OPPs have not been read off" >&2
echo "the device, so pinning would fail silently and the run would look pinned but not be." >&2
echo "$DEVICE_ENV does not carry PROFILE_VERIFIED=1 (it says 0, or says nothing at all): its" >&2
echo "sysfs nodes and OPPs have not been read off the device, so pinning would fail silently" >&2
echo "and the run would look pinned but not be." >&2
echo "Fill in the TODO_VERIFY_ON_DEVICE fields, confirm one pinned window, set PROFILE_VERIFIED=1 -" >&2
echo "or pass --allow-unverified-profile to measure anyway and label the result unpinned." >&2
exit 2
@@ -226,12 +226,32 @@ def read_benchmark(case, backend, run_index):
return report
def series_median(values):
"""The median, by the rule SummarizeSeries uses on the device.
trace_replay_core.cpp's SeriesSummary takes the middle element of an odd window and the
AVERAGE of the two middle elements of an even one, so p50 has to be computed the same way or
the line would print a p50 next to a medianFrameCpuMs that disagreed with it for a reason
nobody could see. (It is the only one of the three that is not a nearest rank: the device's
p95 is.)
"""
if not values:
return -1.0
ordered = sorted(values)
middle = len(ordered) // 2
if len(ordered) % 2 == 1:
return ordered[middle]
return 0.5 * (ordered[middle - 1] + ordered[middle])
def nearest_rank_percentile(values, fraction):
"""Nearest-rank percentile, the same rule SummarizeSeries uses on the device.
"""Nearest-rank percentile, the rule SummarizeSeries uses on the device for p95.
Nearest rank rather than an interpolating percentile so that every number printed here is a
frame that was actually observed, and so that a p95 computed on this side agrees exactly with
the p95 the device reported for the same window.
the p95 the device reported for the same window. The device computes no p99 at all - that is
the whole reason benchmark.json carries the full series - so p99 is this rule extended, and
p50 is NOT computed here (see series_median).
"""
if not values:
return -1.0
@@ -245,11 +265,12 @@ def nearest_rank_percentile(values, fraction):
def cpu_tail(report):
"""The trailing window of the per-frame CPU series, or [] when the run collected none.
benchmark.json carries the WHOLE frameCpuTimesMs[] array precisely so that percentiles the
device does not compute - p50 and p99, which are what the paired A/B publishes - are a
host-side reduction over an artefact that already exists. The window is the same trailing
tailFrames the device summarised, so the numbers below sit beside the device's own without
being about a different set of frames.
benchmark.json carries the WHOLE frameCpuTimesMs[] array precisely so that p99 - which the
device does not compute, and which the paired A/B publishes beside p50 - is a host-side
reduction over an artefact that already exists. The window is the same trailing tailFrames the
device summarised, so the numbers below sit beside the device's own without being about a
different set of frames; p50 is recomputed here by the device's own median rule, so it agrees
with medianFrameCpuMs on the same run rather than merely sitting next to it.
"""
series = report.get("frameCpuTimesMs") or []
if not series:
@@ -277,7 +298,7 @@ def format_benchmark(report):
if window:
line += (
f" | cpu mean={report.get('meanFrameCpuMs', -1):.3f}ms"
f" p50={nearest_rank_percentile(window, 0.50):.3f}ms"
f" p50={series_median(window):.3f}ms"
f" p95={report.get('p95FrameCpuMs', -1):.3f}ms"
f" p99={nearest_rank_percentile(window, 0.99):.3f}ms"
)