From: Kaushlendra Kumar kaushlendra.kumar@intel.com
[ Upstream commit cafb47be3f38ad81306bf894e743bebc2ccf66ab ]
The pmt_telemdir_sort() comparison function was returning a boolean value (0 or 1) instead of the required negative, zero, or positive value for proper sorting. This caused unpredictable and incorrect ordering of telemetry directories named telem0, telem1, ..., telemN. Update the comparison logic to return -1, 0, or 1 based on the numerical value extracted from the directory name, ensuring correct numerical ordering when using scandir.
This change improves stability and correctness when iterating PMT telemetry directories.
Signed-off-by: Kaushlendra Kumar kaushlendra.kumar@intel.com Signed-off-by: Len Brown len.brown@intel.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- What it fixes - The comparator `pmt_telemdir_sort()` returned a boolean (`0`/`1`) instead of a strict ordering value (<0, 0, >0). This violates the `scandir()`/`qsort()` contract and can produce non-deterministic and incorrect ordering. In 6.17 the buggy line is `return aidx >= bidx;` in `tools/power/x86/turbostat/turbostat.c:1893`. - The fix changes it to return -1/0/1 based on numeric comparison of the index in the `telem%u` name, ensuring proper ascending numeric order. Mainline shows the corrected code as `return (aidx > bidx) ? 1 : (aidx < bidx) ? -1 : 0;` in the same function.
- Why it matters to users - Turbostat iterates PMT telemetry directories with `scandir()` using this comparator at `tools/power/x86/turbostat/turbostat.c:1921`. With an invalid comparator, entries like `telem10` vs `telem2` can be mis-ordered. - The order is used to construct a linked list of PMT MMIO regions “preserving the ordering from sysfs” in `pmt_mmio_open()` at `tools/power/x86/turbostat/turbostat.c:9668` and `tools/power/x86/turbostat/turbostat.c:9688`. Wrong order can misassociate telemetry regions with the intended domains. - The order is also used to compute a “sequence” for identifying a specific PMT instance in `pmt_parse_from_path()` at `tools/power/x86/turbostat/turbostat.c:10526`. Mis-ordering yields wrong `seq` values and incorrect counter selection.
- Scope and risk - Change is a one-line, localized fix in a user-space tool (`tools/power/x86/turbostat/turbostat.c`), no architectural changes, no ABI impact, and no dependency on new kernel features. - It directly corrects comparator semantics expected by `scandir()`; risk of regression is minimal and behavior becomes deterministic. - Security impact is negligible; this corrects ordering logic for sysfs directories, not privilege or memory handling.
- History and applicability - The bug was introduced when the PMT directory iterator helper landed (commit “Add PMT directory iterator helper”); 6.17’s turbostat has the buggy `return aidx >= bidx;` line (tools/power/x86/turbostat/turbostat.c:1893). - The mainline fix (“tools/power turbostat: Fix incorrect sorting of PMT telemetry”) updates the comparator to return -1/0/1. - Any stable branch that includes the PMT directory iterator helper should take this fix.
- Stable backport criteria - Fixes a user-visible correctness bug in turbostat’s PMT handling. - Small, self-contained, and low risk. - No features added; purely a bug fix improving stability/correctness. - Touches only a userspace tool under `tools/`, not core kernel subsystems.
Given the clear correctness fix, minimal scope, and direct user impact in PMT telemetry iteration, this is a strong candidate for stable backport.
tools/power/x86/turbostat/turbostat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 72a280e7a9d59..931bad99277fe 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -1890,7 +1890,7 @@ int pmt_telemdir_sort(const struct dirent **a, const struct dirent **b) sscanf((*a)->d_name, "telem%u", &aidx); sscanf((*b)->d_name, "telem%u", &bidx);
- return aidx >= bidx; + return (aidx > bidx) ? 1 : (aidx < bidx) ? -1 : 0; }
const struct dirent *pmt_diriter_next(struct pmt_diriter_t *iter)