From: Len Brown len.brown@intel.com
[ Upstream commit e60a13bcef206795d3ddf82f130fe8f570176d06 ]
/sys/devices/system/cpu/intel_uncore_frequency/package_X_die_Y/ may be readable by all, but /sys/devices/system/cpu/intel_uncore_frequency/package_X_die_Y/current_freq_khz may be readable only by root.
Non-root turbostat users see complaints in this scenario.
Fail probe of the interface if we can't read current_freq_khz.
Reported-by: Artem Bityutskiy artem.bityutskiy@linux.intel.com Original-patch-by: Zhang Rui rui.zhang@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:
Now I have a clear understanding of the issue and fix. Let me analyze the code change:
## Backport Status: **YES**
## Extensive Analysis:
This commit should be backported to stable kernel trees for the following reasons:
### 1. **Bug Fix for Real User Issue** The commit fixes a specific bug where non-root users of turbostat encounter errors when the sysfs permissions are inconsistent. As stated in the commit message: - The directory `/sys/devices/system/cpu/intel_uncore_frequency/package_X_die_Y/` may be readable by all users - But the file `current_freq_khz` within that directory may only be readable by root - This causes turbostat to fail with complaints for non-root users
### 2. **Simple and Contained Fix** The code change is minimal and focused: ```c // Before: Only checked if the directory was readable if (access(path_base, R_OK)) continue;
// After: Check if the actual file we need (current_freq_khz) is readable sprintf(path, "%s/current_freq_khz", path_base); if (access(path, R_OK)) continue; ```
The fix simply moves the permission check from the directory to the actual file that will be read later (line 6763-6765 in the original code reads `current_freq_khz`).
### 3. **Follows Established Pattern** Looking at the `probe_intel_uncore_frequency_cluster()` function (line 6776), it already checks the specific file permission: ```c if (access("/sys/devices/system/cpu/intel_uncore_frequency/uncore00/curr ent_freq_khz", R_OK)) return; ``` This fix makes the legacy function consistent with the cluster function's approach.
### 4. **No Risk of Regression** - The change only affects the probing logic - if the file cannot be read, the function simply skips that uncore frequency entry - This is better than the current behavior where it would try to read the file anyway and fail with errors - The fix maintains backward compatibility - systems where the file is readable continue to work as before
### 5. **Userspace Tool Fix** This is a fix for a userspace tool (turbostat) in the tools/ directory, not kernel code, making it even safer to backport as it cannot cause kernel instability.
### 6. **Clear Problem and Solution** - The problem is well-defined and reported by a real user (Artem Bityutskiy) - The solution is straightforward and logical - The fix prevents error messages that would confuse users running turbostat without root privileges
### 7. **Meets Stable Criteria** According to stable kernel rules, this fix: - Fixes a real bug that affects users ✓ - Is small and contained (2 line change) ✓ - Has minimal risk of introducing new issues ✓ - Doesn't introduce new features ✓ - Doesn't make architectural changes ✓
The commit improves the robustness of turbostat when dealing with permission inconsistencies in sysfs, which is a reasonable scenario that can occur in production systems with specific security configurations.
tools/power/x86/turbostat/turbostat.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 5230e072e414..bf011c2847f2 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -6740,7 +6740,8 @@ static void probe_intel_uncore_frequency_legacy(void) sprintf(path_base, "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d", i, j);
- if (access(path_base, R_OK)) + sprintf(path, "%s/current_freq_khz", path_base); + if (access(path, R_OK)) continue;
BIC_PRESENT(BIC_UNCORE_MHZ);