Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/nouveau/pmu: non-atomic counter read-and-reset causes PMU cycle loss --
commit b1f69f3978548eb48a40027083a2b64535b82115 Author: Hamin Sung <[email protected]> drm/nouveau/pmu/gt215: add graphics engine load counters This commit adds initialization and read functions for the PDAEMON idle counters on GT215 and related GPUs. The counters track PMU clock cycles and graphics engine busy cycles, returning and clearing the values when read. Link: https://envytools.readthedocs.io/en/latest/hw/pm/pdaemon/counter.html > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/gt215.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/gt215.c > index 32cee21ed8585..37310a8ebe818 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/gt215.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/gt215.c [ ... ] > @@ -260,6 +260,44 @@ gt215_pmu_init(struct nvkm_pmu *pmu) > +static void > +gt215_pmu_perfmon_read(struct nvkm_pmu *pmu, u32 *busy, u32 *total) > +{ > + struct nvkm_device *device = pmu->subdev.device; > + > + *busy = nvkm_rd32(device, 0x10a508 + GT215_PMU_COUNTER_GR * 0x10); > + *total = nvkm_rd32(device, 0x10a508 + GT215_PMU_COUNTER_TOTAL * 0x10); > + nvkm_wr32(device, 0x10a508 + GT215_PMU_COUNTER_GR * 0x10, 0x80000000); > + nvkm_wr32(device, 0x10a508 + GT215_PMU_COUNTER_TOTAL * 0x10, > 0x80000000); [Severity: Medium] Does this non-atomic counter read-and-reset sequence cause PMU cycle loss? If a hardware interrupt or CPU preemption occurs between reading the counters and writing the reset values, any hardware cycles that elapse during that window are permanently lost from the statistics. Additionally, a delay between the two reset writes can skew the intervals, which could result in the busy count exceeding the total count. A review of the follow-up devfreq patch shows that the caller gracefully handles the invariant breakage: if (total && busy <= total) ... else stat->busy_time = stat->total_time; This mitigates the risk of integer underflow or metrics exceeding 100%. However, it handles the symptom rather than preventing the permanent loss of PMU cycles during the unprotected preemption window. > + > + *busy &= 0x7fffffff; > + *total &= 0x7fffffff; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
